Merge pull request #189 from ladyada-eagleclaw/gps-use-nmea-framer

Use the shared NMEA framer for GPS reception

extras/tests/gps_framing/gps_framing.ino

@@ -0,0 +1,208 @@
+#include <Arduino.h>
+#include <SPI.h>
+#include <Wire.h>
+// common_init() detaches the transport; reconnect the test stream afterwards.
+#define private public
+#include <Adafruit_GPS.h>
+#undef private
+
+class TestStream : public Stream {
+public:
+  const char *input = "";
+  int available() { return *input != 0; }
+  int read() { return *input ? *input++ : -1; }
+  int peek() { return *input ? *input : -1; }
+  void flush() {}
+  size_t write(uint8_t) { return 1; }
+};
+
+TestStream firstStream, secondStream;
+Adafruit_GPS firstGPS((Stream *)&firstStream);
+Adafruit_GPS secondGPS((Stream *)&secondStream);
+bool testsPassed = false;
+const char navigation[] =
+    "$GPRMC,123519,A,4807.038,N,01131.000,E,022.4,084.4,230394,003.1,W*6A\r\n";
+
+bool receive(TestStream &stream, Adafruit_GPS &gps, const char *text);
+bool checkReinitialization();
+
+void setup() {
+  Serial.begin(115200);
+  Serial.println(F("Adafruit GPS receive framing regression"));
+  if (firstGPS.newNMEAreceived() || firstGPS.lastNMEA()[0] ||
+      firstGPS.available() || firstGPS.read()) {
+    Serial.println(F("FAIL: initial receive state"));
+    return;
+  }
+  if (receive(firstStream, firstGPS, "noise\r\n") ||
+      firstGPS.newNMEAreceived()) {
+    Serial.println(F("FAIL: noise was published as a sentence"));
+    return;
+  }
+  if (!receive(firstStream, firstGPS, navigation) ||
+      !firstGPS.newNMEAreceived() ||
+      strcmp(firstGPS.lastNMEA(), navigation) || firstGPS.newNMEAreceived() ||
+      !firstGPS.parse(firstGPS.lastNMEA())) {
+    Serial.println(F("FAIL: reception, acknowledgement, or parsing"));
+    return;
+  }
+  Serial.println(F("PASS: initial state, noise, byte reads, and normal parsing"));
+
+  if (receive(firstStream, firstGPS, "$interrupted,") ||
+      strcmp(firstGPS.lastNMEA(), navigation) ||
+      !receive(firstStream, firstGPS, navigation) ||
+      strcmp(firstGPS.lastNMEA(), navigation)) {
+    Serial.println(F("FAIL: interrupted line recovery"));
+    return;
+  }
+  if (!receive(firstStream, firstGPS, "$A*00\n") ||
+      strcmp(firstGPS.lastNMEA(), "$A*00\n") ||
+      !receive(firstStream, firstGPS, "!A,1*5C\n") ||
+      strcmp(firstGPS.lastNMEA(), "!A,1*5C\n")) {
+    Serial.println(F("FAIL: invalid raw lines or exclamation start marker"));
+    return;
+  }
+  firstStream.input = "$PQTMVERNO*58\n";
+  if (!firstGPS.waitForSentence("$PQTMVERNO", 1, false, 100) ||
+      strcmp(firstGPS.lastNMEA(), "$PQTMVERNO*58\n")) {
+    Serial.println(F("FAIL: unknown zero-field command response"));
+    return;
+  }
+  Serial.println(F("PASS: restart markers, raw invalid lines, and command waits"));
+
+  char body[MAXLINELENGTH - NMEA_COMMAND_OVERHEAD + 1];
+  memset(body, 'x', sizeof(body) - 1);
+  body[0] = 'A';
+  body[1] = ',';
+  body[sizeof(body) - 1] = 0;
+  char maximum[MAXLINELENGTH];
+  if (Adafruit_NMEA::buildCommand(maximum, sizeof(maximum), body,
+                                   strlen(body)) != MAXLINELENGTH - 1 ||
+      !receive(firstStream, firstGPS, maximum) ||
+      strcmp(firstGPS.lastNMEA(), maximum)) {
+    Serial.println(F("FAIL: maximum-size line"));
+    return;
+  }
+  // Keep the previous complete line while a longer sentence is discarded.
+  char oversized[MAXLINELENGTH + 4];
+  memset(oversized, 'x', sizeof(oversized));
+  oversized[0] = '$';
+  oversized[sizeof(oversized) - 2] = '\n';
+  oversized[sizeof(oversized) - 1] = 0;
+  if (receive(firstStream, firstGPS, oversized) ||
+      firstGPS.newNMEAreceived() || strcmp(firstGPS.lastNMEA(), maximum) ||
+      !receive(firstStream, firstGPS, navigation) ||
+      strcmp(firstGPS.lastNMEA(), navigation)) {
+    Serial.println(F("FAIL: overflow preservation or recovery"));
+    return;
+  }
+  Serial.println(F("PASS: exact capacity, overflow discard, and recovery"));
+
+  firstStream.input = navigation;
+  firstGPS.read();
+  const char *pausedAt = firstStream.input;
+  firstGPS.pause(true);
+  if (firstGPS.available() || firstGPS.read() ||
+      firstStream.input != pausedAt || firstGPS.newNMEAreceived()) {
+    Serial.println(F("FAIL: pause consumed input"));
+    return;
+  }
+  firstGPS.pause(false);
+  while (firstStream.available()) {
+    firstGPS.read();
+  }
+  if (!firstGPS.newNMEAreceived() || strcmp(firstGPS.lastNMEA(), navigation)) {
+    Serial.println(F("FAIL: resume lost partial sentence"));
+    return;
+  }
+  Serial.println(F("PASS: pause and resume preserve partial input"));
+
+  firstStream.input = navigation;
+  secondStream.input = navigation;
+  uint32_t firstStarted = millis();
+  firstGPS.read();
+  delay(60);
+  uint32_t secondStarted = millis();
+  secondGPS.read();
+  delay(60);
+  while (firstStream.available()) {
+    firstGPS.read();
+  }
+  while (secondStream.available()) {
+    secondGPS.read();
+  }
+  if (!firstGPS.parse(firstGPS.lastNMEA()) ||
+      !secondGPS.parse(secondGPS.lastNMEA()) ||
+      abs(firstGPS.secondsSinceTime() - (millis() - firstStarted) / 1000.0) >
+          0.02 ||
+      abs(secondGPS.secondsSinceTime() - (millis() - secondStarted) / 1000.0) >
+          0.02) {
+    Serial.println(F("FAIL: receivers shared a sentence timestamp"));
+    return;
+  }
+  Serial.println(F("PASS: two receivers keep independent sentence timestamps"));
+  if (!checkReinitialization()) {
+    Serial.println(F("FAIL: reinitialization retained input or mixed buffer roles"));
+    return;
+  }
+  Serial.println(F("PASS: reinitialization clears input and preserves buffer roles"));
+  testsPassed = true;
+}
+
+void loop() {
+  if (testsPassed) {
+    Serial.println(F("PASS: GPS receive framing regression"));
+  } else {
+    Serial.println(F("FAIL: GPS receive framing regression"));
+  }
+}
+
+bool receive(TestStream &stream, Adafruit_GPS &gps, const char *text) {
+  stream.input = text;
+  bool completed = false;
+  while (*stream.input) {
+    char expected = *stream.input;
+    if (gps.read() != expected) {
+      Serial.println(F("FAIL: read did not return the incoming byte"));
+      testsPassed = false;
+      return false;
+    }
+    completed = completed || gps.newNMEAreceived();
+  }
+  return completed;
+}
+
+bool checkReinitialization() {
+  // Cover the initial buffer orientation and both roles after completed lines.
+  for (uint8_t completed = 0; completed < 3; completed++) {
+    TestStream stream;
+    Adafruit_GPS gps((Stream *)&stream);
+    for (uint8_t i = 0; i < completed; i++) {
+      if (!receive(stream, gps, navigation)) {
+        return false;
+      }
+    }
+    // Leave any completed line unacknowledged while starting another line.
+    receive(stream, gps, "$stale,");
+    gps.common_init();
+    if (gps.newNMEAreceived() || gps.lastNMEA()[0] ||
+        gps.receiver.lastText().data || gps.receiver.sentenceStartedAt() ||
+        gps.receiver.sentenceReceivedAt()) {
+      return false;
+    }
+    gps.gpsStream = &stream;
+    if (receive(stream, gps, "tail\n") || gps.lastNMEA()[0] ||
+        receive(stream, gps, "$fresh,") || gps.lastNMEA()[0] ||
+        !receive(stream, gps, navigation) || strcmp(gps.lastNMEA(), navigation)) {
+      return false;
+    }
+    if (receive(stream, gps, "$next,") || strcmp(gps.lastNMEA(), navigation)) {
+      return false;
+    }
+    gps.common_init();
+    if (gps.newNMEAreceived() || gps.lastNMEA()[0]) {
+      return false;
+    }
+  }
+  return true;
+}

