stmhal/usbd_cdc_interface: Change CDC RX to use a circular buffer.

This should be a little more efficient (since we anyway scan the input
packet for the interrupt char), and it should also fix any non-atomic read
issues with the buffer state being changed during an interrupt.

Throughput tests show that RX rate is unchanged by this patch.

1 files changed, 30 insertions, 55 deletions

diff --git a/stmhal/usbd_cdc_interface.c b/stmhal/usbd_cdc_interface.c
index 647fe09e99..9410255152 100644
--- a/stmhal/usbd_cdc_interface.c
+++ b/stmhal/usbd_cdc_interface.c
@@ -61,7 +61,7 @@
 
 /* Private typedef -----------------------------------------------------------*/
 /* Private define ------------------------------------------------------------*/
-#define APP_RX_DATA_SIZE  1024 // I think this must be at least CDC_DATA_FS_OUT_PACKET_SIZE=64 (APP_RX_DATA_SIZE was 2048)
+#define APP_RX_DATA_SIZE  1024 // this must be 2 or greater, and a power of 2
 #define APP_TX_DATA_SIZE  1024 // I think this can be any value (was 2048)
 
 /* Private macro -------------------------------------------------------------*/
@@ -69,9 +69,10 @@
 
 static __IO uint8_t dev_is_connected = 0; // indicates if we are connected
 
-static uint8_t UserRxBuffer[APP_RX_DATA_SIZE]; // received data from USB OUT endpoint is stored in this buffer
-static uint16_t UserRxBufCur = 0; // points to next available character in UserRxBuffer
-static uint16_t UserRxBufLen = 0; // counts number of valid characters in UserRxBuffer
+static uint8_t cdc_rx_packet_buf[CDC_DATA_FS_MAX_PACKET_SIZE]; // received data from USB OUT endpoint is stored in this buffer
+static uint8_t cdc_rx_user_buf[APP_RX_DATA_SIZE]; // received data is buffered here until the user reads it
+static volatile uint16_t cdc_rx_buf_put = 0; // circular buffer index
+static uint16_t cdc_rx_buf_get = 0; // circular buffer index
 
 static uint8_t UserTxBuffer[APP_TX_DATA_SIZE]; // data for USB IN endpoind is stored in this buffer
 static uint16_t UserTxBufPtrIn = 0; // increment this pointer modulo APP_TX_DATA_SIZE when new data is available
@@ -141,10 +142,10 @@ static int8_t CDC_Itf_Init(void)
   
     /*##-5- Set Application Buffers ############################################*/
     USBD_CDC_SetTxBuffer(&hUSBDDevice, UserTxBuffer, 0);
-    USBD_CDC_SetRxBuffer(&hUSBDDevice, UserRxBuffer);
+    USBD_CDC_SetRxBuffer(&hUSBDDevice, cdc_rx_packet_buf);
 
-    UserRxBufCur = 0;
-    UserRxBufLen = 0;
+    cdc_rx_buf_put = 0;
+    cdc_rx_buf_get = 0;
   
     return (USBD_OK);
 }
