Skip to content

Commit b7af090

Browse files
SuGliderlucasssvaz
andauthored
Fixes the hardware cdc jtag plugged/unplugged status and related timeout/delay (#9275)
* feat(hw_cdc):fixes the hardware cdc jtag plugged/unplugged status This will use a new IDF 5.1 feature to detect if the USB HW CDC is plugged or not. This can be checked testing HWCDCSerial. It also fixes issues related to timeout or delays while writing to the HW Serial when USB is unplugged. * feat(usb): Creates HWSerial_Events.ino example * feat: adds .skip.esp32 Skips the ESP32 SoC test given that it has no USB * feat: adds .skip.esp32s2 Skips the ESP32S2 because it has no HW CDC JTAG interface * fix: fixes issues with Ubuntu CI Only compiles the example in case it is using Hardware CD and JTAG mode. * feat(serialcdc): non block functions modifies write and flush to do not clock in case CDC host is not connected to the CDC client from the C3/S3/C6/H2 * fix(HWCDC): changes made demands testing for CDC ON BOOT * feat(hwcdc): Improves HWSerial_Events.ino Improves the example by adding more information about USB being plugged and CDC being connected. * feat(hwcdc): solves CDC connection issue Detects correctly when CDC is or not connected. Deals with USB unplugged while the sketch is printing to CDD. * Update cores/esp32/HWCDC.cpp * Update cores/esp32/HWCDC.cpp * Update cores/esp32/HWCDC.cpp * Update cores/esp32/HWCDC.cpp * Update cores/esp32/HWCDC.cpp * Update cores/esp32/HWCDC.cpp * Update cores/esp32/HWCDC.cpp * Update cores/esp32/HWCDC.cpp * Update cores/esp32/HWCDC.cpp * Update cores/esp32/HWCDC.cpp * Update cores/esp32/HWCDC.cpp * Update cores/esp32/HWCDC.cpp * Update cores/esp32/HWCDC.cpp * Update cores/esp32/HWCDC.cpp * Update cores/esp32/HWCDC.cpp * Update cores/esp32/HWCDC.cpp * Apply suggestions from code review --------- Co-authored-by: Lucas Saavedra Vaz <[email protected]>
1 parent c4ad3b7 commit b7af090

File tree

4 files changed

+197
-38
lines changed

4 files changed

+197
-38
lines changed

Diff for: cores/esp32/HWCDC.cpp

+101-38
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2015-2020 Espressif Systems (Shanghai) PTE LTD
1+
// Copyright 2015-2024 Espressif Systems (Shanghai) PTE LTD
22
//
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.
@@ -28,19 +28,19 @@
2828
#include "hal/usb_serial_jtag_ll.h"
2929
#pragma GCC diagnostic warning "-Wvolatile"
3030
#include "rom/ets_sys.h"
31+
#include "driver/usb_serial_jtag.h"
3132

3233
ESP_EVENT_DEFINE_BASE(ARDUINO_HW_CDC_EVENTS);
3334

3435
static RingbufHandle_t tx_ring_buf = NULL;
3536
static QueueHandle_t rx_queue = NULL;
3637
static uint8_t rx_data_buf[64] = {0};
3738
static intr_handle_t intr_handle = NULL;
38-
static volatile bool initial_empty = false;
3939
static SemaphoreHandle_t tx_lock = NULL;
40+
static volatile bool isConnected = false;
4041

41-
// workaround for when USB CDC is not connected
42-
static uint32_t tx_timeout_ms = 0;
43-
static bool tx_timeout_change_request = false;
42+
// timeout has no effect when USB CDC is unplugged
43+
static uint32_t requested_tx_timeout_ms = 100;
4444

4545
static esp_event_loop_handle_t arduino_hw_cdc_event_loop_handle = NULL;
4646

@@ -78,21 +78,17 @@ static void hw_cdc_isr_handler(void *arg) {
7878

7979
if (usbjtag_intr_status & USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY) {
8080
// Interrupt tells us the host picked up the data we sent.
81+
if(!usb_serial_jtag_is_connected()) {
82+
isConnected = false;
83+
usb_serial_jtag_ll_clr_intsts_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY);
84+
// USB is unplugged, nothing to be done here
85+
return;
86+
} else {
87+
isConnected = true;
88+
}
8189
if (usb_serial_jtag_ll_txfifo_writable() == 1) {
8290
// We disable the interrupt here so that the interrupt won't be triggered if there is no data to send.
8391
usb_serial_jtag_ll_disable_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY);
84-
if(!initial_empty){
85-
initial_empty = true;
86-
// First time USB is plugged and the application has not explicitly set TX Timeout, set it to default 100ms.
87-
// Otherwise, USB is still unplugged and the timeout will be kept as Zero in order to avoid any delay in the
88-
// application whenever it uses write() and the TX Queue gets full.
89-
if (!tx_timeout_change_request) {
90-
tx_timeout_ms = 100;
91-
}
92-
//send event?
93-
//ets_printf("CONNECTED\n");
94-
arduino_hw_cdc_event_post(ARDUINO_HW_CDC_EVENTS, ARDUINO_HW_CDC_CONNECTED_EVENT, &event, sizeof(arduino_hw_cdc_event_data_t), &xTaskWoken);
95-
}
9692
size_t queued_size;
9793
uint8_t *queued_buff = (uint8_t *)xRingbufferReceiveUpToFromISR(tx_ring_buf, &queued_size, 64);
9894
// If the hardware fifo is avaliable, write in it. Otherwise, do nothing.
@@ -102,7 +98,7 @@ static void hw_cdc_isr_handler(void *arg) {
10298
usb_serial_jtag_ll_write_txfifo(queued_buff, queued_size);
10399
usb_serial_jtag_ll_txfifo_flush();
104100
vRingbufferReturnItemFromISR(tx_ring_buf, queued_buff, &xTaskWoken);
105-
usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY);
101+
if(isConnected) usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY);
106102
//send event?
107103
//ets_printf("TX:%u\n", queued_size);
108104
event.tx.len = queued_size;
@@ -124,18 +120,15 @@ static void hw_cdc_isr_handler(void *arg) {
124120
break;
125121
}
126122
}
127-
//send event?
128-
//ets_printf("RX:%u/%u\n", i, rx_fifo_len);
129123
event.rx.len = i;
130124
arduino_hw_cdc_event_post(ARDUINO_HW_CDC_EVENTS, ARDUINO_HW_CDC_RX_EVENT, &event, sizeof(arduino_hw_cdc_event_data_t), &xTaskWoken);
125+
isConnected = true;
131126
}
132127

