From 939534d580802a7f02f42f22e4f398cd2c6b7047 Mon Sep 17 00:00:00 2001 From: Edgar Bonet Date: Thu, 20 Feb 2020 19:03:13 +0100 Subject: [PATCH 1/2] Correct handling of millis() rollover millis() returns an unsigned long integer, and C++ guarantees that arithmetics on this type work modulo (ULONG_MAX+1). This behavior ensures that computing a delay as millis() - previousMillis yields the correct result across millis() rollover events, as long as the time actually elapsed is less than about 49.7 days. --- .../GPS_HardwareSerial_LOCUS_Status.ino | 8 ++++---- .../GPS_HardwareSerial_Parsing.ino | 2 -- .../GPS_HardwareSerial_Timing.ino | 3 --- examples/GPS_I2C_Parsing/GPS_I2C_Parsing.ino | 2 -- .../GPS_SoftwareSerial_LOCUS_Status.ino | 8 ++++---- .../GPS_SoftwareSerial_Parsing.ino | 3 --- src/Adafruit_GPS.cpp | 9 +++------ src/NMEA_data.cpp | 2 +- 8 files changed, 12 insertions(+), 25 deletions(-) diff --git a/examples/GPS_HardwareSerial_LOCUS_Status/GPS_HardwareSerial_LOCUS_Status.ino b/examples/GPS_HardwareSerial_LOCUS_Status/GPS_HardwareSerial_LOCUS_Status.ino index 49d5f53..75f20af 100644 --- a/examples/GPS_HardwareSerial_LOCUS_Status/GPS_HardwareSerial_LOCUS_Status.ino +++ b/examples/GPS_HardwareSerial_LOCUS_Status/GPS_HardwareSerial_LOCUS_Status.ino @@ -58,7 +58,7 @@ void setup() Serial.println(" no response :("); } -uint32_t updateTime = 1000; +uint32_t timer = 0; void loop() // run over and over again { @@ -67,9 +67,9 @@ void loop() // run over and over again if ((c) && (GPSECHO)) Serial.write(c); - if (millis() > updateTime) + if (millis() - timer > 1000) { - updateTime = millis() + 1000; + timer = millis(); if (GPS.LOCUS_ReadStatus()) { Serial.print("\n\nLog #"); Serial.print(GPS.LOCUS_serial, DEC); @@ -98,5 +98,5 @@ void loop() // run over and over again Serial.print((int)GPS.LOCUS_percent); Serial.print("% Used "); }//if (GPS.LOCUS_ReadStatus()) - }//if (millis() > updateTime) + }//if (millis() - timer > 1000) }//loop diff --git a/examples/GPS_HardwareSerial_Parsing/GPS_HardwareSerial_Parsing.ino b/examples/GPS_HardwareSerial_Parsing/GPS_HardwareSerial_Parsing.ino index bd64607..2d03491 100644 --- a/examples/GPS_HardwareSerial_Parsing/GPS_HardwareSerial_Parsing.ino +++ b/examples/GPS_HardwareSerial_Parsing/GPS_HardwareSerial_Parsing.ino @@ -76,8 +76,6 @@ void loop() // run over and over again if (!GPS.parse(GPS.lastNMEA())) // this also sets the newNMEAreceived() flag to false return; // we can fail to parse a sentence in which case we should just wait for another } - // if millis() or timer wraps around, we'll just reset it - if (timer > millis()) timer = millis(); // approximately every 2 seconds or so, print out the current stats if (millis() - timer > 2000) { diff --git a/examples/GPS_HardwareSerial_Timing/GPS_HardwareSerial_Timing.ino b/examples/GPS_HardwareSerial_Timing/GPS_HardwareSerial_Timing.ino index de57086..98386f6 100644 --- a/examples/GPS_HardwareSerial_Timing/GPS_HardwareSerial_Timing.ino +++ b/examples/GPS_HardwareSerial_Timing/GPS_HardwareSerial_Timing.ino @@ -101,9 +101,6 @@ void loop() // run over and over again return; // we can fail to parse a sentence in which case we should just // wait for another } - // if millis() or timer wraps around, we'll just reset it - if (timer > millis()) - timer = millis(); // approximately every 2 seconds or so, random intervals, print out the // current stats diff --git a/examples/GPS_I2C_Parsing/GPS_I2C_Parsing.ino b/examples/GPS_I2C_Parsing/GPS_I2C_Parsing.ino index 6e251d3..59d5d1d 100644 --- a/examples/GPS_I2C_Parsing/GPS_I2C_Parsing.ino +++ b/examples/GPS_I2C_Parsing/GPS_I2C_Parsing.ino @@ -64,8 +64,6 @@ void loop() // run over and over again if (!GPS.parse(GPS.lastNMEA())) // this also sets the newNMEAreceived() flag to false return; // we can fail to parse a sentence in which case we should just wait for another } - // if millis() or timer wraps around, we'll just reset it - if (timer > millis()) timer = millis(); // approximately every 2 seconds or so, print out the current stats if (millis() - timer > 2000) { diff --git a/examples/GPS_SoftwareSerial_LOCUS_Status/GPS_SoftwareSerial_LOCUS_Status.ino b/examples/GPS_SoftwareSerial_LOCUS_Status/GPS_SoftwareSerial_LOCUS_Status.ino index bcae4e4..55bba50 100644 --- a/examples/GPS_SoftwareSerial_LOCUS_Status/GPS_SoftwareSerial_LOCUS_Status.ino +++ b/examples/GPS_SoftwareSerial_LOCUS_Status/GPS_SoftwareSerial_LOCUS_Status.ino @@ -68,7 +68,7 @@ void setup() } } -uint32_t updateTime = 1000; +uint32_t timer = 0; void loop() // run over and over again { @@ -77,9 +77,9 @@ void loop() // run over and over again if ((c) && (GPSECHO)) Serial.write(c); - if (millis() > updateTime) + if (millis() - timer > 1000) { - updateTime = millis() + 1000; + timer = millis(); if (GPS.LOCUS_ReadStatus()) { Serial.print("\n\nLog #"); Serial.print(GPS.LOCUS_serial, DEC); @@ -108,7 +108,7 @@ void loop() // run over and over again Serial.print((int)GPS.LOCUS_percent); Serial.print("% Used "); }//if (GPS.LOCUS_ReadStatus()) - }//if (millis() > updateTime) + }//if (millis() - timer > 1000) }//loop diff --git a/examples/GPS_SoftwareSerial_Parsing/GPS_SoftwareSerial_Parsing.ino b/examples/GPS_SoftwareSerial_Parsing/GPS_SoftwareSerial_Parsing.ino index feb8d30..b4ebc85 100644 --- a/examples/GPS_SoftwareSerial_Parsing/GPS_SoftwareSerial_Parsing.ino +++ b/examples/GPS_SoftwareSerial_Parsing/GPS_SoftwareSerial_Parsing.ino @@ -78,9 +78,6 @@ void loop() // run over and over again return; // we can fail to parse a sentence in which case we should just wait for another } - // if millis() or timer wraps around, we'll just reset it - if (timer > millis()) timer = millis(); - // approximately every 2 seconds or so, print out the current stats if (millis() - timer > 2000) { timer = millis(); // reset the timer diff --git a/src/Adafruit_GPS.cpp b/src/Adafruit_GPS.cpp index 2d8f25a..5563c4e 100644 --- a/src/Adafruit_GPS.cpp +++ b/src/Adafruit_GPS.cpp @@ -637,8 +637,7 @@ bool Adafruit_GPS::wakeup(void) { /**************************************************************************/ /*! - @brief Time in seconds since the last position fix was obtained. Will - fail by rolling over to zero after one millis() cycle, about 6-1/2 weeks. + @brief Time in seconds since the last position fix was obtained. @return nmea_float_t value in seconds since last fix. */ /**************************************************************************/ @@ -648,8 +647,7 @@ nmea_float_t Adafruit_GPS::secondsSinceFix() { /**************************************************************************/ /*! - @brief Time in seconds since the last GPS time was obtained. Will fail - by rolling over to zero after one millis() cycle, about 6-1/2 weeks. + @brief Time in seconds since the last GPS time was obtained. @return nmea_float_t value in seconds since last GPS time. */ /**************************************************************************/ @@ -659,8 +657,7 @@ nmea_float_t Adafruit_GPS::secondsSinceTime() { /**************************************************************************/ /*! - @brief Time in seconds since the last GPS date was obtained. Will fail - by rolling over to zero after one millis() cycle, about 6-1/2 weeks. + @brief Time in seconds since the last GPS date was obtained. @return nmea_float_t value in seconds since last GPS date. */ /**************************************************************************/ diff --git a/src/NMEA_data.cpp b/src/NMEA_data.cpp index 72a44ef..008e9d2 100644 --- a/src/NMEA_data.cpp +++ b/src/NMEA_data.cpp @@ -60,7 +60,7 @@ void Adafruit_GPS::newDataValue(nmea_index_t idx, nmea_float_t v) { // weighting factor for smoothing depends on delta t / tau nmea_float_t w = min((nmea_float_t)1.0, - ((nmea_float_t)millis() - val[idx].lastUpdate) / val[idx].response); + (nmea_float_t)(millis() - val[idx].lastUpdate) / val[idx].response); // default smoothing val[idx].smoothed = (1.0 - w) * val[idx].smoothed + w * v; // special smoothing for some angle types From 76526b0b151a488561a3cd45cce912f1caf865eb Mon Sep 17 00:00:00 2001 From: Edgar Bonet Date: Fri, 21 Feb 2020 21:04:56 +0100 Subject: [PATCH 2/2] Document time limits of secondsSince*() The values returned by Adafruit_GPS::secondsSince{Fix,Time,Date}() wrap around if larger than 2^32 ms. --- src/Adafruit_GPS.cpp | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/Adafruit_GPS.cpp b/src/Adafruit_GPS.cpp index 5563c4e..ff44207 100644 --- a/src/Adafruit_GPS.cpp +++ b/src/Adafruit_GPS.cpp @@ -637,7 +637,10 @@ bool Adafruit_GPS::wakeup(void) { /**************************************************************************/ /*! - @brief Time in seconds since the last position fix was obtained. + @brief Time in seconds since the last position fix was obtained. The + time returned is limited to 2^32 milliseconds, which is about 49.7 days. + It will wrap around to zero if no position fix is received + for this long. @return nmea_float_t value in seconds since last fix. */ /**************************************************************************/ @@ -647,7 +650,9 @@ nmea_float_t Adafruit_GPS::secondsSinceFix() { /**************************************************************************/ /*! - @brief Time in seconds since the last GPS time was obtained. + @brief Time in seconds since the last GPS time was obtained. The time + returned is limited to 2^32 milliseconds, which is about 49.7 days. It + will wrap around to zero if no GPS time is received for this long. @return nmea_float_t value in seconds since last GPS time. */ /**************************************************************************/ @@ -657,7 +662,9 @@ nmea_float_t Adafruit_GPS::secondsSinceTime() { /**************************************************************************/ /*! - @brief Time in seconds since the last GPS date was obtained. + @brief Time in seconds since the last GPS date was obtained. The time + returned is limited to 2^32 milliseconds, which is about 49.7 days. It + will wrap around to zero if no GPS date is received for this long. @return nmea_float_t value in seconds since last GPS date. */ /**************************************************************************/