Merge pull request #194 from ladyada-eagleclaw/gps-test-matrix-coverage

Run GPS regressions in all four build configurations

extras/hw_tests/gnss_receive/README.md

@@ -0,0 +1,47 @@
+# GNSS reception test
+
+This engineering sketch exercises the real library receive and parse paths.
+It echoes complete raw sentences and reports byte, valid-frame, checksum-error,
+format-error, and parsed-sentence counts every two seconds. Unsupported sentence
+types count as valid frames but do not increase the parsed count.
+
+No receiver commands are sent. A position fix is reported when available and
+is not required to establish communication. `fix` is true only when the latest
+fix status is valid and less than five seconds old; coordinates use that same
+check, including for RMC/GLL-only output. `ggaQuality` reports the last GGA fix
+quality separately and may be stale when GGA sentences stop arriving. Keep raw
+captures private if they contain location information.
+
+| Fixture | Build selection | Receive wiring |
+| --- | --- | --- |
+| Classic Nano V3 + PA6H | Default AVR build | GPS TX to D8; D7 disconnected; 9600 baud |
+| Classic Nano V3 + SAM-M8Q | Default AVR build | GPS TX to D8; D7 disconnected; 9600 baud |
+| Classic Nano V3 + PA1010D | Define `GNSS_TEST_I2C` | SDA A4, SCL A5, address 0x10 |
+| HILBERT ESP32-S3 | ESP32 build | LC29H(EA) GPIO8 at 460800; ATGM336H GPIO38 at 115200 |
+
+The Nano uses its own USB for upload and serial monitoring while seated in
+Jumperless. Leave Jumperless UART_TX/D0 and UART_RX/D1 disconnected. SoftwareSerial
+TX D7 stays physically disconnected, and the sketch disables the D8 input
+pull-up so it does not pull a 3.3 V receiver output toward the Nano's 5 V rail.
+
+In the established Jumperless fixture, route PA6H TX row 5 **or** SAM-M8Q TX row
+17 to D8, never both. PA1010D SDA is row 24 to A4 and SCL is row 25 to A5. Retain
+the existing physical power and ground jumpers; no power routing is changed by
+this sketch.
+
+Build from the library checkout with `--library .` and one of these selections:
+
+- Nano: `--fqbn arduino:avr:nano:cpu=atmega328`.
+- Nano I2C: the Nano target plus
+  `--build-property compiler.cpp.extra_flags=-DGNSS_TEST_I2C`.
+- HILBERT:
+  `--fqbn esp32:esp32:esp32s3:USBMode=hwcdc,CDCOnBoot=cdc,FlashSize=4M,FlashMode=qio,PSRAM=enabled`.
+
+Read the serial monitor at 115200 baud. Verify that valid and parsed counts
+continue increasing. Record any startup errors separately and compare error
+counts during steady reception; do not hide increasing errors. HILBERT reports
+both receivers independently. A missing fix is different from missing UART
+or I2C traffic.
+
+These hardware tests are compiled and run explicitly. The host regression
+runner exercises synthetic parser tests without connected boards.

extras/hw_tests/gnss_receive/gnss_receive.ino