133128
if (usbjtag_intr_status & USB_SERIAL_JTAG_INTR_BUS_RESET) {
134129
usb_serial_jtag_ll_clr_intsts_mask(USB_SERIAL_JTAG_INTR_BUS_RESET);
135-
initial_empty = false;
136-
usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY);
137-
//ets_printf("BUS_RESET\n");
138130
arduino_hw_cdc_event_post(ARDUINO_HW_CDC_EVENTS, ARDUINO_HW_CDC_BUS_RESET_EVENT, &event, sizeof(arduino_hw_cdc_event_data_t), &xTaskWoken);
131+
isConnected = false;
139132
}
140133

141134
if (xTaskWoken == pdTRUE) {
@@ -144,12 +137,16 @@ static void hw_cdc_isr_handler(void *arg) {
144137
}
145138

146139
static void ARDUINO_ISR_ATTR cdc0_write_char(char c) {
140+
uint32_t tx_timeout_ms = 0;
141+
if(usb_serial_jtag_is_connected()) {
142+
tx_timeout_ms = requested_tx_timeout_ms;
143+
}
147144
if(xPortInIsrContext()){
148145
xRingbufferSendFromISR(tx_ring_buf, (void*) (&c), 1, NULL);
149146
} else {
150147
xRingbufferSend(tx_ring_buf, (void*) (&c), 1, tx_timeout_ms / portTICK_PERIOD_MS);
151148
}
152-
usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY);
149+
usb_serial_jtag_ll_txfifo_flush();
153150
}
154151

