mikalhart / TinyGPSPlus

A new, customizable Arduino NMEA parsing library
http://arduiniana.org
1.08k stars 491 forks source link

Handling of GPS week number rollover for older GPS modules #75

Open dl9sec opened 4 years ago

dl9sec commented 4 years ago

I am using this library at the Elektor ESP32 NTP server project (https://github.com/ElektorLabs/180662-mini-NTP-ESP32) together with a Perthold ST22 GPS module (Skytraq Venus 6 based). Unfortunately the module suffers from the GPS week number rollover (https://en.wikipedia.org/wiki/GPS_Week_Number_Rollover), so it shows 14.10.2000 instead of 30.05.2020. The solution seems to be easy: the given date from the GPRMC sentence must be added 1024 weeks (or 1024 multiplied by 7 days) to get the correct date. It would be great to have a configuration define (e.g. #define WK_ROLLOVER_MUL 1) with a multpilier for 1024 weeks which is then added to the date from the NMEA sentence. So for new GPS modules 1024 multiplied by 0 weeks are added, for the last generation of GPS modules 1024 multiplied by 1 weeks are added and for the next rollover in 2038 a number of 1024 multiplied by 2 could be added.

There are already a lot of forks, maybe someone took already care of it. I will try to find a coded solution, but maybe someone is faster than me with an efficient solution... :-)

Regards, Thorsten

CaptianNemo commented 3 years ago

I tweaked the code a little bit, and as of 10/29/2020 it's working correctly.

Change line 371 of file "TinyGPS++.cpp" to "date = newDate + 140719;"

TD-er commented 3 years ago

Isn't it a bit more elegant to check the reported year and if that's before the build time, then add a multiple of 1024 weeks to set the date past the build time?

So a quick fix for now could be to check if the year is < 2020 and then add it. For 2038 we probably have more things to fix as the (32 bit signed) Unix timestamp then also overflows.

dl9sec commented 3 years ago

In any case the correction should be configurable, because not all GPS modules have the rollover problem...

TD-er commented 3 years ago

In any case the correction should be configurable, because not all GPS modules have the rollover problem...

That's why I suggest to look at the returned data from the GPS. If that's a date before the current compile date then you know the unit has this specific issue. Since we now are at the "start" of this 1024 weeks period, all code now compiled should report a year of at least 2020.

dl9sec commented 3 years ago

That's why I suggest to look at the returned data from the GPS.

Ah ok, that wasn't clear for me. But in this case that would make sense...

CaptianNemo commented 3 years ago

What do you think about this?

void TinyGPSDate::commit() 
{ 
  if (newDate % 100 < 20) {
    //check if GPS is using old, and rolled over the date. 
    date = newDate + 140719; 
  } else {
    //if GPS isn't old and did not roll over, use normal value. 
    date = newDate; 
  }
  lastCommitTime = millis(); 
  valid = updated = true;
}

Edit: Github destroyed my formatting. Edit2: Fixed the formatting.

TD-er commented 3 years ago

For the formatting, use 3x a backtick (left to the "1" on your keyboard) This is a code block marker for both the start and end.

if you add a language next to the first 3 backticks you also get syntax highlighting, So your code then looks like this: (I added scope braces)

void TinyGPSDate::commit() 
{ 
  if (newDate % 100 < 20) {
    //check if GPS is using old, and rolled over the date. 
    date = newDate + 140719; 
  } else {
    //if GPS isn't old and did not roll over, use normal value. 
    date = newDate; 
  }
  lastCommitTime = millis(); 
  valid = updated = true;
}

To make it more readable, I would not use a "magic number" like 140719, but 1024 * 7

CaptianNemo commented 3 years ago

For the formatting, use 3x a backtick (left to the "1" on your keyboard) This is a code block marker for both the start and end.

if you add a language next to the first 3 backticks you also get syntax highlighting, So your code then looks like this: (I added scope braces)

Thank you. I'll edit my post again.

To make it more readable, I would not use a "magic number" like 140719, but 1024 * 7

The "magic number" is the only thing I could get to work. "1024 * 7" doesn't work, and every permutation I could think of doesn't work either.

TD-er commented 3 years ago

Hmm, that's even more strange. Then I really would like to know what this value represents. Or is it a specific notation, where you have a day/month/year notation? If so, then I expect this will fail after some time in the future as you will end up with some notation like days > 31, months > 12 etc.

CaptianNemo commented 3 years ago

It's day month year. Correctly formatted, "the 31st of October 2020", would be represented as "311020".

TD-er commented 3 years ago

OK, then your fix to add this number will result in a number that isn't a valid date representation. Maybe it works now, but it won't work on all dates.

TD-er commented 3 years ago

Just played around a bit with the code (including some sanity check for the date received)

void TinyGPSDate::commit()
{
   const uint32_t olddate = date;
   date = newDate;
   valid = false;

   {
     const uint8_t newday = day();
     if (newday > 31 || newday == 0) {
       // Day of month
       date = olddate;
       return;
     }
   }
   {
     const uint8_t newmonth = month();
     if (newmonth > 12 || newmonth == 0) {
       date = olddate;
       return;
     }
   }
   {
     const uint16_t newyear = year();
     if (newyear < 2020) {
       // Looks like it is an old GPS unit not working well with the 1024 week rollover.
       struct tm t;
       t.tm_year=newyear-1900;
       t.tm_mon=month()-1;
       t.tm_mday=day();
       t.tm_mday += (7 * 1024);
       mktime(&t);

       date = t.tm_year + 1900;
       date += t.tm_mon * 100;
       date += t.tm_mday * 10000;
     }
   }
   lastCommitTime = millis();
   valid = true;
   updated = true;
}

