GPS: set the clock only from the RMC sentence that was validated

.github/workflows/run-unit-tests.yml

@@ -20,7 +20,7 @@ jobs:
         uses: ./.github/actions/setup-build-environment
 
       - name: Run Unit Tests
-        run: pio test -e native -e native_kiss_modem -vv
+        run: pio test -e native -e native_kiss_modem -e native_gps -vv
 
       - name: Upload Test Results
         # Upload test results even if the test step failed.

platformio.ini

@@ -167,7 +167,7 @@ build_flags = -std=c++17
   -I src
   -I test/mocks
 test_build_src = yes
-test_ignore = test_kiss_modem
+test_ignore = test_kiss_modem, test_gps_nmea
 build_src_filter =
   -<*>
   +<../src/Utils.cpp>
@@ -191,3 +191,26 @@ build_src_filter =
   +<../examples/kiss_modem/KissModem.cpp>
 lib_deps =
   google/googletest @ 1.17.0
+
+; GPS/NMEA provider tests compile the real provider header against the real
+; MicroNMEA library. The mocks must shadow src/ (same ordering as the KISS
+; modem suite). MicroNMEA is an Arduino-format library, so compatibility
+; checks are off to let it build for the native platform. Sanitizers make
+; out-of-bounds reads in the parser fail the test instead of passing silently.
+[env:native_gps]
+platform = native
+test_framework = googletest
+build_flags = -std=c++17
+  -I test/mocks
+  -I src
+  -fsanitize=address,undefined
+  -fno-sanitize-recover=all
+  -fno-omit-frame-pointer
+test_build_src = yes
+test_filter = test_gps_nmea
+build_src_filter =
+  -<*>
+lib_compat_mode = off
+lib_deps =
+  google/googletest @ 1.17.0
+  stevemarple/MicroNMEA @ 2.0.6

src/helpers/sensors/MicroNMEALocationProvider.h

@@ -48,8 +48,17 @@ class MicroNMEALocationProvider : public LocationProvider {
     int _pin_reset;
     int _pin_en;
     unsigned long next_check = 0;
-    long time_valid = 0;
     unsigned long _last_time_sync = 0;
+    // The clock is set only from the time and date text of an RMC sentence
+    // that passed its checksum, once consecutive sentences agree with
+    // millis(). MicroNMEA's own date/time fields can still hold values from
+    // an earlier sentence when the latest one is malformed.
+    uint32_t _clock_sample = 0;      // UTC seconds from the last accepted RMC
+    uint32_t _clock_sample_ms = 0;   // millis() when that second was first seen
+    uint8_t _clock_samples = 0;      // consecutive samples that agreed
+    static const uint8_t CLOCK_SAMPLES_REQUIRED = 3;
+    static const uint32_t CLOCK_SAMPLE_MAX_AGE_MS = 2000;
+    static const int32_t CLOCK_SAMPLE_TOLERANCE_S = 2;
     static const unsigned long TIME_SYNC_INTERVAL = 1800000; // Re-sync every 30 minutes
 
 #ifdef GPS_L76K
@@ -88,6 +97,80 @@ class MicroNMEALocationProvider : public LocationProvider {
         }
     }
 