155152
HWCDC::HWCDC() {
@@ -160,9 +157,33 @@ HWCDC::~HWCDC(){
160157
end();
161158
}
162159

160+
161+
// It should return <true> just when USB is plugged and CDC is connected.
163162
HWCDC::operator bool() const
164163
{
165-
return initial_empty;
164+
static bool running = false;
165+
166+
// USB may be unplugged
167+
if (usb_serial_jtag_is_connected() == false) {
168+
isConnected = false;
169+
running = false;
170+
return false;
171+
}
172+
173+
if (isConnected) {
174+
running = false;
175+
return true;
176+
}
177+
178+
if (running == false && !isConnected) { // enables it only once!
179+
usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY);
180+
}
181+
// this will feed CDC TX FIFO to trigger IN_EMPTY
182+
uint8_t c = '\0';
183+
usb_serial_jtag_ll_write_txfifo(&c, sizeof(c));
184+
usb_serial_jtag_ll_txfifo_flush();
185+
running = true;
186+
return false;
166187
}
167188

168189
void HWCDC::onEvent(esp_event_handler_t callback){
@@ -206,9 +227,9 @@ void HWCDC::begin(unsigned long baud)
206227
log_e("HW CDC RX Buffer error");
207228
}
208229
}
209-
//TX Buffer default has 256 bytes if not preset
230+
//TX Buffer default has 16 bytes if not preset
210231
if (tx_ring_buf == NULL) {
211-
if (!setTxBufferSize(256)) {
232+
if (!setTxBufferSize(16)) {
212233
log_e("HW CDC TX Buffer error");
213234
}
214235
}
@@ -227,8 +248,6 @@ void HWCDC::begin(unsigned long baud)
227248
} else {
228249
log_e("Serial JTAG Pins can't be set into Peripheral Manager.");
229250
}
230-
231-
usb_serial_jtag_ll_txfifo_flush();
232251
}
233252

234253
void HWCDC::end()
@@ -248,13 +267,11 @@ void HWCDC::end()
248267
arduino_hw_cdc_event_loop_handle = NULL;
249268
}
250269
HWCDC::deinit(this);
270+
isConnected = false;
251271
}
252272

253273
void HWCDC::setTxTimeoutMs(uint32_t timeout){
254-
tx_timeout_ms = timeout;
255-
// it registers that the user has explicitly requested to use a value as TX timeout
256-
// used for the workaround with unplugged USB and TX Queue Full that causes a delay on every write()
257-
tx_timeout_change_request = true;
274+
requested_tx_timeout_ms = timeout;
258275
}
259276

