Skip to content

Commit e29b383

Browse files
committed
shared/tinyusb: Fix hang from new tx_overwritabe_if_not_connected flag.
This flag is in the main branch of TinyUSB, included in Espressif since their v0.18.0~3 component release (but it's not actually in TinyUSB v0.18.0 release). Setting the flag is needed for the USB device not to block waiting for space in the FIFO if the host is disconnected. Running mp_usbd_task() unconditionally in the loop may not be necessary, but avoids the risk of a race condition where tud_cdc_connected() returns true, the FIFO is in blocking mode, but the host has actually disconnected (it just hasn't been processed by TinyUSB task yet). This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton <angus@redyak.com.au>
1 parent bbd0a3a commit e29b383

File tree

2 files changed

+36
-18
lines changed

2 files changed

+36
-18
lines changed

shared/tinyusb/mp_usbd.h

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,18 @@
4343
// Initialise TinyUSB device.
4444
static inline void mp_usbd_init_tud(void) {
4545
tusb_init();
46-
tud_cdc_configure_fifo_t cfg = { .rx_persistent = 0, .tx_persistent = 1 };
46+
tud_cdc_configure_fifo_t cfg = { .rx_persistent = 0,
47+
.tx_persistent = 1,
48+
49+
// This config flag is unreleased in TinyUSB >v0.18.0
50+
// but included in Espressif's TinyUSB component since v0.18.0~3
51+
//
52+
// Versioning issue reported as
53+
// https://github.com/espressif/esp-usb/issues/236
54+
#if TUSB_VERSION_NUMBER > 1800 || defined(ESP_PLATFORM)
55+
.tx_overwritabe_if_not_connected = 1,
56+
#endif
57+
};
4758
tud_cdc_configure_fifo(&cfg);
4859
}
4960

shared/tinyusb/mp_usbd_cdc.c

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -98,32 +98,39 @@ mp_uint_t mp_usbd_cdc_tx_strn(const char *str, mp_uint_t len) {
9898
if (!tusb_inited()) {
9999
return 0;
100100
}
101+
mp_uint_t last_write = mp_hal_ticks_ms();
101102
size_t i = 0;
102-
while (i < len) {
103+
while (i < len && (mp_uint_t)(mp_hal_ticks_ms() - last_write) < MICROPY_HW_USB_CDC_TX_TIMEOUT) {
103104
uint32_t n = len - i;
104-
if (n > CFG_TUD_CDC_EP_BUFSIZE) {
105-
n = CFG_TUD_CDC_EP_BUFSIZE;
106-
}
107-
if (tud_cdc_connected()) {
108-
// If CDC port is connected but the buffer is full, wait for up to USC_CDC_TIMEOUT ms.
109-
mp_uint_t t0 = mp_hal_ticks_ms();
110-
while (n > tud_cdc_write_available() && (mp_uint_t)(mp_hal_ticks_ms() - t0) < MICROPY_HW_USB_CDC_TX_TIMEOUT) {
111-
mp_event_wait_ms(1);
112105

113-
// Explicitly run the USB stack as the scheduler may be locked (eg we
114-
// are in an interrupt handler), while there is data pending.
115-
mp_usbd_task();
116-
}
106+
if (tud_cdc_connected()) {
117107
// Limit write to available space in tx buffer when connected.
108+
// (Otherwise we always write to usb fifo, expecting it to overwrite
109+
// old data so it will have latest data buffered when host connects.)
118110
n = MIN(n, tud_cdc_write_available());
119-
if (n == 0) {
120-
break;
121-
}
122111
}
123-
// When not connected we always write to usb fifo, ensuring it has latest data.
112+
124113
uint32_t n2 = tud_cdc_write(str + i, n);
125114
tud_cdc_write_flush();
126115
i += n2;
116+
117+
if (i < len) {
118+
if (n2 > 0) {
119+
// reset the timeout each time we successfully write to the FIFO
120+
last_write = mp_hal_ticks_ms();
121+
}
122+
123+
if (tud_cdc_connected()) {
124+
// If we know we're connected then we can wait for host to make
125+
// more space
126+
mp_event_wait_ms(1);
127+
}
128+
129+
// Always explicitly run the USB stack as the scheduler may be
130+
// locked (eg we are in an interrupt handler), while there is data
131+
// or a state change pending.
132+
mp_usbd_task();
133+
}
127134
}
128135
return i;
129136
}

0 commit comments

Comments
 (0)