Merge pull request #191 from ladyada-eagleclaw/gps-coordinate-precision

Preserve coordinate precision with shared GNSS decoding

extras/tests/gnss_coordinate/gnss_coordinate.cpp

@@ -0,0 +1,100 @@
+#include "Adafruit_GNSS.h"
+#include <assert.h>
+#include <stdio.h>
+#include <string.h>
+
+static gnss_coordinate_t decode(const char *text, const char *direction) {
+  nmea_span_t coordinate = {text, text ? strlen(text) : 0};
+  nmea_span_t hemisphere = {direction, direction ? strlen(direction) : 0};
+  return Adafruit_GNSS::parseCoordinate(coordinate, hemisphere);
+}
+
+static void invalid(const char *text, const char *direction,
+                    nmea_number_status_t status) {
+  gnss_coordinate_t value = decode(text, direction);
+  assert(value.status == status);
+  assert(value.degreesE7 == 0 && value.degrees == 0 && value.minutes == 0 &&
+         value.fractionalMinutes == 0 && value.hemisphere == 0);
+}
+
+int main() {
+  gnss_coordinate_t value = decode("4807.038", "N");
+  assert(value.status == NMEA_NUMBER_VALID && value.degreesE7 == 481173000);
+  assert(value.degrees == 48 && value.minutes == 7 &&
+         value.fractionalMinutes == 38000000 && value.hemisphere == 'N');
+  assert(decode("4807.038", "S").degreesE7 == -481173000);
+  assert(decode("01131.000", "E").degreesE7 == 115166666);
+  assert(decode("01131.000", "W").degreesE7 == -115166666);
+  assert(decode("0000", "N").status == NMEA_NUMBER_VALID);
+  assert(decode("9000.000", "S").degreesE7 == -900000000);
+  assert(decode("18000", "E").degreesE7 == 1800000000);
+  assert(decode("18000.0000000000000000", "W").degreesE7 == -1800000000);
+  puts("PASS: standard coordinates, hemispheres, zero, poles, and date line");
+
+  invalid(NULL, "N", NMEA_NUMBER_MISSING);
+  invalid("4807.038", NULL, NMEA_NUMBER_MISSING);
+  invalid("", NULL, NMEA_NUMBER_MISSING);
+  invalid("", "N", NMEA_NUMBER_EMPTY);
+  invalid("4807.038", "", NMEA_NUMBER_EMPTY);
+  const char *bad[] = {"4807.",
+                       "4807.1x",
+                       "4807.1.2",
+                       "4807e1",
+                       "+4807.1",
+                       "-4807.1",
+                       " 4807.1",
+                       "4807.1 ",
+                       "480.1",
+                       "04807.1",
+                       "480a.1",
+                       "4807,1",
+                       "4807.0000000000000x"};
+  for (size_t i = 0; i < sizeof(bad) / sizeof(bad[0]); i++)
+    invalid(bad[i], "N", NMEA_NUMBER_BAD_FORMAT);
+  invalid("4807.038", "North", NMEA_NUMBER_BAD_FORMAT);
+  invalid("4807.038", "n", NMEA_NUMBER_BAD_FORMAT);
+  invalid("4807.038", "X", NMEA_NUMBER_BAD_FORMAT);
+  invalid("4807.038", "E", NMEA_NUMBER_BAD_FORMAT);
+  invalid("01131.000", "N", NMEA_NUMBER_BAD_FORMAT);
+  invalid("4860.0", "N", NMEA_NUMBER_OUT_OF_RANGE);
+  invalid("9100.0", "S", NMEA_NUMBER_OUT_OF_RANGE);
+  invalid("18100.0", "W", NMEA_NUMBER_OUT_OF_RANGE);
+  invalid("9000.0000000000001", "N", NMEA_NUMBER_OUT_OF_RANGE);
+  invalid("18000.0000000000001", "E", NMEA_NUMBER_OUT_OF_RANGE);
+  invalid("65536.0", "E", NMEA_NUMBER_OUT_OF_RANGE);
+  invalid("99999.0", "W", NMEA_NUMBER_OUT_OF_RANGE);
+  puts("PASS: missing, empty, syntax, overflow, and range failures clear "
+       "outputs");
+
+  // The buffers need not be NUL-terminated; bytes outside each span are
+  // ignored.
+  const char coordinate[] = {'4', '8', '0', '7', '.', '0', '3', '8', 'x'};
+  const char hemisphere[] = {'N', 'x'};
+  value = Adafruit_GNSS::parseCoordinate({coordinate, 8}, {hemisphere, 1});
+  assert(value.status == NMEA_NUMBER_VALID && value.degreesE7 == 481173000);
+  assert(decode("1234.123456789987654321", "N").fractionalMinutes == 123456789);
+  assert(decode("0000.000005999999999", "N").degreesE7 == 0);
+  assert(decode("0000.000006", "N").degreesE7 == 1);
+  assert(decode("8959.999999999999", "N").degreesE7 == 899999999);
+  puts("PASS: bounded spans, retained precision, and truncation boundaries");
+
+  const uint32_t fractions[] = {0, 1, 599, 600, 123456789, 999999999};
+  for (unsigned degrees = 0; degrees < 180; degrees += 7) {
+    for (unsigned minutes = 0; minutes < 60; minutes++) {
+      for (size_t i = 0; i < sizeof(fractions) / sizeof(fractions[0]); i++) {
+        char text[24];
+        snprintf(text, sizeof(text), "%03u%02u.%09lu", degrees, minutes,
+                 (unsigned long)fractions[i]);
+        int32_t expected =
+            degrees * 10000000L +
+            ((uint64_t)minutes * 1000000000 + fractions[i]) / 6000;
+        value = decode(text, "E");
+        assert(value.status == NMEA_NUMBER_VALID &&
+               value.degreesE7 == expected);
+        assert(decode(text, "W").degreesE7 == -expected);
+      }
+    }
+  }
+  puts("PASS: fixed-point output matches an independent 64-bit calculation");
+  return 0;
+}

