Fix WireProtocol Receive processing

- Update declaration of WP_ReceiveBytes to pass position argument by reference and remove return. - Update code and declarations on all platform implementations. - Remove unnused globals in WP Message. - Fix code in ReceiveState_ReadingHeader state. - Add cast to ReceiveState enum to make sure of correct type.
nanoframework · Aug 16, 2021 · e28718d · e28718d
1 parent acc6150
commit e28718d
Show file tree

Hide file tree

Showing 8 changed files with 34 additions and 105 deletions.
diff --git a/src/CLR/Include/WireProtocol.h b/src/CLR/Include/WireProtocol.h
@@ -58,13 +58,13 @@ typedef enum WP_Flags
 // enum with machine states for Wire Procotol receiver
 typedef enum ReceiveState
 {
-    ReceiveState_Idle             = 1,
-    ReceiveState_Initialize       = 2,
-    ReceiveState_WaitingForHeader = 3,
-    ReceiveState_ReadingHeader    = 4,
-    ReceiveState_CompleteHeader   = 5,
-    ReceiveState_ReadingPayload   = 6,
-    ReceiveState_CompletePayload  = 7,
+    ReceiveState_Idle             = (uint8_t)1,
+    ReceiveState_Initialize       = (uint8_t)2,
+    ReceiveState_WaitingForHeader = (uint8_t)3,
+    ReceiveState_ReadingHeader    = (uint8_t)4,
+    ReceiveState_CompleteHeader   = (uint8_t)5,
+    ReceiveState_ReadingPayload   = (uint8_t)6,
+    ReceiveState_CompletePayload  = (uint8_t)7,
 }ReceiveState;
 
 // enum with CLR monitor commands

diff --git a/src/CLR/Include/WireProtocol_HAL_Interface.h b/src/CLR/Include/WireProtocol_HAL_Interface.h
@@ -15,9 +15,8 @@
 ///
 /// @param ptr Pointer to the buffer that will hold the received bytes.
 /// @param size Number of bytes to read. On return it will have the number of bytes actually received.
-/// @return bool true if any bytes where received, false otherwise.
 ///
-uint8_t WP_ReceiveBytes(uint8_t *ptr, uint32_t *size);
+void WP_ReceiveBytes(uint8_t **ptr, uint32_t *size);
 
 ///
 /// @brief Sends a message through the Wire Protocol channel.

diff --git a/src/CLR/WireProtocol/WireProtocol_HAL_Interface.c b/src/CLR/WireProtocol/WireProtocol_HAL_Interface.c
@@ -16,13 +16,10 @@
 /////////////////////////////////////////////////////////////////////////////////////////////////
 
 // provided as weak to be replaced by actual implementation by HAL interface
-__nfweak uint8_t WP_ReceiveBytes(uint8_t *ptr, uint32_t *size)
+__nfweak void WP_ReceiveBytes(uint8_t **ptr, uint32_t *size)
 {
     (void)(ptr);
     (void)(size);
-
-    // default to false
-    return false;
 }
 
 // provided as weak to be replaced by actual implementation by HAL interface

diff --git a/src/CLR/WireProtocol/WireProtocol_Message.c b/src/CLR/WireProtocol/WireProtocol_Message.c
@@ -15,12 +15,10 @@
 
 static uint16_t _lastOutboundMessage = 65535;
 static uint64_t _receiveExpiryTicks;
-static uint8_t *_marker;
+static uint8_t _rxState;
 static uint8_t *_pos;
 static uint32_t _size;
-static uint8_t _rxState;
 static WP_Message _inboundMessage;
-static uint8_t *buf = (uint8_t *)&(_inboundMessage.m_header);
 
 #ifdef DEBUG
 volatile uint8_t _rxStatePrev;
