Keep validated navigation parsing within AVR logger flash limits

Co-authored-by: Limor Fried <ladyada@users.noreply.github.com>

examples/shield_sdlog/shield_sdlog.ino

@@ -37,7 +37,9 @@ bool usingInterrupt = false;
 #define ledPin 13
 
 SdFat SD;
-File logfile;
+// Raw NMEA logging only needs the base file API. Avoid the Arduino Stream
+// wrapper, whose virtual methods also pull unused read/seek code into flash.
+SdBaseFile logfile;
 
 // read a Hex value and return the decimal equivalent
 uint8_t parseHex(char c) {
@@ -98,15 +100,19 @@ void setup() {
   }
   char filename[15];
   strcpy(filename, "GPSLOG0000.TXT");
-  for (uint16_t i = 0; i < 10000; i++) {
-    filename[6] = '0' + i/1000;
-    filename[7] = '0' + (i/100)%10;
-    filename[8] = '0' + (i/10)%10;
-    filename[9] = '0' + i%10;
-    // Find an unused long filename, then create it without opening an old log.
-    if (! SD.exists(filename)) {
+  // Find an unused long filename, then create it without opening an old log.
+  while (SD.exists(filename)) {
+    // Carry through the four digits, keeping the filename as the counter.
+    int8_t digit = 9;
+    while (digit >= 6 && filename[digit] == '9') {
+      filename[digit] = '0';
+      digit--;
+    }
+    if (digit < 6) {
+      // All 10000 names exist. Exclusive creation below will fail safely.
       break;
     }
+    filename[digit]++;
   }
 
   if (!logfile.open(filename, O_WRONLY | O_CREAT | O_EXCL)) {
@@ -219,7 +225,7 @@ void loop() {
     uint8_t stringsize = strlen(stringptr);
     if (stringsize != logfile.write((uint8_t *)stringptr, stringsize))    //write the string to the SD file
         error(4);
-    if (strstr(stringptr, "RMC") || strstr(stringptr, "GGA"))   logfile.flush();
+    if (strstr(stringptr, "RMC") || strstr(stringptr, "GGA"))   logfile.sync();
     Serial.println();
   }
 }

extras/tests/nmea_utilities/nmea_utilities.cpp

@@ -28,6 +28,9 @@ int main() {
                  {"-.5", -5, 1},
                  {"1.", 1, 0},
                  {"00012.0300", 120300, 4},
+                 {"0009223372036854775807", INT64_MAX, 0},
+                 {"-0009223372036854775808", INT64_MIN, 0},
+                 {"1000000000000000000", INT64_C(1000000000000000000), 0},
                  {"4807.038123456", INT64_C(4807038123456), 9},
                  {"9223372036854775807", INT64_MAX, 0},
                  {"-9223372036854775808", INT64_MIN, 0},
@@ -44,7 +47,8 @@ int main() {
   for (const char *text : badNumbers)
     expectNumber({text, strlen(text)}, NMEA_NUMBER_BAD_FORMAT);
   const char *overflow[] = {"9223372036854775808", "-9223372036854775809",
-                            "99999999999999999999999999"};
+                            "99999999999999999999999999",
+                            "10000000000000000000", "0009223372036854775808"};
   for (const char *text : overflow)
     expectNumber({text, strlen(text)}, NMEA_NUMBER_OUT_OF_RANGE);
   expectNumber({NULL, 100}, NMEA_NUMBER_MISSING);
@@ -121,6 +125,11 @@ static void expectNumber(nmea_span_t field, nmea_number_status_t status,
                          int64_t coefficient, uint8_t decimalPlaces) {
   nmea_decimal_t result = Adafruit_NMEA::parseDecimal(field);
   assert(result.status == status);
+  assert(Adafruit_NMEA::validateDecimal(field) == status);
+  nmea_number_status_t nonnegative = status;
+  if (status == NMEA_NUMBER_VALID && coefficient < 0)
+    nonnegative = NMEA_NUMBER_OUT_OF_RANGE;
+  assert(Adafruit_NMEA::validateDecimal(field, false) == nonnegative);
   assert(result.coefficient == coefficient);
   assert(result.decimalPlaces == decimalPlaces);
 }

src/Adafruit_GNSS.cpp

@@ -195,14 +195,12 @@ static nmea_number_status_t parseSixDigits(nmea_span_t field, uint8_t *pairs) {
     return NMEA_NUMBER_EMPTY;
   if (field.length != 6)
     return NMEA_NUMBER_BAD_FORMAT;
-  for (uint8_t i = 0; i < 6; i++) {
-    char c = field.data[i];
-    if (c < '0' || c > '9')
+  for (uint8_t i = 0; i < 3; i++) {
+    uint8_t tens = field.data[2 * i] - '0';
+    uint8_t ones = field.data[2 * i + 1] - '0';
+    if (tens > 9 || ones > 9)
       return NMEA_NUMBER_BAD_FORMAT;
-    if (i % 2 == 0)
-      pairs[i / 2] = (c - '0') * 10;
-    else
-      pairs[i / 2] += c - '0';
+    pairs[i] = tens * 10 + ones;
   }
   return NMEA_NUMBER_VALID;
 }
@@ -231,22 +229,27 @@ gnss_validation_t Adafruit_GNSS::validateNavigation(nmea_span_t type,
   gnss_validation_t result = {GNSS_SENTENCE_UNSUPPORTED, 0};
   if (!type.data || type.length != 3)
     return result;
-  bool gga = type.data[0] == 'G' && type.data[1] == 'G' && type.data[2] == 'A';
-  bool rmc = type.data[0] == 'R' && type.data[1] == 'M' && type.data[2] == 'C';
-  bool gll = type.data[0] == 'G' && type.data[1] == 'L' && type.data[2] == 'L';
-  bool gsa = type.data[0] == 'G' && type.data[1] == 'S' && type.data[2] == 'A';
-  if (!gga && !rmc && !gll && !gsa)
+  uint8_t sentence = UNSUPPORTED;
+  if (type.data[0] == 'G' && type.data[1] == 'G' && type.data[2] == 'A')
+    sentence = GGA;
+  else if (type.data[0] == 'R' && type.data[1] == 'M' && type.data[2] == 'C')
+    sentence = RMC;
+  else if (type.data[0] == 'G' && type.data[1] == 'L' && type.data[2] == 'L')
+    sentence = GLL;
+  else if (type.data[0] == 'G' && type.data[1] == 'S' && type.data[2] == 'A')
+    sentence = GSA;
+  if (sentence == UNSUPPORTED)
     return result;
   uint8_t required = 6, latitudeField = 1, timeField = 5;
-  if (gga) {
+  if (sentence == GGA) {
     required = 11;
     latitudeField = 2;
     timeField = 1;
-  } else if (rmc) {
+  } else if (sentence == RMC) {
     required = 9;
     latitudeField = 3;
     timeField = 1;
-  } else if (gsa) {
+  } else if (sentence == GSA) {
     required = 17;
     timeField = 0;
   }
@@ -256,7 +259,7 @@ gnss_validation_t Adafruit_GNSS::validateNavigation(nmea_span_t type,
     result.status = GNSS_SENTENCE_MISSING_FIELDS;
     if (!field.data)
       return result;
-    if (!gsa && (i == latitudeField || i == latitudeField + 2)) {
+    if (sentence != GSA && (i == latitudeField || i == latitudeField + 2)) {
       nmea_span_t hemisphere = Adafruit_NMEA::nextField(fields);
       if (!hemisphere.data) {
         result.field = i + 1;
@@ -279,28 +282,28 @@ gnss_validation_t Adafruit_GNSS::validateNavigation(nmea_span_t type,
     if (!field.length)
       continue;
     result.status = GNSS_SENTENCE_INVALID_FIELD;
-    if (gsa && (i == 1 || (i >= 3 && i <= 14))) {
+    if (sentence == GSA && (i == 1 || (i >= 3 && i <= 14))) {
       continue; // Selection mode and satellite IDs are not decoded by GPS.
     } else if (i == timeField) {
       if (parseTime(field).status != NMEA_NUMBER_VALID)
         return result;
-    } else if (rmc && i == 9) {
+    } else if (sentence == RMC && i == 9) {
       if (parseDate(field).status != NMEA_NUMBER_VALID)
         return result;
-    } else if ((rmc && i == 2) || (gll && i == 6)) {
+    } else if ((sentence == RMC && i == 2) || (sentence == GLL && i == 6)) {
       if (field.length != 1 || (field.data[0] != 'A' && field.data[0] != 'V'))
         return result;
-    } else if ((gga && (i == 6 || i == 7)) || (gsa && i == 2)) {
+    } else if ((sentence == GGA && (i == 6 || i == 7)) ||
+               (sentence == GSA && i == 2)) {
       if (!validUnsignedByte(field))
         return result;
-    } else if (gga && i == 10) {
+    } else if (sentence == GGA && i == 10) {
       if (field.length != 1 || field.data[0] != 'M')
         return result;
     } else {
-      nmea_decimal_t number = Adafruit_NMEA::parseDecimal(field);
-      bool signedValue = gga && (i == 9 || i == 11);
-      if (number.status != NMEA_NUMBER_VALID ||
-          (!signedValue && number.coefficient < 0))
+      bool signedValue = sentence == GGA && (i == 9 || i == 11);
+      if (Adafruit_NMEA::validateDecimal(field, signedValue) !=
+          NMEA_NUMBER_VALID)
         return result;
     }
   }

src/Adafruit_GNSS.h

@@ -68,6 +68,16 @@ public:
   static gnss_date_t parseDate(nmea_span_t field);
   static gnss_validation_t validateNavigation(nmea_span_t type,
                                               nmea_span_t fields);
+
+private:
+  /// Sentence kinds handled by the navigation validator.
+  enum {
+    UNSUPPORTED, ///< Sentence has no standard navigation validation.
+    GGA,         ///< Fix time, position, and altitude.
+    RMC,         ///< Recommended minimum navigation data.
+    GLL,         ///< Geographic position and time.
+    GSA          ///< Fix type and dilution of precision.
+  };
 };
 
 #endif // ADAFRUIT_GNSS_H

src/Adafruit_GPS.h

@@ -250,6 +250,7 @@ private:
                   nmea_float_t *angle = NULL, int32_t *angle_fixed = NULL,
                   char *dir = NULL);
   char *parseStr(char *buff, char *p, int n);
+  char *parseCoordinates(char *p);
   bool parseTime(char *);
   bool parseFix(char *);
   bool parseAntenna(char *);

src/Adafruit_NMEA.cpp

@@ -313,6 +313,57 @@ nmea_span_t Adafruit_NMEA::nextField(nmea_span_t &remaining) {
   return field;
 }
 
+static nmea_number_status_t scanDecimal(nmea_span_t field, uint8_t &places,
+                                        bool &negative) {
+  if (!field.data)
+    return NMEA_NUMBER_MISSING;
+  if (!field.length)
+    return NMEA_NUMBER_EMPTY;
+  size_t start = 0;
+  negative = field.data[0] == '-';
+  if (negative || field.data[0] == '+')
+    start = 1;
+  bool decimalPoint = false, hasDigit = false, overflow = false;
+  uint8_t significant = 0;
+  int8_t comparison = 0;
+  places = 0;
+  // Compare significant digits with INT64_MAX's magnitude. This also accepts
+  // INT64_MIN's extra final digit without requiring any 64-bit arithmetic.
+  const char *limit = "9223372036854775807";
+  for (size_t i = start; i < field.length; i++) {
+    char c = field.data[i];
+    if (c == '.' && !decimalPoint) {
+      decimalPoint = true;
+      continue;
+    }
+    if (c < '0' || c > '9')
+      return NMEA_NUMBER_BAD_FORMAT;
+    hasDigit = true;
+    if (decimalPoint) {
+      if (places == UINT8_MAX)
+        overflow = true;
+      else
+        places++;
+    }
+    if (significant || c != '0') {
+      if (significant < 20)
+        significant++;
+      if (significant <= 19 && !comparison) {
+        char bound =
+            significant == 19 && negative ? '8' : limit[significant - 1];
+        if (c != bound)
+          comparison = c < bound ? -1 : 1;
+      }
+    }
+  }
+  if (!hasDigit)
+    return NMEA_NUMBER_BAD_FORMAT;
+  if (overflow || significant > 19 || (significant == 19 && comparison > 0))
+    return NMEA_NUMBER_OUT_OF_RANGE;
+  negative = negative && significant; // Negative zero remains nonnegative.
+  return NMEA_NUMBER_VALID;
+}
+
 /**************************************************************************/
 /*!
     @brief Convert a complete field to an exact signed decimal value.
@@ -330,72 +381,47 @@ nmea_span_t Adafruit_NMEA::nextField(nmea_span_t &remaining) {
 /**************************************************************************/
 nmea_decimal_t Adafruit_NMEA::parseDecimal(nmea_span_t field) {
   nmea_decimal_t result = {NMEA_NUMBER_BAD_FORMAT, 0, 0};
-  if (!field.data) {
-    result.status = NMEA_NUMBER_MISSING;
-    return result;
-  }
-  if (!field.length) {
-    result.status = NMEA_NUMBER_EMPTY;
-    return result;
-  }
-
-  size_t start = 0;
+  uint8_t places = 0;
   bool negative = false;
-  if (field.data[0] == '-' || field.data[0] == '+') {
-    negative = field.data[0] == '-';
-    start = 1;
-  }
-
-  // Accumulate negatively: INT64_MIN has no positive int64_t counterpart.
-  int64_t limit = -INT64_MAX;
-  if (negative)
-    limit = INT64_MIN;
-  int64_t cutoff = limit / 10;
-  uint8_t lastDigitLimit = (uint8_t)(-(limit % 10));
+  result.status = scanDecimal(field, places, negative);
+  if (result.status != NMEA_NUMBER_VALID)
+    return result;
+  // The complete field is already checked, so accumulation cannot overflow.
+  // Accumulate negatively because INT64_MIN has no positive counterpart.
   int64_t coefficient = 0;
-  uint8_t decimalPlaces = 0;
-  bool decimalPoint = false;
-  bool hasDigit = false;
-  bool overflow = false;
-  for (size_t i = start; i < field.length; i++) {
+  for (size_t i = 0; i < field.length; i++) {
     char c = field.data[i];
-    if (c == '.' && !decimalPoint) {
-      decimalPoint = true;
-      continue;
-    }
-    if (c < '0' || c > '9')
-      return result;
-    hasDigit = true;
-    uint8_t digit = c - '0';
-    if (decimalPoint) {
-      if (decimalPlaces == UINT8_MAX)
-        overflow = true;
-      else
-        decimalPlaces++;
-    }
-    if (!overflow) {
-      if (coefficient < cutoff ||
-          (coefficient == cutoff && digit > lastDigitLimit))
-        overflow = true;
-      else
-        coefficient = coefficient * 10 - digit;
-    }
+    if (c >= '0' && c <= '9')
+      coefficient = coefficient * 10 - (c - '0');
   }
-  if (!hasDigit)
-    return result;
-  if (overflow) {
-    result.status = NMEA_NUMBER_OUT_OF_RANGE;
-    return result;
-  }
-
-  result.status = NMEA_NUMBER_VALID;
-  result.coefficient = coefficient;
-  if (!negative)
-    result.coefficient = -coefficient;
-  result.decimalPlaces = decimalPlaces;
+  result.coefficient = negative ? coefficient : -coefficient;
+  result.decimalPlaces = places;
   return result;
 }
 
+/**************************************************************************/
+/*!
+    @brief Check an exact decimal without constructing its numeric value.
+    @param field Borrowed readable text, with no NUL terminator required.
+    @param allowNegative Whether to accept negative nonzero values.
+    @return The same syntax and range status as parseDecimal(). Negative
+    nonzero values give OUT_OF_RANGE when allowNegative is false.
+
+    Retains parseDecimal's signed 64-bit coefficient and fractional digit
+    limits, including malformed-syntax precedence. Uses no 64-bit arithmetic
+    or allocation. Negative zero is accepted in either mode.
+*/
+/**************************************************************************/
+nmea_number_status_t Adafruit_NMEA::validateDecimal(nmea_span_t field,
+                                                    bool allowNegative) {
+  uint8_t places = 0;
+  bool negative = false;
+  nmea_number_status_t status = scanDecimal(field, places, negative);
+  if (status == NMEA_NUMBER_VALID && negative && !allowNegative)
+    return NMEA_NUMBER_OUT_OF_RANGE;
+  return status;
+}
+
 /**************************************************************************/
 /*!
     @brief Build a checksummed command in caller-provided storage.

src/Adafruit_NMEA.h

@@ -88,6 +88,8 @@ public:
   static nmea_sentence_t validate(const char *data, size_t length);
   static nmea_span_t nextField(nmea_span_t &remaining);
   static nmea_decimal_t parseDecimal(nmea_span_t field);
+  static nmea_number_status_t validateDecimal(nmea_span_t field,
+                                              bool allowNegative = true);
   static size_t buildCommand(char *output, size_t capacity, const char *body,
                              size_t bodyLength);
 

src/NMEA_parse.cpp

@@ -102,21 +102,10 @@ bool Adafruit_GPS::parse(char *nmea) {
   // pruning excess code easier. Otherwise, keep them alphabetical for ease of
   // reading.
   if (!strcmp(thisSentence, "GGA")) { //************************************GGA
-    if (fields < 11)
-      return false;
     // Adafruit from Actisense NGW-1 from SH CP150C
     parseTime(p);
     p = strchr(p, ',') + 1; // parse time with specialized function
-    // parse out both latitude and direction, then go to next field, or fail
-    if (parseCoord(p, &latitudeDegrees, &latitude, &latitude_fixed, &lat))
-      newDataValue(NMEA_LAT, latitudeDegrees);
-    p = strchr(p, ',') + 1;
-    p = strchr(p, ',') + 1;
-    // parse out both longitude and direction, then go to next field, or fail
-    if (parseCoord(p, &longitudeDegrees, &longitude, &longitude_fixed, &lon))
-      newDataValue(NMEA_LON, longitudeDegrees);
-    p = strchr(p, ',') + 1;
-    p = strchr(p, ',') + 1;
+    p = parseCoordinates(p);
     if (!isEmpty(p)) { // if it's a , (or a * at end of sentence) the value is
                        // not included
       fixquality = atoi(p); // needs additional processing
@@ -142,23 +131,12 @@ bool Adafruit_GPS::parse(char *nmea) {
       geoidheight = atof(p); // skip the rest
 
   } else if (!strcmp(thisSentence, "RMC")) { //*****************************RMC
-    if (fields < 9)
-      return false;
     // in Adafruit from Actisense NGW-1 from SH CP150C
     parseTime(p);
     p = strchr(p, ',') + 1;
     parseFix(p);
     p = strchr(p, ',') + 1;
-    // parse out both latitude and direction, then go to next field, or fail
-    if (parseCoord(p, &latitudeDegrees, &latitude, &latitude_fixed, &lat))
-      newDataValue(NMEA_LAT, latitudeDegrees);
-    p = strchr(p, ',') + 1;
-    p = strchr(p, ',') + 1;
-    // parse out both longitude and direction, then go to next field, or fail
-    if (parseCoord(p, &longitudeDegrees, &longitude, &longitude_fixed, &lon))
-      newDataValue(NMEA_LON, longitudeDegrees);
-    p = strchr(p, ',') + 1;
-    p = strchr(p, ',') + 1;
+    p = parseCoordinates(p);
     if (!isEmpty(p))
       newDataValue(NMEA_SOG, speed = atof(p));
     p = strchr(p, ',') + 1;
@@ -175,26 +153,13 @@ bool Adafruit_GPS::parse(char *nmea) {
     } // skip the rest
 
   } else if (!strcmp(thisSentence, "GLL")) { //*****************************GLL
-    if (fields < 6)
-      return false;
     // in Adafruit from Actisense NGW-1 from SH CP150C
-    // parse out both latitude and direction, then go to next field, or fail
-    if (parseCoord(p, &latitudeDegrees, &latitude, &latitude_fixed, &lat))
-      newDataValue(NMEA_LAT, latitudeDegrees);
-    p = strchr(p, ',') + 1;
-    p = strchr(p, ',') + 1;
-    // parse out both longitude and direction, then go to next field, or fail
-    if (parseCoord(p, &longitudeDegrees, &longitude, &longitude_fixed, &lon))
-      newDataValue(NMEA_LON, longitudeDegrees);
-    p = strchr(p, ',') + 1;
-    p = strchr(p, ',') + 1;
+    p = parseCoordinates(p);
     parseTime(p);
     p = strchr(p, ',') + 1;
     parseFix(p); // skip the rest
 
   } else if (!strcmp(thisSentence, "GSA")) { //*****************************GSA
-    if (fields < 17)
-      return false;
     // in Adafruit from Actisense NGW-1
     p = strchr(p, ',') + 1; // skip selection mode
     if (!isEmpty(p))
@@ -792,6 +757,28 @@ bool Adafruit_GPS::parseCoord(char *pStart, nmea_float_t *angleDegrees,
   return true;
 }
 
+/**************************************************************************/
+/*!
+    @brief Update the adjacent latitude and longitude fields of a sentence.
+    @param p First coordinate field in an already validated GGA, RMC, or GLL.
+    @return Field following longitude and its hemisphere.
+
+    The caller must validate all four fields before calling. Empty coordinate
+    pairs leave their previous values unchanged. Use the same exact conversion
+    and data update path for each navigation sentence.
+*/
+/**************************************************************************/
+char *Adafruit_GPS::parseCoordinates(char *p) {
+  if (parseCoord(p, &latitudeDegrees, &latitude, &latitude_fixed, &lat))
+    newDataValue(NMEA_LAT, latitudeDegrees);
+  p = strchr(p, ',') + 1;
+  p = strchr(p, ',') + 1;
+  if (parseCoord(p, &longitudeDegrees, &longitude, &longitude_fixed, &lon))
+    newDataValue(NMEA_LON, longitudeDegrees);
+  p = strchr(p, ',') + 1;
+  return strchr(p, ',') + 1;
+}
+
 /**************************************************************************/
 /*!
     @brief Parse a string token from pointer p to the next comma, asterisk