extras/tests/gps_coordinates/gps_coordinates.ino

@@ -0,0 +1,71 @@
+#include <Arduino.h>
+#include <SPI.h>
+#include <Wire.h>
+#define private public
+#include <Adafruit_GPS.h>
+#undef private
+
+Adafruit_GPS GPS;
+bool testsPassed = false;
+
+void setup() {
+  Serial.begin(115200);
+  Serial.println(F("Adafruit GPS shared coordinate regression"));
+  char sentence[120] =
+      "$GPRMC,123519,A,4807.038123456,N,01131.123456789,E,1.2,3.4,150926,,,A";
+  GPS.addChecksum(sentence);
+  if (!GPS.parse(sentence) || GPS.latitude_fixed != 481173020 ||
+      GPS.longitude_fixed != 115187242 || GPS.lat != 'N' || GPS.lon != 'E') {
+    Serial.println(F("FAIL: exact coordinates through the RMC decoder"));
+    return;
+  }
+  double tolerance = sizeof(nmea_float_t) > 4 ? 1e-11 : 1e-5;
+  if (abs(GPS.latitudeDegrees - (48.0 + 7.038123456 / 60)) > tolerance ||
+      abs(GPS.longitudeDegrees - (11.0 + 31.123456789 / 60)) > tolerance) {
+    Serial.println(F("FAIL: decimal degrees lost precision through E7"));
+    return;
+  }
+  Serial.println(F("PASS: exact fixed point and full convenience-float precision"));
+
+  const char *bad[] = {"4860.0,N", "4807.1x,N", "4807.0,North", "9000.00001,N",
+                       "18000.1,E", "4807.0,E", "-4807.0,S", "4807.0,"};
+  for (uint8_t i = 0; i < sizeof(bad) / sizeof(bad[0]); i++) {
+    char text[32];
+    strcpy(text, bad[i]);
+    nmea_float_t degrees = 12, angle = 34;
+    int32_t fixed = 56;
+    char direction = 'X';
+    if (GPS.parseCoord(text, &degrees, &angle, &fixed, &direction) ||
+        degrees != 12 || angle != 34 || fixed != 56 || direction != 'X') {
+      Serial.println(F("FAIL: bad coordinate changed outputs"));
+      return;
+    }
+  }
+  if (GPS.parseCoord(NULL)) {
+    Serial.println(F("FAIL: NULL coordinate accepted"));
+    return;
+  }
+  Serial.println(F("PASS: invalid fields leave every requested output intact"));
+
+  char west[] = "18000,W";
+  int32_t fixed = 0;
+  if (!GPS.parseCoord(west, NULL, NULL, &fixed) || fixed != -1800000000) {
+    Serial.println(F("FAIL: integral minutes or optional outputs"));
+    return;
+  }
+  char south[] = "8959.999999999999,S";
+  if (!GPS.parseCoord(south, NULL, NULL, &fixed) || fixed != -899999999) {
+    Serial.println(F("FAIL: high precision near a pole"));
+    return;
+  }
+  Serial.println(F("PASS: hemispheres, boundary values, and optional outputs"));
+  testsPassed = true;
+}
+
+void loop() {
+  if (testsPassed) {
+    Serial.println(F("PASS: GPS shared coordinate regression"));
+  } else {
+    Serial.println(F("FAIL: GPS shared coordinate regression"));
+  }
+}