extras/tests/nmea_framing/nmea_framing.cpp

@@ -20,6 +20,7 @@ int main() {
                 "Receivers must not copy receive state");
   char first[32], second[32];
   Adafruit_NMEA receiver(first, second, sizeof(first));
+  assert(!receiver.lastText().data && !receiver.lastText().length);
   nmea_sentence_t sentence = receiver.lastSentence();
   assert(sentence.status == NMEA_FRAME_INCOMPLETE);
   assert(!sentence.text.data && !sentence.address.data &&
@@ -36,6 +37,7 @@ int main() {
   assert(feedLine(receiver, "$A*41\r\n", 0) == NMEA_FRAME_VALID);
   sentence = receiver.lastSentence();
   expectText(sentence.text, "$A*41\r\n");
+  expectText(receiver.lastText(), "$A*41\r\n");
   expectText(sentence.address, "A");
   assert(sentence.fields.data == NULL);
   assert(sentence.text.data == first && first[sentence.text.length] == 0);

src/Adafruit_GPS.cpp

@@ -159,9 +159,9 @@ void Adafruit_GPS::common_init(void) {
   gpsSPI = NULL;
   recvdflag = false;
   paused = false;
-  lineidx = 0;
-  currentline = line1;
-  lastline = line2;
+  // Reset clears both buffers without swapping them. Keep lastline pointing
+  // at the completed buffer, which may be either line1 or line2 by now.
+  receiver.reset();
 
   hour = minute = seconds = year = month = day = fixquality = fixquality_3d =
       satellites = antenna = 0; // uint8_t
@@ -277,8 +277,7 @@ size_t Adafruit_GPS::write(uint8_t c) {
 */
 /**************************************************************************/
 char Adafruit_GPS::read(void) {
-  static uint32_t firstChar = 0; // first character received in current sentence
-  uint32_t tStart = millis();    // as close as we can get to time char was sent
+  uint32_t tStart = millis(); // as close as we can get to time char was sent
   char c = 0;
 
   if (paused || noComms)
@@ -351,36 +350,15 @@ char Adafruit_GPS::read(void) {
   }
   // Serial.print(c);
 
-  currentline[lineidx] = c;
-  lineidx = lineidx + 1;
-  if (lineidx >= MAXLINELENGTH)
-    lineidx = MAXLINELENGTH -
-              1; // ensure there is someplace to put the next received character
-
-  if (c == '\n') {
-    currentline[lineidx] = 0;
-
-    if (currentline == line1) {
-      currentline = line2;
-      lastline = line1;
-    } else {
-      currentline = line1;
-      lastline = line2;
-    }
-
-    // Serial.println("----");
-    // Serial.println((char *)lastline);
-    // Serial.println("----");
-    lineidx = 0;
+  nmea_frame_status_t status = receiver.feed((uint8_t)c, tStart);
+  if (c == '\n' && status != NMEA_FRAME_INCOMPLETE &&
+      status != NMEA_FRAME_OVERFLOW) {
+    // Publish only after the buffer and timestamp are ready. Keep read() as
+    // the sole framer owner so interrupt-driven sketches consume lastline.
+    lastline = receiver.lastText().data;
+    sentTime = receiver.sentenceStartedAt();
     recvdflag = true;
-    recvdTime = millis(); // time we got the end of the string
-    sentTime = firstChar;
-    firstChar = 0; // there are no characters yet
-    return c;      // wait until next character to set time
   }
-
-  if (firstChar == 0)
-    firstChar = tStart;
   return c;
 }
 

src/Adafruit_GPS.h

@@ -60,6 +60,7 @@
 #ifdef USE_SW_SERIAL
 #include <SoftwareSerial.h>
 #endif
+#include <Adafruit_NMEA.h>
 #include <Adafruit_PMTK.h>
 #include <NMEA_data.h>
 #include <SPI.h>
@@ -266,8 +267,6 @@ private:
   uint32_t lastFix = 2000000000L;  ///< millis() when last fix received
   uint32_t lastTime = 2000000000L; ///< millis() when last time received
   uint32_t lastDate = 2000000000L; ///< millis() when last date received
-  uint32_t recvdTime =
-      2000000000L; ///< millis() when last full sentence received
   uint32_t sentTime = 2000000000L; ///< millis() when first character of last
                                    ///< full sentence received
   bool paused;
@@ -292,11 +291,11 @@ private:
   volatile char line1[MAXLINELENGTH]; ///< We double buffer: read one line in
                                       ///< and leave one for the main program
   volatile char line2[MAXLINELENGTH]; ///< Second buffer
-  volatile uint8_t lineidx = 0; ///< our index into filling the current line
-  volatile char *currentline;   ///< Pointer to current line buffer
-  volatile char *lastline;      ///< Pointer to previous line buffer
-  volatile bool recvdflag;      ///< Received flag
-  volatile bool inStandbyMode;  ///< In standby flag
+  Adafruit_NMEA receiver{line1, line2, MAXLINELENGTH}; ///< Bounded byte framing
+  const char *volatile lastline =
+      (const char *)line2;     ///< Completed line published by read()
+  volatile bool recvdflag;     ///< Received flag
+  volatile bool inStandbyMode; ///< In standby flag
 };
 /**************************************************************************/
 

src/Adafruit_NMEA.cpp

@@ -23,8 +23,8 @@
     buffers are initialized to empty strings. No memory is allocated.
 */
 /**************************************************************************/
-Adafruit_NMEA::Adafruit_NMEA(char *firstBuffer, char *secondBuffer,
-                             size_t capacity)
+Adafruit_NMEA::Adafruit_NMEA(volatile char *firstBuffer,
+                             volatile char *secondBuffer, size_t capacity)
     : _buffer(firstBuffer), _lastBuffer(secondBuffer), _capacity(capacity) {
   if (!firstBuffer || !secondBuffer || capacity < 2) {
     _capacity = 0;
@@ -103,7 +103,7 @@ nmea_frame_status_t Adafruit_NMEA::feed(uint8_t byte, uint32_t receivedAtMs) {
 
   // Publish by swapping buffers, leaving the new completed line undisturbed
   // while the next line is assembled.
-  char *previous = _lastBuffer;
+  volatile char *previous = _lastBuffer;
   _lastBuffer = _buffer;
   _buffer = previous;
   _lastLength = _length;
@@ -111,7 +111,26 @@ nmea_frame_status_t Adafruit_NMEA::feed(uint8_t byte, uint32_t receivedAtMs) {
   _lastReceived = receivedAtMs;
   _length = 0;
   _buffer[0] = '\0';
-  return validate(_lastBuffer, _lastLength).status;
+  return validate((const char *)_lastBuffer, _lastLength).status;
+}
+
+/**************************************************************************/
+/*!
+    @brief Get the latest complete raw line without validating it again.
+    @return Borrowed text and its length, or an absent span before completion.
+
+    Includes invalid lines and their LF, with a NUL after the stored length.
+    The view expires on the next completed line or reset(). As with
+    lastSentence(), callers must synchronize access with feed().
+*/
+/**************************************************************************/
+nmea_span_t Adafruit_NMEA::lastText() const {
+  nmea_span_t text = {NULL, 0};
+  if (_lastLength) {
+    text.data = (const char *)_lastBuffer;
+    text.length = _lastLength;
+  }
+  return text;
 }
 
 /**************************************************************************/
@@ -127,7 +146,7 @@ nmea_frame_status_t Adafruit_NMEA::feed(uint8_t byte, uint32_t receivedAtMs) {
 /**************************************************************************/
 nmea_sentence_t Adafruit_NMEA::lastSentence() const {
   if (_lastLength)
-    return validate(_lastBuffer, _lastLength);
+    return validate((const char *)_lastBuffer, _lastLength);
   nmea_sentence_t result = {
       NMEA_FRAME_INCOMPLETE, {NULL, 0}, {NULL, 0}, {NULL, 0}};
   return result;

src/Adafruit_NMEA.h

@@ -69,7 +69,8 @@ typedef struct {
  */
 class Adafruit_NMEA {
 public:
-  Adafruit_NMEA(char *firstBuffer, char *secondBuffer, size_t capacity);
+  Adafruit_NMEA(volatile char *firstBuffer, volatile char *secondBuffer,
+                size_t capacity);
   /// @brief Copying is disabled to prevent sharing writable receive buffers.
   /// @param other Receiver that cannot be copied.
   Adafruit_NMEA(const Adafruit_NMEA &other) = delete;
@@ -79,6 +80,7 @@ public:
   Adafruit_NMEA &operator=(const Adafruit_NMEA &other) = delete;
   void reset();
   nmea_frame_status_t feed(uint8_t byte, uint32_t receivedAtMs);
+  nmea_span_t lastText() const;
   nmea_sentence_t lastSentence() const;
   uint32_t sentenceStartedAt() const;
   uint32_t sentenceReceivedAt() const;
@@ -90,8 +92,8 @@ public:
                              size_t bodyLength);
 
 private:
-  char *_buffer;          ///< Buffer currently receiving bytes.
-  char *_lastBuffer;      ///< Buffer holding the latest complete line.
+  volatile char *_buffer;     ///< Buffer currently receiving bytes.
+  volatile char *_lastBuffer; ///< Buffer holding the latest complete line.
   size_t _capacity;       ///< Bytes per buffer, or zero for invalid storage.
   size_t _length;         ///< Current line length; zero while awaiting a start.
   size_t _lastLength;     ///< Latest complete line length, excluding NUL.