AeroRust / nmea

NMEA 0183 - for communication between marine electronics such as echo sounder, sonars, anemometer, gyrocompass, autopilot, GNSS receivers and many other types of instruments. Defined and controlled by the National Marine Electronics Association (NMEA)
https://crates.io/crates/nmea
Other
60 stars 41 forks source link

Implement `PGRMZ` support #59

Closed Turbo87 closed 2 years ago

Turbo87 commented 2 years ago

$PGRMZ is used by a lot of gliding computers like the https://gliding.lxnav.com/products/lx9000/ to transmit the current barometric altitude.

This PR implements basic support for it. It is currently implemented to output the raw altitude in feet without any conversions.

elpiel commented 2 years ago

Great progress for the support of RMZ messages.

There are a few points that need addressing:

codecov[bot] commented 2 years ago

Codecov Report

Base: 76.80% // Head: 76.76% // Decreases project coverage by -0.03% :warning:

Coverage data is based on head (f65c8cc) compared to base (03695a6). Patch coverage: 75.00% of modified lines in pull request are covered.

:exclamation: Current head f65c8cc differs from pull request most recent head 7b0ef1f. Consider uploading reports for the commit 7b0ef1f to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #59 +/- ## ========================================== - Coverage 76.80% 76.76% -0.04% ========================================== Files 20 21 +1 Lines 776 792 +16 ========================================== + Hits 596 608 +12 - Misses 180 184 +4 ``` | [Impacted Files](https://codecov.io/gh/AeroRust/nmea/pull/59?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AeroRust) | Coverage Δ | | |---|---|---| | [src/parser.rs](https://codecov.io/gh/AeroRust/nmea/pull/59/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AeroRust#diff-c3JjL3BhcnNlci5ycw==) | `79.91% <ø> (ø)` | | | [src/sentences/mod.rs](https://codecov.io/gh/AeroRust/nmea/pull/59/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AeroRust#diff-c3JjL3NlbnRlbmNlcy9tb2QucnM=) | `0.00% <ø> (ø)` | | | [src/sentences/rmz.rs](https://codecov.io/gh/AeroRust/nmea/pull/59/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AeroRust#diff-c3JjL3NlbnRlbmNlcy9ybXoucnM=) | `75.00% <75.00%> (ø)` | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AeroRust). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AeroRust)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

Turbo87 commented 2 years ago
  • Add new sentence type to the fn parse()

tbh I'm not convinced that we should be merging this data with the rest in the parse() function. the PGRMZ sentence usually contains a barometric or pressure altitude, while the regular NMEA sentences contain GPS altitudes, which can often have significant offsets due to the current weather situation. if we would merge them and pretend like they contain the same information then the altitude will regularly jump all over the place.

elpiel commented 2 years ago

@Turbo87 I was referring to the parse_str(I made a typo) which is able to parse all types of sentences, not like the NMEA::parse():

https://github.com/AeroRust/nmea/blob/03695a6d0dacdf39f16ba83338d040418ef6fd54/src/parse.rs#L134

Turbo87 commented 2 years ago

Ah, I see. That makes sense :)

I'll update the PR in the next couple of days once EuroRust is over

elpiel commented 2 years ago

Awesome, thank you 🥳

elpiel commented 2 years ago

Thank you 🥳