Skip to content

The CRC check doesn't catch all bad messages #55

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

Closed
lesamouraipourpre opened this issue Feb 22, 2021 · 4 comments · Fixed by #57
Closed

The CRC check doesn't catch all bad messages #55

lesamouraipourpre opened this issue Feb 22, 2021 · 4 comments · Fixed by #57

Comments

@lesamouraipourpre
Copy link
Contributor

Because, the CRC value in the NMEA sentence is only 8-bit, it is fairly easy for a bad message to get past the CRC check.
By observation, with my GPS on the window it takes on average ~3 hours before a bad message crashes the program.

This is usually triggered by _parse_int and _parse_float functions and can happen elsewhere.

@lesamouraipourpre
Copy link
Contributor Author

I intend to fix this by making the _parse_int etc. methods always return either a valid value or None and never raise a ValueError.

I also intend to put checks into the _parse_gpgll etc functions which will fail quickly if the wrong number of comma separated parts has been received.

@tannewt
Copy link
Member

tannewt commented Feb 23, 2021

Would it be possible to fail in the same way as CRC if parse int or float fail? That way you still get an error but you only have one type of error to handle.

@lesamouraipourpre
Copy link
Contributor Author

I'm aiming to keep it so that the update() method behaviour stays the same, specifically
Returns True if new data was processed, and False if nothing new was received.
Returning False in the event of an unparseable sentence will match the behaviour of a CRC failure.

@tannewt
Copy link
Member

tannewt commented Feb 23, 2021

Ok, that sounds perfect! Thank you!

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 a pull request may close this issue.

2 participants