Skip to content

Commit 541cef9

Browse files
me-no-devme-no-dev
authored
[USB CDC] Fix data might not be transmitted until more is written (espressif#5652)
Depending on `tud_cdc_tx_complete_cb` can cause in some cases the last packet to not be transmitted until more data is written and flushed. It's a rare case, but if the other end is expecting those last bytes, transmission will hang. This PR also fixes debug output on CDC
1 parent 6dfaf6c commit 541cef9

File tree

2 files changed

+89
-55
lines changed

2 files changed

+89
-55
lines changed

‎cores/esp32/USBCDC.cpp‎

Lines changed: 82 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -62,63 +62,40 @@ void tud_cdc_rx_cb(uint8_t itf)
6262

6363
// Invoked when received send break
6464
void tud_cdc_send_break_cb(uint8_t itf, uint16_t duration_ms){
65-
//isr_log_v("itf: %u, duration_ms: %u", itf, duration_ms);
65+
//log_v("itf: %u, duration_ms: %u", itf, duration_ms);
6666
}
6767

6868
// Invoked when space becomes available in TX buffer
6969
void tud_cdc_tx_complete_cb(uint8_t itf){
70-
if(itf < MAX_USB_CDC_DEVICES && devices[itf] != NULL && devices[itf]->tx_sem != NULL){
71-
xSemaphoreGive(devices[itf]->tx_sem);
70+
if(itf < MAX_USB_CDC_DEVICES && devices[itf] != NULL){
7271
devices[itf]->_onTX();
7372
}
7473
}
7574

76-
static size_t tinyusb_cdc_write(uint8_t itf, const uint8_t *buffer, size_t size){
77-
if(itf >= MAX_USB_CDC_DEVICES || devices[itf] == NULL || devices[itf]->tx_sem == NULL){
78-
return 0;
75+
static void ARDUINO_ISR_ATTR cdc0_write_char(char c){
76+
if(devices[0] != NULL){
77+
devices[0]->write(c);
7978
}
80-
if(!tud_cdc_n_connected(itf)){
81-
return 0;
82-
}
83-
size_t tosend = size, sofar = 0;
84-
while(tosend){
85-
uint32_t space = tud_cdc_n_write_available(itf);
86-
if(!space){
87-
//make sure that we do not get previous semaphore
88-
xSemaphoreTake(devices[itf]->tx_sem, 0);
89-
//wait for tx_complete
90-
if(xSemaphoreTake(devices[itf]->tx_sem, 200 / portTICK_PERIOD_MS) == pdTRUE){
91-
space = tud_cdc_n_write_available(itf);
92-
}
93-
if(!space){
94-
return sofar;
95-
}
96-
}
97-
if(tosend < space){
98-
space = tosend;
99-
}
100-
uint32_t sent = tud_cdc_n_write(itf, buffer + sofar, space);
101-
if(!sent){
102-
return sofar;
103-
}
104-
sofar += sent;
105-
tosend -= sent;
106-
tud_cdc_n_write_flush(itf);
107-
//xSemaphoreTake(devices[itf]->tx_sem, portMAX_DELAY);
108-
}
109-
return sofar;
110-
}
111-
112-
static void ARDUINO_ISR_ATTR cdc0_write_char(char c)
113-
{
114-
tinyusb_cdc_write(0, (const uint8_t *)&c, 1);
11579
}
11680

11781
static void usb_unplugged_cb(void* arg, esp_event_base_t event_base, int32_t event_id, void* event_data){
11882
((USBCDC*)arg)->_onUnplugged();
11983
}
12084

121-
USBCDC::USBCDC(uint8_t itfn) : itf(itfn), bit_rate(0), stop_bits(0), parity(0), data_bits(0), dtr(false), rts(false), connected(false), reboot_enable(true), rx_queue(NULL), tx_sem(NULL) {
85+
USBCDC::USBCDC(uint8_t itfn)
86+
: itf(itfn)
87+
, bit_rate(0)
88+
, stop_bits(0)
89+
, parity(0)
90+
, data_bits(0)
91+
, dtr(false)
92+
, rts(false)
93+
, connected(false)
94+
, reboot_enable(true)
95+
, rx_queue(NULL)
96+
, tx_lock(NULL)
97+
, tx_timeout_ms(250)
98+
{
12299
tinyusb_enable_interface(USB_INTERFACE_CDC, TUD_CDC_DESC_LEN, load_cdc_descriptor);
123100
if(itf < MAX_USB_CDC_DEVICES){
124101
arduino_usb_event_handler_register_with(ARDUINO_USB_EVENTS, ARDUINO_USB_STOPPED_EVENT, usb_unplugged_cb, this);
@@ -153,9 +130,8 @@ size_t USBCDC::setRxBufferSize(size_t rx_queue_len){
153130

154131
void USBCDC::begin(unsigned long baud)
155132
{
156-
if(tx_sem == NULL){
157-
tx_sem = xSemaphoreCreateBinary();
158-
xSemaphoreTake(tx_sem, 0);
133+
if(tx_lock == NULL) {
134+
tx_lock = xSemaphoreCreateMutex();
159135
}
160136
setRxBufferSize(256);//default if not preset
161137
devices[itf] = this;
@@ -166,12 +142,15 @@ void USBCDC::end()
166142
connected = false;
167143
devices[itf] = NULL;
168144
setRxBufferSize(0);
169-
if (tx_sem != NULL) {
170-
vSemaphoreDelete(tx_sem);
171-
tx_sem = NULL;
145+
if(tx_lock != NULL) {
146+
vSemaphoreDelete(tx_lock);
172147
}
173148
}
174149

150+
void USBCDC::setTxTimeoutMs(uint32_t timeout){
151+
tx_timeout_ms = timeout;
152+
}
153+
175154
void USBCDC::_onUnplugged(void){
176155
if(connected){
177156
connected = false;
@@ -336,23 +315,73 @@ size_t USBCDC::read(uint8_t *buffer, size_t size)
336315

337316
void USBCDC::flush(void)
338317
{
339-
if(itf >= MAX_USB_CDC_DEVICES || tx_sem == NULL){
318+
if(itf >= MAX_USB_CDC_DEVICES || tx_lock == NULL || !tud_cdc_n_connected(itf)){
319+
return;
320+
}
321+
if(xSemaphoreTake(tx_lock, tx_timeout_ms / portTICK_PERIOD_MS) != pdPASS){
340322
return;
341323
}
342324
tud_cdc_n_write_flush(itf);
325+
xSemaphoreGive(tx_lock);
343326
}
344327

345328
int USBCDC::availableForWrite(void)
346329
{
347-
if(itf >= MAX_USB_CDC_DEVICES || tx_sem == NULL){
348-
return -1;
330+
if(itf >= MAX_USB_CDC_DEVICES || tx_lock == NULL || !tud_cdc_n_connected(itf)){
331+
return 0;
349332
}
350-
return tud_cdc_n_write_available(itf);
333+
if(xSemaphoreTake(tx_lock, tx_timeout_ms / portTICK_PERIOD_MS) != pdPASS){
334+
return 0;
335+
}
336+
size_t a = tud_cdc_n_write_available(itf);
337+
xSemaphoreGive(tx_lock);
338+
return a;
351339
}
352340

353341
size_t USBCDC::write(const uint8_t *buffer, size_t size)
354342
{
355-
return tinyusb_cdc_write(itf, buffer, size);
343+
if(itf >= MAX_USB_CDC_DEVICES || tx_lock == NULL || buffer == NULL || size == 0 || !tud_cdc_n_connected(itf)){
344+
return 0;
345+
}
346+
if(xPortInIsrContext()){
347+
BaseType_t taskWoken = false;
348+
if(xSemaphoreTakeFromISR(tx_lock, &taskWoken) != pdPASS){
349+
return 0;
350+
}
351+
} else if(xSemaphoreTake(tx_lock, tx_timeout_ms / portTICK_PERIOD_MS) != pdPASS){
352+
return 0;
353+
}
354+
size_t to_send = size, so_far = 0;
355+
while(to_send){
356+
if(!tud_cdc_n_connected(itf)){
357+
size = so_far;
358+
break;
359+
}
360+
size_t space = tud_cdc_n_write_available(itf);
361+
if(!space){
362+
tud_cdc_n_write_flush(itf);
363+
continue;
364+
}
365+
if(space > to_send){
366+
space = to_send;
367+
}
368+
size_t sent = tud_cdc_n_write(itf, buffer+so_far, space);
369+
if(sent){
370+
so_far += sent;
371+
to_send -= sent;
372+
tud_cdc_n_write_flush(itf);
373+
} else {
374+
size = so_far;
375+
break;
376+
}
377+
}
378+
if(xPortInIsrContext()){
379+
BaseType_t taskWoken = false;
380+
xSemaphoreGiveFromISR(tx_lock, &taskWoken);
381+
} else {
382+
xSemaphoreGive(tx_lock);
383+
}
384+
return size;
356385
}
357386

358387
size_t USBCDC::write(uint8_t c)

‎cores/esp32/USBCDC.h‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@
1818

1919
#include <inttypes.h>
2020
#include "esp_event.h"
21+
#include "freertos/FreeRTOS.h"
22+
#include "freertos/queue.h"
23+
#include "freertos/semphr.h"
2124
#include "Stream.h"
2225

2326
ESP_EVENT_DECLARE_BASE(ARDUINO_USB_CDC_EVENTS);
@@ -58,7 +61,8 @@ class USBCDC: public Stream
5861
void onEvent(esp_event_handler_t callback);
5962
void onEvent(arduino_usb_cdc_event_t event, esp_event_handler_t callback);
6063

61-
size_t setRxBufferSize(size_t);
64+
size_t setRxBufferSize(size_t size);
65+
void setTxTimeoutMs(uint32_t timeout);
6266
void begin(unsigned long baud=0);
6367
void end();
6468

@@ -113,7 +117,6 @@ class USBCDC: public Stream
113117
void _onRX(void);
114118
void _onTX(void);
115119
void _onUnplugged(void);
116-
xSemaphoreHandle tx_sem;
117120

118121
protected:
119122
uint8_t itf;
@@ -126,6 +129,8 @@ class USBCDC: public Stream
126129
bool connected;
127130
bool reboot_enable;
128131
xQueueHandle rx_queue;
132+
xSemaphoreHandle tx_lock;
133+
uint32_t tx_timeout_ms;
129134

130135
};
131136

0 commit comments

Comments
 (0)