@@ -111,10 +109,7 @@ void WP_Message_PrepareRequest(
     uint32_t payloadSize,
     uint8_t *payload)
 {
-    memcpy(
-        &message->m_header.m_signature,
-        _marker ? _marker : (uint8_t *)MARKER_PACKET_V1,
-        sizeof(message->m_header.m_signature));
+    memcpy(&message->m_header.m_signature, (uint8_t *)MARKER_PACKET_V1, sizeof(message->m_header.m_signature));
 
 #if defined(WP_IMPLEMENTS_CRC32)
     message->m_header.m_crcData = SUPPORT_ComputeCRC(payload, payloadSize, 0);
@@ -144,10 +139,7 @@ void WP_Message_PrepareReply(
     uint32_t payloadSize,
     uint8_t *payload)
 {
-    memcpy(
-        &message->m_header.m_signature,
-        _marker ? _marker : (uint8_t *)MARKER_PACKET_V1,
-        sizeof(message->m_header.m_signature));
+    memcpy(&message->m_header.m_signature, (uint8_t *)MARKER_PACKET_V1, sizeof(message->m_header.m_signature));
 
 #if defined(WP_IMPLEMENTS_CRC32)
     message->m_header.m_crcData = SUPPORT_ComputeCRC(payload, payloadSize, 0);
@@ -256,20 +248,14 @@ void WP_Message_Process()
             case ReceiveState_WaitingForHeader:
                 TRACE0(TRACE_STATE, "RxState==WaitForHeader\n");
 
-                if (!WP_ReceiveBytes(_pos, &_size))
-                {
-                    // didn't receive the expected amount of bytes, returning false
-                    TRACE0(TRACE_NODATA, "ReceiveBytes returned false - bailing out\n");
-
-                    return;
-                }
+                WP_ReceiveBytes(&_pos, &_size);
 
                 // Synch to the start of a message by looking for a valid MARKER
                 while (true)
                 {
                     len = sizeof(_inboundMessage.m_header) - _size;
 
-                    if (len == 0)
+                    if (len <= 0)
                     {
                         break;
                     }
@@ -285,18 +271,17 @@ void WP_Message_Process()
                         break;
                     }
 
-                    memmove(&buf[0], &buf[1], len - 1);
+                    // move buffer 1 position down
+                    memmove(
+                        (uint8_t *)&(_inboundMessage.m_header),
+                        ((uint8_t *)&(_inboundMessage.m_header) + 1),
+                        len - 1);
 
                     _pos--;
                     _size++;
                 }
 
-                if (_size == 0)
-                {
-                    // header reception completed
-                    _rxState = ReceiveState_CompleteHeader;
-                }
-                else if (len >= sizeof(_inboundMessage.m_header.m_signature))
+                if (len >= sizeof(_inboundMessage.m_header.m_signature))
                 {
                     // still missing some bytes for the header
                     _rxState = ReceiveState_ReadingHeader;
@@ -315,12 +300,7 @@ void WP_Message_Process()
 
                 if (_receiveExpiryTicks > now)
                 {
-                    if (!WP_ReceiveBytes(_pos, &_size))
-                    {
-                        // didn't receive the expected amount of bytes, returning false
-                        TRACE0(TRACE_NODATA, "ReceiveBytes returned false - bailing out\n");
-                        return;
-                    }
+                    WP_ReceiveBytes(&_pos, &_size);
 
                     if (_size == 0)
                     {
@@ -393,12 +373,7 @@ void WP_Message_Process()
 
                 if (_receiveExpiryTicks > now)
                 {
-                    if (!WP_ReceiveBytes(_pos, &_size))
-                    {
-                        // didn't receive the expected amount of bytes, returning false
-                        TRACE0(TRACE_NODATA, "ReceiveBytes returned false - bailing out\n");
-                        return;
-                    }
+                    WP_ReceiveBytes(&_pos, &_size);
 
                     if (_size == 0)
                     {

diff --git a/targets/ChibiOS/_common/WireProtocol_HAL_Interface.c b/targets/ChibiOS/_common/WireProtocol_HAL_Interface.c
@@ -18,7 +18,7 @@
 
 #if (HAL_USE_SERIAL_USB == TRUE)
 
-uint8_t WP_ReceiveBytes(uint8_t *ptr, uint32_t *size)
+void WP_ReceiveBytes(uint8_t **ptr, uint32_t *size)
 {
     // save for later comparison
     uint32_t requestedSize = *size;
@@ -28,28 +28,18 @@ uint8_t WP_ReceiveBytes(uint8_t *ptr, uint32_t *size)
     if (*size)
     {
         // read from serial stream with 100ms timeout
-        size_t read = chnReadTimeout(&SDU1, ptr, requestedSize, TIME_MS2I(100));
+        size_t read = chnReadTimeout(&SDU1, *ptr, requestedSize, TIME_MS2I(100));
 
-        // check if any bytes where read
-        if (read == 0)
-        {
-            return false;
-        }
-
-        ptr += read;
+        *ptr += read;
         *size -= read;
 
         TRACE(TRACE_STATE, "RXMSG: Expecting %d bytes, received %d.\n", requestedSize, read);
     }
-
-    return true;
 }
 #elif (HAL_USE_SERIAL == TRUE)
 
-uint8_t WP_ReceiveBytes(uint8_t *ptr, uint32_t *size)
+void WP_ReceiveBytes(uint8_t **ptr, uint32_t *size)
 {
-    volatile uint32_t read;
-
     // save for later comparison
     uint32_t requestedSize = *size;
     (void)requestedSize;
@@ -58,21 +48,13 @@ uint8_t WP_ReceiveBytes(uint8_t *ptr, uint32_t *size)
     if (*size)
     {
         // non blocking read from serial port with 100ms timeout
-        read = chnReadTimeout(&SERIAL_DRIVER, ptr, requestedSize, TIME_MS2I(20));
+        size_t read = chnReadTimeout(&SERIAL_DRIVER, *ptr, requestedSize, TIME_MS2I(100));
 
         TRACE(TRACE_STATE, "RXMSG: Expecting %d bytes, received %d.\n", requestedSize, read);
 
-        // check if any bytes where read
-        if (read == 0)
-        {
-            return false;
-        }
-
-        ptr += read;
+        *ptr += read;
         *size -= read;
     }
-
-    return true;
 }
 
 #else

diff --git a/targets/FreeRTOS/NXP/_common/WireProtocol_HAL_Interface.c b/targets/FreeRTOS/NXP/_common/WireProtocol_HAL_Interface.c
@@ -46,7 +46,7 @@ bool WP_Initialise()
     return WP_Port_Intitialised;
 }
 
-uint8_t WP_ReceiveBytes(uint8_t *ptr, uint32_t *size)
+void WP_ReceiveBytes(uint8_t **ptr, uint32_t *size)
 {
     // TODO: Initialise Port if not already done, Wire Protocol should be calling this directly at startup
     if (!WP_Port_Intitialised)
@@ -63,13 +63,7 @@ uint8_t WP_ReceiveBytes(uint8_t *ptr, uint32_t *size)
         size_t read = 0;
         LPUART_RTOS_Receive(&handle, ptr, requestedSize, &read);
 
-        // check if any bytes where read
-        if (read == 0)
-        {
-            return false;
-        }
-
-        ptr += read;
+        *ptr += read;
         *size -= read;
     }
 

diff --git a/targets/FreeRTOS_ESP32/ESP32_WROOM_32/common/WireProtocol_HAL_Interface.c b/targets/FreeRTOS_ESP32/ESP32_WROOM_32/common/WireProtocol_HAL_Interface.c
@@ -82,7 +82,7 @@ bool WP_Initialise(COM_HANDLE port)
     return true;
 }
 
-uint8_t WP_ReceiveBytes(uint8_t *ptr, uint32_t *size)
+void WP_ReceiveBytes(uint8_t **ptr, uint32_t *size)
 {
     // TODO: Initialise Port if not already done, Wire Protocol should be calling this directly at startup
     if (!WP_Port_Intitialised)
@@ -99,19 +99,7 @@ uint8_t WP_ReceiveBytes(uint8_t *ptr, uint32_t *size)
         // non blocking read from serial port with 100ms timeout
         size_t read = uart_read_bytes(WP_Port, ptr, (uint32_t)requestedSize, (TickType_t)100 / portTICK_PERIOD_MS);
 
-        // check if any bytes where read
-        if (read == 0)
-        {
-            return false;
-        }
-
-        // check if any bytes where read
-        if (read == 0)
-        {
-            return false;
-        }
-
-        ptr += read;
+        *ptr += read;
         *size -= read;
     }
 

diff --git a/targets/TI-SimpleLink/_common/WireProtocol_HAL_Interface.c b/targets/TI-SimpleLink/_common/WireProtocol_HAL_Interface.c
@@ -17,7 +17,7 @@
 
 UART2_Handle uart = NULL;
 
-uint8_t WP_ReceiveBytes(uint8_t *ptr, uint32_t *size)
+void WP_ReceiveBytes(uint8_t **ptr, uint32_t *size)
 {
     // save for later comparison
     uint32_t requestedSize = *size;
@@ -29,13 +29,7 @@ uint8_t WP_ReceiveBytes(uint8_t *ptr, uint32_t *size)
         // non blocking read from serial port with 500ms timeout
         UART2_readTimeout(uart, ptr, requestedSize, &read, UART_TIMEOUT_MILLISECONDS / Clock_tickPeriod);
 
-        // check if any bytes where read
-        if (read == 0)
-        {
-            return false;
-        }
-
-        ptr += read;
+        *ptr += read;
         *size -= read;
     }