Various USB fixes (espressif#5422)

* Fix compile archive arguments for the new toolchain * Add menu to S2 for picking through which port to upload Internal USB CDC requires to reset and wait for the new port (because persistence is not yet stable) * USB CDC should also be started in main * Fix URL and USB version for WebUSB * Update vendor callback API * Update CDC::write to use TX_DONE semaphore * Update USB_Serial example
WebDust21 · Jul 20, 2021 · 268595c · 268595c
1 parent 4a0305a
commit 268595c
Show file tree

Hide file tree

Showing 8 changed files with 106 additions and 47 deletions.
diff --git a/boards.txt b/boards.txt
@@ -1,5 +1,6 @@
 menu.UploadSpeed=Upload Speed
 menu.SerialMode=Serial Connected To
+menu.UploadMode=Upload Mode
 menu.CPUFreq=CPU Frequency
 menu.FlashFreq=Flash Frequency
 menu.FlashMode=Flash Mode
@@ -158,8 +159,8 @@ esp32s2.upload.maximum_size=1310720
 esp32s2.upload.maximum_data_size=327680
 esp32s2.upload.flags=
 esp32s2.upload.extra_flags=
-esp32s2.upload.use_1200bps_touch=true
-esp32s2.upload.wait_for_upload_port=true
+esp32s2.upload.use_1200bps_touch=false
+esp32s2.upload.wait_for_upload_port=false
 
 esp32s2.serial.disableDTR=false
 esp32s2.serial.disableRTS=false
@@ -186,6 +187,13 @@ esp32s2.menu.SerialMode.default.build.serial=0
 esp32s2.menu.SerialMode.cdc=USB CDC
 esp32s2.menu.SerialMode.cdc.build.serial=1
 
+esp32s2.menu.UploadMode.default=UART0
+esp32s2.menu.UploadMode.default.upload.use_1200bps_touch=false
+esp32s2.menu.UploadMode.default.upload.wait_for_upload_port=false
+esp32s2.menu.UploadMode.cdc=Internal USB
+esp32s2.menu.UploadMode.cdc.upload.use_1200bps_touch=true
+esp32s2.menu.UploadMode.cdc.upload.wait_for_upload_port=true
+
 esp32s2.menu.PSRAM.disabled=Disabled
 esp32s2.menu.PSRAM.disabled.build.defines=
 esp32s2.menu.PSRAM.enabled=Enabled

diff --git a/cores/esp32/USB.cpp b/cores/esp32/USB.cpp
@@ -121,7 +121,7 @@ ESPUSB::ESPUSB(size_t task_stack_size, uint8_t event_task_priority)
 ,usb_attributes(TUSB_DESC_CONFIG_ATT_SELF_POWERED)
 ,usb_power_ma(500)
 ,webusb_enabled(false)
-,webusb_url("espressif.github.io/arduino-esp32/webusb.html")
+,webusb_url("https://espressif.github.io/arduino-esp32/webusb.html")
 ,_started(false)
 ,_task_stack_size(task_stack_size)
 ,_event_task_priority(event_task_priority)
@@ -282,6 +282,9 @@ uint8_t ESPUSB::usbAttributes(void){
 bool ESPUSB::webUSB(bool enabled){
     if(!_started){
         webusb_enabled = enabled;
+        if(enabled && usb_version < 0x0210){
+            usb_version = 0x0210;
+        }
     }
     return !_started;
 }

diff --git a/cores/esp32/USBCDC.cpp b/cores/esp32/USBCDC.cpp
@@ -38,27 +38,46 @@ static uint16_t load_cdc_descriptor(uint8_t * dst, uint8_t * itf)
     return TUD_CDC_DESC_LEN;
 }
 
+// Invoked when line state DTR & RTS are changed via SET_CONTROL_LINE_STATE
 void tud_cdc_line_state_cb(uint8_t itf, bool dtr, bool rts)
 {
+    //isr_log_v("itf: %u, dtr: %u, rts: %u", itf, dtr, rts);
     if(itf < MAX_USB_CDC_DEVICES && devices[itf] != NULL){
         devices[itf]->_onLineState(dtr, rts);
     }
 }
 
+// Invoked when line coding is change via SET_LINE_CODING
 void tud_cdc_line_coding_cb(uint8_t itf, cdc_line_coding_t const* p_line_coding)
 {
+    //isr_log_v("itf: %u, bit_rate: %u, data_bits: %u, stop_bits: %u, parity: %u", itf, p_line_coding->bit_rate, p_line_coding->data_bits, p_line_coding->stop_bits, p_line_coding->parity);
     if(itf < MAX_USB_CDC_DEVICES && devices[itf] != NULL){
         devices[itf]->_onLineCoding(p_line_coding->bit_rate, p_line_coding->stop_bits, p_line_coding->parity, p_line_coding->data_bits);
     }
 }
 
+// Invoked when received new data
 void tud_cdc_rx_cb(uint8_t itf)
 {
+    //isr_log_v("itf: %u", itf);
     if(itf < MAX_USB_CDC_DEVICES && devices[itf] != NULL){
         devices[itf]->_onRX();
     }
 }
 
+// Invoked when received send break
+void tud_cdc_send_break_cb(uint8_t itf, uint16_t duration_ms){
+    //isr_log_v("itf: %u, duration_ms: %u", itf, duration_ms);
+}
+
+// Invoked when space becomes available in TX buffer
+void tud_cdc_tx_complete_cb(uint8_t itf){
+    //isr_log_v("itf: %u", itf);
+    if(itf < MAX_USB_CDC_DEVICES && devices[itf] != NULL){
+        xSemaphoreGive(devices[itf]->tx_sem);
+        devices[itf]->_onTX();
+    }
+}
 
 static size_t tinyusb_cdc_write(uint8_t itf, const uint8_t *buffer, size_t size){
     if(itf >= MAX_USB_CDC_DEVICES){
@@ -84,6 +103,7 @@ static size_t tinyusb_cdc_write(uint8_t itf, const uint8_t *buffer, size_t size)
         sofar += sent;
         tosend -= sent;
         tud_cdc_n_write_flush(itf);
+        xSemaphoreTake(devices[itf]->tx_sem, portMAX_DELAY);
     }
     return sofar;
 }
@@ -103,6 +123,7 @@ USBCDC::USBCDC(uint8_t itfn) : itf(itfn), bit_rate(0), stop_bits(0), parity(0),
     tinyusb_enable_interface(USB_INTERFACE_CDC, TUD_CDC_DESC_LEN, load_cdc_descriptor);
     if(itf < MAX_USB_CDC_DEVICES){
         devices[itf] = this;
+        tx_sem = NULL;
         arduino_usb_event_handler_register_with(ARDUINO_USB_EVENTS, ARDUINO_USB_STOPPED_EVENT, usb_unplugged_cb, this);
     }
 }
