From c5477c10b804424ae72c68b758b93dd639623ec8 Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Wed, 21 Feb 2024 22:31:15 -0300 Subject: [PATCH 01/26] 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. --- cores/esp32/HWCDC.cpp | 50 +++++++++++++++++++++---------------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/cores/esp32/HWCDC.cpp b/cores/esp32/HWCDC.cpp index f0539d69b5d..9ebc1eceb55 100644 --- a/cores/esp32/HWCDC.cpp +++ b/cores/esp32/HWCDC.cpp @@ -1,4 +1,4 @@ -// Copyright 2015-2020 Espressif Systems (Shanghai) PTE LTD +// Copyright 2015-2024 Espressif Systems (Shanghai) PTE LTD // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -28,6 +28,7 @@ #include "hal/usb_serial_jtag_ll.h" #pragma GCC diagnostic warning "-Wvolatile" #include "rom/ets_sys.h" +#include "driver/usb_serial_jtag.h" ESP_EVENT_DEFINE_BASE(ARDUINO_HW_CDC_EVENTS); @@ -35,12 +36,10 @@ static RingbufHandle_t tx_ring_buf = NULL; static QueueHandle_t rx_queue = NULL; static uint8_t rx_data_buf[64] = {0}; static intr_handle_t intr_handle = NULL; -static volatile bool initial_empty = false; static SemaphoreHandle_t tx_lock = NULL; -// workaround for when USB CDC is not connected -static uint32_t tx_timeout_ms = 0; -static bool tx_timeout_change_request = false; +// workaround for when USB CDC is unplugged +static uint32_t requested_tx_timeout_ms = 100; static esp_event_loop_handle_t arduino_hw_cdc_event_loop_handle = NULL; @@ -81,18 +80,6 @@ static void hw_cdc_isr_handler(void *arg) { if (usb_serial_jtag_ll_txfifo_writable() == 1) { // We disable the interrupt here so that the interrupt won't be triggered if there is no data to send. usb_serial_jtag_ll_disable_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY); - if(!initial_empty){ - initial_empty = true; - // First time USB is plugged and the application has not explicitly set TX Timeout, set it to default 100ms. - // Otherwise, USB is still unplugged and the timeout will be kept as Zero in order to avoid any delay in the - // application whenever it uses write() and the TX Queue gets full. - if (!tx_timeout_change_request) { - tx_timeout_ms = 100; - } - //send event? - //ets_printf("CONNECTED\n"); - arduino_hw_cdc_event_post(ARDUINO_HW_CDC_EVENTS, ARDUINO_HW_CDC_CONNECTED_EVENT, &event, sizeof(arduino_hw_cdc_event_data_t), &xTaskWoken); - } size_t queued_size; uint8_t *queued_buff = (uint8_t *)xRingbufferReceiveUpToFromISR(tx_ring_buf, &queued_size, 64); // If the hardware fifo is avaliable, write in it. Otherwise, do nothing. @@ -124,18 +111,18 @@ static void hw_cdc_isr_handler(void *arg) { break; } } - //send event? - //ets_printf("RX:%u/%u\n", i, rx_fifo_len); event.rx.len = i; arduino_hw_cdc_event_post(ARDUINO_HW_CDC_EVENTS, ARDUINO_HW_CDC_RX_EVENT, &event, sizeof(arduino_hw_cdc_event_data_t), &xTaskWoken); } if (usbjtag_intr_status & USB_SERIAL_JTAG_INTR_BUS_RESET) { usb_serial_jtag_ll_clr_intsts_mask(USB_SERIAL_JTAG_INTR_BUS_RESET); - initial_empty = false; usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY); //ets_printf("BUS_RESET\n"); arduino_hw_cdc_event_post(ARDUINO_HW_CDC_EVENTS, ARDUINO_HW_CDC_BUS_RESET_EVENT, &event, sizeof(arduino_hw_cdc_event_data_t), &xTaskWoken); + if(usb_serial_jtag_is_connected()) { + arduino_hw_cdc_event_post(ARDUINO_HW_CDC_EVENTS, ARDUINO_HW_CDC_CONNECTED_EVENT, &event, sizeof(arduino_hw_cdc_event_data_t), &xTaskWoken); + } } if (xTaskWoken == pdTRUE) { @@ -144,6 +131,10 @@ static void hw_cdc_isr_handler(void *arg) { } static void ARDUINO_ISR_ATTR cdc0_write_char(char c) { + uint32_t tx_timeout_ms = 0; // if not connected, no timeout + if(usb_serial_jtag_is_connected()) { + tx_timeout_ms = requested_tx_timeout_ms; // once connected, restores the TX timeout + } if(xPortInIsrContext()){ xRingbufferSendFromISR(tx_ring_buf, (void*) (&c), 1, NULL); } else { @@ -162,7 +153,7 @@ HWCDC::~HWCDC(){ HWCDC::operator bool() const { - return initial_empty; + return usb_serial_jtag_is_connected(); } void HWCDC::onEvent(esp_event_handler_t callback){ @@ -251,10 +242,7 @@ void HWCDC::end() } void HWCDC::setTxTimeoutMs(uint32_t timeout){ - tx_timeout_ms = timeout; - // it registers that the user has explicitly requested to use a value as TX timeout - // used for the workaround with unplugged USB and TX Queue Full that causes a delay on every write() - tx_timeout_change_request = true; + requested_tx_timeout_ms = timeout; } /* @@ -278,9 +266,13 @@ size_t HWCDC::setTxBufferSize(size_t tx_queue_len){ int HWCDC::availableForWrite(void) { + uint32_t tx_timeout_ms = 0; // if not connected, no timeout if(tx_ring_buf == NULL || tx_lock == NULL){ return 0; } + if(usb_serial_jtag_is_connected()) { + tx_timeout_ms = requested_tx_timeout_ms; // once connected, restores the TX timeout + } if(xSemaphoreTake(tx_lock, tx_timeout_ms / portTICK_PERIOD_MS) != pdPASS){ return 0; } @@ -291,9 +283,13 @@ int HWCDC::availableForWrite(void) size_t HWCDC::write(const uint8_t *buffer, size_t size) { + uint32_t tx_timeout_ms = 0; // if not connected, no timeout if(buffer == NULL || size == 0 || tx_ring_buf == NULL || tx_lock == NULL){ return 0; } + if(usb_serial_jtag_is_connected()) { + tx_timeout_ms = requested_tx_timeout_ms; // once connected, restores the TX timeout + } if(xSemaphoreTake(tx_lock, tx_timeout_ms / portTICK_PERIOD_MS) != pdPASS){ return 0; } @@ -339,9 +335,13 @@ size_t HWCDC::write(uint8_t c) void HWCDC::flush(void) { + uint32_t tx_timeout_ms = 0; // if not connected, no timeout if(tx_ring_buf == NULL || tx_lock == NULL){ return; } + if(usb_serial_jtag_is_connected()) { + tx_timeout_ms = requested_tx_timeout_ms; // once connected, restores the TX timeout + } if(xSemaphoreTake(tx_lock, tx_timeout_ms / portTICK_PERIOD_MS) != pdPASS){ return; } From 112e0c2a8aea8eafb591831daaabf5561461a545 Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Wed, 21 Feb 2024 23:06:45 -0300 Subject: [PATCH 02/26] feat(usb): Creates HWSerial_Events.ino example --- .../HWSerial_Events/HWSerial_Events.ino | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 libraries/ESP32/examples/HWSerial_Events/HWSerial_Events.ino diff --git a/libraries/ESP32/examples/HWSerial_Events/HWSerial_Events.ino b/libraries/ESP32/examples/HWSerial_Events/HWSerial_Events.ino new file mode 100644 index 00000000000..27b3d0a2281 --- /dev/null +++ b/libraries/ESP32/examples/HWSerial_Events/HWSerial_Events.ino @@ -0,0 +1,60 @@ +/* + * This Example demonstrates how to receive Hardware Serial Events + * This USB interface is available for the ESP32-S3, ESP32-C3, ESP32-C6 and ESP32-H2 + * + * It will log all events and USB status (plugged/unplugged) into UART0 + * Any data read from UART0 will be sent to the USB CDC + * Any data read from USB CDC will be sent to the UART0 + * + * A suggestion is to use Arduino Serial Monitor for the UART0 port + * and some other serial monitor application for the USB CDC port + * in order to see the exchanged data and the Hardware Serial Events + * + */ + +// USB Event Callback Function that will log CDC events into UART0 +static void usbEventCallback(void* arg, esp_event_base_t event_base, int32_t event_id, void* event_data) { + if (event_base == ARDUINO_HW_CDC_EVENTS) { + switch (event_id) { + case ARDUINO_HW_CDC_CONNECTED_EVENT: + Serial0.println("CDC EVENT:: ARDUINO_HW_CDC_CONNECTED_EVENT"); + break; + case ARDUINO_HW_CDC_BUS_RESET_EVENT: + Serial0.println("CDC EVENT:: ARDUINO_HW_CDC_BUS_RESET_EVENT"); + break; + case ARDUINO_HW_CDC_RX_EVENT: + Serial0.println("\nCDC EVENT:: ARDUINO_HW_CDC_RX_EVENT"); + // sends all bytes read from USB Hardware Serial to UART0 + while (HWCDCSerial.available()) Serial0.write(HWCDCSerial.read()); + break; + case ARDUINO_HW_CDC_TX_EVENT: + Serial0.println("CDC EVENT:: ARDUINO_HW_CDC_TX_EVENT"); + break; + + default: + break; + } + } +} + +void setup() { + Serial0.begin(115200); + Serial0.setDebugOutput(true); + + HWCDCSerial.begin(); + HWCDCSerial.onEvent(usbEventCallback); + Serial0.println("Starting..."); +} + +void loop() { + static uint32_t counter = 0; + Serial0.print(counter++); + if (HWCDCSerial) { + Serial0.println(" +++ USB CDC JTAG Plugged"); + } else { + Serial0.println(" --- USB CDC JTAG Unplugged"); + } + // sends all bytes read from UART0 to USB Hardware Serial + while (Serial0.available()) HWCDCSerial.write(Serial0.read()); + delay(1000); +} From 742cbb438660ac5e5f297331365962261301e186 Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Wed, 21 Feb 2024 23:57:54 -0300 Subject: [PATCH 03/26] feat: adds .skip.esp32 Skips the ESP32 SoC test given that it has no USB --- libraries/ESP32/examples/HWSerial_Events/.skip.esp32 | 1 + 1 file changed, 1 insertion(+) create mode 100644 libraries/ESP32/examples/HWSerial_Events/.skip.esp32 diff --git a/libraries/ESP32/examples/HWSerial_Events/.skip.esp32 b/libraries/ESP32/examples/HWSerial_Events/.skip.esp32 new file mode 100644 index 00000000000..8b137891791 --- /dev/null +++ b/libraries/ESP32/examples/HWSerial_Events/.skip.esp32 @@ -0,0 +1 @@ + From ec05cd1c4ee874970620d091a3a16d47f55f2329 Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Wed, 21 Feb 2024 23:58:55 -0300 Subject: [PATCH 04/26] feat: adds .skip.esp32s2 Skips the ESP32S2 because it has no HW CDC JTAG interface --- libraries/ESP32/examples/HWSerial_Events/.skip.esp32s2 | 1 + 1 file changed, 1 insertion(+) create mode 100644 libraries/ESP32/examples/HWSerial_Events/.skip.esp32s2 diff --git a/libraries/ESP32/examples/HWSerial_Events/.skip.esp32s2 b/libraries/ESP32/examples/HWSerial_Events/.skip.esp32s2 new file mode 100644 index 00000000000..8b137891791 --- /dev/null +++ b/libraries/ESP32/examples/HWSerial_Events/.skip.esp32s2 @@ -0,0 +1 @@ + From f7e8b9fc346c6b0b66e75eb1af89abf18b1a2ae5 Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Thu, 22 Feb 2024 00:05:44 -0300 Subject: [PATCH 05/26] fix: fixes issues with Ubuntu CI Only compiles the example in case it is using Hardware CD and JTAG mode. --- .../ESP32/examples/HWSerial_Events/HWSerial_Events.ino | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/libraries/ESP32/examples/HWSerial_Events/HWSerial_Events.ino b/libraries/ESP32/examples/HWSerial_Events/HWSerial_Events.ino index 27b3d0a2281..f1a5a289a8c 100644 --- a/libraries/ESP32/examples/HWSerial_Events/HWSerial_Events.ino +++ b/libraries/ESP32/examples/HWSerial_Events/HWSerial_Events.ino @@ -12,6 +12,14 @@ * */ +#ifndef ARDUINO_USB_MODE +#error This ESP32 SoC has no Native USB interface +#elif ARDUINO_USB_MODE == 0 +#warning This sketch should be used when USB is in Hardware CDC and JTAG mode +void setup(){} +void loop(){} +#else + // USB Event Callback Function that will log CDC events into UART0 static void usbEventCallback(void* arg, esp_event_base_t event_base, int32_t event_id, void* event_data) { if (event_base == ARDUINO_HW_CDC_EVENTS) { @@ -58,3 +66,4 @@ void loop() { while (Serial0.available()) HWCDCSerial.write(Serial0.read()); delay(1000); } +#endif From 52dec6996ec3c0404d66c82bbb007d598483d0a3 Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Sun, 25 Feb 2024 01:14:11 -0300 Subject: [PATCH 06/26] 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 --- cores/esp32/HWCDC.cpp | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/cores/esp32/HWCDC.cpp b/cores/esp32/HWCDC.cpp index 9ebc1eceb55..d2edf0cbd12 100644 --- a/cores/esp32/HWCDC.cpp +++ b/cores/esp32/HWCDC.cpp @@ -97,6 +97,7 @@ static void hw_cdc_isr_handler(void *arg) { } } else { usb_serial_jtag_ll_clr_intsts_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY); + // should we just flush internal CDC buffer?? } } @@ -281,9 +282,23 @@ int HWCDC::availableForWrite(void) return a; } +static void flushTXBuffer() +{ + if (!tx_ring_buf) return; + UBaseType_t uxItemsWaiting = 0; + vRingbufferGetInfo(tx_ring_buf, NULL, NULL, NULL, NULL, &uxItemsWaiting); + + size_t queued_size = 0; + uint8_t *queued_buff = (uint8_t *)xRingbufferReceiveUpTo(tx_ring_buf, &queued_size, 0, uxItemsWaiting); + if (queued_size && queued_buff != NULL) { + vRingbufferReturnItem(tx_ring_buf, (void *)queued_buff); + } + // flush internal CDC +} + size_t HWCDC::write(const uint8_t *buffer, size_t size) { - uint32_t tx_timeout_ms = 0; // if not connected, no timeout + uint32_t tx_timeout_ms = 0; // if not connected, no timeout if(buffer == NULL || size == 0 || tx_ring_buf == NULL || tx_lock == NULL){ return 0; } @@ -301,6 +316,7 @@ size_t HWCDC::write(const uint8_t *buffer, size_t size) space = size; } // Non-Blocking method, Sending data to ringbuffer, and handle the data in ISR. + // USB may be plugged, but CDC may be not connected ==> do not block and flush TX buffer, keeping in the buffer just the lastest data if(xRingbufferSend(tx_ring_buf, (void*) (buffer), space, 0) != pdTRUE){ size = 0; } else { @@ -324,6 +340,8 @@ size_t HWCDC::write(const uint8_t *buffer, size_t size) usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY); } } + // there is no more space in CDC Endpoint internal buffer ==> flush all data from TX buffer and only keep last written data + if(to_send && !usb_serial_jtag_ll_txfifo_writable()) flushTXBuffer(); xSemaphoreGive(tx_lock); return size; } @@ -345,16 +363,21 @@ void HWCDC::flush(void) if(xSemaphoreTake(tx_lock, tx_timeout_ms / portTICK_PERIOD_MS) != pdPASS){ return; } + // USB may be plugged, but CDC may be not connected ==> do not block and flush TX buffer, keeping in the buffer just the lastest data UBaseType_t uxItemsWaiting = 0; vRingbufferGetInfo(tx_ring_buf, NULL, NULL, NULL, NULL, &uxItemsWaiting); if(uxItemsWaiting){ // Now trigger the ISR to read data from the ring buffer. usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY); } - while(uxItemsWaiting){ + uint8_t tries = 3; + while(tries && uxItemsWaiting){ delay(5); + UBaseType_t lastUxItemsWaiting = uxItemsWaiting; // is it flushing CDC? vRingbufferGetInfo(tx_ring_buf, NULL, NULL, NULL, NULL, &uxItemsWaiting); + if (lastUxItemsWaiting == uxItemsWaiting) tries--; // avoids locking when USB is plugged, but CDC is not connected } + if (!tries) flushTXBuffer(); // flush TX Buffer xSemaphoreGive(tx_lock); } From b1d6108f9cd1d649e6467dd156010c5be0759848 Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Tue, 27 Feb 2024 14:40:00 -0300 Subject: [PATCH 07/26] fix(HWCDC): changes made demands testing for CDC ON BOOT --- libraries/ESP32/examples/HWSerial_Events/HWSerial_Events.ino | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libraries/ESP32/examples/HWSerial_Events/HWSerial_Events.ino b/libraries/ESP32/examples/HWSerial_Events/HWSerial_Events.ino index f1a5a289a8c..9cbb06a977d 100644 --- a/libraries/ESP32/examples/HWSerial_Events/HWSerial_Events.ino +++ b/libraries/ESP32/examples/HWSerial_Events/HWSerial_Events.ino @@ -20,6 +20,9 @@ void setup(){} void loop(){} #else +#if !ARDUINO_USB_CDC_ON_BOOT +HWCDC HWCDCSerial; +#endif // USB Event Callback Function that will log CDC events into UART0 static void usbEventCallback(void* arg, esp_event_base_t event_base, int32_t event_id, void* event_data) { if (event_base == ARDUINO_HW_CDC_EVENTS) { From a203d4236c640d853466583ace9d7715425169c0 Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Wed, 28 Feb 2024 01:26:05 -0300 Subject: [PATCH 08/26] feat(hwcdc): Improves HWSerial_Events.ino Improves the example by adding more information about USB being plugged and CDC being connected. --- .../HWSerial_Events/HWSerial_Events.ino | 30 ++++++++++++++++--- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/libraries/ESP32/examples/HWSerial_Events/HWSerial_Events.ino b/libraries/ESP32/examples/HWSerial_Events/HWSerial_Events.ino index 9cbb06a977d..2838c627936 100644 --- a/libraries/ESP32/examples/HWSerial_Events/HWSerial_Events.ino +++ b/libraries/ESP32/examples/HWSerial_Events/HWSerial_Events.ino @@ -23,6 +23,9 @@ void loop(){} #if !ARDUINO_USB_CDC_ON_BOOT HWCDC HWCDCSerial; #endif + +#include "driver/usb_serial_jtag.h" + // USB Event Callback Function that will log CDC events into UART0 static void usbEventCallback(void* arg, esp_event_base_t event_base, int32_t event_id, void* event_data) { if (event_base == ARDUINO_HW_CDC_EVENTS) { @@ -48,6 +51,23 @@ static void usbEventCallback(void* arg, esp_event_base_t event_base, int32_t eve } } +bool isPlugged() { + return usb_serial_jtag_is_connected(); +} + +const char* _hwcdc_status[] = { + " USB Plugged but CDC is not connected\r\n", + " USB Plugged and CDC is connected\r\n", + " USB Unplugged and CDC not connected\r\n", + " USB Unplugged BUT CDC is connected :: PROBLEM\r\n", +}; + +const char* HWCDC_Status() { + int i = isPlugged() ? 0 : 2; + if(HWCDCSerial) i += 1; + return _hwcdc_status[i]; +} + void setup() { Serial0.begin(115200); Serial0.setDebugOutput(true); @@ -59,14 +79,16 @@ void setup() { void loop() { static uint32_t counter = 0; - Serial0.print(counter++); + + Serial0.print(counter); + Serial0.print(HWCDC_Status()); + if (HWCDCSerial) { - Serial0.println(" +++ USB CDC JTAG Plugged"); - } else { - Serial0.println(" --- USB CDC JTAG Unplugged"); + HWCDCSerial.printf(" [%ld] connected\n\r", counter); } // sends all bytes read from UART0 to USB Hardware Serial while (Serial0.available()) HWCDCSerial.write(Serial0.read()); delay(1000); + counter++; } #endif From 774bb9a8cfe0d129760c0a1864a4f3cba10b8286 Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Wed, 28 Feb 2024 01:48:58 -0300 Subject: [PATCH 09/26] 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. --- cores/esp32/HWCDC.cpp | 98 +++++++++++++++++++++++++++++++++---------- 1 file changed, 75 insertions(+), 23 deletions(-) diff --git a/cores/esp32/HWCDC.cpp b/cores/esp32/HWCDC.cpp index ad9e03dd395..bfb5e25d996 100644 --- a/cores/esp32/HWCDC.cpp +++ b/cores/esp32/HWCDC.cpp @@ -37,8 +37,9 @@ static QueueHandle_t rx_queue = NULL; static uint8_t rx_data_buf[64] = {0}; static intr_handle_t intr_handle = NULL; static SemaphoreHandle_t tx_lock = NULL; +static volatile bool isConnected = false; -// workaround for when USB CDC is unplugged +// timeout has no effect when USB CDC is unplugged static uint32_t requested_tx_timeout_ms = 100; static esp_event_loop_handle_t arduino_hw_cdc_event_loop_handle = NULL; @@ -75,8 +76,17 @@ static void hw_cdc_isr_handler(void *arg) { arduino_hw_cdc_event_data_t event = {0}; usbjtag_intr_status = usb_serial_jtag_ll_get_intsts_mask(); + // enabling USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY only happen when CDC is connected if (usbjtag_intr_status & USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY) { // Interrupt tells us the host picked up the data we sent. + if(!usb_serial_jtag_is_connected()) { + isConnected = false; // reset it when USB is unplugged + usb_serial_jtag_ll_clr_intsts_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY); + // USB is unplugged, nothing to be done here + return; + } else { + isConnected = true; + } if (usb_serial_jtag_ll_txfifo_writable() == 1) { // We disable the interrupt here so that the interrupt won't be triggered if there is no data to send. usb_serial_jtag_ll_disable_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY); @@ -89,12 +99,12 @@ static void hw_cdc_isr_handler(void *arg) { usb_serial_jtag_ll_write_txfifo(queued_buff, queued_size); usb_serial_jtag_ll_txfifo_flush(); vRingbufferReturnItemFromISR(tx_ring_buf, queued_buff, &xTaskWoken); - usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY); + if(isConnected) usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY); //send event? //ets_printf("TX:%u\n", queued_size); event.tx.len = queued_size; arduino_hw_cdc_event_post(ARDUINO_HW_CDC_EVENTS, ARDUINO_HW_CDC_TX_EVENT, &event, sizeof(arduino_hw_cdc_event_data_t), &xTaskWoken); - } + } } else { usb_serial_jtag_ll_clr_intsts_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY); // should we just flush internal CDC buffer?? @@ -114,16 +124,13 @@ static void hw_cdc_isr_handler(void *arg) { } event.rx.len = i; arduino_hw_cdc_event_post(ARDUINO_HW_CDC_EVENTS, ARDUINO_HW_CDC_RX_EVENT, &event, sizeof(arduino_hw_cdc_event_data_t), &xTaskWoken); + isConnected = true; // receiving data also means that CDC is connected! } if (usbjtag_intr_status & USB_SERIAL_JTAG_INTR_BUS_RESET) { usb_serial_jtag_ll_clr_intsts_mask(USB_SERIAL_JTAG_INTR_BUS_RESET); - usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY); - //ets_printf("BUS_RESET\n"); arduino_hw_cdc_event_post(ARDUINO_HW_CDC_EVENTS, ARDUINO_HW_CDC_BUS_RESET_EVENT, &event, sizeof(arduino_hw_cdc_event_data_t), &xTaskWoken); - if(usb_serial_jtag_is_connected()) { - arduino_hw_cdc_event_post(ARDUINO_HW_CDC_EVENTS, ARDUINO_HW_CDC_CONNECTED_EVENT, &event, sizeof(arduino_hw_cdc_event_data_t), &xTaskWoken); - } + isConnected = false; // TX/RX only takes place after USB BUS resets (it was just plugged) } if (xTaskWoken == pdTRUE) { @@ -141,7 +148,7 @@ static void ARDUINO_ISR_ATTR cdc0_write_char(char c) { } else { xRingbufferSend(tx_ring_buf, (void*) (&c), 1, tx_timeout_ms / portTICK_PERIOD_MS); } - usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY); + usb_serial_jtag_ll_txfifo_flush(); // flushes HW Serial CDC and sets IN_EMPTY when Host reads data } HWCDC::HWCDC() { @@ -152,9 +159,35 @@ HWCDC::~HWCDC(){ end(); } + +// It should return just when USB is plugged and CDC is connected. HWCDC::operator bool() const { - return usb_serial_jtag_is_connected(); + // deals when this functions is called many times + // typically with something like `while(!Serial) delay(10);` + static bool running = false; + + // USB may be unplugged + if (usb_serial_jtag_is_connected() == false) { + isConnected = false; + running = false; // reset it for when it will plugged in + return false; + } + + if (isConnected) { + running = false; + return true; + } + + if (running == false && !isConnected) { // enable it only once! + usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY); + } + // this will feed CDC TX FIFO and then trigger when CDC is connected + uint8_t c = '\0'; + usb_serial_jtag_ll_write_txfifo(&c, sizeof(c)); + usb_serial_jtag_ll_txfifo_flush(); + running = true; + return false; } void HWCDC::onEvent(esp_event_handler_t callback){ @@ -198,9 +231,9 @@ void HWCDC::begin(unsigned long baud) log_e("HW CDC RX Buffer error"); } } - //TX Buffer default has 256 bytes if not preset + //TX Buffer default has 16 bytes if not preset if (tx_ring_buf == NULL) { - if (!setTxBufferSize(256)) { + if (!setTxBufferSize(16)) { log_e("HW CDC TX Buffer error"); } } @@ -219,8 +252,11 @@ void HWCDC::begin(unsigned long baud) } else { log_e("Serial JTAG Pins can't be set into Peripheral Manager."); } - - usb_serial_jtag_ll_txfifo_flush(); + //isConnected = false; + // trying to detect if it is already connected by filling CDC FIFO + //uint8_t c = '0'; + //usb_serial_jtag_ll_write_txfifo(&c, 1); + //usb_serial_jtag_ll_txfifo_flush(); } void HWCDC::end() @@ -240,6 +276,7 @@ void HWCDC::end() arduino_hw_cdc_event_loop_handle = NULL; } HWCDC::deinit(this); + isConnected = false; } void HWCDC::setTxTimeoutMs(uint32_t timeout){ @@ -293,7 +330,8 @@ static void flushTXBuffer() if (queued_size && queued_buff != NULL) { vRingbufferReturnItem(tx_ring_buf, (void *)queued_buff); } - // flush internal CDC + // flushes CDC FIFO + usb_serial_jtag_ll_txfifo_flush(); } size_t HWCDC::write(const uint8_t *buffer, size_t size) @@ -304,6 +342,8 @@ size_t HWCDC::write(const uint8_t *buffer, size_t size) } if(usb_serial_jtag_is_connected()) { tx_timeout_ms = requested_tx_timeout_ms; // once connected, restores the TX timeout + } else { + isConnected = false; } if(xSemaphoreTake(tx_lock, tx_timeout_ms / portTICK_PERIOD_MS) != pdPASS){ return 0; @@ -323,8 +363,9 @@ size_t HWCDC::write(const uint8_t *buffer, size_t size) to_send -= space; so_far += space; // Now trigger the ISR to read data from the ring buffer. - usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY); - + usb_serial_jtag_ll_txfifo_flush(); // flushes HW Serial CDC and sets IN_EMPTY when Host reads data + if(isConnected) usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY); + while(to_send){ if(max_size > to_send){ max_size = to_send; @@ -337,11 +378,16 @@ size_t HWCDC::write(const uint8_t *buffer, size_t size) so_far += max_size; to_send -= max_size; // Now trigger the ISR to read data from the ring buffer. - usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY); + usb_serial_jtag_ll_txfifo_flush(); // flushes HW Serial CDC and sets IN_EMPTY when Host reads data + if(isConnected) usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY); } } - // there is no more space in CDC Endpoint internal buffer ==> flush all data from TX buffer and only keep last written data - if(to_send && !usb_serial_jtag_ll_txfifo_writable()) flushTXBuffer(); + // there is no more space in CDC Endpoint FIFO. It is very possible that CDC is diconnected! + // ==> flush all data from TX buffer and only keep last written data + if(to_send && !usb_serial_jtag_ll_txfifo_writable()) { + isConnected = false; // we consider that CDC connection has been terminated + flushTXBuffer(); // flush TX Ringbuffer + } xSemaphoreGive(tx_lock); return size; } @@ -359,6 +405,8 @@ void HWCDC::flush(void) } if(usb_serial_jtag_is_connected()) { tx_timeout_ms = requested_tx_timeout_ms; // once connected, restores the TX timeout + } else { + isConnected = false; } if(xSemaphoreTake(tx_lock, tx_timeout_ms / portTICK_PERIOD_MS) != pdPASS){ return; @@ -368,7 +416,8 @@ void HWCDC::flush(void) vRingbufferGetInfo(tx_ring_buf, NULL, NULL, NULL, NULL, &uxItemsWaiting); if(uxItemsWaiting){ // Now trigger the ISR to read data from the ring buffer. - usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY); + usb_serial_jtag_ll_txfifo_flush(); // flushes HW Serial CDC and sets IN_EMPTY when Host reads data + if(isConnected) usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY); } uint8_t tries = 3; while(tries && uxItemsWaiting){ @@ -377,7 +426,10 @@ void HWCDC::flush(void) vRingbufferGetInfo(tx_ring_buf, NULL, NULL, NULL, NULL, &uxItemsWaiting); if (lastUxItemsWaiting == uxItemsWaiting) tries--; // avoids locking when USB is plugged, but CDC is not connected } - if (!tries) flushTXBuffer(); // flush TX Buffer + if (tries == 0) { // it is very possible that CDC isn't connected anymore... + isConnected = false; // we consider that CDC connection has been terminated + flushTXBuffer(); // flush TX Ringbuffer + } xSemaphoreGive(tx_lock); } @@ -459,7 +511,7 @@ void HWCDC::setDebugOutput(bool en) } } -#if ARDUINO_USB_MODE && ARDUINO_USB_CDC_ON_BOOT // Hardware JTAG CDC selected +#if ARDUINO_USB_MODE && ARDUINO_USB_CDC_ON_BOOT // Hardware JTAG CDC selected // USBSerial is always available to be used HWCDC HWCDCSerial; #endif From 98bad3ecacef04a4d82f9d08823adffbb9a42641 Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Wed, 28 Feb 2024 02:08:01 -0300 Subject: [PATCH 10/26] Update cores/esp32/HWCDC.cpp --- cores/esp32/HWCDC.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cores/esp32/HWCDC.cpp b/cores/esp32/HWCDC.cpp index bfb5e25d996..dfc6344c0af 100644 --- a/cores/esp32/HWCDC.cpp +++ b/cores/esp32/HWCDC.cpp @@ -104,7 +104,7 @@ static void hw_cdc_isr_handler(void *arg) { //ets_printf("TX:%u\n", queued_size); event.tx.len = queued_size; arduino_hw_cdc_event_post(ARDUINO_HW_CDC_EVENTS, ARDUINO_HW_CDC_TX_EVENT, &event, sizeof(arduino_hw_cdc_event_data_t), &xTaskWoken); - } + } } else { usb_serial_jtag_ll_clr_intsts_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY); // should we just flush internal CDC buffer?? From 06b8a08823c7e3d9498c2d4d619787e08f20890a Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Wed, 28 Feb 2024 02:08:21 -0300 Subject: [PATCH 11/26] Update cores/esp32/HWCDC.cpp --- cores/esp32/HWCDC.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/cores/esp32/HWCDC.cpp b/cores/esp32/HWCDC.cpp index dfc6344c0af..445ea20ecd5 100644 --- a/cores/esp32/HWCDC.cpp +++ b/cores/esp32/HWCDC.cpp @@ -107,7 +107,6 @@ static void hw_cdc_isr_handler(void *arg) { } } else { usb_serial_jtag_ll_clr_intsts_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY); - // should we just flush internal CDC buffer?? } } From c218f507c36d7020f78423b00debbdd30791ce47 Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Wed, 28 Feb 2024 02:08:29 -0300 Subject: [PATCH 12/26] Update cores/esp32/HWCDC.cpp --- cores/esp32/HWCDC.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cores/esp32/HWCDC.cpp b/cores/esp32/HWCDC.cpp index 445ea20ecd5..a4b4f2467f2 100644 --- a/cores/esp32/HWCDC.cpp +++ b/cores/esp32/HWCDC.cpp @@ -162,7 +162,7 @@ HWCDC::~HWCDC(){ // It should return just when USB is plugged and CDC is connected. HWCDC::operator bool() const { - // deals when this functions is called many times + // deals when this function is called many times // typically with something like `while(!Serial) delay(10);` static bool running = false; From ca0a9f586ec79d24eb7bb17a93c222f2e743f592 Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Wed, 28 Feb 2024 02:08:36 -0300 Subject: [PATCH 13/26] Update cores/esp32/HWCDC.cpp --- cores/esp32/HWCDC.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cores/esp32/HWCDC.cpp b/cores/esp32/HWCDC.cpp index a4b4f2467f2..5d7e8b88196 100644 --- a/cores/esp32/HWCDC.cpp +++ b/cores/esp32/HWCDC.cpp @@ -169,7 +169,7 @@ HWCDC::operator bool() const // USB may be unplugged if (usb_serial_jtag_is_connected() == false) { isConnected = false; - running = false; // reset it for when it will plugged in + running = false; return false; } From f7d66d2a2df8226b7de01149a6b7460b625559af Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Wed, 28 Feb 2024 02:08:47 -0300 Subject: [PATCH 14/26] Update cores/esp32/HWCDC.cpp --- cores/esp32/HWCDC.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cores/esp32/HWCDC.cpp b/cores/esp32/HWCDC.cpp index 5d7e8b88196..085d20c6a4a 100644 --- a/cores/esp32/HWCDC.cpp +++ b/cores/esp32/HWCDC.cpp @@ -178,7 +178,7 @@ HWCDC::operator bool() const return true; } - if (running == false && !isConnected) { // enable it only once! + if (running == false && !isConnected) { // enables it only once! usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY); } // this will feed CDC TX FIFO and then trigger when CDC is connected From 980f7b182ffbde6eb558166fec62405ee618bd0a Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Wed, 28 Feb 2024 02:08:56 -0300 Subject: [PATCH 15/26] Update cores/esp32/HWCDC.cpp --- cores/esp32/HWCDC.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cores/esp32/HWCDC.cpp b/cores/esp32/HWCDC.cpp index 085d20c6a4a..6a120094b10 100644 --- a/cores/esp32/HWCDC.cpp +++ b/cores/esp32/HWCDC.cpp @@ -410,7 +410,7 @@ void HWCDC::flush(void) if(xSemaphoreTake(tx_lock, tx_timeout_ms / portTICK_PERIOD_MS) != pdPASS){ return; } - // USB may be plugged, but CDC may be not connected ==> do not block and flush TX buffer, keeping in the buffer just the lastest data + // USB may be plugged, but CDC may be not connected ==> do not block and flush TX buffer, keeping just the lastest data buffered UBaseType_t uxItemsWaiting = 0; vRingbufferGetInfo(tx_ring_buf, NULL, NULL, NULL, NULL, &uxItemsWaiting); if(uxItemsWaiting){ From 8e6eda4fc99f617174f94889306b470267200ae6 Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Wed, 28 Feb 2024 02:09:03 -0300 Subject: [PATCH 16/26] Update cores/esp32/HWCDC.cpp --- cores/esp32/HWCDC.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cores/esp32/HWCDC.cpp b/cores/esp32/HWCDC.cpp index 6a120094b10..8b284bb5c22 100644 --- a/cores/esp32/HWCDC.cpp +++ b/cores/esp32/HWCDC.cpp @@ -415,7 +415,7 @@ void HWCDC::flush(void) vRingbufferGetInfo(tx_ring_buf, NULL, NULL, NULL, NULL, &uxItemsWaiting); if(uxItemsWaiting){ // Now trigger the ISR to read data from the ring buffer. - usb_serial_jtag_ll_txfifo_flush(); // flushes HW Serial CDC and sets IN_EMPTY when Host reads data + usb_serial_jtag_ll_txfifo_flush(); if(isConnected) usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY); } uint8_t tries = 3; From 43075e4244285521ea57b365783717e272fa0a53 Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Wed, 28 Feb 2024 02:09:13 -0300 Subject: [PATCH 17/26] Update cores/esp32/HWCDC.cpp --- cores/esp32/HWCDC.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cores/esp32/HWCDC.cpp b/cores/esp32/HWCDC.cpp index 8b284bb5c22..ef2ff4f12f1 100644 --- a/cores/esp32/HWCDC.cpp +++ b/cores/esp32/HWCDC.cpp @@ -425,7 +425,7 @@ void HWCDC::flush(void) vRingbufferGetInfo(tx_ring_buf, NULL, NULL, NULL, NULL, &uxItemsWaiting); if (lastUxItemsWaiting == uxItemsWaiting) tries--; // avoids locking when USB is plugged, but CDC is not connected } - if (tries == 0) { // it is very possible that CDC isn't connected anymore... + if (tries == 0) { // CDC isn't connected anymore... isConnected = false; // we consider that CDC connection has been terminated flushTXBuffer(); // flush TX Ringbuffer } From 3d2d18dfa8beca9cb9c4bbf2b608e1dd964ad603 Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Wed, 28 Feb 2024 02:09:21 -0300 Subject: [PATCH 18/26] Update cores/esp32/HWCDC.cpp --- cores/esp32/HWCDC.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cores/esp32/HWCDC.cpp b/cores/esp32/HWCDC.cpp index ef2ff4f12f1..cec9422fddc 100644 --- a/cores/esp32/HWCDC.cpp +++ b/cores/esp32/HWCDC.cpp @@ -426,7 +426,7 @@ void HWCDC::flush(void) if (lastUxItemsWaiting == uxItemsWaiting) tries--; // avoids locking when USB is plugged, but CDC is not connected } if (tries == 0) { // CDC isn't connected anymore... - isConnected = false; // we consider that CDC connection has been terminated + isConnected = false; flushTXBuffer(); // flush TX Ringbuffer } xSemaphoreGive(tx_lock); From 467fe9dbda4747f94fcddf06160e4b5c558437fe Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Wed, 28 Feb 2024 02:09:28 -0300 Subject: [PATCH 19/26] Update cores/esp32/HWCDC.cpp --- cores/esp32/HWCDC.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cores/esp32/HWCDC.cpp b/cores/esp32/HWCDC.cpp index cec9422fddc..52b5e2c4a6c 100644 --- a/cores/esp32/HWCDC.cpp +++ b/cores/esp32/HWCDC.cpp @@ -510,7 +510,7 @@ void HWCDC::setDebugOutput(bool en) } } -#if ARDUINO_USB_MODE && ARDUINO_USB_CDC_ON_BOOT // Hardware JTAG CDC selected +#if ARDUINO_USB_MODE && ARDUINO_USB_CDC_ON_BOOT // Hardware JTAG CDC selected // USBSerial is always available to be used HWCDC HWCDCSerial; #endif From 576d675c1e882667f80b7d9b4b7f5996d595ec3b Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Wed, 28 Feb 2024 02:09:48 -0300 Subject: [PATCH 20/26] Update cores/esp32/HWCDC.cpp --- cores/esp32/HWCDC.cpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/cores/esp32/HWCDC.cpp b/cores/esp32/HWCDC.cpp index 52b5e2c4a6c..1d7918d8e0c 100644 --- a/cores/esp32/HWCDC.cpp +++ b/cores/esp32/HWCDC.cpp @@ -251,11 +251,6 @@ void HWCDC::begin(unsigned long baud) } else { log_e("Serial JTAG Pins can't be set into Peripheral Manager."); } - //isConnected = false; - // trying to detect if it is already connected by filling CDC FIFO - //uint8_t c = '0'; - //usb_serial_jtag_ll_write_txfifo(&c, 1); - //usb_serial_jtag_ll_txfifo_flush(); } void HWCDC::end() From ab157cf4d0a8dd89eabfd6d066bab5917c2ac032 Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Wed, 28 Feb 2024 02:10:04 -0300 Subject: [PATCH 21/26] Update cores/esp32/HWCDC.cpp --- cores/esp32/HWCDC.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cores/esp32/HWCDC.cpp b/cores/esp32/HWCDC.cpp index 1d7918d8e0c..998e37ee7ba 100644 --- a/cores/esp32/HWCDC.cpp +++ b/cores/esp32/HWCDC.cpp @@ -350,7 +350,7 @@ size_t HWCDC::write(const uint8_t *buffer, size_t size) space = size; } // Non-Blocking method, Sending data to ringbuffer, and handle the data in ISR. - // USB may be plugged, but CDC may be not connected ==> do not block and flush TX buffer, keeping in the buffer just the lastest data + // USB may be plugged, but CDC may be not connected ==> do not block and flush TX buffer, keeping just the lastest data buffered if(xRingbufferSend(tx_ring_buf, (void*) (buffer), space, 0) != pdTRUE){ size = 0; } else { From 5d2cb6965e9bd2fbaeb397e8d45e40e9fa9cc5cc Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Wed, 28 Feb 2024 02:10:31 -0300 Subject: [PATCH 22/26] Update cores/esp32/HWCDC.cpp --- cores/esp32/HWCDC.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cores/esp32/HWCDC.cpp b/cores/esp32/HWCDC.cpp index 998e37ee7ba..3a281ae8d61 100644 --- a/cores/esp32/HWCDC.cpp +++ b/cores/esp32/HWCDC.cpp @@ -357,7 +357,7 @@ size_t HWCDC::write(const uint8_t *buffer, size_t size) to_send -= space; so_far += space; // Now trigger the ISR to read data from the ring buffer. - usb_serial_jtag_ll_txfifo_flush(); // flushes HW Serial CDC and sets IN_EMPTY when Host reads data + usb_serial_jtag_ll_txfifo_flush(); if(isConnected) usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY); while(to_send){ From 500093919d005be9db45a47896dad7fcc254e982 Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Wed, 28 Feb 2024 02:10:42 -0300 Subject: [PATCH 23/26] Update cores/esp32/HWCDC.cpp --- cores/esp32/HWCDC.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cores/esp32/HWCDC.cpp b/cores/esp32/HWCDC.cpp index 3a281ae8d61..dee7fe19941 100644 --- a/cores/esp32/HWCDC.cpp +++ b/cores/esp32/HWCDC.cpp @@ -372,7 +372,7 @@ size_t HWCDC::write(const uint8_t *buffer, size_t size) so_far += max_size; to_send -= max_size; // Now trigger the ISR to read data from the ring buffer. - usb_serial_jtag_ll_txfifo_flush(); // flushes HW Serial CDC and sets IN_EMPTY when Host reads data + usb_serial_jtag_ll_txfifo_flush(); if(isConnected) usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY); } } From a0c7b415786d2b0fd30f1a13606bfdfd5077edfe Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Wed, 28 Feb 2024 02:10:58 -0300 Subject: [PATCH 24/26] Update cores/esp32/HWCDC.cpp --- cores/esp32/HWCDC.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cores/esp32/HWCDC.cpp b/cores/esp32/HWCDC.cpp index dee7fe19941..1f5ab422453 100644 --- a/cores/esp32/HWCDC.cpp +++ b/cores/esp32/HWCDC.cpp @@ -376,8 +376,7 @@ size_t HWCDC::write(const uint8_t *buffer, size_t size) if(isConnected) usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY); } } - // there is no more space in CDC Endpoint FIFO. It is very possible that CDC is diconnected! - // ==> flush all data from TX buffer and only keep last written data + // CDC is diconnected ==> flush all data from TX buffer if(to_send && !usb_serial_jtag_ll_txfifo_writable()) { isConnected = false; // we consider that CDC connection has been terminated flushTXBuffer(); // flush TX Ringbuffer From d4eca05df7320c56f9905708f21dfe079d25a03a Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Wed, 28 Feb 2024 02:11:08 -0300 Subject: [PATCH 25/26] Update cores/esp32/HWCDC.cpp --- cores/esp32/HWCDC.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cores/esp32/HWCDC.cpp b/cores/esp32/HWCDC.cpp index 1f5ab422453..fac0a2f9ed1 100644 --- a/cores/esp32/HWCDC.cpp +++ b/cores/esp32/HWCDC.cpp @@ -378,7 +378,7 @@ size_t HWCDC::write(const uint8_t *buffer, size_t size) } // CDC is diconnected ==> flush all data from TX buffer if(to_send && !usb_serial_jtag_ll_txfifo_writable()) { - isConnected = false; // we consider that CDC connection has been terminated + isConnected = false; flushTXBuffer(); // flush TX Ringbuffer } xSemaphoreGive(tx_lock); From 7b42a8f1b09204efe60fdc7b97d1d09fe7e1d71d Mon Sep 17 00:00:00 2001 From: Rodrigo Garcia Date: Wed, 28 Feb 2024 02:43:55 -0300 Subject: [PATCH 26/26] Apply suggestions from code review --- cores/esp32/HWCDC.cpp | 39 +++++++++++++++++---------------------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/cores/esp32/HWCDC.cpp b/cores/esp32/HWCDC.cpp index fac0a2f9ed1..e593d7ff371 100644 --- a/cores/esp32/HWCDC.cpp +++ b/cores/esp32/HWCDC.cpp @@ -76,11 +76,10 @@ static void hw_cdc_isr_handler(void *arg) { arduino_hw_cdc_event_data_t event = {0}; usbjtag_intr_status = usb_serial_jtag_ll_get_intsts_mask(); - // enabling USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY only happen when CDC is connected if (usbjtag_intr_status & USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY) { // Interrupt tells us the host picked up the data we sent. if(!usb_serial_jtag_is_connected()) { - isConnected = false; // reset it when USB is unplugged + isConnected = false; usb_serial_jtag_ll_clr_intsts_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY); // USB is unplugged, nothing to be done here return; @@ -123,13 +122,13 @@ static void hw_cdc_isr_handler(void *arg) { } event.rx.len = i; arduino_hw_cdc_event_post(ARDUINO_HW_CDC_EVENTS, ARDUINO_HW_CDC_RX_EVENT, &event, sizeof(arduino_hw_cdc_event_data_t), &xTaskWoken); - isConnected = true; // receiving data also means that CDC is connected! + isConnected = true; } if (usbjtag_intr_status & USB_SERIAL_JTAG_INTR_BUS_RESET) { usb_serial_jtag_ll_clr_intsts_mask(USB_SERIAL_JTAG_INTR_BUS_RESET); arduino_hw_cdc_event_post(ARDUINO_HW_CDC_EVENTS, ARDUINO_HW_CDC_BUS_RESET_EVENT, &event, sizeof(arduino_hw_cdc_event_data_t), &xTaskWoken); - isConnected = false; // TX/RX only takes place after USB BUS resets (it was just plugged) + isConnected = false; } if (xTaskWoken == pdTRUE) { @@ -138,16 +137,16 @@ static void hw_cdc_isr_handler(void *arg) { } static void ARDUINO_ISR_ATTR cdc0_write_char(char c) { - uint32_t tx_timeout_ms = 0; // if not connected, no timeout + uint32_t tx_timeout_ms = 0; if(usb_serial_jtag_is_connected()) { - tx_timeout_ms = requested_tx_timeout_ms; // once connected, restores the TX timeout + tx_timeout_ms = requested_tx_timeout_ms; } if(xPortInIsrContext()){ xRingbufferSendFromISR(tx_ring_buf, (void*) (&c), 1, NULL); } else { xRingbufferSend(tx_ring_buf, (void*) (&c), 1, tx_timeout_ms / portTICK_PERIOD_MS); } - usb_serial_jtag_ll_txfifo_flush(); // flushes HW Serial CDC and sets IN_EMPTY when Host reads data + usb_serial_jtag_ll_txfifo_flush(); } HWCDC::HWCDC() { @@ -162,8 +161,6 @@ HWCDC::~HWCDC(){ // It should return just when USB is plugged and CDC is connected. HWCDC::operator bool() const { - // deals when this function is called many times - // typically with something like `while(!Serial) delay(10);` static bool running = false; // USB may be unplugged @@ -181,7 +178,7 @@ HWCDC::operator bool() const if (running == false && !isConnected) { // enables it only once! usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY); } - // this will feed CDC TX FIFO and then trigger when CDC is connected + // this will feed CDC TX FIFO to trigger IN_EMPTY uint8_t c = '\0'; usb_serial_jtag_ll_write_txfifo(&c, sizeof(c)); usb_serial_jtag_ll_txfifo_flush(); @@ -298,12 +295,12 @@ size_t HWCDC::setTxBufferSize(size_t tx_queue_len){ int HWCDC::availableForWrite(void) { - uint32_t tx_timeout_ms = 0; // if not connected, no timeout + uint32_t tx_timeout_ms = 0; if(tx_ring_buf == NULL || tx_lock == NULL){ return 0; } if(usb_serial_jtag_is_connected()) { - tx_timeout_ms = requested_tx_timeout_ms; // once connected, restores the TX timeout + tx_timeout_ms = requested_tx_timeout_ms; } if(xSemaphoreTake(tx_lock, tx_timeout_ms / portTICK_PERIOD_MS) != pdPASS){ return 0; @@ -330,12 +327,12 @@ static void flushTXBuffer() size_t HWCDC::write(const uint8_t *buffer, size_t size) { - uint32_t tx_timeout_ms = 0; // if not connected, no timeout + uint32_t tx_timeout_ms = 0; if(buffer == NULL || size == 0 || tx_ring_buf == NULL || tx_lock == NULL){ return 0; } if(usb_serial_jtag_is_connected()) { - tx_timeout_ms = requested_tx_timeout_ms; // once connected, restores the TX timeout + tx_timeout_ms = requested_tx_timeout_ms; } else { isConnected = false; } @@ -350,7 +347,6 @@ size_t HWCDC::write(const uint8_t *buffer, size_t size) space = size; } // Non-Blocking method, Sending data to ringbuffer, and handle the data in ISR. - // USB may be plugged, but CDC may be not connected ==> do not block and flush TX buffer, keeping just the lastest data buffered if(xRingbufferSend(tx_ring_buf, (void*) (buffer), space, 0) != pdTRUE){ size = 0; } else { @@ -379,7 +375,7 @@ size_t HWCDC::write(const uint8_t *buffer, size_t size) // CDC is diconnected ==> flush all data from TX buffer if(to_send && !usb_serial_jtag_ll_txfifo_writable()) { isConnected = false; - flushTXBuffer(); // flush TX Ringbuffer + flushTXBuffer(); } xSemaphoreGive(tx_lock); return size; @@ -392,19 +388,18 @@ size_t HWCDC::write(uint8_t c) void HWCDC::flush(void) { - uint32_t tx_timeout_ms = 0; // if not connected, no timeout + uint32_t tx_timeout_ms = 0; if(tx_ring_buf == NULL || tx_lock == NULL){ return; } if(usb_serial_jtag_is_connected()) { - tx_timeout_ms = requested_tx_timeout_ms; // once connected, restores the TX timeout + tx_timeout_ms = requested_tx_timeout_ms; } else { isConnected = false; } if(xSemaphoreTake(tx_lock, tx_timeout_ms / portTICK_PERIOD_MS) != pdPASS){ return; } - // USB may be plugged, but CDC may be not connected ==> do not block and flush TX buffer, keeping just the lastest data buffered UBaseType_t uxItemsWaiting = 0; vRingbufferGetInfo(tx_ring_buf, NULL, NULL, NULL, NULL, &uxItemsWaiting); if(uxItemsWaiting){ @@ -415,13 +410,13 @@ void HWCDC::flush(void) uint8_t tries = 3; while(tries && uxItemsWaiting){ delay(5); - UBaseType_t lastUxItemsWaiting = uxItemsWaiting; // is it flushing CDC? + UBaseType_t lastUxItemsWaiting = uxItemsWaiting; vRingbufferGetInfo(tx_ring_buf, NULL, NULL, NULL, NULL, &uxItemsWaiting); - if (lastUxItemsWaiting == uxItemsWaiting) tries--; // avoids locking when USB is plugged, but CDC is not connected + if (lastUxItemsWaiting == uxItemsWaiting) tries--; } if (tries == 0) { // CDC isn't connected anymore... isConnected = false; - flushTXBuffer(); // flush TX Ringbuffer + flushTXBuffer(); } xSemaphoreGive(tx_lock); }