+    static bool isDigits(const char* s, int n) {
+        for (int i = 0; i < n; i++) {
+            if (s[i] < '0' || s[i] > '9') return false;
+        }
+        return true;
+    }
+
+    static uint8_t twoDigits(const char* s) { return (s[0] - '0') * 10 + (s[1] - '0'); }
+
+    static uint8_t daysInMonth(uint16_t year, uint8_t month) {
+        static const uint8_t days[] = { 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31 };
+        bool leap = (year % 4 == 0 && year % 100 != 0) || year % 400 == 0;
+        return month == 2 && leap ? 29 : days[month - 1];
+    }
+
+    // UTC seconds from the time and date fields of an RMC sentence, or 0 when
+    // the sentence does not carry a complete time, an 'A' (valid) status and a
+    // complete date. The checksum covers only what comes before the first
+    // '*', and the date is the last field used, so it must end before it.
+    static uint32_t rmcEpoch(const char* sentence) {
+        const char* star = strchr(sentence, '*');
+        const char* time = strchr(sentence, ',');
+        if (star == NULL || time == NULL) return 0;
+        time++;
+        const char* time_end = strchr(time, ',');
+        if (time_end == NULL || !isDigits(time, 6)) return 0;  // hhmmss
+        if (time_end - time > 6                                 // optional .s+
+            && (time[6] != '.' || time_end - time == 7 || !isDigits(time + 7, time_end - time - 7))) return 0;
+        const char* status = time_end + 1;
+        if (status[0] != 'A' || status[1] != ',') return 0;
+        const char* date = status + 2;
+        for (int field = 3; field <= 8; field++) {  // latitude .. course
+            date = strchr(date, ',');
+            if (date == NULL) return 0;
+            date++;
+        }
+        if (date + 6 > star || !isDigits(date, 6) || (date[6] != ',' && date[6] != '*')) return 0;  // ddmmyy
+
+        const uint8_t hour = twoDigits(time), minute = twoDigits(time + 2), second = twoDigits(time + 4);
+        const uint8_t day = twoDigits(date), month = twoDigits(date + 2);
+        const uint16_t year = 2000 + twoDigits(date + 4);
+        if (hour > 23 || minute > 59 || second > 59 || month < 1 || month > 12
+            || day < 1 || day > daysInMonth(year, month)) return 0;
+        return DateTime(year, month, day, hour, minute, second).unixtime();
+    }
+
+    // Count consecutive RMC samples whose seconds advance with millis(). A
+    // sentence without a usable time, or a repeat of the same second
+    // (receivers faster than 1 Hz), is ignored. A sample that disagrees by
+    // more than the tolerance (loop stalls, lost sentences) starts a new run.
+    void noteClockSample(uint32_t epoch, uint32_t now) {
+        if (epoch == 0) return;
+        if (_clock_samples > 0) {
+            const int32_t gps_s = (int32_t)(epoch - _clock_sample);
+            const int32_t wall_s = (int32_t)((now - _clock_sample_ms) / 1000);
+            const int32_t drift = gps_s - wall_s;
+            if (gps_s >= 0 && drift >= -CLOCK_SAMPLE_TOLERANCE_S && drift <= CLOCK_SAMPLE_TOLERANCE_S) {
+                if (gps_s == 0) return;
+                if (_clock_samples < CLOCK_SAMPLES_REQUIRED) _clock_samples++;
+                _clock_sample = epoch;
+                _clock_sample_ms = now;
+                return;
+            }
+        }
+        _clock_samples = 1;
+        _clock_sample = epoch;
+        _clock_sample_ms = now;
+    }
+
+    bool clockSampleReady() const {
+        return _clock_samples >= CLOCK_SAMPLES_REQUIRED
+            && (uint32_t)(millis() - _clock_sample_ms) <= CLOCK_SAMPLE_MAX_AGE_MS;
+    }
+
 public :
     MicroNMEALocationProvider(Stream& ser, mesh::RTCClock* clock = NULL, int pin_reset = GPS_RESET, int pin_en = GPS_EN,RefCountedDigitalPin* peripher_power=NULL) :
     nmea(_nmeaBuffer, sizeof(_nmeaBuffer)), _clock(clock), _gps_serial(&ser), _peripher_power(peripher_power), _pin_reset(pin_reset), _pin_en(pin_en) {
@@ -157,7 +240,11 @@ public :
         }
     }
 
-    void syncTime() override { nmea.clear(); LocationProvider::syncTime(); }
+    void syncTime() override {
+        nmea.clear();
+        _clock_samples = 0;
+        LocationProvider::syncTime();
+    }
     long getLatitude() override { return nmea.getLatitude(); }
     long getLongitude() override { return nmea.getLongitude(); }
     long getAltitude() override { 
@@ -228,30 +315,28 @@ public :
                         _last_fix_ms = millis();
                         _fix_seen = true;
                     }
+                    if (strcmp(message_id, "RMC") == 0) {
+                        noteClockSample(rmcEpoch(nmea.getSentence()), millis());
+                    }
                 } else {
                     _nmea_checksum_bad++;
                 }
             }
         }
 
-        if (!isValid()) time_valid = 0;
-
         if ((long)(millis() - next_check) > 0) {
             next_check = millis() + 1000;
             // Re-enable time sync periodically when GPS has valid fix
             if (!_time_sync_needed && _clock != NULL && (millis() - _last_time_sync) > TIME_SYNC_INTERVAL) {
                 _time_sync_needed = true;
             }
-            if (_time_sync_needed && time_valid > 2) {
+            if (_time_sync_needed && clockSampleReady()) {
                 if (_clock != NULL) {
-                    _clock->setCurrentTime(getTimestamp());
+                    _clock->setCurrentTime(_clock_sample);
                     _time_sync_needed = false;
                     _last_time_sync = millis();
                 }
             }
-            if (isValid()) {
-                time_valid ++;
-            }
         }
     }
 

test/mocks/Arduino.h

@@ -2,6 +2,7 @@
 
 #include <cstdint>
 #include <cmath>
+#include <ctype.h>  // Arduino.h provides the C character classification functions
 #include "Stream.h"
 
 inline uint32_t g_mock_millis = 0;