@@ -128,10 +149,18 @@ size_t USBCDC::setRxBufferSize(size_t rx_queue_len){
 void USBCDC::begin(unsigned long baud)
 {
     setRxBufferSize(256);//default if not preset
+    if(tx_sem == NULL){
+        tx_sem = xSemaphoreCreateBinary();
+        xSemaphoreTake(tx_sem, 0);
+    }
 }
 
 void USBCDC::end()
 {
+    if (tx_sem != NULL) {
+        vSemaphoreDelete(tx_sem);
+        tx_sem = NULL;
+    }
 }
 
 void USBCDC::_onUnplugged(void){
@@ -228,6 +257,11 @@ void USBCDC::_onRX(){
     arduino_usb_event_post(ARDUINO_USB_CDC_EVENTS, ARDUINO_USB_CDC_RX_EVENT, &p, sizeof(arduino_usb_cdc_event_data_t), portMAX_DELAY);
 }
 
+void USBCDC::_onTX(){
+    arduino_usb_cdc_event_data_t p = {0};
+    arduino_usb_event_post(ARDUINO_USB_CDC_EVENTS, ARDUINO_USB_CDC_TX_EVENT, &p, sizeof(arduino_usb_cdc_event_data_t), portMAX_DELAY);
+}
+
 void  USBCDC::enableReboot(bool enable){
     reboot_enable = enable;
 }

diff --git a/cores/esp32/USBCDC.h b/cores/esp32/USBCDC.h
@@ -30,6 +30,7 @@ typedef enum {
     ARDUINO_USB_CDC_LINE_STATE_EVENT,
     ARDUINO_USB_CDC_LINE_CODING_EVENT,
     ARDUINO_USB_CDC_RX_EVENT,
+    ARDUINO_USB_CDC_TX_EVENT,
     ARDUINO_USB_CDC_MAX_EVENT,
 } arduino_usb_cdc_event_t;
 
@@ -110,7 +111,9 @@ class USBCDC: public Stream
     void _onLineState(bool _dtr, bool _rts);
     void _onLineCoding(uint32_t _bit_rate, uint8_t _stop_bits, uint8_t _parity, uint8_t _data_bits);
     void _onRX(void);
+    void _onTX(void);
     void _onUnplugged(void);
+    xSemaphoreHandle tx_sem;
 
 protected:
     uint8_t  itf;

diff --git a/cores/esp32/esp32-hal-tinyusb.c b/cores/esp32/esp32-hal-tinyusb.c
@@ -228,7 +228,7 @@ typedef struct TU_ATTR_PACKED {
 static tinyusb_desc_webusb_url_t tinyusb_url_descriptor = {
         .bLength         = 3,
         .bDescriptorType = 3, // WEBUSB URL type
-        .bScheme         = 1, // URL Scheme Prefix: 0: "http://", 1: "https://", 255: ""
+        .bScheme         = 255, // URL Scheme Prefix: 0: "http://", 1: "https://", 255: ""
         .url             = ""
 };
 
@@ -317,12 +317,11 @@ uint16_t const *tud_descriptor_string_cb(uint8_t index, uint16_t langid)
  */
 uint8_t const * tud_descriptor_bos_cb(void)
 {
-    //log_d("");
+    //log_v("");
     return tinyusb_bos_descriptor;
 }
 
-__attribute__ ((weak)) bool tinyusb_vendor_control_request_cb(uint8_t rhport, tusb_control_request_t const * request){ return false; }
-__attribute__ ((weak)) bool tinyusb_vendor_control_complete_cb(uint8_t rhport, tusb_control_request_t const * request){ return true; }
+__attribute__ ((weak)) bool tinyusb_vendor_control_request_cb(uint8_t rhport, uint8_t stage, tusb_control_request_t const * request){ return false; }
 
 /**
  * @brief Handle WebUSB and Vendor requests.
@@ -331,30 +330,26 @@ bool tud_vendor_control_xfer_cb(uint8_t rhport, uint8_t stage, tusb_control_requ
 {
     if(WEBUSB_ENABLED && (request->bRequest == VENDOR_REQUEST_WEBUSB
             || (request->bRequest == VENDOR_REQUEST_MICROSOFT && request->wIndex == 7))){
-        if(request->bRequest == VENDOR_REQUEST_WEBUSB){
-            // match vendor request in BOS descriptor
-            // Get landing page url
-            tinyusb_url_descriptor.bLength = 3 + strlen(WEBUSB_URL);
-            snprintf(tinyusb_url_descriptor.url, 127, "%s", WEBUSB_URL);
-            return tud_control_xfer(rhport, request, (void*) &tinyusb_url_descriptor, tinyusb_url_descriptor.bLength);
+        // we only care for SETUP stage
+        if (stage == CONTROL_STAGE_SETUP) {
+            if(request->bRequest == VENDOR_REQUEST_WEBUSB){
+                // match vendor request in BOS descriptor
+                // Get landing page url
+                tinyusb_url_descriptor.bLength = 3 + strlen(WEBUSB_URL);
+                snprintf(tinyusb_url_descriptor.url, 127, "%s", WEBUSB_URL);
+                return tud_control_xfer(rhport, request, (void*) &tinyusb_url_descriptor, tinyusb_url_descriptor.bLength);
+            }
+            // Get Microsoft OS 2.0 compatible descriptor
+            uint16_t total_len;
+            memcpy(&total_len, tinyusb_ms_os_20_descriptor + 8, 2);
+            return tud_control_xfer(rhport, request, (void*) tinyusb_ms_os_20_descriptor, total_len);
         }
-        // Get Microsoft OS 2.0 compatible descriptor
-        uint16_t total_len;
-        memcpy(&total_len, tinyusb_ms_os_20_descriptor + 8, 2);
-        return tud_control_xfer(rhport, request, (void*) tinyusb_ms_os_20_descriptor, total_len);
+        return true;
     }
-    return tinyusb_vendor_control_request_cb(rhport, request);
+    log_v("rhport: %u, stage: %u, type: 0x%x, request: 0x%x", rhport, stage, request->bmRequestType_bit.type, request->bRequest);
+    return tinyusb_vendor_control_request_cb(rhport, stage, request);
 }
 
-// bool tud_vendor_control_complete_cb(uint8_t rhport, tusb_control_request_t const * request)
-// {
-//     if(!WEBUSB_ENABLED || !(request->bRequest == VENDOR_REQUEST_WEBUSB
-//             || (request->bRequest == VENDOR_REQUEST_MICROSOFT && request->wIndex == 7))){
-//         return tinyusb_vendor_control_complete_cb(rhport, request);
-//     }
-//     return true;
-// }
-
 /*
  * Required Callbacks
  * */
@@ -537,6 +532,9 @@ static void IRAM_ATTR usb_persist_shutdown_handler(void)
             REG_WRITE(RTC_CNTL_OPTION1_REG, RTC_CNTL_FORCE_DOWNLOAD_BOOT);
         } else if (usb_persist_mode == RESTART_BOOTLOADER_DFU) {
             //DFU Download
+            // Reset USB Core
+            USB0.grstctl |= USB_CSFTRST;
+            while ((USB0.grstctl & USB_CSFTRST) == USB_CSFTRST){}
             chip_usb_set_persist_flags(USBDC_BOOT_DFU);
             REG_WRITE(RTC_CNTL_OPTION1_REG, RTC_CNTL_FORCE_DOWNLOAD_BOOT);
         } else if (usb_persist_enabled) {

diff --git a/cores/esp32/main.cpp b/cores/esp32/main.cpp
@@ -49,6 +49,7 @@ extern "C" void app_main()
 {
 #if ARDUINO_SERIAL_PORT //Serial used for USB CDC
     USB.begin();
+    Serial.begin();
 #endif
     loopTaskWDTEnabled = false;
     initArduino();

diff --git a/libraries/USB/examples/USBSerial/USBSerial.ino b/libraries/USB/examples/USBSerial/USBSerial.ino
@@ -1,21 +1,28 @@
 #include "USB.h"
+
+#if ARDUINO_SERIAL_PORT
+#define HWSerial Serial0
+#define USBSerial Serial
+#else
+#define HWSerial Serial
 USBCDC USBSerial;
+#endif
 
 static void usbEventCallback(void* arg, esp_event_base_t event_base, int32_t event_id, void* event_data){
   if(event_base == ARDUINO_USB_EVENTS){
     arduino_usb_event_data_t * data = (arduino_usb_event_data_t*)event_data;
     switch (event_id){
       case ARDUINO_USB_STARTED_EVENT:
-        Serial.println("USB PLUGGED");
+        HWSerial.println("USB PLUGGED");
         break;
       case ARDUINO_USB_STOPPED_EVENT:
-        Serial.println("USB UNPLUGGED");
+        HWSerial.println("USB UNPLUGGED");
         break;
       case ARDUINO_USB_SUSPEND_EVENT:
-        Serial.printf("USB SUSPENDED: remote_wakeup_en: %u\n", data->suspend.remote_wakeup_en);
+        HWSerial.printf("USB SUSPENDED: remote_wakeup_en: %u\n", data->suspend.remote_wakeup_en);
         break;
       case ARDUINO_USB_RESUME_EVENT:
-        Serial.println("USB RESUMED");
+        HWSerial.println("USB RESUMED");
         break;
 
       default:
@@ -25,24 +32,25 @@ static void usbEventCallback(void* arg, esp_event_base_t event_base, int32_t eve
     arduino_usb_cdc_event_data_t * data = (arduino_usb_cdc_event_data_t*)event_data;
     switch (event_id){
       case ARDUINO_USB_CDC_CONNECTED_EVENT:
-        Serial.println("CDC CONNECTED");
+        HWSerial.println("CDC CONNECTED");
         break;
       case ARDUINO_USB_CDC_DISCONNECTED_EVENT:
-        Serial.println("CDC DISCONNECTED");
+        HWSerial.println("CDC DISCONNECTED");
         break;
       case ARDUINO_USB_CDC_LINE_STATE_EVENT:
-        Serial.printf("CDC LINE STATE: dtr: %u, rts: %u\n", data->line_state.dtr, data->line_state.rts);
+        HWSerial.printf("CDC LINE STATE: dtr: %u, rts: %u\n", data->line_state.dtr, data->line_state.rts);
         break;
       case ARDUINO_USB_CDC_LINE_CODING_EVENT:
-        Serial.printf("CDC LINE CODING: bit_rate: %u, data_bits: %u, stop_bits: %u, parity: %u\n", data->line_coding.bit_rate, data->line_coding.data_bits, data->line_coding.stop_bits, data->line_coding.parity);
+        HWSerial.printf("CDC LINE CODING: bit_rate: %u, data_bits: %u, stop_bits: %u, parity: %u\n", data->line_coding.bit_rate, data->line_coding.data_bits, data->line_coding.stop_bits, data->line_coding.parity);
         break;
       case ARDUINO_USB_CDC_RX_EVENT:
-        Serial.printf("CDC RX: %u\n", data->rx.len);
+        HWSerial.printf("CDC RX [%u]:", data->rx.len);
         {
             uint8_t buf[data->rx.len];
             size_t len = USBSerial.read(buf, data->rx.len);
-            Serial.write(buf, len);
+            HWSerial.write(buf, len);
         }
+        HWSerial.println();
         break;
 
       default:
@@ -51,24 +59,28 @@ static void usbEventCallback(void* arg, esp_event_base_t event_base, int32_t eve
   }
 }
 
-
 void setup() {
-  Serial.begin(115200);
-  Serial.setDebugOutput(true);
+  HWSerial.begin(115200);
+  HWSerial.setDebugOutput(true);
 
   USB.onEvent(usbEventCallback);
+  USBSerial.onEvent(usbEventCallback);
+
+#if !ARDUINO_SERIAL_PORT
+  USB.enableDFU();
+  USB.webUSB(true);
+  USB.webUSBURL("http://localhost/webusb");
   USB.productName("ESP32S2-USB");
   USB.begin();
-
-  USBSerial.onEvent(usbEventCallback);
   USBSerial.begin();
+#endif
 }
 
 void loop() {
-  while(Serial.available()){
-    size_t l = Serial.available();
+  while(HWSerial.available()){
+    size_t l = HWSerial.available();
     uint8_t b[l];
-    l = Serial.read(b, l);
+    l = HWSerial.read(b, l);
     USBSerial.write(b, l);
   }
 }
diff --git a/platform.txt b/platform.txt
@@ -29,7 +29,7 @@ compiler.c.flags.esp32=-mlongcalls -Wno-frame-address -ffunction-sections -fdata
 compiler.cpp.flags.esp32=-mlongcalls -Wno-frame-address -ffunction-sections -fdata-sections -Wno-error=unused-function -Wno-error=unused-variable -Wno-error=deprecated-declarations -Wno-unused-parameter -Wno-sign-compare -ggdb -O2 -Wwrite-strings -fstack-protector -fstrict-volatile-bitfields -Wno-error=unused-but-set-variable -fno-jump-tables -fno-tree-switch-conversion -std=gnu++11 -fexceptions -fno-rtti  -MMD -c
 compiler.S.flags.esp32=-ffunction-sections -fdata-sections -Wno-error=unused-function -Wno-error=unused-variable -Wno-error=deprecated-declarations -Wno-unused-parameter -Wno-sign-compare -ggdb -O2 -Wwrite-strings -fstack-protector -fstrict-volatile-bitfields -Wno-error=unused-but-set-variable -fno-jump-tables -fno-tree-switch-conversion  -x assembler-with-cpp -MMD -c
 compiler.c.elf.flags.esp32=-T esp32.rom.redefined.ld -T esp32.rom.ld -T esp32.rom.api.ld -T esp32.rom.libgcc.ld -T esp32.rom.newlib-data.ld -T esp32.rom.syscalls.ld -T esp32_out.ld -T esp32.project.ld -T esp32.peripherals.ld  -mlongcalls -Wno-frame-address -Wl,--cref -Wl,--gc-sections -fno-rtti -fno-lto -u _Z5setupv -u _Z4loopv -Wl,--wrap=mbedtls_mpi_exp_mod -u esp_app_desc -u pthread_include_pthread_impl -u pthread_include_pthread_cond_impl -u pthread_include_pthread_local_storage_impl -u ld_include_panic_highint_hdl -u start_app -u start_app_other_cores -u __ubsan_include -Wl,--wrap=longjmp -u __assert_func -u vfs_include_syscalls_impl -u call_user_start_cpu0 -Wl,--undefined=uxTopUsedPriority -u app_main -u newlib_include_heap_impl -u newlib_include_syscalls_impl -u newlib_include_pthread_impl -u __cxa_guard_dummy 
-compiler.ar.flags.esp32=cru
+compiler.ar.flags.esp32=cr
 build.extra_flags.esp32=-DARDUINO_SERIAL_PORT=0
 #
 # ESP32 Support End
@@ -44,7 +44,7 @@ compiler.c.flags.esp32s2=-mlongcalls -ffunction-sections -fdata-sections -Wno-er
 compiler.cpp.flags.esp32s2=-mlongcalls -ffunction-sections -fdata-sections -Wno-error=unused-function -Wno-error=unused-variable -Wno-error=deprecated-declarations -Wno-unused-parameter -Wno-sign-compare -ggdb -O2 -Wwrite-strings -fstack-protector -fstrict-volatile-bitfields -Wno-error=unused-but-set-variable -fno-jump-tables -fno-tree-switch-conversion -std=gnu++11 -fexceptions -fno-rtti  -MMD -c
 compiler.S.flags.esp32s2=-ffunction-sections -fdata-sections -Wno-error=unused-function -Wno-error=unused-variable -Wno-error=deprecated-declarations -Wno-unused-parameter -Wno-sign-compare -ggdb -O2 -Wwrite-strings -fstack-protector -fstrict-volatile-bitfields -Wno-error=unused-but-set-variable -fno-jump-tables -fno-tree-switch-conversion  -x assembler-with-cpp -MMD -c
 compiler.c.elf.flags.esp32s2=-T esp32s2.rom.ld -T esp32s2.rom.api.ld -T esp32s2.rom.libgcc.ld -T esp32s2.rom.newlib-funcs.ld -T esp32s2.rom.newlib-data.ld -T esp32s2.rom.spiflash.ld -T esp32s2_out.ld -T esp32s2.project.ld -T esp32s2.peripherals.ld  -mlongcalls -Wl,--cref -Wl,--gc-sections -fno-rtti -fno-lto -u _Z5setupv -u _Z4loopv -u esp_app_desc -u pthread_include_pthread_impl -u pthread_include_pthread_cond_impl -u pthread_include_pthread_local_storage_impl -u ld_include_panic_highint_hdl -u start_app -u __ubsan_include -Wl,--wrap=longjmp -u __assert_func -u vfs_include_syscalls_impl -u call_user_start_cpu0 -Wl,--undefined=uxTopUsedPriority -u app_main -u newlib_include_heap_impl -u newlib_include_syscalls_impl -u newlib_include_pthread_impl -u __cxa_guard_dummy 
-compiler.ar.flags.esp32s2=cru
+compiler.ar.flags.esp32s2=cr
 build.extra_flags.esp32s2=-DARDUINO_SERIAL_PORT={build.serial}
 #
 # ESP32S2 Support End