From 620a3c3be4bc171ee9d03ce093f2bf06b2baedfb Mon Sep 17 00:00:00 2001 From: Bruce Luckcuck Date: Sat, 9 Feb 2019 21:00:10 -0500 Subject: [PATCH] VCP HAL transmit buffer fixes Fixes issues that could cause the tail of the ring buffer to be moved before data transmission is complete thus allowing the head to overwrite unsent data. Also adds 512 byte blocking when queueing the output data with the endpoint (helps with slow receivers). Fixes a bug in the transmit buffer logic if the TX and RX buffers were different sizes. --- src/main/vcp_hal/usbd_cdc_interface.c | 64 +++++++++++++++------------ src/main/vcp_hal/usbd_cdc_interface.h | 2 +- 2 files changed, 36 insertions(+), 30 deletions(-) diff --git a/src/main/vcp_hal/usbd_cdc_interface.c b/src/main/vcp_hal/usbd_cdc_interface.c index 4d39821882..529a327ce2 100644 --- a/src/main/vcp_hal/usbd_cdc_interface.c +++ b/src/main/vcp_hal/usbd_cdc_interface.c @@ -61,6 +61,8 @@ #define APP_RX_DATA_SIZE 2048 #define APP_TX_DATA_SIZE 2048 +#define APP_TX_BLOCK_SIZE 512 + /* Private macro -------------------------------------------------------------*/ /* Private variables ---------------------------------------------------------*/ USBD_CDC_LineCodingTypeDef LineCoding = @@ -241,35 +243,42 @@ static int8_t CDC_Itf_Control (uint8_t cmd, uint8_t* pbuf, uint16_t length) */ void HAL_TIM_PeriodElapsedCallback(TIM_HandleTypeDef *htim) { - if (htim->Instance != TIMusb) return; + if (htim->Instance != TIMusb) { + return; + } - uint32_t buffptr; uint32_t buffsize; + static uint32_t lastBuffsize = 0; - if (UserTxBufPtrOut != UserTxBufPtrIn) - { - if (UserTxBufPtrOut > UserTxBufPtrIn) /* Roll-back */ - { - buffsize = APP_RX_DATA_SIZE - UserTxBufPtrOut; - } - else - { - buffsize = UserTxBufPtrIn - UserTxBufPtrOut; - } + USBD_CDC_HandleTypeDef *hcdc = (USBD_CDC_HandleTypeDef*)USBD_Device.pCDC_ClassData; - buffptr = UserTxBufPtrOut; - - USBD_CDC_SetTxBuffer(&USBD_Device, (uint8_t*)&UserTxBuffer[buffptr], buffsize); - - if (USBD_CDC_TransmitPacket(&USBD_Device) == USBD_OK) - { - UserTxBufPtrOut += buffsize; - if (UserTxBufPtrOut == APP_TX_DATA_SIZE) - { + if (hcdc->TxState == 0) { + // endpoint has finished transmitting previous block + if (lastBuffsize) { + // move the ring buffer tail based on the previous succesful transmission + UserTxBufPtrOut += lastBuffsize; + if (UserTxBufPtrOut == APP_TX_DATA_SIZE) { UserTxBufPtrOut = 0; } + lastBuffsize = 0; } - } + if (UserTxBufPtrOut != UserTxBufPtrIn) { + if (UserTxBufPtrOut > UserTxBufPtrIn) { /* Roll-back */ + buffsize = APP_TX_DATA_SIZE - UserTxBufPtrOut; + } else { + buffsize = UserTxBufPtrIn - UserTxBufPtrOut; + } + if (buffsize > APP_TX_BLOCK_SIZE) { + buffsize = APP_TX_BLOCK_SIZE; + } + + USBD_CDC_SetTxBuffer(&USBD_Device, (uint8_t*)&UserTxBuffer[UserTxBufPtrOut], buffsize); + + if (USBD_CDC_TransmitPacket(&USBD_Device) == USBD_OK) { + lastBuffsize = buffsize; + } + } + } } /** @@ -379,16 +388,13 @@ uint32_t CDC_Send_FreeBytes(void) */ uint32_t CDC_Send_DATA(const uint8_t *ptrBuffer, uint32_t sendLength) { - USBD_CDC_HandleTypeDef *hcdc = (USBD_CDC_HandleTypeDef*)USBD_Device.pCDC_ClassData; - while (hcdc->TxState != 0); - - for (uint32_t i = 0; i < sendLength; i++) - { - UserTxBuffer[UserTxBufPtrIn] = ptrBuffer[i]; - UserTxBufPtrIn = (UserTxBufPtrIn + 1) % APP_TX_DATA_SIZE; + for (uint32_t i = 0; i < sendLength; i++) { while (CDC_Send_FreeBytes() == 0) { + // block until there is free space in the ring buffer delay(1); } + UserTxBuffer[UserTxBufPtrIn] = ptrBuffer[i]; + UserTxBufPtrIn = (UserTxBufPtrIn + 1) % APP_TX_DATA_SIZE; } return sendLength; } diff --git a/src/main/vcp_hal/usbd_cdc_interface.h b/src/main/vcp_hal/usbd_cdc_interface.h index 3674dc410f..f7f0aa20f6 100644 --- a/src/main/vcp_hal/usbd_cdc_interface.h +++ b/src/main/vcp_hal/usbd_cdc_interface.h @@ -65,7 +65,7 @@ /* Periodically, the state of the buffer "UserTxBuffer" is checked. The period depends on CDC_POLLING_INTERVAL */ -#define CDC_POLLING_INTERVAL 10 /* in ms. The max is 65 and the min is 1 */ +#define CDC_POLLING_INTERVAL 5 /* in ms. The max is 65 and the min is 1 */ /* Exported typef ------------------------------------------------------------*/ /* The following structures groups all needed parameters to be configured for the