extras/tests/run_tests.py

@@ -34,7 +34,8 @@ def main():
                        support / "Arduino.cpp", support / "sketch_main.cpp"]
             extra_flags = ["-I" + str(support), "-DNMEA_EXTRAS=1"]
         else:
-            sources = [root / "src/Adafruit_NMEA.cpp"]
+            sources = [root / "src/Adafruit_NMEA.cpp",
+                       root / "src/Adafruit_GNSS.cpp"]
             extra_flags = []
         try:
             subprocess.run([compiler, *flags, *extra_flags,

src/Adafruit_GNSS.cpp

@@ -0,0 +1,98 @@
+/**************************************************************************/
+/*!
+  @file Adafruit_GNSS.cpp
+  @brief Transport-independent decoding of standard GNSS fields.
+
+  Written for Adafruit Industries. BSD license; see license.txt.
+*/
+/**************************************************************************/
+
+#include "Adafruit_GNSS.h"
+
+/**************************************************************************/
+/*!
+    @brief Decode a latitude or longitude field and its hemisphere.
+    @param coordinate DDMM[.fraction] latitude or DDDMM[.fraction] longitude.
+    @param hemisphere Exactly one N/S for latitude or E/W for longitude.
+    @return Validated components and signed decimal degrees times 10000000.
+
+    Missing spans take precedence over empty spans, then malformed syntax,
+    then range errors. Signs, spaces, exponents, and a bare decimal point are
+    invalid. Minutes must be below 60; latitude is limited to 90 degrees and
+    longitude to 180, with zero minutes at either boundary. Fractional minutes
+    retain the first nine digits, padded with zeros; further digits are
+    validated and truncated. degreesE7 is exact at its stated resolution,
+    without rounding or intermediate floating-point arithmetic.
+*/
+/**************************************************************************/
+gnss_coordinate_t Adafruit_GNSS::parseCoordinate(nmea_span_t coordinate,
+                                                 nmea_span_t hemisphere) {
+  gnss_coordinate_t result = {};
+  result.status = NMEA_NUMBER_MISSING;
+  if (!coordinate.data || !hemisphere.data)
+    return result;
+  result.status = NMEA_NUMBER_EMPTY;
+  if (!coordinate.length || !hemisphere.length)
+    return result;
+  result.status = NMEA_NUMBER_BAD_FORMAT;
+  if (hemisphere.length != 1)
+    return result;
+  char direction = hemisphere.data[0];
+  bool latitude = direction == 'N' || direction == 'S';
+  if (!latitude && direction != 'E' && direction != 'W')
+    return result;
+
+  size_t wholeDigits = latitude ? 4 : 5;
+  if (coordinate.length < wholeDigits)
+    return result;
+  uint32_t whole = 0;
+  for (size_t i = 0; i < wholeDigits; i++) {
+    char c = coordinate.data[i];
+    if (c < '0' || c > '9')
+      return result;
+    whole = whole * 10 + c - '0';
+  }
+  uint32_t fraction = 0;
+  uint8_t retained = 0;
+  bool nonzeroFraction = false;
+  if (coordinate.length > wholeDigits) {
+    if (coordinate.data[wholeDigits] != '.' ||
+        coordinate.length == wholeDigits + 1)
+      return result;
+    for (size_t i = wholeDigits + 1; i < coordinate.length; i++) {
+      char c = coordinate.data[i];
+      if (c < '0' || c > '9')
+        return result;
+      nonzeroFraction = nonzeroFraction || c != '0';
+      if (retained < 9) {
+        fraction = fraction * 10 + c - '0';
+        retained++;
+      }
+    }
+  }
+  while (retained < 9) {
+    fraction *= 10;
+    retained++;
+  }
+  uint16_t degrees = whole / 100;
+  uint8_t minutes = whole % 100;
+  uint16_t limit = latitude ? 90 : 180;
+  result.status = NMEA_NUMBER_OUT_OF_RANGE;
+  if (minutes >= 60 || degrees > limit ||
+      (degrees == limit && (minutes || nonzeroFraction)))
+    return result;
+
+  // Keeping seven fractional-minute digits is sufficient for an exact E7
+  // truncation after division by 60. The largest intermediate fits uint32_t.
+  uint32_t minuteE7 = minutes * 10000000UL + fraction / 100;
+  int32_t fixed = degrees * 10000000L + minuteE7 / 60;
+  if (direction == 'S' || direction == 'W')
+    fixed = -fixed;
+  result.status = NMEA_NUMBER_VALID;
+  result.degreesE7 = fixed;
+  result.degrees = degrees;
+  result.minutes = minutes;
+  result.fractionalMinutes = fraction;
+  result.hemisphere = direction;
+  return result;
+}

src/Adafruit_GNSS.h

@@ -0,0 +1,38 @@
+/**************************************************************************/
+/*!
+  @file Adafruit_GNSS.h
+  @brief Transport-independent decoding of standard GNSS fields.
+
+  Written for Adafruit Industries. BSD license; see license.txt.
+*/
+/**************************************************************************/
+
+#ifndef ADAFRUIT_GNSS_H
+#define ADAFRUIT_GNSS_H
+
+#include "Adafruit_NMEA.h"
+
+/** Validated coordinate components, with no floating-point conversion.
+ *  Every value except status is zero on failure. Southern and western
+ *  coordinates have negative degreesE7; the other components are unsigned.
+ */
+typedef struct {
+  nmea_number_status_t status; ///< Result of decoding the coordinate pair.
+  int32_t degreesE7; ///< Decimal degrees times 10000000, truncated toward zero.
+  uint16_t degrees;  ///< Whole unsigned degrees, 0 through 180.
+  uint8_t minutes;   ///< Whole minutes, 0 through 59.
+  uint32_t fractionalMinutes; ///< Fraction of a minute times 1000000000.
+  char hemisphere;            ///< Exactly N, S, E, or W on success.
+} gnss_coordinate_t;
+
+/** Stateless standard GNSS decoding shared by receiver implementations.
+ *  Inputs are borrowed spans. Decoding neither allocates memory nor performs
+ *  transport I/O, and does not alter the supplied text or receiver state.
+ */
+class Adafruit_GNSS {
+public:
+  static gnss_coordinate_t parseCoordinate(nmea_span_t coordinate,
+                                           nmea_span_t hemisphere);
+};
+
+#endif // ADAFRUIT_GNSS_H

src/NMEA_parse.cpp

@@ -20,6 +20,7 @@
 */
 /**************************************************************************/
 
+#include <Adafruit_GNSS.h>
 #include <Adafruit_GPS.h>
 
 const char PROGMEM Adafruit_GPS::sources[][4] = {"II", "WI", "GP", "PG", "GL",
@@ -723,8 +724,8 @@ bool Adafruit_GPS::onList(char *nmea, const char **list) {
     Works for either DDMM.mmmm,N (latitude) or DDDMM.mmmm,W (longitude) format.
     Insensitive to number of decimal places present. Only fills the variables
     if it succeeds and the variable pointer is not NULL. This allows calling
-    to fill only the variables of interest. Does rudimentary validation on
+    to fill only the variables of interest. Validates the complete coordinate
-    angle range.
+    and hemisphere fields, including minute and degree limits.
 
     Supersedes private functions parseLat(), parseLon(), parseLatDir(),
     parseLonDir(), all previously called from parse().
@@ -743,62 +744,40 @@ bool Adafruit_GPS::onList(char *nmea, const char **list) {
 bool Adafruit_GPS::parseCoord(char *pStart, nmea_float_t *angleDegrees,
                               nmea_float_t *angle, int32_t *angle_fixed,
                               char *dir) {
-  char *p = pStart;
+  if (!pStart)
-  if (!isEmpty(p)) {
+    return false;
-    // get the number in DDDMM.mmmm format and break into components
+  // Bound just the coordinate and hemisphere, stopping before the next field.
-    char degreebuff[10] = {0}; // Ensure string is terminated after copying
+  // Character comparisons avoid placing a delimiter string in AVR RAM.
-    char *e = strchr(p, '.');
+  char *end = pStart;
-    char *comma = strchr(p, ',');
+  uint8_t commas = 0;
-    if (e == NULL || comma == NULL || e > comma || e - p > 6)
+  while (*end && *end != '*') {
-      return false;               // no decimal point in range
+    if (*end == ',' && ++commas == 2)
-    memcpy(degreebuff, p, e - p); // get DDDMM
+      break;
-    long dddmm = atol(degreebuff);
+    end++;
-    long degrees = (dddmm / 100);         // truncate the minutes
+  }
-    long minutes = dddmm - degrees * 100; // remove the degrees
+  nmea_span_t remaining = {pStart, (size_t)(end - pStart)};
-    p = e;                                // start from the decimal point
+  nmea_span_t value = Adafruit_NMEA::nextField(remaining);
-    nmea_float_t decminutes = atof(e); // the fraction after the decimal point
+  nmea_span_t hemisphere = Adafruit_NMEA::nextField(remaining);
-    p = comma + 1;                     // go to the direction field
+  gnss_coordinate_t coordinate =
+      Adafruit_GNSS::parseCoordinate(value, hemisphere);
+  if (coordinate.status != NMEA_NUMBER_VALID)
+    return false;
 
-    // get the NSEW direction as a character
+  // Derive convenience floats only after exact fixed-point conversion. Keep
-    char nsew = 'X';
+  // their available precision instead of rebuilding them from the E7 value.
-    if (!isEmpty(p))
+  nmea_float_t minutes = coordinate.minutes + coordinate.fractionalMinutes /
-      nsew = *p; // field is not empty
+                                                  (nmea_float_t)1000000000.0;
-    else
+  nmea_float_t degrees = coordinate.degrees + minutes / 60;
-      return false; // no direction provided
+  if (coordinate.hemisphere == 'S' || coordinate.hemisphere == 'W')
-
+    degrees = -degrees;
-    // set the various numerical formats to their values
+  if (angle)
-    long fixed = degrees * 10000000 + (minutes * 10000000) / 60 +
+    *angle = coordinate.degrees * 100 + minutes;
-                 (decminutes * 10000000) / 60;
+  if (angle_fixed)
-    nmea_float_t ang = degrees * 100 + minutes + decminutes;
+    *angle_fixed = coordinate.degreesE7;
-    nmea_float_t deg = fixed / (nmea_float_t)10000000.;
+  if (angleDegrees)
-    if (nsew == 'S' ||
+    *angleDegrees = degrees;
-        nsew == 'W') { // fixed and deg are signed, but DDDMM.mmmm is not
+  if (dir)
-      fixed = -fixed;
+    *dir = coordinate.hemisphere;
-      deg = -deg;
-    }
-
-    // reject directions that are not NSEW
-    if (nsew != 'N' && nsew != 'S' && nsew != 'E' && nsew != 'W')
-      return false;
-
-    // reject angles that are out of range
-    if (nsew == 'N' || nsew == 'S')
-      if (abs(deg) > 90)
-        return false;
-    if (abs(deg) > 180)
-      return false;
-
-    // store in locations passed as args
-    if (angle != NULL)
-      *angle = ang;
-    if (angle_fixed != NULL)
-      *angle_fixed = fixed;
-    if (angleDegrees != NULL)
-      *angleDegrees = deg;
-    if (dir != NULL)
-      *dir = nsew;
-  } else
-    return false; // no number
   return true;
 }