SlashDevin / NeoGPS

NMEA and ublox GPS parser for Arduino, configurable to use as few as 10 bytes of RAM
GNU General Public License v3.0
694 stars 193 forks source link

Issue with Height Sigma Error #80

Closed Phill76 closed 6 years ago

Phill76 commented 6 years ago

Hi i think i have detected an issue with the Height Sigma Error information in GST message : it seems that in some conditions the value is divided by ten --> see attached excel file in which i have logged the Height Sigma Error value and the char data before it is parsed by NEOGPS (i've modified the source to have this raw char information).

thanks for your feedback and the great job with NEOGPS

LOG GST.xlsx

SlashDevin commented 6 years ago

Sorry, but I won't download a spreadsheet. Simply embed the data in a code block by putting three grave accents before and after the data, like this:

``` 234,456,678,78,9 456,567,89,234,5 ```

This will display like this:

234,456,678,78,9
456,567,89,234,5

I do appreciate the feedback, though. I'll take a closer look until you get a chance to post your data.

Phill76 commented 6 years ago

I forgot to mention it, altitude is constant and around 40m.

Satelittes Number in fix

Altitude (m) Sigma Altitude (m) NMEA GST Sigma Alt CHAR 3, 127.1999969, 7.10, 71,1,00,, 3, 128.1000061, 6.60, 66,1,00,, 3, 128.1999969, 6.50, 65,1,00,, 3, 129.1000061, 6.50, 65,1,00,, 3, 129.1999969, 6.40, 64,1,00,, 3, 129.3999938, 6.40, 64,1,00,, 3, 128.6000061, 6.30, 63,1,00,, 3, 127.0000000, 6.30, 63,1,00,, 3, 126.0999984, 6.30, 63,1,00,, 3, 126.0000000, 6.30, 63,1,00,, 3, 125.4000015, 6.30, 63,1,00,,

SlashDevin commented 6 years ago

I'm not sure I know what the last fields mean. Is this correct?

fix.satellitesfix.altitude() NMEA GST
raw data
fix.alt_err_cm???
3127.19999697.1071100
3128.10000616.6066100
3128.19999696.5065100
3129.10000616.5065100
3129.19999696.4064100

It's a little suspicious that you have reception from 3 satellites, but it shows altitude and altitude error. Normally that requires 4 satellites. Could this be fix.status instead?

If I use this test sentence:

$GPGST,082356.00,1.8,,,,1.7,1.3,7.10*48

NeoGPS appears to parse it correctly to fix.alt_err_cm == 710.

I guess I'll need more information:

  1. Are you using the default configuration, except for enabling GST and ALT_ERR?
  2. Are you checking the validity flags for each field?
  3. Are you getting each fix with the normal loop structure?
  if (gps.available( gpsPort )) {
    fix = gps.read();

I would suggest using the github website to reply, so you can format your replies nicely (or I can do it for you). Email replies do not allow formatting.

Phill76 commented 6 years ago

Are you using the default configuration, except for enabling GST and ALT_ERR?

yes.

Are you checking the validity flags for each field?

[I am checking] fix.valid.location

Are you getting each fix with the normal loop structure?

yes

regarding the number of satellites, gps.sat_count gives higher number of satellites.

to take an example :

fix.satellitesfix.altitude()fix.alt_err()NMEA GST
raw data
3127.19999697.10"71,1,00,,"

i have obtained what i call "NMEA raw data" by modifying this function like that :

bool NMEAGPS::parseGST( char chr )
{
#ifdef NMEAGPS_PARSE_GST
    switch (fieldIndex) {
      case 1: return parseTime( chr );
      case 6: return parse_lat_err( chr );
      case 7: return parse_lon_err( chr );
      case 8: 
        chr_sd[cpt]=chr;
        cpt=cpt+1;
        return parse_alt_err( chr );
    }
  #endif
  return true;
}

what i call "NMEA GST raw data" is the content of chr_sd .

it seems to me that the right value for fix.alt_err() is not 7.1 meter but 71meter. the real altitude is 41m. the sigma error is consistent with real error.

if you look at data a bit farther in time then you get :

fix.satellitesfix.altitude()fix.alt_err()NMEA GST
raw data
644.50000009.80"9.8,,00,,"

which is ok !

if you are plotting the fix.alt_err() you will better understand the issue. sometimes the error seems ok and sometimes not.

capture d ecran 2018-04-05 a 00 32 36

SlashDevin commented 6 years ago

There are two things:

  1. Your example data is not valid for a GST sentence. The alt_err field should have a decimal point in it somewhere. I suspect that you have lost a character in this sentence.
  2. There is a subtle bug in the parseFloat routine for these kinds of numbers. It only manifests when a decimal point character is not received. I will check in a fix for that.

But to address the first problem, you need to figure out where that '.' character went. You should probably run NMEAorder.ino to make sure that the correct LAST_SENTENCE has been chosen.

If it is incorrect, your sketch may be trying to print too much information while the GST is still being received (i.e., it is not the GPS quiet time yet). That can cause characters to be dropped because the input buffer overflows.

If you are using the correct LAST_SENTENCE, you might be performing some time-consuming operation (or delay) that takes too long. You might need to break that process up into smaller steps and call gps.available() more frequently. Or, you may need to reorder your main processing loop. Try to coordinate everything with the gps.available() arrival. You could also try the INTERRUPT processing style. This would process the characters in the background, which prevents losing any characters.

Phill76 commented 6 years ago

Hi, To answer your questions, i’ve used the NMEAorder.ino script. Last sentence is correctly chosen. I’m printing information during the quiet time in an sd file on an sdcard. So I’m not sure i have character drop.

I perform processing as soon as the while loop on gps_available is finished.

To be sure i have correctly understood, the subtle bug in parsefloat function could explain that sometimes the information is correctly parsed and sometimes not ? If yes I’m happy that you found it !

SlashDevin commented 6 years ago

the subtle bug in parsefloat function could explain that sometimes the information is correctly parsed and sometimes not ?

Yes, I'm doing some testing now.

But like I said, "71" is not a valid altitude error estimate field. Without the decimal point, it would be interpreted as 7100cm, or 71m. Highly doubtful. All example data that I can find show a floating-point value for this field.

Most importantly, I forgot to mention that you should test fix.valid.alt_err before using it.

I wonder if the GST sentence does not pass the checksum test (because of the dropped character), but the RMC sentence does. This would cause a valid location value (parsed from the RMC sentence), but fix.alt_err could have an invalid value (partially parsed from an invalid GST sentence).

This could also explain a 3-satellite fix with altitude -- suspicious. You might want to review all uses of fix members to make sure you are checking their corresponding validity flags.

Phill76 commented 6 years ago

Ok i will test again with fix.valid.alt_err Thanks

SlashDevin commented 6 years ago

I’m printing information during the quiet time in an sd file

This can cause data loss, because some SD operations can take "too long". I have seen some cards take about 1 second to complete a write, after about 1MB had been written to the log file. This kind of write delay may require you to use the INTERRUPT processing style.

There is some mention of this in the NMEASDlog example program. It also logs the SD write times from the variable lastLoggingTime. You might want to add that to your file, just to see if you're getting caught by a SD card performance issue.

Phill76 commented 6 years ago

Hello i have implemented a test on fix.valid.alt_err before using the alt_err value i've also measured the time between end of gps acquisition (end of while gps.available( gpsPort )) and end of log. it is around 250ms including processing.

and i still got the issue. any news regarding the bug fix ? thanks

SlashDevin commented 6 years ago

Well, I am still skeptical of the "71m" value. The code you used to print the raw data:

bool NMEAGPS::parseGST( char chr )
{
#ifdef NMEAGPS_PARSE_GST
    switch (fieldIndex) {
      case 1: return parseTime( chr );
      case 6: return parse_lat_err( chr );
      case 7: return parse_lon_err( chr );
      case 8: 
        chr_sd[cpt]=chr;
        cpt=cpt+1;
        return parse_alt_err( chr );
    }
  #endif
  return true;
}

... would not print this:

9.8,,00,,

That contains multiple fields, so I'm not sure what that data represents. Try this:

bool NMEAGPS::parseGST( char chr )
{
#ifdef NMEAGPS_PARSE_GST
    switch (fieldIndex) {
      case 1: return parseTime( chr );
      case 6:
        if (chrCount == 0)
          cpt = 0;
        chr_sd[cpt]=chr;
        cpt=cpt+1;
        return parse_lat_err( chr );
      case 7:
        chr_sd[cpt]=chr;
        cpt=cpt+1;
        return parse_lon_err( chr );
      case 8: 
        chr_sd[cpt]=chr;
        cpt=cpt+1;
        return parse_alt_err( chr );
    }
  #endif
  return true;
}

This will show the raw data for all 3 fields. Perhaps you weren't resetting cpt correctly?

I have confirmed that an incorrect checksum will reject all fields from a bad GST sentence. As long are you are testing fix.valid.alt_error before using fix.alt_error(), you should be fine.

Another note is that the NMEA checksum is fairly weak. If two '.' characters are dropped, the checksum will still pass. Actually, dropping any two of the same character, like two '0' characters, would also not be detected. If the raw data for all three fields shows that you are occasionally dropping 2 decimal points from those 3 fields, that might be the real problem.

I would suggest printing gps.statistics.errors to see if some sentences are being rejected. As long as it is always 0, you have a good connection. NMEA.ino prints it as "RX errors", so that would be a good test.

And if you're not getting any RX errors, and the RAW data shows the 3 fields sometimes do not have decimal points, I'll be glad this fixes your problem. Surprised, but glad.

Phill76 commented 6 years ago

i will try what you proposed and i .

i feel "sad" you closed the issue because if you look at the parsed valid data (before i made a modification to log values) there is something non physical :

look at this snap. it talks well !

can you do something ? you talked of a subtle bug . can it explain ?

SlashDevin commented 6 years ago

i feel "sad" you closed the issue

When I check in a fix for an issue, it automatically closes the issue. I do think the "subtle bug" is fixed, so I was ok closing it. You can still add more comments, with or without reopening the issue.

And if the problem is not fixed by the new NMEAGPS.cpp, please "Reopen" this issue. I assume you have that button. If not, I can do it for you.

the error is very small and not consistent with real altitude (decade factor at least)

Remember, the alt_err is a 1-sigma Standard Deviation value. I think this means that 68% of a "random sample" would be at the reported altitude() ± alt_err(). So a STD of 71m is huge. That's why I'm skeptical.

i’m pretty sure a shift in the error somewhere explains that.

I don't know what you mean by "a shift in the error". When the '.' is missing, NeoGPS will produce a large value, perhaps 100 times the other values. This is another reason I think characters are being dropped.

look at this snap. it talks well !

Please don't use email for comments. I can't see the picture, because it was attached to your email. You have to use the website interface to insert a link to your image. Email comments also prevent me from fixing the format of your comments. :-/

Phill76 commented 6 years ago

Hi i'm new to github and i misunderstood the email when i received it saying that the issue was closed. i will test new version. thanks for it.

here is the snapshot i was mentioning in my last exchange issueerrz

Phill76 commented 6 years ago

i've tested new version and unfortunately it does not correct the issue. i have struggled to perform complementary log without disturbing the parser. i've buffered all the GPS information sent by GPS on Serial Link. i've flushed this information during idle time on sd.

here is what i get for one cycle : raw data :

$GPGGA,171209.00,4752.05938,N,00313.63944,E,1,03,16.94,126.5,M,46.1,M,,*6A
$GPGST,171209.00,4.4,,,,33,58,71*50

parsed altitude : 126.5m which is consistent with raw data parsed altitude error: 7.1m whereas raw data indicates 71m (if i'm not wrong) there is the same issue on lat/long error.

what is your view ?

SlashDevin commented 6 years ago

All NMEA sentences include a checksum at the end, to make sure characters weren't lost or corrupted. The checksum is preceeded by an asterisk, so I have edited your comment to put the GPS data in a "code block" (3 grave accents `` before and after the data). The github comment field interprets everything between asterisks a *italics*. <-- I typeditalics`. Now you can see the checksums.

If you copy each line, between the $ and the * into this checksum calculator, it will confirm that the values should be 6A and 50. (Do not paste the $ nor the *.)

However, I do not believe that the GST sentence is correct. It passes the checksum, but the NMEA checksum is not very good at detecting errors. If you paste this sentence (omit the $) into the calculator:

$GPGST,171209.00,4.4,,,,33,5.8,7.1

... you will see that the checksum is STILL 50. The checksum did not detect the addition of 2 identical characters, nor does it detect the loss of 2 identical characters. There appear to be 3 missing decimal points, so I can't explain that.

I am still of the opinion that some characters are being dropped, and that the data loss is not detected by the NMEA checksum.

I would suggest using a simple echo sketch to display some of the raw GPS data in the Serial Monitor window:

void setup()
{
  Serial.begin( 9600 );
  gpsPort.begin( 9600 );
}

void loop()
{
  if (gpsPort.available())
    Serial.write( gpsPort.read() );
}

Look for GST sentences that have decimal points and compare them with GST sentences that do not have decimal points. Use the checksum calculator to verify the checksums. Please show me some of this GST data.

What GPS device are you using? As I have mentioned, I have never seen a GST sentence without decimal points in these error fields.

Phill76 commented 6 years ago

i understand your demonstration. the trouble with printing on serial monitor is that from my desk in my apartment, i don't manage to get a valid localization. so it will be hard for me to provide what you suggest.

for information here is the code i'm using to log the raw gps information. it is similar to what you are proposing :

while (gpsPort.available())
  {
    bool_flush_sd = 0;
    char c = gpsPort.read();

    if (cpt_debug<cpt_debug_max){
      GPS_Sentence_TabChar[cpt_debug]=c;
    }
    cpt_debug = cpt_debug +1;

look at these new samples at two seconds interval :

Curtime= 495.584 NB CHAR recus GPS= 107
$GPGGA,184637.00,4752.07692,N,00313.56170,E,1,04,7.14,29.1,M,46.1,M,,*68
$GPGST,184637.00,54,,,,37,21,10*71

NeoGPS parser decodes information as follows :

altitude : 29.1000003
altitude error : 1.0

Curtime= 496.584 NB CHAR received GPS= 108
$GPGGA,184638.00,4752.07688,N,00313.56142,E,1,04,7.14,29.1,M,46.1,M,,*6D
$GPGST,184638.00,57,,,,36,20,9.9*52

NeoGPS parser decodes information as follows :

altitude : 29.1000003
altitude error : 9.9

--> this means that NEOGPS decodes correctly altitude error when there is a comma and not correctly when there is not any comma.

i think that there is not any character drop because the two raw error values are consistent.

i don't know why sometimes there is not a comma. i'm using two different GPS neo-6M and neo-7M

SlashDevin commented 6 years ago

i think that there is not any character drop because the two raw error values are consistent.

Ok, I'll take another look. Sample data will help!

SlashDevin commented 6 years ago

The example sketch NMEA.ino produces 71m for the raw input "71". When Streamers.cpp has USE_FLOAT uncommented, it prints 71.0, in units of meters. That appears to match what you are expecting.

Did you get the one file for fix #80? You needed to get NMEAGPS.cpp, because I did not do a complete release for the Arduino Library Manager.

I just issued release 4.2.7, which can be downloaded with the Arduino Library Manager. Maybe you just didn't have the latest files.

Phill76 commented 6 years ago

i thought i had latest version of NMEAGPS.cpp but it was not the case. i've updated it with 4.2.7 and now it works :) i need to further test but it sounds ok !