260277
/*
@@ -278,9 +295,13 @@ size_t HWCDC::setTxBufferSize(size_t tx_queue_len){
278295

279296
int HWCDC::availableForWrite(void)
280297
{
298+
uint32_t tx_timeout_ms = 0;
281299
if(tx_ring_buf == NULL || tx_lock == NULL){
282300
return 0;
283301
}
302+
if(usb_serial_jtag_is_connected()) {
303+
tx_timeout_ms = requested_tx_timeout_ms;
304+
}
284305
if(xSemaphoreTake(tx_lock, tx_timeout_ms / portTICK_PERIOD_MS) != pdPASS){
285306
return 0;
286307
}
@@ -289,11 +310,32 @@ int HWCDC::availableForWrite(void)
289310
return a;
290311
}
291312

313+
static void flushTXBuffer()
314+
{
315+
if (!tx_ring_buf) return;
316+
UBaseType_t uxItemsWaiting = 0;
317+
vRingbufferGetInfo(tx_ring_buf, NULL, NULL, NULL, NULL, &uxItemsWaiting);
318+
319+
size_t queued_size = 0;
320+
uint8_t *queued_buff = (uint8_t *)xRingbufferReceiveUpTo(tx_ring_buf, &queued_size, 0, uxItemsWaiting);
321+
if (queued_size && queued_buff != NULL) {
322+
vRingbufferReturnItem(tx_ring_buf, (void *)queued_buff);
323+
}
324+
// flushes CDC FIFO
325+
usb_serial_jtag_ll_txfifo_flush();
326+
}
327+
292328
size_t HWCDC::write(const uint8_t *buffer, size_t size)
293329
{
330+
uint32_t tx_timeout_ms = 0;
294331
if(buffer == NULL || size == 0 || tx_ring_buf == NULL || tx_lock == NULL){
295332
return 0;
296333
}
334+
if(usb_serial_jtag_is_connected()) {
335+
tx_timeout_ms = requested_tx_timeout_ms;
336+
} else {
337+
isConnected = false;
338+
}
297339
if(xSemaphoreTake(tx_lock, tx_timeout_ms / portTICK_PERIOD_MS) != pdPASS){
298340
return 0;
299341
}
@@ -311,8 +353,9 @@ size_t HWCDC::write(const uint8_t *buffer, size_t size)
311353
to_send -= space;
312354
so_far += space;
313355
// Now trigger the ISR to read data from the ring buffer.
314-
usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY);
315-
356+
usb_serial_jtag_ll_txfifo_flush();
357+
if(isConnected) usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY);
358+
316359
while(to_send){
317360
if(max_size > to_send){
318361
max_size = to_send;
@@ -325,9 +368,15 @@ size_t HWCDC::write(const uint8_t *buffer, size_t size)
325368
so_far += max_size;
326369
to_send -= max_size;
327370
// Now trigger the ISR to read data from the ring buffer.
328-
usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY);
371+
usb_serial_jtag_ll_txfifo_flush();
372+
if(isConnected) usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY);
329373
}
330374
}
375+
// CDC is diconnected ==> flush all data from TX buffer
376+
if(to_send && !usb_serial_jtag_ll_txfifo_writable()) {
377+
isConnected = false;
378+
flushTXBuffer();
379+
}
331380
xSemaphoreGive(tx_lock);
332381
return size;
333382
}
@@ -339,21 +388,35 @@ size_t HWCDC::write(uint8_t c)
339388

340389
void HWCDC::flush(void)
341390
{
391+
uint32_t tx_timeout_ms = 0;
342392
if(tx_ring_buf == NULL || tx_lock == NULL){
343393
return;
344394
}
395+
if(usb_serial_jtag_is_connected()) {
396+
tx_timeout_ms = requested_tx_timeout_ms;
397+
} else {
398+
isConnected = false;
399+
}
345400
if(xSemaphoreTake(tx_lock, tx_timeout_ms / portTICK_PERIOD_MS) != pdPASS){
346401
return;
347402
}
348403
UBaseType_t uxItemsWaiting = 0;
349404
vRingbufferGetInfo(tx_ring_buf, NULL, NULL, NULL, NULL, &uxItemsWaiting);
350405
if(uxItemsWaiting){
351406
// Now trigger the ISR to read data from the ring buffer.
352-
usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY);
407+
usb_serial_jtag_ll_txfifo_flush();
408+
if(isConnected) usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY);
353409
}
354-
while(uxItemsWaiting){
410+
uint8_t tries = 3;
411+
while(tries && uxItemsWaiting){
355412
delay(5);
413+
UBaseType_t lastUxItemsWaiting = uxItemsWaiting;
356414
vRingbufferGetInfo(tx_ring_buf, NULL, NULL, NULL, NULL, &uxItemsWaiting);
415+
if (lastUxItemsWaiting == uxItemsWaiting) tries--;
416+
}
417+
if (tries == 0) { // CDC isn't connected anymore...
418+
isConnected = false;
419+
flushTXBuffer();
357420
}
358421
xSemaphoreGive(tx_lock);
359422
}

Diff for: libraries/ESP32/examples/HWSerial_Events/.skip.esp32

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
/*
2+
* This Example demonstrates how to receive Hardware Serial Events
3+
* This USB interface is available for the ESP32-S3, ESP32-C3, ESP32-C6 and ESP32-H2
4+
*
5+
* It will log all events and USB status (plugged/unplugged) into UART0
6+
* Any data read from UART0 will be sent to the USB CDC
7+
* Any data read from USB CDC will be sent to the UART0
8+
*
9+
* A suggestion is to use Arduino Serial Monitor for the UART0 port
10+
* and some other serial monitor application for the USB CDC port
11+
* in order to see the exchanged data and the Hardware Serial Events
12+
*
13+
*/
14+
15+
#ifndef ARDUINO_USB_MODE
16+
#error This ESP32 SoC has no Native USB interface
17+
#elif ARDUINO_USB_MODE == 0
18+
#warning This sketch should be used when USB is in Hardware CDC and JTAG mode
19+
void setup(){}
20+
void loop(){}
21+
#else
22+
23+
#if !ARDUINO_USB_CDC_ON_BOOT
24+
HWCDC HWCDCSerial;
25+
#endif
26+
27+
#include "driver/usb_serial_jtag.h"
28+
29+
// USB Event Callback Function that will log CDC events into UART0
30+
static void usbEventCallback(void* arg, esp_event_base_t event_base, int32_t event_id, void* event_data) {
31+
if (event_base == ARDUINO_HW_CDC_EVENTS) {
32+
switch (event_id) {
33+
case ARDUINO_HW_CDC_CONNECTED_EVENT:
34+
Serial0.println("CDC EVENT:: ARDUINO_HW_CDC_CONNECTED_EVENT");
35+
break;
36+
case ARDUINO_HW_CDC_BUS_RESET_EVENT:
37+
Serial0.println("CDC EVENT:: ARDUINO_HW_CDC_BUS_RESET_EVENT");
38+
break;
39+
case ARDUINO_HW_CDC_RX_EVENT:
40+
Serial0.println("\nCDC EVENT:: ARDUINO_HW_CDC_RX_EVENT");
41+
// sends all bytes read from USB Hardware Serial to UART0
42+
while (HWCDCSerial.available()) Serial0.write(HWCDCSerial.read());
43+
break;
44+
case ARDUINO_HW_CDC_TX_EVENT:
45+
Serial0.println("CDC EVENT:: ARDUINO_HW_CDC_TX_EVENT");
46+
break;
47+
48+
default:
49+
break;
50+
}
51+
}
52+
}
53+
54+
bool isPlugged() {
55+
return usb_serial_jtag_is_connected();
56+
}
57+
58+
const char* _hwcdc_status[] = {
59+
" USB Plugged but CDC is not connected\r\n",
60+
" USB Plugged and CDC is connected\r\n",
61+
" USB Unplugged and CDC not connected\r\n",
62+
" USB Unplugged BUT CDC is connected :: PROBLEM\r\n",
63+
};
64+
65+
const char* HWCDC_Status() {
66+
int i = isPlugged() ? 0 : 2;
67+
if(HWCDCSerial) i += 1;
68+
return _hwcdc_status[i];
69+
}
70+
71+
void setup() {
72+
Serial0.begin(115200);
73+
Serial0.setDebugOutput(true);
74+
75+
HWCDCSerial.begin();
76+
HWCDCSerial.onEvent(usbEventCallback);
77+
Serial0.println("Starting...");
78+
}
79+
80+
void loop() {
81+
static uint32_t counter = 0;
82+
83+
Serial0.print(counter);
84+
Serial0.print(HWCDC_Status());
85+
86+
if (HWCDCSerial) {
87+
HWCDCSerial.printf(" [%ld] connected\n\r", counter);
88+
}
89+
// sends all bytes read from UART0 to USB Hardware Serial
90+
while (Serial0.available()) HWCDCSerial.write(Serial0.read());
91+
delay(1000);
92+
counter++;
93+
}
94+
#endif

0 commit comments

Comments
 (0)