@@ -0,0 +1,141 @@
+// Engineering reception test. No commands are sent to any GNSS module.
+// Classic Nano: default SoftwareSerial RX D8, TX D7 left disconnected.
+// Define GNSS_TEST_I2C for PA1010D at 0x10 on A4/A5 instead.
+// HILBERT ESP32-S3: LC29H(EA) GPIO8/460800 and ATGM336H GPIO38/115200.
+// Keep receiver power wiring unchanged. A position fix is reported, not required.
+#include <Adafruit_GPS.h>
+
+struct ReceiveCounts {
+  uint32_t bytes = 0;
+  uint32_t valid = 0;
+  uint32_t checksumErrors = 0;
+  uint32_t formatErrors = 0;
+  uint32_t parsed = 0;
+};
+
+#if defined(ARDUINO_ARCH_ESP32)
+HardwareSerial firstPort(1), secondPort(2);
+Adafruit_GPS firstGPS(&firstPort), secondGPS(&secondPort);
+ReceiveCounts firstCounts, secondCounts;
+#elif defined(GNSS_TEST_I2C)
+Adafruit_GPS firstGPS(&Wire);
+ReceiveCounts firstCounts;
+#else
+SoftwareSerial firstPort(8, 7);
+Adafruit_GPS firstGPS(&firstPort);
+ReceiveCounts firstCounts;
+#endif
+
+uint32_t lastReport = 0;
+bool receiverReady = false;
+void receiveGPS(Adafruit_GPS &gps, ReceiveCounts &counts,
+                const __FlashStringHelper *name);
+void reportGPS(Adafruit_GPS &gps, ReceiveCounts &counts,
+               const __FlashStringHelper *name);
+
+void setup() {
+  Serial.begin(115200);
+  while (!Serial && millis() < 3000) {
+    delay(10);
+  }
+  delay(250);
+  Serial.println(F("Adafruit GNSS receive regression"));
+#if defined(ARDUINO_ARCH_ESP32)
+  firstPort.setRxBufferSize(4096);
+  secondPort.setRxBufferSize(4096);
+  if (!firstPort.setPins(8, -1) || !secondPort.setPins(38, -1) ||
+      !firstGPS.begin(460800) || !secondGPS.begin(115200)) {
+    Serial.println(F("FAIL: HILBERT receive UART initialization"));
+    return;
+  }
+#elif defined(GNSS_TEST_I2C)
+  if (!firstGPS.begin(0x10)) {
+    Serial.println(F("FAIL: PA1010D did not acknowledge I2C address 0x10"));
+    return;
+  }
+#else
+  firstGPS.begin(9600);
+  // The module drives this line at 3.3 V; disable the Nano's 5 V pull-up.
+  pinMode(8, INPUT);
+#endif
+  receiverReady = true;
+  Serial.println(F("Receive interface initialized; waiting for NMEA"));
+}
+
+void loop() {
+  if (!receiverReady) {
+    delay(1000);
+    return;
+  }
+#if defined(ARDUINO_ARCH_ESP32)
+  receiveGPS(firstGPS, firstCounts, F("LC29H"));
+  receiveGPS(secondGPS, secondCounts, F("ATGM"));
+#else
+  receiveGPS(firstGPS, firstCounts, F("NANO"));
+#endif
+  if (millis() - lastReport >= 2000) {
+    lastReport = millis();
+#if defined(ARDUINO_ARCH_ESP32)
+    reportGPS(firstGPS, firstCounts, F("LC29H"));
+    reportGPS(secondGPS, secondCounts, F("ATGM"));
+#else
+    reportGPS(firstGPS, firstCounts, F("NANO"));
+#endif
+  }
+}
+
+void receiveGPS(Adafruit_GPS &gps, ReceiveCounts &counts,
+                const __FlashStringHelper *name) {
+  // Limit each turn so two busy UARTs can both make progress.
+  for (uint16_t i = 0; i < 256 && gps.available(); i++) {
+    if (gps.read()) {
+      counts.bytes++;
+    }
+    if (!gps.newNMEAreceived()) {
+      continue;
+    }
+    char *line = gps.lastNMEA();
+    Serial.print(name);
+    Serial.print(' ');
+    Serial.print(line);
+    nmea_sentence_t sentence = Adafruit_NMEA::validate(line, strlen(line));
+    if (sentence.status == NMEA_FRAME_VALID) {
+      counts.valid++;
+    } else if (sentence.status == NMEA_FRAME_BAD_CHECKSUM) {
+      counts.checksumErrors++;
+    } else {
+      counts.formatErrors++;
+    }
+    if (gps.parse(line)) {
+      counts.parsed++;
+    }
+  }
+}
+
+void reportGPS(Adafruit_GPS &gps, ReceiveCounts &counts,
+               const __FlashStringHelper *name) {
+  Serial.print(F("STATUS "));
+  Serial.print(name);
+  Serial.print(F(" bytes="));
+  Serial.print(counts.bytes);
+  Serial.print(F(" valid="));
+  Serial.print(counts.valid);
+  Serial.print(F(" checksumErrors="));
+  Serial.print(counts.checksumErrors);
+  Serial.print(F(" formatErrors="));
+  Serial.print(counts.formatErrors);
+  Serial.print(F(" parsed="));
+  Serial.print(counts.parsed);
+  bool freshFix = gps.fix && gps.secondsSinceFix() < 5;
+  Serial.print(F(" fix="));
+  Serial.print(freshFix);
+  Serial.print(F(" ggaQuality="));
+  Serial.print(gps.fixquality);
+  if (freshFix) {
+    Serial.print(F(" latitudeE7="));
+    Serial.print(gps.latitude_fixed);
+    Serial.print(F(" longitudeE7="));
+    Serial.print(gps.longitude_fixed);
+  }
+  Serial.println();
+}

extras/tests/README.md

@@ -16,11 +16,15 @@ All tests use AddressSanitizer and UndefinedBehaviorSanitizer. Compiler
 warnings, failed checks, sanitizer findings, and a 30-second execution timeout
 fail the job. Other tests still run after an individual failure.
 
