From 3b447ede78970df04e08eeb1cc549a574a733667 Mon Sep 17 00:00:00 2001 From: Damien George Date: Tue, 4 Apr 2017 16:23:25 +1000 Subject: [PATCH] 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. --- stmhal/usbd_cdc_interface.c | 85 +++++++++++++------------------------ 1 file 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