Since you've apparently got one of those units with issues regarding the 1024 week rollover, can you test it?

CaptianNemo commented 3 years ago

\Documents\Arduino\libraries\TinyGPSPlus-1.0.2b\src\TinyGPS++.cpp: In member function 'void TinyGPSDate::commit()': \Documents\Arduino\libraries\TinyGPSPlus-1.0.2b\src\TinyGPS++.cpp:391:18: error: aggregate 'tm t' has incomplete type and cannot be defined 391 | struct tm t; | ^ \Documents\Arduino\libraries\TinyGPSPlus-1.0.2b\src\TinyGPS++.cpp:396:8: error: 'mktime' was not declared in this scope; did you mean 'mktemp'? 396 | mktime(&t); | ^~ | mktemp exit status 1

CaptianNemo commented 3 years ago

Added #include to includes.

DATE Fix Age=1ms Raw=114045 Year=2045 Month=40 Day=11

CaptianNemo commented 3 years ago

I think I fixed it:

#include <time.h>
void TinyGPSDate::commit()
{
   const uint32_t olddate = date;
   date = newDate;
   valid = false;

   {
     const uint16_t newyear = year();
     if (newyear < 2020) {
       // Looks like it is an old GPS unit not working well with the 1024 week rollover.
       struct tm tm;
       tm.tm_year=(date % 100) + 100;
       tm.tm_mon=(date / 100) % 100;
       tm.tm_mday= date / 10000;
       tm.tm_sec=0;
       tm.tm_min=0;
       tm.tm_hour=0;
       tm.tm_mday += 7137;
       mktime(&tm);
       date = tm.tm_year;
       date += tm.tm_mon * 100;
       date += tm.tm_mday * 10000;
     }
   }
   lastCommitTime = millis();
   valid = true;
   updated = true;
}
TD-er commented 3 years ago

I don't understand the tm.tm_mday += 7137; Are you sure the months are OK? (months start counting at 0 in struct tm) (7*1024) - 7137 = 31 days. So I guess there is some offset of 1 month somewhere.

tm.tm_mon=(date / 100) % 100; and date += tm.tm_mon * 100; should have an offset of 1 month.

It should probably be something like this: tm.tm_mon=((date / 100) % 100) - 1; and date += (tm.tm_mon + 1) * 100;

Can you print out the received date of today for that GPS, so I can also test it here?

Edit: I noticed there is also a bug in the years (also in my suggested code).

struct tm counts from year 1900. So we should also correct for it when creating the new date value.

date = tm.tm_year % 100;  // (+1900 - 2000) % 100, which can be simplified to %100
CaptianNemo commented 3 years ago

Taken a few minutes ago:

DATE Fix Age=1ms Raw=21120 Year=2020 Month=11 Day=2 TIME Fix Age=3ms Raw=2053070 Hour=2 Minute=5 Second=30 Hundredths=70

Raw passthrough: $GPRMC,777777.7,A,4777.777777,N,07777.777777,W,0.0,,190301,,,A*56

I changed the location data for privacy.

TD-er commented 3 years ago

And what correction code did you use for this?

CaptianNemo commented 3 years ago

This:

#include <time.h>
void TinyGPSDate::commit()
{
   const uint32_t olddate = date;
   date = newDate;
   valid = false;

   {
     const uint16_t newyear = year();
     if (newyear < 2020) {
       // Looks like it is an old GPS unit not working well with the 1024 week rollover.
       struct tm tm;
       tm.tm_year=(date % 100) + 100;
       tm.tm_mon=(date / 100) % 100;
       tm.tm_mday= date / 10000;
       tm.tm_sec=0;
       tm.tm_min=0;
       tm.tm_hour=0;
       tm.tm_mday += 7137;
       mktime(&tm);
       date = tm.tm_year;
       date += tm.tm_mon * 100;
       date += tm.tm_mday * 10000;
     }
   }
   lastCommitTime = millis();
   valid = true;
   updated = true;
}
TD-er commented 3 years ago

It should be something like this (still have to test it myself)

void TinyGPSDate::commit()
{
   const uint32_t olddate = date;
   date = newDate;
   valid = false;

   {
     const uint16_t newyear = year();
     if (newyear < 2020) {
       // Looks like it is an old GPS unit not working well with the 1024 week rollover.
       struct tm tm;
       tm.tm_year=(date % 100) + 100;
       tm.tm_mon=((date / 100) % 100) - 1;
       tm.tm_mday= date / 10000;
       tm.tm_sec=0;
       tm.tm_min=0;
       tm.tm_hour=0;
       tm.tm_mday += 7*1024;
       mktime(&tm);
       date = tm.tm_year;
       date += (tm.tm_mon + 1) * 100;
       date += tm.tm_mday * 10000;
     }
   }
   lastCommitTime = millis();
   valid = true;
   updated = true;
}

Like I said (and linked), the months start counting at 0 in struct tm and the week offset you use simply cannot be correct as it is 7*1024 - 31

TD-er commented 3 years ago

See also this date calculator

From maandag 19 maart 2001 Added 1024 weeks Normalized to 7168 days Result: maandag 2 november 2020