-The Arduino sketches use the real library sources with NMEA extensions enabled,
-including the marine and RMB checks. Each runs with both `NMEA_FLOAT_T=float`
-and `NMEA_FLOAT_T=double`, applied consistently to the sketch and library.
-`extras/test_support` provides serial
-output and a real monotonic clock; the wakeup timeout test takes ten seconds.
+The Arduino sketches use the real library sources in four configurations:
+`NMEA_EXTRAS=0` for the basic GPS API and `NMEA_EXTRAS=1` for marine extensions,
+each with `NMEA_FLOAT_T=float` and `NMEA_FLOAT_T=double`. Definitions apply
+consistently to the sketch and library. The RMB test checks that basic builds
+reject the unsupported sentence and extended builds decode it without partial
+updates on error. Standalone C++ core tests run once.
+
+`extras/test_support` provides serial output and a real monotonic clock; the
+wakeup timeout test takes ten seconds per configuration.
 I2C, SPI, GPIO, and hardware serial input abort if used. This runs the parser
 and mock-stream regressions, not physical GPS/SD hardware or MCU emulation.
 The sketches remain usable on Arduino boards with the normal Arduino core.
@@ -32,3 +36,7 @@ Those positions deliberately share the same legacy E7 coordinate. The default
 float fields and E7 fields are not full-precision RTK outputs. Future receiver
 APIs and logging examples must preserve the core precision through storage,
 public results, and formatting without requiring a global float-type override.
+
+Physical reception tests live separately in `extras/hw_tests/gnss_receive`.
+Build and run them explicitly on the documented Nano or HILBERT fixture; the
+host test runner does not operate attached boards.

extras/tests/rmb_state/rmb_state.ino

@@ -13,6 +13,13 @@ void setup() {
   Serial.println("Adafruit GPS RMB state regression");
 
   char good[] = "$GPRMB,A,0.10,R,DEST,START,4807.038,N,01131.000,E,1.0,84.4,2.5,A*44";
+#ifndef NMEA_EXTENSIONS
+  if (GPS.parse(good)) {
+    Serial.println("FAIL: basic GPS build accepted an unsupported RMB sentence");
+    return;
+  }
+  Serial.println("PASS: basic GPS build rejects unsupported RMB sentences");
+#else
   if (!GPS.parse(good)) {
     Serial.println("FAIL: valid RMB sentence was rejected");
     return;
@@ -36,6 +43,7 @@ void setup() {
     return;
   }
   Serial.println("PASS: previous waypoint data was preserved");
+#endif
   testsPassed = true;
 }
 

extras/tests/run_tests.py

@@ -23,25 +23,29 @@ def main():
         "-fsanitize=address,undefined", "-fno-sanitize-recover=all",
         "-fno-omit-frame-pointer", "-I" + str(root / "src"),
     ]
-    # Apply the same type to the sketch and every library translation unit.
-    # The default float API and optional double API must both remain usable.
+    # Host builds otherwise enable extensions automatically. Cover the basic
+    # GPS API and marine extensions, each with consistent float/double types
+    # across the sketch and every library translation unit.
     cases = []
     for test in tests:
-        modes = ("float", "double") if test.suffix == ".ino" else (None,)
-        cases.extend((test, mode) for mode in modes)
+        if test.suffix == ".ino":
+            for extras in (0, 1):
+                for float_type in ("float", "double"):
+                    cases.append((test, extras, float_type))
+        else:
+            cases.append((test, None, None))
     failures = []
-    for index, (test, mode) in enumerate(cases, 1):
+    for index, (test, extras, float_type) in enumerate(cases, 1):
         name = str(test.relative_to(tests_dir))
-        if mode is not None:
-            name += f" (NMEA_FLOAT_T={mode})"
+        if extras is not None:
+            name += f" (NMEA_EXTRAS={extras}, NMEA_FLOAT_T={float_type})"
         print(f"\n[{index}/{len(cases)}] {name}", flush=True)
         binary = build / str(index)
         if test.suffix == ".ino":
-            # Enable marine extensions so RMB and conditional marine checks run.
             sources = [*sorted((root / "src").glob("*.cpp")),
                        support / "Arduino.cpp", support / "sketch_main.cpp"]
-            extra_flags = ["-I" + str(support), "-DNMEA_EXTRAS=1",
-                           f"-DNMEA_FLOAT_T={mode}"]
+            extra_flags = ["-I" + str(support), f"-DNMEA_EXTRAS={extras}",
+                           f"-DNMEA_FLOAT_T={float_type}"]
         else:
             sources = [root / "src/Adafruit_NMEA.cpp",
                        root / "src/Adafruit_GNSS.cpp"]

src/NMEA_data.cpp

@@ -91,6 +91,9 @@ void Adafruit_GPS::newDataValue(nmea_index_t idx, nmea_float_t v) {
       val[idx].hist->lastHistory = millis();
     }
   }
+#else
+  (void)idx;
+  (void)v;
 #endif // NMEA_EXTENSIONS
 }