@@ -313,7 +314,7 @@ void HAL_PCD_SOFCallback(PCD_HandleTypeDef *hpcd) {
   * @param  Buf: Buffer of data received
   * @param  Len: Number of data received (in bytes)
   * @retval Result of the opeartion: USBD_OK if all operations are OK else USBD_FAIL
-  * @note   The buffer we are passed here is just UserRxBuffer, so we are
+  * @note   The buffer we are passed here is just cdc_rx_packet_buf, so we are
   *         free to modify it.
   */
 static int8_t CDC_Itf_Receive(uint8_t* Buf, uint32_t *Len) {
@@ -322,54 +323,23 @@ static int8_t CDC_Itf_Receive(uint8_t* Buf, uint32_t *Len) {
     HAL_UART_Transmit_DMA(&UartHandle, Buf, *Len);
 #endif
 
-    // TODO improve this function to implement a circular buffer
-
-    // if we have processed all the characters, reset the buffer counters
-    if (UserRxBufCur > 0 && UserRxBufCur >= UserRxBufLen) {
-        memmove(UserRxBuffer, UserRxBuffer + UserRxBufLen, *Len);
-        UserRxBufCur = 0;
-        UserRxBufLen = 0;
-    }
-
-    uint32_t delta_len;
-
-    if (mp_interrupt_char == -1) {
-        // no special interrupt character
-        delta_len = *Len;
-
-    } else {
-        // filter out special interrupt character from the buffer
-        bool char_found = false;
-        uint8_t *dest = Buf;
-        uint8_t *src = Buf;
-        uint8_t *buf_top = Buf + *Len;
-        for (; src < buf_top; src++) {
-            if (*src == mp_interrupt_char) {
-                char_found = true;
-                // raise KeyboardInterrupt when interrupts are finished
-                pendsv_kbd_intr();
-            } else {
-                if (char_found) {
-                    *dest = *src;
-                }
-                dest++;
+    // copy the incoming data into the circular buffer
+    for (uint8_t *src = Buf, *top = Buf + *Len; src < top; ++src) {
+        if (mp_interrupt_char != -1 && *src == mp_interrupt_char) {
+            pendsv_kbd_intr();
+        } else {
+            uint16_t next_put = (cdc_rx_buf_put + 1) & (APP_RX_DATA_SIZE - 1);
+            if (next_put == cdc_rx_buf_get) {
+                // overflow, we just discard the rest of the chars
+                break;
             }
+            cdc_rx_user_buf[cdc_rx_buf_put] = *src;
+            cdc_rx_buf_put = next_put;
         }
-
-        // length of remaining characters
-        delta_len = dest - Buf;
-    }
-
-    if (UserRxBufLen + delta_len + CDC_DATA_FS_MAX_PACKET_SIZE > APP_RX_DATA_SIZE) {
-        // if we keep this data then the buffer can overflow on the next USB rx
-        // so we don't increment the length, and throw this data away
-    } else {
-        // data fits, leaving room for another CDC_DATA_FS_OUT_PACKET_SIZE
-        UserRxBufLen += delta_len;
     }
 
-    // initiate next USB packet transfer, to append to existing data in buffer
-    USBD_CDC_SetRxBuffer(&hUSBDDevice, UserRxBuffer + UserRxBufLen);
+    // initiate next USB packet transfer
+    USBD_CDC_SetRxBuffer(&hUSBDDevice, cdc_rx_packet_buf);
     USBD_CDC_ReceivePacket(&hUSBDDevice);
 
     return USBD_OK;
@@ -463,7 +433,11 @@ void USBD_CDC_TxAlways(const uint8_t *buf, uint32_t len) {
 
 // Returns number of bytes in the rx buffer.
 int USBD_CDC_RxNum(void) {
-    return UserRxBufLen - UserRxBufCur;
+    int32_t rx_waiting = (int32_t)cdc_rx_buf_put - (int32_t)cdc_rx_buf_get;
+    if (rx_waiting < 0) {
+        rx_waiting += APP_RX_DATA_SIZE;
+    }
+    return rx_waiting;
 }
 
 // timout in milliseconds.
@@ -473,7 +447,7 @@ int USBD_CDC_Rx(uint8_t *buf, uint32_t len, uint32_t timeout) {
     for (uint32_t i = 0; i < len; i++) {
         // Wait until we have at least 1 byte to read
         uint32_t start = HAL_GetTick();
-        while (UserRxBufLen == UserRxBufCur) {
+        while (cdc_rx_buf_put == cdc_rx_buf_get) {
             // Wraparound of tick is taken care of by 2's complement arithmetic.
             if (HAL_GetTick() - start >= timeout) {
                 // timeout
@@ -487,7 +461,8 @@ int USBD_CDC_Rx(uint8_t *buf, uint32_t len, uint32_t timeout) {
         }
 
         // Copy byte from device to user buffer
-        buf[i] = UserRxBuffer[UserRxBufCur++];
+        buf[i] = cdc_rx_user_buf[cdc_rx_buf_get];
+        cdc_rx_buf_get = (cdc_rx_buf_get + 1) & (APP_RX_DATA_SIZE - 1);
     }
 
     // Success, return number of bytes read