Adds MWV and MDA parsers#63
Conversation
|
Thank you for working on this! I will make sure to review the PR in the coming days. |
Codecov ReportBase: 79.50% // Head: 79.50% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## main #63 +/- ##
=======================================
Coverage 79.50% 79.50%
=======================================
Files 22 22
Lines 854 854
=======================================
Hits 679 679
Misses 175 175 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
elpiel
left a comment
There was a problem hiding this comment.
The PR looks great so far. There are a few tweaks and small changes + fixing the CI build but other than that great contribution <3 !
Would you mind adding both example sentences to the test case all_supported_messages too?
|
Alrighty - still getting the hang of Rust's docs and linting, but Edit - forgot the pub members on MDA - fixed now. Give it a whirl Shirl |
clemarescx
left a comment
There was a problem hiding this comment.
Looks good to me, just the nitpick about using preceded(char(','), <...>) instead of let (i, _) = char(',')(i)?; in the MWV parser
|
|
||
| fn do_parse_mwv(i: &str) -> IResult<&str, MwvData> { | ||
| let (i, direction) = opt(float)(i)?; | ||
| let (i, reference_type) = opt(preceded(char(','), one_of("RT")))(i)?; |
There was a problem hiding this comment.
To handle the comma separators, you should parse each field separated with let (i, _) = char(',')(i)?; instead of using preceded(char(','), <...>)(i)?; when parsing the field itself, like you did in the whole MDA parser.
This is not a big deal, mostly for the sake of consistency with the rest of the crate.
According to the benchmarks (run cargo bench in the project), neither seem to outperform the other in a significant way.
Adds a pair of parsers for WIMDA and WIMWV. No GPS Fix related messaging parsing for this set as they are both weather station based messages.