My Asset Shield and Electron seems to be working fine as far as publishing GPS coordinates. But when I made a serial connection to COM3: (9600,N,8,1) the output seemed erratic like bits and pieces were missing.
I stepped down the transfer rate to 2400, just to make sure I wasn’t losing data, but that didn’t seem to make the problem better or worse.
I added some more strings to the main loop of the default 1-gps-features.ino example. The text I added always comes across the same, which is good, but the full NMEA sentence seems irregular.
Does the routine which prints the last NMEA sentence have a problem? Do I simply misunderstand what it should be printing when it is dumping the full NMEA sentence? I don’t know. I’ve included my modification to the main loop below, and I’ve included some sample output (to serial) below.
void loop() {
// You'll need to run this every loop to capture the GPS output
t.updateGPS();
// if the current time - the last time we published is greater than your set delay...
if(millis()-lastPublish > delayMinutes*60*1000){
// Remember when we published
lastPublish = millis();
//String pubAccel = String::format("%d,%d,%d",t.readX(),t.readY(),t.readZ());
//Serial.println(pubAccel);
//Particle.publish("A", pubAccel, 60, PRIVATE);
// Dumps the full NMEA sentence to serial in case you're curious
Serial.println("Full NMEA sentence --------------------------------------------------------------");
Serial.println(t.preNMEA());
// GPS requires a "fix" on the satellites to give good data,
// so we should only publish data if there's a fix
if(t.gpsFix()){
// Only publish if we're in transmittingData mode 1;
if(transmittingData){
// Short publish names save data!
Particle.publish("G", t.readLatLon(), 60, PRIVATE);
}
// but always report the data over serial for local development
Serial.println("Lattitude, Longitude ------------------------------------------------------------");
Serial.println(t.readLatLon());
Serial.println(" ");
}
}
}
Here is a sample of the output (set to 1 minute updates). Latitude and longitude adjusted (in raw NMEA and formatted output) for my own privacy:
Full NMEA sentence --------------------------------------------------------------
$GPGGA,205118.000,2206.2392,N,03356.2124,W,1,8,0.95,223.8,M,-27.1,M,,*G,00,,
Lattitude, Longitude ------------------------------------------------------------
22.103989,-33.936867
Full NMEA sentence --------------------------------------------------------------
$GPGGA,205215.000,2206.2368,N,03356.2130,W,1,8,0.95,223.5,M,1
Lattitude, Longitude ------------------------------------------------------------
22.103947,-33.936882
Full NMEA sentence --------------------------------------------------------------
$GPGGA,205319.000,2206.2346,N,03356.2131,W,1,7,1.13,223.3,M,-27.1,M,,*P26859G010724
Lattitude, Longitude ------------------------------------------------------------
22.103909,-33.936882
Full NMEA sentence --------------------------------------------------------------
$GPGGA,205419.000,2206.2350,N,03356.2118,W,1,8,0.96,223.0,M,-27.1,M,,*6,36,,*S2,3,,1G,1,724
Lattitude, Longitude ------------------------------------------------------------
22.103920,-33.936867
Full NMEA sentence --------------------------------------------------------------
33,,,,7,04,,,
Lattitude, Longitude ------------------------------------------------------------
22.103909,-33.936813
Full NMEA sentence --------------------------------------------------------------
,13*M069,11
Lattitude, Longitude ------------------------------------------------------------
22.103882,-33.936813
Full NMEA sentence --------------------------------------------------------------
$.04
Lattitude, Longitude ------------------------------------------------------------
22.103897,-33.936813
I modified the library AssetTracker.h by adding in a println(nmea) into the Adafruit_GPS::parse function. What I saw is that malform lines were being fed into the parse function. Worse, it looks like if the “*” isn’t in the right place, it doesn’t attempt to confirm the line’s checksum, and it goes forward with processing the line.
I could be stupid, but I’m starting to think there is quite a few things wrong here?
boolean Adafruit_GPS::parse(char *nmea) {
// do checksum check
// first look if we even have one
if (nmea[strlen(nmea)-4] == '*') {
uint16_t sum = parseHex(nmea[strlen(nmea)-3]) * 16;
sum += parseHex(nmea[strlen(nmea)-2]);
// check checksum
for (uint8_t i=2; i < (strlen(nmea)-4); i++) {
sum ^= nmea[i];
}
if (sum != 0) {
// bad checksum :(
return false;
}
}
int32_t degree;
long minutes;
char degreebuff[10];
// look for a few common sentences
// DEBUG LINE FROM JOSH
Serial.println(nmea);
Sample output (which passed the checksum and went into the parser):
This shouldn’t be happening… right? This shouldn’t even reach the parser. But then, we seem to regularly be sending the parser some strings which are partially invalid?
I’ve spent more time on this than I’d like. I believe that I’ve found a number of significant problems with the Asset Tracker library.
1] There seems to be an input problem in the serial data from the GPS. I haven’t been able to completely diagnose that, but the problem gets a bit better when you go from PMTK_SET_NMEA_OUTPUT_RMCGGA to a reduced data set with PMTK_SET_NMEA_OUTPUT_RMCONLY so I’m thinking that it looks like a buffer overrun? I don’t know.
2] Each NMEA sentence should have a checksum which is a * followed by two hex digits. However, if the checksum symbol * isn’t there, or is in the wrong place, then it will go ahead and skip the checksum and process the line as though it had passed the check. Ooops!
3] These two problem combined actually hides the fact that the checksum calculator code itself is bad. It starts the loop at i=2 when it should start at i=1. It isn’t running a checksum on the complete string between the $ and the *. So it’ll fail any line that is actually good. Ha!
4] I’d have to go back and check, but I think someone may have misunderstood what to do on the character input routine when a $ comes in. It should clear out the line and become the first character.
5] I’m suspecting that the alternate GPRMC sentence processing may be unreliable (I have good data but most of the time, the parser isn’t pulling out longitude and latitude) but I haven’t investigated why.
All in all, I think this library needs a very serious review. But I’m a newbie, so I’d really appreciate someone looking over my findings.
I think you are having serial buffer overrun problems that are confusing your debugging. The baud rate for the USB serial connection does nothing to the data flow, so that step didn’t help. Try printing less (shorter separator lines) and see what happens–I believe the character buffer for serial is 40 or 64 bytes, so try to stay under that.
I don’t have an asset tracker shield to test, but some of your points just don’t add up. For instance in the GPS parse code, the very first statement tests to see if a character four from the end of the input is a “*” and the entire rest of the parser is skipped if it is not. The code then parses the hex checksum and sums the NMEA string and if the sum is not zero, it returns false.
A NMEA checksum is calculated from the character after the “$” unto the “*”, so starting from 2 seems more appropriate to me (although I am not sure what is in the array at 0).
I see this isn’t going to be an easy sell. Your reply focused on the checksum routine, let’s start with that.
Let’s assume that we’re going into the parser function with a string that is missing its checksum. The ‘*’ character is NOT in the expected place near the end of the line. I used Serial.println statements to trace it, but I think we can just look at the code and eyeball it.
Line 151: [comment]
Line 152: [blank line]
Line 153: [comment]
Line 154: [if statement - condition check fails]
Line 167: […continues into main parser routine…]
There is a missing else statement as a counterpart for line 154. If the ‘*’ is not found at that position, then it doesn’t try to calculate checksum, and it continues on with the parser.
OK, thanks, I see what you mean now. The code in github is indented as if it worked in the fashion I described, but when I throw it into an editor, the closing “}” for the if is not where I would have put it.
I have opened a github issue for that library and will ping @jensck here to let him know. The original Adafruit library has this problem as well.
One you fix that problem, I think the AssetTracker library is going to be broken. At least for me, the fact that it is not processing the checksum is actually covering up a deeper problem. There seems to be an overflow of data coming from the GPS module and so the strings it is trying to parse are very often corrupt or truncated.
I’ll see if I can’t put in a few debug statements which better illustrate this.
GPS is serial and requires that you not read character by character to avoid dropping data.
Adding more serial print debug statements may not help you and may actually make things worse since the ISR that services one serial port can block the other serial port from service.
I am still not clear what your actual problem is?
Does your code work correctly without serial debug statements?
To be fair, it is the library itself which is reading character by character in the Adafruit_GPS::read function. But I'll accept your statement that my own debug statements could compound the problem.
[quote="bko, post:10, topic:21356, full:true"]
I am still not clear what your actual problem is?
Does your code work correctly without serial debug statements?[/quote]
I believe the AssetTracker library manages to at least function, despite a number of faults, because it isn't correctly processing the checksum. We haven't discussed this additional part in detail, but I believe that when it does process the checksum, it is starting from the wrong offset at the start of the NMEA sentence (one character too far to the right).
The result of both problems? NMEA sentences that are invalid (because the checksum indicator '*' is in the wrong place) actually are processed. NMEA sentences with the checksum character and the checksum value that ARE correct are flagged as invalid and are not processed because the Adafruit_GPS::parse function has a bad checksum verification routine.
// check checksum
for (uint8_t i=2; i < (strlen(nmea)-4); i++) {
sum ^= nmea[i];
}
...should be changed to...
// check checksum
for (uint8_t i=1; i < (strlen(nmea)-4); i++) {
sum ^= nmea[i];
}
The change is: i=2 --> i=1
I realize the burden of proof is on me to show that the library isn't functioning correctly, but this is a lot of effort! Can you walk through what the library is doing here, or what kind of data can I provide which better illustrates where it is going wrong?
The Adafruit parser was actually changed to start from 2 since the read function puts a carriage return (\n) at the start of the string, so since C strings start from index 0, it skips the \n and the $ in the checksum calculations.
Huh. I'm not actually seeing a carriage return at the start of an NMEA sentence that is being passed to the parser.
I think what is going on is that the library's behavior is very inconsistent in delivering the strings to the parser. The position of the carriage return varies from line to line. Depending on where it ends up (end of a sentence that is passed to the parser, missing, or at the beginning), it will either put the '*' character in the wrong offset, or it will make the checksum calculation begin at the wrong offset.
The end result is that, in the official AssetTracker library, a lot of NMEA sentences are being discarded and are not processed. The ones that do get processed are the ones with the '*' that is missing or at the wrong offset.
I don't use GitHub, so it has been a struggle to use to update the library. But I have created a test library here with some debug statements. Also, I added code so that it'll reject lines with a '*' character that is missing or in the wrong position, and that has a big impact. Hopefully this is public...
For those that have the Asset Tracker shield, you can run if for yourself and see that it is being inconsistent with what is being delivered to the parser. For others, here's some sample output from my own run, with GPS coordinates altered (after the fact) for my own privacy... so if you manually compute a checksum on this output, it won't add up. Please assume that the debug statements on the output aren't being mangled. They look absolutely great. You can skip past the $P lines at the beginning:
DEBUG: Evaluating NMEA sentence ----------------------
<ENDN>$PGACK,103*40
------------------------------------------------
DEBUG: Checksum is invalid
DEBUG: Evaluating NMEA sentence ----------------------
<ENDN>$PGACK,105*46
------------------------------------------------
DEBUG: Checksum is invalid
DEBUG: Evaluating NMEA sentence ----------------------
<ENDN>$PMTK011,MTKGPS*08
------------------------------------------------
DEBUG: Checksum is invalid
DEBUG: Evaluating NMEA sentence ----------------------
<ENDN>$GPRMC,154459.320,V,,,,,0.12,207.25,230316,,,N*40
------------------------------------------------
DEBUG: Checksum is invalid
DEBUG: Evaluating NMEA sentence ----------------------
BEGIN>$GPRMC,154608.000,A,2206.2412,N,03356.2202,W,0.05,123.04,230316,,,A*7D<END
------------------------------------------------
DEBUG: Checksum missing, character in checksum position is: A
DEBUG: Evaluating NMEA sentence ----------------------
BEGIN>$GPRMC,154618.000,A,2206.2402,N,03356.2184,W,0.16,155.81,230316,,,A*7E<END
------------------------------------------------
DEBUG: Checksum missing, character in checksum position is: A
DEBUG: Evaluating NMEA sentence ----------------------
BEGIN>$GPRMC,154838.000,A,2206.2392,N,03356.2323,W,0.27,281.09,230316,,,A*7B<END
------------------------------------------------
DEBUG: Checksum missing, character in checksum position is: A
DEBUG: Evaluating NMEA sentence ----------------------
<ENDN>$GPRMC,155018.000,A,2206.2407,N,03356.2374,W,0.17,9.61,230316,,,A*76
------------------------------------------------
DEBUG: Checksum is invalid
DEBUG: Evaluating NMEA sentence ----------------------
<ENDN>$GPRMC,155148.000,A,2206.2491,N,03356.2394,W,0.28,21.30,230316,,,A*41
------------------------------------------------
DEBUG: Checksum is invalid
DEBUG: Evaluating NMEA sentence ----------------------
BEGIN>$C0.9W86<END
------------------------------------------------
DEBUG: Checksum missing, character in checksum position is: 9
$C0.9W86
DEBUG: Evaluating NMEA sentence ----------------------
<ENDN>$GPRMC,155459.000,A,2206.2312,N,03356.2152,W,0.24,41.13,230316,,,A*4B
------------------------------------------------
DEBUG: Checksum is invalid
DEBUG: Evaluating NMEA sentence ----------------------
<ENDN>$GPRMC,155519.000,A,2206.2329,N,03356.2152,W,0.16,0.47,230316,,,A*73
------------------------------------------------
DEBUG: Checksum is invalid
DEBUG: Evaluating NMEA sentence ----------------------
<ENDN>$GPRMC,155530.000,A,2206.2348,N,03356.2161,W,0.60,1.31,230316,,,A*7E
------------------------------------------------
DEBUG: Checksum is invalid
DEBUG: Evaluating NMEA sentence ----------------------
<ENDN>$GPRMC,155610.000,A,2206.2420,N,03356.2165,W,0.50,17.70,230316,,,A*43
------------------------------------------------
DEBUG: Checksum is invalid
DEBUG: Evaluating NMEA sentence ----------------------
<ENDN>$GPRMC,155620.000,A,2206.2433,N,03356.2160,W,0.23,77.91,230316,,,A*4A
------------------------------------------------
DEBUG: Checksum is invalid
DEBUG: Evaluating NMEA sentence ----------------------
<ENDN>$GPRMC,155630.000,A,2206.2435,N,03356.2152,W,0.42,47.78,230316,,,A*4F
------------------------------------------------
DEBUG: Checksum is invalid
DEBUG: Evaluating NMEA sentence ----------------------
<ENDN>$GPRMC,155701.000,A,2206.2443,N,03356.2178,W,0.31,7.77,230316,,,A*7A
------------------------------------------------
DEBUG: Checksum is invalid
DEBUG: Evaluating NMEA sentence ----------------------
<ENDN>$GPRMC,155721.000,A,2206.2463,N,03356.2195,W,0.34,4.73,230316,,,A*7B
------------------------------------------------
DEBUG: Checksum is invalid
DEBUG: Evaluating NMEA sentence ----------------------
<ENDN>$GPRMC,155731.000,A,2206.2472,N,03356.2193,W,0.31,7.29,230316,,,A*75
------------------------------------------------
DEBUG: Checksum is invalid
DEBUG: Evaluating NMEA sentence ----------------------
<ENDN>$GPRMC,155852.000,A,2206.2486,N,03356.2150,W,0.40,55.12,230316,,,A*42
------------------------------------------------
DEBUG: Checksum is invalid
DEBUG: Evaluating NMEA sentence ----------------------
<ENDN>$GPRMC,155932.000,A,2206.2512,N,03356.2119,W,0.38,43.84,230316,,,A*43
------------------------------------------------
DEBUG: Checksum is invalid
DEBUG: Evaluating NMEA sentence ----------------------
<ENDN>$GPRMC,155952.000,A,2206.2512,N,03356.2095,W,0.74,46.77,230316,,,A*41
------------------------------------------------
DEBUG: Checksum is invalid
DEBUG: Evaluating NMEA sentence ----------------------
BEGIN>$GPRMC,160032.000,A,2206.2494,N,03356.2073,W,0.20,356.64,230316,,,A*7E<END
------------------------------------------------
DEBUG: Checksum missing, character in checksum position is: A
DEBUG: Evaluating NMEA sentence ----------------------
BEGIN>$GPRMC,160102.000,A,2206.2502,N,03356.2080,W,0.34,247.86,230316,,,A*76<END
------------------------------------------------
DEBUG: Checksum missing, character in checksum position is: A
DEBUG: Evaluating NMEA sentence ----------------------
BEGIN>$GPRMC,160212.000,A,2206.2454,N,03356.2127,W,0.23,357.03,230316,,,A*71<END
------------------------------------------------
DEBUG: Checksum missing, character in checksum position is: A
DEBUG: Evaluating NMEA sentence ----------------------
<ENDN>$GPRMC,160222.000,A,2206.2453,N,03356.2129,W,0.36,8.78,230316,,,A*7A
------------------------------------------------
DEBUG: Checksum is invalid
DEBUG: Evaluating NMEA sentence ----------------------
<ENDN>$GPRMC,160252.000,A,2206.2472,N,03356.2145,W,0.16,44.97,230316,,,A*4F
------------------------------------------------
DEBUG: Checksum is invalid
DEBUG: Evaluating NMEA sentence ----------------------
BEGIN>$GPRMC,160332.000,A,2206.2496,N,03356.2134,W,0.06,111.11,230316,,,A*7A<END
------------------------------------------------
DEBUG: Checksum missing, character in checksum position is: A
DEBUG: Evaluating NMEA sentence ----------------------
<ENDN>$GPRMC,160342.000,A,2206.2495,N,03356.2132,W,0.45,69.25,230316,,,A*46
------------------------------------------------
DEBUG: Checksum is invalid
DEBUG: Evaluating NMEA sentence ----------------------
<ENDN>$GPRMC,160352.000,A,2206.2499,N,03356.2118,W,0.26,43.47,230316,,,A*4A
------------------------------------------------
DEBUG: Checksum is invalid
Well I wish I had one of these to test–maybe @Hypnopompia or @Dave can test it for us? I know @Dave had some ideas to improve the serial reading in that library.
I will say that in my sampling of your printed results above, some of the checksums were correct, such as this one:
$GPRMC,154459.320,V,,,,,0.12,207.25,230316,,,N*40
This seems to happen when the CRLF buffering is not right based on your BEGIN> and <END tags.
And some of the checksums were wrong. When they are wrong, they seem to fit a pattern in the last hex digit but I don’t know if that means anything or not.
Don’t be fooled: there are a whole bunch of online NMEA checksum calculators that are wrong in that they compute the decimal and not the hex checksum. I validated my testing code against the samples here.
[quote=“bko, post:14, topic:21356, full:true”]
I will say that in my sampling of your printed results above, some of the checksums were correct, such as this one…[/quote]
As mentioned, the lines with an actual GPS location will have a bad checksum because, after the fact, I altered my GPS coordinates in the output to protect my privacy.
When I looked at the checksums being delivered to the code before I modified them, they were correct (but the official library would either not evaluate the checksum at all and blindly pass them through because the ‘*’ was at an unexpected offset, or it would incorrectly flag a valid checksum as bad and discard it because the library starts calculating its own checksum at one character too late into the start of the sentence). If it is useful, I can privately deliver some unmodified output to you for examination.
[quote=“bko, post:14, topic:21356, full:true”]
This seems to happen when the CRLF buffering is not right based on your BEGIN> and <END tags.[/quote]
Correct. So only the lines that say, “DEBUG: Checksum missing” would be the only ones that are processed in the official library. For me, this is a minority. Most of my valid NMEA sentences are being thrown away.
Someone else might see this in terms of it taking a long time to finally deliver lattitude/longitude after a signal fix, or they might see their coordinates not always updating as consistently as it should, and not due to signal acquisition issues.
The Adafruit_GPS::read function is not delivering consistent results (particularly in regards to placement of the CR).
Yeah, I seem to be missing data from the GPS serial read in places that I shouldn’t be. I’ve got some test code setup to only read (no output debugging or publish events) until a full NMEA sentence comes in, and I’m missing characters in the middle of a sentence. I’m slowing trimming down code in the adafruit library to simplify this. I’ll report back results when I have more info. Might just be something I’m doing wrong.
[quote=“bko, post:16, topic:21356, full:true”]
There is a comment in the Asset Tracker code that echos my concern over using serial debug printing…[/quote]
We got a few things going on here, right?
Back to the original complaint! Awesome. But despite the caution expressed inside the library, the sample code 1_GPS_Features.cpp has serial debugging hard-coded into the main loop, and that’s what initially grabbed my attention:
// Dumps the full NMEA sentence to serial in case you're curious
Serial.println(t.preNMEA());
// but always report the data over serial for local development
Serial.println(t.readLatLon());
[quote=“bko, post:16, topic:21356, full:true”]
I know that @Hypnopompia is looking at this now as well and he has seen some strange results…[/quote]
Naturally, as a newbie, I don’t know who that is, but it sounds reassuring.
[quote=“Hypnopompia, post:17, topic:21356, full:true”]
Yeah, I seem to be missing data from the GPS serial read in places that I shouldn’t be.[/quote]
[waves] Hi! Thanks for looking into this!
I can confirm that I’m getting strange results as well. I went ahead and made the most simplistic set of code that I could to reproduce this. I’m not using the adafruit library at all in this case.
I’m simply reading from the GPS serial port as fast as possible and looking for NMEA sentence characters. ($ and \n). If I get something I’m not expecting, I start over with the next sentence. I try not to do anything like output to USB serial unless there’s a read error, or I have a complete sentence.
I’m seeing a high error rate on data coming in from the tracker shield. Most sentences look good until I get a $GPGGA sentence and it just looks corrupted towards the end. Many times the newline character is just simply missing. The adafruit library does indeed seem to not care much, which is why you can get some good readings every now and then.
@Dave, @bko: I’m not sure where to go from here. I think this is over my head now and probably goes into issues with the electron serial firmware and/or the gps shield. Do you guys see any obvious issues with my code in how I’m reading from the serial GPS port?
OK, @dave got back to me and gave me some hints. The way the asset tracker example is reading from the serial port is too slow. The read loop on the serial port needs to be much tighter. He has an example in his fancy tracker repository:
