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

If checksum is corrupt don't assume message is OK #67

Closed
snorfalorpagus opened this issue Aug 7, 2017 · 4 comments
Closed

If checksum is corrupt don't assume message is OK #67

snorfalorpagus opened this issue Aug 7, 2017 · 4 comments

Comments

@snorfalorpagus
Copy link

@snorfalorpagus snorfalorpagus commented Aug 7, 2017

If an NMEA checksum is missing the library will assume everything is OK and continue to parse the message. This can cause problems if the message is so corrupt that the checksum itself is also corrupted, i.e. the * is missing or in the wrong place. I think a better approach would be to skip the message if the checksum is missing. The GPS unit on the v3 board always seems to have a checksum, so I can't see a benefit to ignoring missing values.

Relevant code here: https://github.com/adafruit/Adafruit_GPS/blob/master/Adafruit_GPS.cpp#L36

@SamuelWAnderson45
Copy link

@SamuelWAnderson45 SamuelWAnderson45 commented Nov 16, 2017

I've had to manually patch this for myself to prevent collecting bad data from the GPS. It would be good to see this fixed on master

@billpugh
Copy link

@billpugh billpugh commented Sep 10, 2018

I agree. I encountered several situations in which the strings coming from the GPS unit were corrupted such that there wasn't a * in the expected column and thus the checksum check was ignored.

@snorfalorpagus
Copy link
Author

@snorfalorpagus snorfalorpagus commented Sep 11, 2018

This repository hasn't seen a commit for a year and I think it's fair to say it's stagnant. There are plenty of other NMEA parsers available for Arduino. I have had some luck with TinyGPS.

@drak7
Copy link
Contributor

@drak7 drak7 commented Apr 10, 2019

Good idea, adding this to the library.

@drak7 drak7 closed this Apr 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants