Enforce stop-bit count detection strictly.

plerup · Nov 13, 2021 · d017e7e · d017e7e
1 parent 1a611e7
commit d017e7e
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 41 deletions.
diff --git a/library.json b/library.json
@@ -1,6 +1,6 @@
 {
     "name": "EspSoftwareSerial",
-    "version": "6.14.2",
+    "version": "6.15.0",
     "description": "Implementation of the Arduino software serial for ESP8266/ESP32.",
     "keywords": [
         "serial", "io", "softwareserial"

diff --git a/library.properties b/library.properties
@@ -1,5 +1,5 @@
 name=EspSoftwareSerial
-version=6.14.2
+version=6.15.0
 author=Dirk Kaar, Peter Lerup
 maintainer=Dirk Kaar <[email protected]>
 sentence=Implementation of the Arduino software serial for ESP8266/ESP32.

diff --git a/src/SoftwareSerial.cpp b/src/SoftwareSerial.cpp
@@ -469,8 +469,8 @@ void SoftwareSerial::rxBits() {
     // and there was also no next start bit yet, so one word may be pending.
     // Check that there was no new ISR data received in the meantime, inserting an
     // extraneous stop level bit out of sequence breaks rx.
-    if (m_rxCurBit >= -1 && m_rxCurBit < m_pduBits - m_stopBits) {
-        const uint32_t detectionCycles = (m_pduBits - m_stopBits - m_rxCurBit) * m_bitCycles;
+    if (m_rxCurBit >= -1 && m_rxCurBit < m_pduBits - 1) {
+        const uint32_t detectionCycles = (m_pduBits - 1 - m_rxCurBit) * m_bitCycles;
         if (!m_isrBuffer->available() && ESP.getCycleCount() - m_isrLastCycle > detectionCycles) {
             // Produce faux stop bit level, prevents start bit maldetection
             // cycle's LSB is repurposed for the level bit
@@ -479,8 +479,8 @@ void SoftwareSerial::rxBits() {
     }
 }
 
-void SoftwareSerial::rxBits(uint32_t isrCycle) {
-    bool level = (m_isrLastCycle & 1) ^ m_invert;
+void SoftwareSerial::rxBits(const uint32_t isrCycle) {
+    const bool level = (m_isrLastCycle & 1) ^ m_invert;
 
     // error introduced by edge value in LSB of isrCycle is negligible
     uint32_t cycles = isrCycle - m_isrLastCycle;
@@ -491,7 +491,7 @@ void SoftwareSerial::rxBits(uint32_t isrCycle) {
     while (bits > 0) {
         // start bit detection
         if (m_rxCurBit >= (m_pduBits - 1)) {
-            // leading edge of start bit
+            // leading edge of start bit?
             if (level) break;
             m_rxCurBit = -1;
             --bits;
@@ -514,44 +514,35 @@ void SoftwareSerial::rxBits(uint32_t isrCycle) {
             continue;
         }
         // stop bits
-        if (m_rxCurBit < (m_pduBits - m_stopBits - 1)) {
-            ++m_rxCurBit;
-            --bits;
-            continue;
-        }
-        if (m_rxCurBit == (m_pduBits - m_stopBits - 1)) {
-            // Store the received value in the buffer unless we have an overflow
-            // if not high stop bit level, discard word
-            if (level)
-            {
-                m_rxCurByte >>= (sizeof(uint8_t) * 8 - m_dataBits);
-                if (!m_buffer->push(m_rxCurByte)) {
-                    m_overflow = true;
-                }
-                else {
-                    if (m_parityBuffer)
+        // Store the received value in the buffer unless we have an overflow
+        // if not high stop bit level, discard word
+        if (bits >= static_cast<uint32_t>(m_pduBits - 1 - m_rxCurBit) && level) {
+            m_rxCurByte >>= (sizeof(uint8_t) * 8 - m_dataBits);
+            if (!m_buffer->push(m_rxCurByte)) {
+                m_overflow = true;
+            }
+            else {
+                if (m_parityBuffer)
+                {
+                    if (m_rxCurParity) {
+                        m_parityBuffer->pushpeek() |= m_parityInPos;
+                    }
+                    else {
+                        m_parityBuffer->pushpeek() &= ~m_parityInPos;
+                    }
+                    m_parityInPos <<= 1;
+                    if (!m_parityInPos)
                     {
-                        if (m_rxCurParity) {
-                            m_parityBuffer->pushpeek() |= m_parityInPos;
-                        }
-                        else {
-                            m_parityBuffer->pushpeek() &= ~m_parityInPos;
-                        }
-                        m_parityInPos <<= 1;
-                        if (!m_parityInPos)
-                        {
-                            m_parityBuffer->push();
-                            m_parityInPos = 1;
-                        }
+                        m_parityBuffer->push();
+                        m_parityInPos = 1;
                     }
                 }
             }
-            m_rxCurBit = m_pduBits;
-            // reset to 0 is important for masked bit logic
-            m_rxCurByte = 0;
-            m_rxCurParity = false;
-            break;
         }
+        m_rxCurBit = m_pduBits - 1;
+        // reset to 0 is important for masked bit logic
+        m_rxCurByte = 0;
+        m_rxCurParity = false;
         break;
     }
 }

diff --git a/src/SoftwareSerial.h b/src/SoftwareSerial.h
@@ -228,7 +228,7 @@ class SoftwareSerial : public Stream {
     void setRxGPIOPullUp();
     /* check m_rxValid that calling is safe */
     void rxBits();
-    void rxBits(uint32_t isrCycle);
+    void rxBits(const uint32_t isrCycle);
 
     static void rxBitISR(SoftwareSerial* self);
     static void rxBitSyncISR(SoftwareSerial* self);