Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added the additional [optional] types to the RMC message including: #144

Merged
merged 5 commits into from
Jul 28, 2022

Conversation

barbacbd
Copy link
Contributor

  • Mode Indicator
  • Navigation Status

The tests were added as a single test case with many subcases.

Fix for #118

- Mode Indicator
- Navigation Status

The tests were added as a single test case with many subcases.
@coveralls
Copy link

coveralls commented Jul 13, 2022

Coverage Status

Coverage increased (+0.02%) to 98.363% when pulling c9a6ee0 on barbacbd:RMC_additional_optional_fields into a0d60b6 on Knio:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 98.363% when pulling e9fe977 on barbacbd:RMC_additional_optional_fields into a0d60b6 on Knio:master.

@barbacbd
Copy link
Contributor Author

@Knio Any way to add you as a reviewer?

@Knio
Copy link
Owner

Knio commented Jul 27, 2022

Can you fix the failing tests?

@barbacbd
Copy link
Contributor Author

@Knio Looks like it is good to go. Apparently I forgot to format the super() call for python2.7

@Knio Knio merged commit 988c297 into Knio:master Jul 28, 2022
@wodny
Copy link

wodny commented Apr 14, 2023

@Knio
I think that the new is_valid implementation should contain a warning in a docstring as it causes a regression for multiple device models (eg. modems with GPS) when upgrading from 1.18 to 1.19. It seems that some of the devices always return V for the navigational status as they do not implement it but still return messages in the extended NMEA 4.10 format. For those devices status == "A" seems better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants