Fixes the hardware cdc jtag plugged/unplugged status and related timeout/delay

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,19 +28,19 @@
 #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);
 
 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;
+static volatile bool isConnected = false;
 
-// workaround for when USB CDC is not connected
-static uint32_t tx_timeout_ms = 0;
-static bool tx_timeout_change_request = false;
+// 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;
 
@@ -76,23 +76,20 @@ 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);
-            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.
@@ -102,14 +99,15 @@ 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??
         }
     }
 
@@ -124,18 +122,15 @@ 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);
+        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);
-        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);
+        isConnected = false; // TX/RX only takes place after USB BUS resets (it was just plugged)
     }
 
     if (xTaskWoken == pdTRUE) {
@@ -144,12 +139,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
+    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 {
         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() {
@@ -160,9 +159,35 @@ HWCDC::~HWCDC(){
     end();
 }
 
+
+// It should return <true> just when USB is plugged and CDC is connected.
 HWCDC::operator bool() const
 {
-    return initial_empty;
+    // <running> 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){
@@ -206,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");
         }    
     }
@@ -227,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()
@@ -248,13 +276,11 @@ void HWCDC::end()
         arduino_hw_cdc_event_loop_handle = NULL;
     }
     HWCDC::deinit(this);
+    isConnected = false;
 }
 
 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 +304,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;
     }
@@ -289,11 +319,32 @@ 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);
+    }
+    // flushes CDC FIFO
+    usb_serial_jtag_ll_txfifo_flush();
+}
+
 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
+    } else {
+        isConnected = false;
+    }
     if(xSemaphoreTake(tx_lock, tx_timeout_ms / portTICK_PERIOD_MS) != pdPASS){
         return 0;
     }
@@ -305,14 +356,16 @@ 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 {
         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;
@@ -325,9 +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 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;
 }
@@ -339,21 +399,36 @@ 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
+    } 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 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);
+        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(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 == 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);
 }
@@ -436,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

libraries/ESP32/examples/HWSerial_Events/.skip.esp32

@@ -0,0 +1 @@
+

libraries/ESP32/examples/HWSerial_Events/.skip.esp32s2

@@ -0,0 +1 @@
+