feat (hwcdc): ports changes made in 2.0.15 (#9565)

* feat (hwcdc): ports changes made in 2.0.15

Ports many changes, fixes and improvements made in 2.0.15:
- correct use of timeout
- avoids problems with CDC ISR not reading data
- fixes problems with transmitting many bytes to USB Host
- changes how USB SOF and CDC connection is detected

* feat (HWCDC) : port 2.0.15 

Changed header for a few functions.

* feat (HWCDC): port 2.0.15 upwards

Fixes include file that is not necessary any more.

- SOF is used directly now.

* fix (HWCDC): removes left over

Removes a left over function from previous 3.0.0 code.

- just removing unused code.

* ci(pre-commit): Apply automatic fixes

* fix: typo and commentaries

This fixes a few commentaries.

Just a typo error.

---------

Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com>

cores/esp32/HWCDC.cpp

@@ -29,7 +29,6 @@
 #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);
 
@@ -40,8 +39,11 @@ static intr_handle_t intr_handle = NULL;
 static SemaphoreHandle_t tx_lock = NULL;
 static volatile bool connected = false;
 
+static volatile unsigned long lastSOF_ms;
+static volatile uint8_t SOF_TIMEOUT;
+
 // timeout has no effect when USB CDC is unplugged
-static uint32_t requested_tx_timeout_ms = 100;
+static uint32_t tx_timeout_ms = 100;
 
 static esp_event_loop_handle_t arduino_hw_cdc_event_loop_handle = NULL;
 
@@ -77,7 +79,7 @@ static void hw_cdc_isr_handler(void *arg) {
 
   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()) {
+    if (!HWCDC::isPlugged()) {
       connected = false;
       usb_serial_jtag_ll_clr_intsts_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY);
       // USB is unplugged, nothing to be done here
@@ -132,19 +134,31 @@ static void hw_cdc_isr_handler(void *arg) {
     connected = false;
   }
 
+  if (usbjtag_intr_status & USB_SERIAL_JTAG_INTR_SOF) {
+    usb_serial_jtag_ll_clr_intsts_mask(USB_SERIAL_JTAG_INTR_SOF);
+    lastSOF_ms = millis();
+  }
+
   if (xTaskWoken == pdTRUE) {
     portYIELD_FROM_ISR();
   }
 }
 
+inline bool HWCDC::isPlugged(void) {
+  return (lastSOF_ms + SOF_TIMEOUT) >= millis();
+}
+
 bool HWCDC::isCDC_Connected() {
   static bool running = false;
 
   // USB may be unplugged
-  if (usb_serial_jtag_is_connected() == false) {
+  if (!isPlugged()) {
     connected = false;
     running = false;
+    SOF_TIMEOUT = 5;  // SOF timeout when unplugged
     return false;
+  } else {
+    SOF_TIMEOUT = 50;  // SOF timeout when plugged
   }
 
   if (connected) {
@@ -155,21 +169,72 @@ bool HWCDC::isCDC_Connected() {
   if (running == false && !connected) {  // enables it only once!
     usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY);
   }
+
   // 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();
   running = true;
   return false;
 }
 
+static void flushTXBuffer(const uint8_t *buffer, size_t size) {
+  if (!tx_ring_buf) {
+    return;
+  }
+  UBaseType_t uxItemsWaiting = 0;
+  vRingbufferGetInfo(tx_ring_buf, NULL, NULL, NULL, NULL, &uxItemsWaiting);
+  size_t freeSpace = xRingbufferGetCurFreeSize(tx_ring_buf);
+  size_t ringbufferLength = freeSpace + uxItemsWaiting;
+
+  if (buffer == NULL) {
+    // just flush the whole ring buffer and exit - used by HWCDC::flush()
+    size_t queued_size = 0;
+    uint8_t *queued_buff = (uint8_t *)xRingbufferReceiveUpTo(tx_ring_buf, &queued_size, 0, ringbufferLength);
+    if (queued_size && queued_buff != NULL) {
+      vRingbufferReturnItem(tx_ring_buf, (void *)queued_buff);
+    }
+    return;
+  }
+  if (size == 0) {
+    return;  // nothing to do
+  }
+  if (freeSpace >= size) {
+    // there is enough space, just add the data to the ring buffer
+    if (xRingbufferSend(tx_ring_buf, (void *)buffer, size, 0) != pdTRUE) {
+      return;
+    }
+  } else {
+    // how many byte should be flushed to make space for the new data
+    size_t to_flush = size - freeSpace;
+    if (to_flush > ringbufferLength) {
+      to_flush = ringbufferLength;
+    }
+    size_t queued_size = 0;
+    uint8_t *queued_buff = (uint8_t *)xRingbufferReceiveUpTo(tx_ring_buf, &queued_size, 0, to_flush);
+    if (queued_size && queued_buff != NULL) {
+      vRingbufferReturnItem(tx_ring_buf, (void *)queued_buff);
+    }
+    // now add the new data that fits into the ring buffer
+    uint8_t *bptr = (uint8_t *)buffer;
+    if (size >= ringbufferLength) {
+      size = ringbufferLength;
+      bptr = (uint8_t *)buffer + (size - ringbufferLength);
+    }
+    if (xRingbufferSend(tx_ring_buf, (void *)bptr, size, 0) != pdTRUE) {
+      return;
+    }
+  }
+  // flushes CDC FIFO
+  usb_serial_jtag_ll_txfifo_flush();
+}
+
 static void ARDUINO_ISR_ATTR cdc0_write_char(char c) {
   if (tx_ring_buf == NULL) {
     return;
   }
-  uint32_t tx_timeout_ms = 0;
+  if (!HWCDC::isConnected()) {
-  if (HWCDC::isConnected()) {
+    // just pop/push RingBuffer and apply FIFO policy
-    tx_timeout_ms = requested_tx_timeout_ms;
+    flushTXBuffer((const uint8_t *)&c, 1);
+    return;
   }
   if (xPortInIsrContext()) {
     xRingbufferSendFromISR(tx_ring_buf, (void *)(&c), 1, NULL);
@@ -182,6 +247,8 @@ static void ARDUINO_ISR_ATTR cdc0_write_char(char c) {
 HWCDC::HWCDC() {
   perimanSetBusDeinit(ESP32_BUS_TYPE_USB_DM, HWCDC::deinit);
   perimanSetBusDeinit(ESP32_BUS_TYPE_USB_DP, HWCDC::deinit);
+  lastSOF_ms = 0;
+  SOF_TIMEOUT = 5;
 }
 
 HWCDC::~HWCDC() {
@@ -234,9 +301,9 @@ void HWCDC::begin(unsigned long baud) {
       log_e("HW CDC RX Buffer error");
     }
   }
-  //TX Buffer default has 16 bytes if not preset
+  //TX Buffer default has 256 bytes if not preset
   if (tx_ring_buf == NULL) {
-    if (!setTxBufferSize(16)) {
+    if (!setTxBufferSize(256)) {
       log_e("HW CDC TX Buffer error");
     }
   }
@@ -265,7 +332,9 @@ void HWCDC::begin(unsigned long baud) {
   // Enable USB pad function
   USB_SERIAL_JTAG.conf0.usb_pad_enable = 1;
   usb_serial_jtag_ll_disable_intr_mask(USB_SERIAL_JTAG_LL_INTR_MASK);
-  usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY | USB_SERIAL_JTAG_INTR_SERIAL_OUT_RECV_PKT | USB_SERIAL_JTAG_INTR_BUS_RESET);
+  usb_serial_jtag_ll_ena_intr_mask(
+    USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY | USB_SERIAL_JTAG_INTR_SERIAL_OUT_RECV_PKT | USB_SERIAL_JTAG_INTR_BUS_RESET | USB_SERIAL_JTAG_INTR_SOF
+  );
   if (!intr_handle && esp_intr_alloc(ETS_USB_SERIAL_JTAG_INTR_SOURCE, 0, hw_cdc_isr_handler, NULL, &intr_handle) != ESP_OK) {
     isr_log_e("HW USB CDC failed to init interrupts");
     end();
@@ -300,7 +369,7 @@ void HWCDC::end() {
 }
 
 void HWCDC::setTxTimeoutMs(uint32_t timeout) {
-  requested_tx_timeout_ms = timeout;
+  tx_timeout_ms = timeout;
 }
 
 /*
@@ -323,13 +392,9 @@ size_t HWCDC::setTxBufferSize(size_t tx_queue_len) {
 }
 
 int HWCDC::availableForWrite(void) {
-  uint32_t tx_timeout_ms = 0;
   if (tx_ring_buf == NULL || tx_lock == NULL) {
     return 0;
   }
-  if (HWCDC::isCDC_Connected()) {
-    tx_timeout_ms = requested_tx_timeout_ms;
-  }
   if (xSemaphoreTake(tx_lock, tx_timeout_ms / portTICK_PERIOD_MS) != pdPASS) {
     return 0;
   }
@@ -338,35 +403,17 @@ 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 (buffer == NULL || size == 0 || tx_ring_buf == NULL || tx_lock == NULL) {
     return 0;
   }
-  if (HWCDC::isCDC_Connected()) {
-    tx_timeout_ms = requested_tx_timeout_ms;
-  } else {
-    connected = false;
-  }
   if (xSemaphoreTake(tx_lock, tx_timeout_ms / portTICK_PERIOD_MS) != pdPASS) {
     return 0;
   }
+  if (!isCDC_Connected()) {
+    // just pop/push RingBuffer and apply FIFO policy
+    flushTXBuffer(buffer, size);
+  } else {
     size_t space = xRingbufferGetCurFreeSize(tx_ring_buf);
     size_t to_send = size, so_far = 0;
 
@@ -384,8 +431,10 @@ size_t HWCDC::write(const uint8_t *buffer, size_t size) {
       if (connected) {
         usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY);
       }
-
+      // tracks CDC trasmission progress to avoid hanging if CDC is unplugged while still sending data
-    while (to_send) {
+      size_t last_toSend = to_send;
+      uint32_t tries = tx_timeout_ms;  // waits 1ms per sending data attempt, in case CDC is unplugged
+      while (connected && to_send) {
         space = xRingbufferGetCurFreeSize(tx_ring_buf);
         if (space > to_send) {
           space = to_send;
@@ -393,6 +442,7 @@ size_t HWCDC::write(const uint8_t *buffer, size_t size) {
         // Blocking method, Sending data to ringbuffer, and handle the data in ISR.
         if (xRingbufferSend(tx_ring_buf, (void *)(buffer + so_far), space, tx_timeout_ms / portTICK_PERIOD_MS) != pdTRUE) {
           size = so_far;
+          log_w("write failed due to ring buffer full - timeout");
           break;
         }
         so_far += space;
@@ -402,12 +452,26 @@ size_t HWCDC::write(const uint8_t *buffer, size_t size) {
         if (connected) {
           usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY);
         }
+        if (last_toSend == to_send) {
+          // no progress in sending data... USB CDC is probably unplugged
+          tries--;
+          delay(1);
+        } else {
+          last_toSend = to_send;
+          tries = tx_timeout_ms;  // reset the timeout
+        }
+        if (tries == 0) {  // CDC isn't connected anymore...
+          size = so_far;
+          log_w("write failed due to waiting USB Host - timeout");
+          connected = false;
         }
       }
-  // CDC is disconnected ==> flush all data from TX buffer
+    }
+    // CDC was diconnected while sending data ==> flush the TX buffer keeping the last data
     if (to_send && !usb_serial_jtag_ll_txfifo_writable()) {
       connected = false;
-    flushTXBuffer();
+      flushTXBuffer(buffer + so_far, to_send);
+    }
   }
   xSemaphoreGive(tx_lock);
   return size;
@@ -418,18 +482,15 @@ size_t HWCDC::write(uint8_t c) {
 }
 
 void HWCDC::flush(void) {
-  uint32_t tx_timeout_ms = 0;
   if (tx_ring_buf == NULL || tx_lock == NULL) {
     return;
   }
-  if (HWCDC::isCDC_Connected()) {
-    tx_timeout_ms = requested_tx_timeout_ms;
-  } else {
-    connected = false;
-  }
   if (xSemaphoreTake(tx_lock, tx_timeout_ms / portTICK_PERIOD_MS) != pdPASS) {
     return;
   }
+  if (!isCDC_Connected()) {
+    flushTXBuffer(NULL, 0);
+  } else {
     UBaseType_t uxItemsWaiting = 0;
     vRingbufferGetInfo(tx_ring_buf, NULL, NULL, NULL, NULL, &uxItemsWaiting);
     if (uxItemsWaiting) {
@@ -439,18 +500,22 @@ void HWCDC::flush(void) {
         usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY);
       }
     }
-  uint8_t tries = 3;
+    uint32_t tries = tx_timeout_ms;  // waits 1ms per ISR sending data attempt, in case CDC is unplugged
-  while (tries && uxItemsWaiting) {
+    while (connected && tries && uxItemsWaiting) {
-    delay(5);
+      delay(1);
       UBaseType_t lastUxItemsWaiting = uxItemsWaiting;
       vRingbufferGetInfo(tx_ring_buf, NULL, NULL, NULL, NULL, &uxItemsWaiting);
       if (lastUxItemsWaiting == uxItemsWaiting) {
         tries--;
       }
+      if (connected) {
+        usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY);
+      }
     }
     if (tries == 0) {  // CDC isn't connected anymore...
       connected = false;
-    flushTXBuffer();
+      flushTXBuffer(NULL, 0);  // flushes all TX Buffer
+    }
   }
   xSemaphoreGive(tx_lock);
 }

cores/esp32/HWCDC.h

@@ -21,7 +21,6 @@
 #include <inttypes.h>
 #include "esp_event.h"
 #include "Stream.h"
-#include "driver/usb_serial_jtag.h"
 
 ESP_EVENT_DECLARE_BASE(ARDUINO_HW_CDC_EVENTS);
 
@@ -70,10 +69,7 @@ public:
   size_t write(const uint8_t *buffer, size_t size);
   void flush(void);
 
-  inline static bool isPlugged(void) {
+  static bool isPlugged(void);
-    return usb_serial_jtag_is_connected();
-  }
-
   inline static bool isConnected(void) {
     return isCDC_Connected();
   }