Bug in read() routine

[srob1]

According to the NMEA specification, sentences start with a '$' character and end with a CR-LF pair.

The read() routine in the Adafruit GPS library terminates received NMEA sentences with a CR rather than a CR-LF pair. In addition, all returned NMEA sentences except the first begin with a LF rather than a '$'. (This LF is actually the final LF of the prior NMEA sentence received.)

When the read() routine sees the LF character ('\n'), it null terminates the current NMEA sentence in the buffer pointed to by currentline (which is either line1 or line2), and then switches currentline to point to the other buffer all before storing the LF character in the buffer.

The issue is that it then stores the LF character in the new buffer as the first character of the new buffer, rather than the last character of the prior buffer.

As a result, most NMEA sentences returned by lastNMEA() start with a LF character and end with a CR character rather than starting with a '$' and ending with a CR-LF pair. ( I say most because unlike all other NMEA setences, the first NMEA sentence returned by the parser does not start with a LF because there was no trailing LF from a previous NMEA sentence stored at the start of the first buffer.)

When/if you fix this incorrect behavior, you will need to make changes to other parts of the library. For example, the parse() routine relies on the fact that there is a single CR character at the end of NMEA sentences it receives rather than the standard CR-LF pair that is part of the specification. This dependency is in the checksum code at the top of the parse() routine. In particular, it looks for an asterisk character ('*') four characters from the end of the NMEA sentence rather than five characters from the end. The rest of the indices used for the checksum will also need to be adjusted.

Also, due to this bug, I believe the parse routine will always incorrectly determine that that the checksum on the first received NMEA sentence is incorrect, because it starts checksumming at offset 2 into the NMEA sentence. As noted above, the very first NMEA sentence buffered by the read() routine does NOT start with a LF since there was no previous NMEA sentence.

(I am opening a separate issue regarding the repeated use of strlen() in the checksum code at the top of parse. Processing NMEA sentences is somewhat time critical in order to keep up with them. So it is a bit ridiculous that strlen is used in the condition clause of the checksum for-loop. That invokes strlen once for every character in the NMEA sentence! The length of a NMEA sentence is invariant inside parse, so strlen should be evaluated once at the start of the routine and stored in a local variable.)

[ladyada]

thanks for the note, we should probably just drop all \n and \r's - is that something you can change and submit a PR?