@@ -15,3 +16,26 @@ inline uint32_t millis() {
 inline void delay(uint32_t ms) {
   g_mock_millis += ms;
 }
+
+// Busy-wait loops call yield(); let mock time move so they can time out.
+inline void yield() {
+  g_mock_millis += 1;
+}
+
+// Minimal digital I/O so headers that drive enable/reset pins compile natively.
+constexpr uint8_t LOW = 0;
+constexpr uint8_t HIGH = 1;
+constexpr uint8_t INPUT = 0;
+constexpr uint8_t OUTPUT = 1;
+
+inline uint8_t g_mock_pin_levels[64] = {};
+
+inline void pinMode(uint8_t, uint8_t) {}
+
+inline void digitalWrite(uint8_t pin, uint8_t level) {
+  if (pin < 64) g_mock_pin_levels[pin] = level;
+}
+
+inline int digitalRead(uint8_t pin) {
+  return pin < 64 ? g_mock_pin_levels[pin] : LOW;
+}

test/mocks/Mesh.h

@@ -15,6 +15,13 @@ class Radio {
   virtual int16_t getNoiseFloor() { return -120; }
 };
 
+class RTCClock {
+public:
+  virtual ~RTCClock() = default;
+  virtual uint32_t getCurrentTime() = 0;
+  virtual void setCurrentTime(uint32_t time) = 0;
+};
+
 class MainBoard {
 public:
   virtual ~MainBoard() = default;

test/mocks/RTClib.h

@@ -0,0 +1,39 @@
+#pragma once
+
+#include <stdint.h>
+
+// Minimal DateTime stand-in for native tests. unixtime() matches RTClib for
+// valid dates from 2000 to 2099, the range RTClib supports (seconds since
+// 1970-01-01 UTC). Out-of-range fields map to a deterministic value instead of
+// indexing past the month table, so tests can compare two parses of the same
+// bytes; such values are not what RTClib would return.
+class DateTime {
+  uint16_t _year;
+  uint8_t _month, _day, _hour, _minute, _second;
+
+  static bool leap(uint16_t year) {
+    return year % 4 == 0 && (year % 100 != 0 || year % 400 == 0);
+  }
+
+public:
+  DateTime(uint16_t year, uint8_t month, uint8_t day, uint8_t hour,
+           uint8_t minute, uint8_t second)
+      : _year(year), _month(month), _day(day), _hour(hour),
+        _minute(minute), _second(second) {}
+
+  uint32_t unixtime() const {
+    uint32_t days = 0;
+    for (uint16_t year = 1970; year < _year; ++year)
+      days += leap(year) ? 366 : 365;
+    if (_month >= 1 && _month <= 12) {
+      const uint8_t month_days[] = {31, 28, 31, 30, 31, 30,
+                                    31, 31, 30, 31, 30, 31};
+      for (uint8_t month = 1; month < _month; ++month)
+        days += month_days[month - 1] + (month == 2 && leap(_year) ? 1 : 0);
+      days += _day - 1;
+    } else {
+      days += 31UL * _month + _day;  // deterministic, not calendar-correct
+    }
+    return days * 86400UL + _hour * 3600UL + _minute * 60UL + _second;
+  }
+};

test/test_gps_nmea/gps_test_support.h

@@ -0,0 +1,49 @@
+#pragma once
+
+// Shared helpers for the native GPS/NMEA tests: the real
+// MicroNMEALocationProvider and MicroNMEA, fed through a mock UART.
+
+#include <gtest/gtest.h>
+
+#include <MicroNMEA.h>
+#include <helpers/sensors/MicroNMEALocationProvider.h>
+
+#include <cstdio>
+#include <cstring>
+#include <deque>
+#include <string>
+
+
+struct MockSerial : Stream {
+  std::deque<char> queue;
+  int available() override { return (int)queue.size(); }
+  int read() override {
+    if (queue.empty()) return -1;
+    char c = queue.front();
+    queue.pop_front();
+    return (unsigned char)c;
+  }
+  void feed(const std::string& text) {
+    for (char c : text) queue.push_back(c);
+  }
+};
+
+// "$" + body + "*" + two hex digits, the checksum MicroNMEA expects.
+inline std::string withChecksum(const std::string& body) {
+  uint8_t sum = 0;
+  for (char c : body) sum ^= (uint8_t)c;
+  char hex[4];
+  snprintf(hex, sizeof(hex), "%02X", sum);
+  return "$" + body + "*" + hex;
+}
+
+inline void feedAndLoop(MicroNMEALocationProvider& gps, MockSerial& serial,
+                 const std::string& line) {
+  serial.feed(line + "\r\n");
+  gps.loop();
+}
+
+inline constexpr const char* kGoodRmc =
+    "GPRMC,120034.00,A,4716.1234,N,12233.5678,W,0.12,34.5,171026,003.1,W,A";
+inline constexpr const char* kGoodGga =
+    "GPGGA,120034.00,4716.1234,N,12233.5678,W,1,08,0.9,125.0,M,-17.0,M,,";