adafruit/Adafruit_GPS

Parse function checksum issue

Closed this issue · 2 comments

CJayW commented

Hi,
I've recently been looking through this library with the idea of using it for a project, but I have noticed that there seems to be a slight issue with the check sum.
When calculating the check sum value, the start of the sentence found using the strchr and the "$" sign at line 65.
However, after the check sum is finished, its assumed that nmea[0] is the start of the sentence, this could possible cause data to be lost.

I don't currently have any hardware for this so i cannot fix it and make a PR, or know if this even has any effect.
The best fix that I can think of is to set the start of the sentence to "p" from line 65.

Thanks

If the NMEA sentence is well formed, starts with $ and ends with a checksum, then everything works fine. The checksum code is also robust even if there is trash before the $ or after the checksum.

The rest of the code would be more robust to prepended trash with nmea = p inserted between lines 67 and 68, but that has the stylistic fault of changing the value of an argument in the middle of a function. nmeaStart = p, then using that variable through the rest of the parsing would be tidier, but more work as a modification.

A badly formed sentence might pass the checksum test and fail the parsing, still generating the right outcome. If I was looking for work, I would start by building validation into the various parseXX() functions to make sure the results are in range.

I think I have a fix for this one that I will send up on my next pull request. It creates a new function check() that independently validates that the string starts with a $ and ends with a valid check sum. parse() can then run check() before getting into the parsing process.