Merge pull request #33 from 107-systems/fix-always-complete-message-i…

…n-parse-buffer

Fix: Always have a complete NMEA message in buffer

extras/test/src/test_ArduinoNmeaParser.cpp

@@ -238,3 +238,10 @@ TEST_CASE("Multiple NMEA messages received", "[Parser-06]")
 
   REQUIRE(parser.error() == ArduinoNmeaParser::Error::None);
 }
+
+TEST_CASE("NMEA message with no checksum received", "[Parser-07]")
+{
+  ArduinoNmeaParser parser(nullptr, nullptr);
+  std::string const GPRMC = "79\r\n"; /* This should not lead to a segmentation violation. */
+  encode(parser, GPRMC);
+}

src/ArduinoNmeaParser.cpp

@@ -26,8 +26,8 @@
 ArduinoNmeaParser::ArduinoNmeaParser(OnRmcUpdateFunc on_rmc_update,
                                      OnGgaUpdateFunc on_gga_update)
 : _error{Error::None}
-, _parser_state{ParserState::Synching}
-, _parser_buf{{0}, 0}
+, _parser_buf{0}
+, _parser_buf_elems{0}
 , _rmc{nmea::INVALID_RMC}
 , _gga{nmea::INVALID_GGA}
 , _on_rmc_update{on_rmc_update}
@@ -42,15 +42,12 @@ ArduinoNmeaParser::ArduinoNmeaParser(OnRmcUpdateFunc on_rmc_update,
 
 void ArduinoNmeaParser::encode(char const c)
 {
-  /* Wait for the first '$' to be received which
-   * indicates the start of a NMEA message.
+  /* Flash the whole parser buffer every time we encounter
+   * a '$' sign. This way the parser buffer always starts
+   * with a valid NMEA message.
    */
-  if (_parser_state == ParserState::Synching) {
-    if (c == '$')
-      _parser_state = ParserState::Synced;
-    else
-      return;
-  }
+  if (c == '$')
+    flushParserBuffer();
 
   if (!isParseBufferFull())
     addToParserBuffer(c);
@@ -70,15 +67,15 @@ void ArduinoNmeaParser::encode(char const c)
   terminateParserBuffer();
 
   /* Verify if the checksum of the NMEA message is correct. */
-  if (!nmea::util::isChecksumOk(_parser_buf.buf)) {
+  if (!nmea::util::isChecksumOk(_parser_buf)) {
     _error = Error::Checksum;
     flushParserBuffer();
     return;
   }
 
   /* Parse the various NMEA messages. */
-  if      (nmea::util::rmc_isGxRMC(_parser_buf.buf)) parseGxRMC();
-  else if (nmea::util::rmc_isGxGGA(_parser_buf.buf)) parseGxGGA();
+  if      (nmea::util::rmc_isGxRMC(_parser_buf)) parseGxRMC();
+  else if (nmea::util::rmc_isGxGGA(_parser_buf)) parseGxGGA();
 
   /* The NMEA message has been fully processed and all
    * values updates so its time to flush the parser
@@ -93,29 +90,50 @@ void ArduinoNmeaParser::encode(char const c)
 
 bool ArduinoNmeaParser::isParseBufferFull()
 {
-  return (_parser_buf.elems_in_buf >= (NMEA_PARSE_BUFFER_SIZE - 1));
+  return (_parser_buf_elems >= (NMEA_PARSE_BUFFER_SIZE - 1));
 }
 
 void ArduinoNmeaParser::addToParserBuffer(char const c)
 {
-  _parser_buf.buf[_parser_buf.elems_in_buf] = c;
-  _parser_buf.elems_in_buf++;
+  _parser_buf[_parser_buf_elems] = c;
+  _parser_buf_elems++;
 }
 
 void ArduinoNmeaParser::flushParserBuffer()
 {
-  _parser_buf.elems_in_buf = 0;
+  _parser_buf_elems = 0;
 }
 
 bool ArduinoNmeaParser::isCompleteNmeaMessageInParserBuffer()
 {
-  if (_parser_buf.elems_in_buf < 8) /* $GPxxx\r\n = 8 */
+  /* Temporarily terminate string with a '\0' terminator in
+   * order to be able to use string library functions.
+   */
+  _parser_buf[_parser_buf_elems] = '\0';
+
+  /* Determine whether or not there exists a '$' marking
+   * the start of a NMEA message.
+   */
+  char const * nmea_start = strchr(_parser_buf, '$');
+  if (!nmea_start)
     return false;
 
-  char const prev_last = _parser_buf.buf[_parser_buf.elems_in_buf - 2];
-  char const      last = _parser_buf.buf[_parser_buf.elems_in_buf - 1];
+  /* Now that we've got a '$' marking the start of a NMEA
+   * message it is time to check if there's also an end
+   * to it.
+   */
+  char const * nmea_stop_cr = strchr(nmea_start, '\r');
+  if (!nmea_stop_cr)
+    return false;
 
-  return ((prev_last == '\r') && (last == '\n'));
+  char const * nmea_stop_lf = strchr(nmea_start, '\n');
+  if (!nmea_stop_lf)
+    return false;
+
+  /* Lastly: check if the LF is really directly following
+   * the CR within the NMEA message.
+   */
+  return ((nmea_stop_cr + 1) == nmea_stop_lf);
 }
 
 void ArduinoNmeaParser::terminateParserBuffer()
@@ -125,15 +143,15 @@ void ArduinoNmeaParser::terminateParserBuffer()
 
 void ArduinoNmeaParser::parseGxRMC()
 {
-  nmea::GxRMC::parse(_parser_buf.buf, _rmc);
+  nmea::GxRMC::parse(_parser_buf, _rmc);
 
   if (_on_rmc_update)
     _on_rmc_update(_rmc);
 }
 
 void ArduinoNmeaParser::parseGxGGA()
 {
-  nmea::GxGGA::parse(_parser_buf.buf, _gga);
+  nmea::GxGGA::parse(_parser_buf, _gga);
 
   if (_on_gga_update)
     _on_gga_update(_gga);

src/ArduinoNmeaParser.h

@@ -58,20 +58,9 @@ class ArduinoNmeaParser
 
   static size_t constexpr NMEA_PARSE_BUFFER_SIZE = 82 + 1; /* Leave space for the '\0' terminator */
 
-  typedef struct
-  {
-    char buf[NMEA_PARSE_BUFFER_SIZE];
-    size_t elems_in_buf;
-  } ParserBuffer;
-
-  enum class ParserState
-  {
-    Synching, Synced
-  };
-
   Error _error;
-  ParserState _parser_state;
-  ParserBuffer _parser_buf;
+  char _parser_buf[NMEA_PARSE_BUFFER_SIZE];
+  size_t _parser_buf_elems;
   nmea::RmcData _rmc;
   nmea::GgaData _gga;
   OnRmcUpdateFunc _on_rmc_update;