From 2beba21e098b4fba890949d198eb16e10c79ea3c Mon Sep 17 00:00:00 2001 From: me-no-dev Date: Wed, 8 Sep 2021 15:12:20 +0300 Subject: [PATCH 1/6] [HWCDC] make sure that ISR is running and will pick up any leftover data --- cores/esp32/HWCDC.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cores/esp32/HWCDC.cpp b/cores/esp32/HWCDC.cpp index 05cb816fd47..662eefea8b1 100644 --- a/cores/esp32/HWCDC.cpp +++ b/cores/esp32/HWCDC.cpp @@ -187,6 +187,10 @@ void HWCDC::flush(void) } 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){ delay(5); vRingbufferGetInfo(tx_ring_buf, NULL, NULL, NULL, NULL, &uxItemsWaiting); From 97eea6c4e9889019e6b0a344cd2a5a8d783c399c Mon Sep 17 00:00:00 2001 From: me-no-dev Date: Wed, 15 Sep 2021 18:15:16 +0300 Subject: [PATCH 2/6] Fix not being able to send data, larger than MaxItemSize Also adds semaphore for the sending buffer --- cores/esp32/HWCDC.cpp | 75 +++++++++++++++++++++++++++++++++++++------ cores/esp32/HWCDC.h | 1 + 2 files changed, 66 insertions(+), 10 deletions(-) diff --git a/cores/esp32/HWCDC.cpp b/cores/esp32/HWCDC.cpp index 662eefea8b1..55e748c9feb 100644 --- a/cores/esp32/HWCDC.cpp +++ b/cores/esp32/HWCDC.cpp @@ -29,6 +29,8 @@ static xQueueHandle rx_queue = NULL; static uint8_t rx_data_buf[64]; static intr_handle_t intr_handle = NULL; static volatile bool initial_empty = false; +static xSemaphoreHandle tx_lock = NULL; +static uint32_t tx_timeout_ms = 200; static void hw_cdc_isr_handler(void *arg) { portBASE_TYPE xTaskWoken = 0; @@ -95,7 +97,7 @@ static void ARDUINO_ISR_ATTR cdc0_write_char(char c) { if(xPortInIsrContext()){ xRingbufferSendFromISR(tx_ring_buf, (void*) (&c), 1, NULL); } else { - xRingbufferSend(tx_ring_buf, (void*) (&c), 1, 0); + 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); } @@ -113,8 +115,13 @@ HWCDC::operator bool() const return initial_empty; } +static void dummy_putc(char c){} + void HWCDC::begin(unsigned long baud) { + if(tx_lock == NULL) { + tx_lock = xSemaphoreCreateMutex(); + } setRxBufferSize(256);//default if not preset setTxBufferSize(256);//default if not preset @@ -123,6 +130,7 @@ void HWCDC::begin(unsigned long baud) if(!intr_handle && esp_intr_alloc(ETS_USB_INTR_SOURCE/*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(); + return; } } @@ -132,10 +140,17 @@ void HWCDC::end() usb_serial_jtag_ll_disable_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY | USB_SERIAL_JTAG_INTR_SERIAL_OUT_RECV_PKT | USB_SERIAL_JTAG_INTR_BUS_RESET); esp_intr_free(intr_handle); intr_handle = NULL; + if(tx_lock != NULL) { + vSemaphoreDelete(tx_lock); + } setRxBufferSize(0); setTxBufferSize(0); } +void HWCDC::setTxTimeoutMs(uint32_t timeout){ + tx_timeout_ms = timeout; +} + /* * WRITING */ @@ -157,21 +172,57 @@ size_t HWCDC::setTxBufferSize(size_t tx_queue_len){ int HWCDC::availableForWrite(void) { - if(tx_ring_buf == NULL){ - return -1; + if(tx_ring_buf == NULL || tx_lock == NULL){ + return 0; + } + if(xSemaphoreTake(tx_lock, tx_timeout_ms / portTICK_PERIOD_MS) != pdPASS){ + return 0; } - return xRingbufferGetCurFreeSize(tx_ring_buf); + size_t a = xRingbufferGetCurFreeSize(tx_ring_buf); + xSemaphoreGive(tx_lock); + return a; } 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), size, 200 / portTICK_PERIOD_MS) != pdTRUE){ - log_e("Write Failed"); + if(buffer == NULL || size == 0 || tx_ring_buf == NULL || tx_lock == NULL){ return 0; } - // 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); + if(xSemaphoreTake(tx_lock, tx_timeout_ms / portTICK_PERIOD_MS) != pdPASS){ + return 0; + } + size_t max_size = xRingbufferGetMaxItemSize(tx_ring_buf); + size_t space = xRingbufferGetCurFreeSize(tx_ring_buf); + size_t to_send = size, so_far = 0; + + if(space > size){ + space = size; + } + // Non-Blocking method, Sending data to ringbuffer, and handle the data in ISR. + 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); + + while(to_send){ + if(max_size > to_send){ + max_size = to_send; + } + // Blocking method, Sending data to ringbuffer, and handle the data in ISR. + if(xRingbufferSend(tx_ring_buf, (void*) (buffer+so_far), max_size, tx_timeout_ms / portTICK_PERIOD_MS) != pdTRUE){ + size = so_far; + break; + } + 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); + } + } + xSemaphoreGive(tx_lock); return size; } @@ -182,7 +233,10 @@ size_t HWCDC::write(uint8_t c) void HWCDC::flush(void) { - if(tx_ring_buf == NULL){ + if(tx_ring_buf == NULL || tx_lock == NULL){ + return; + } + if(xSemaphoreTake(tx_lock, tx_timeout_ms / portTICK_PERIOD_MS) != pdPASS){ return; } UBaseType_t uxItemsWaiting = 0; @@ -195,6 +249,7 @@ void HWCDC::flush(void) delay(5); vRingbufferGetInfo(tx_ring_buf, NULL, NULL, NULL, NULL, &uxItemsWaiting); } + xSemaphoreGive(tx_lock); } /* diff --git a/cores/esp32/HWCDC.h b/cores/esp32/HWCDC.h index 6551425d57f..e8fe6c70cea 100644 --- a/cores/esp32/HWCDC.h +++ b/cores/esp32/HWCDC.h @@ -27,6 +27,7 @@ class HWCDC: public Stream size_t setRxBufferSize(size_t); size_t setTxBufferSize(size_t); + void setTxTimeoutMs(uint32_t timeout); void begin(unsigned long baud=0); void end(); From e81630219cccec8a51e4fbeecfba947353b39c01 Mon Sep 17 00:00:00 2001 From: me-no-dev Date: Wed, 15 Sep 2021 18:16:32 +0300 Subject: [PATCH 3/6] Fix Serial::setDebugOutput(true) not replacing the default function --- cores/esp32/esp32-hal-uart.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/cores/esp32/esp32-hal-uart.c b/cores/esp32/esp32-hal-uart.c index 1d8c280b687..c18b0533ea4 100644 --- a/cores/esp32/esp32-hal-uart.c +++ b/cores/esp32/esp32-hal-uart.c @@ -368,13 +368,9 @@ void uartSetDebug(uart_t* uart) { if(uart == NULL || uart->num >= SOC_UART_NUM) { s_uart_debug_nr = -1; - //ets_install_putc1(NULL); - //return; - } else - if(s_uart_debug_nr == uart->num) { - return; - } else - s_uart_debug_nr = uart->num; + } else { + s_uart_debug_nr = uart->num; + } uart_install_putc(); } From 9420f83a0a26681c66c2f7421f2cef3faf60c64b Mon Sep 17 00:00:00 2001 From: me-no-dev Date: Wed, 15 Sep 2021 18:18:44 +0300 Subject: [PATCH 4/6] ESP32-C3 does not need to use 1200 bps touch for reboot --- boards.txt | 7 ------- 1 file changed, 7 deletions(-) diff --git a/boards.txt b/boards.txt index fb1854d5ec8..5d133d147e6 100644 --- a/boards.txt +++ b/boards.txt @@ -57,13 +57,6 @@ esp32c3.menu.CDCOnBoot.default.build.cdc_on_boot=0 esp32c3.menu.CDCOnBoot.cdc=Enabled esp32c3.menu.CDCOnBoot.cdc.build.cdc_on_boot=1 -esp32c3.menu.UploadMode.default=UART0 -esp32c3.menu.UploadMode.default.upload.use_1200bps_touch=false -esp32c3.menu.UploadMode.default.upload.wait_for_upload_port=false -esp32c3.menu.UploadMode.cdc=Internal USB -esp32c3.menu.UploadMode.cdc.upload.use_1200bps_touch=true -esp32c3.menu.UploadMode.cdc.upload.wait_for_upload_port=true - esp32c3.menu.PartitionScheme.default=Default 4MB with spiffs (1.2MB APP/1.5MB SPIFFS) esp32c3.menu.PartitionScheme.default.build.partitions=default esp32c3.menu.PartitionScheme.defaultffat=Default 4MB with ffat (1.2MB APP/1.5MB FATFS) From 3d3c9398f581834b537b709b692c7afa75af1773 Mon Sep 17 00:00:00 2001 From: me-no-dev Date: Wed, 15 Sep 2021 19:08:27 +0300 Subject: [PATCH 5/6] Add events support --- cores/esp32/HWCDC.cpp | 49 +++++++++++++++++++++++++++++++++++++++++-- cores/esp32/HWCDC.h | 24 +++++++++++++++++++++ 2 files changed, 71 insertions(+), 2 deletions(-) diff --git a/cores/esp32/HWCDC.cpp b/cores/esp32/HWCDC.cpp index 55e748c9feb..1baf0f49f4a 100644 --- a/cores/esp32/HWCDC.cpp +++ b/cores/esp32/HWCDC.cpp @@ -24,6 +24,8 @@ #include "soc/periph_defs.h" #include "hal/usb_serial_jtag_ll.h" +ESP_EVENT_DEFINE_BASE(ARDUINO_HW_CDC_EVENTS); + static RingbufHandle_t tx_ring_buf = NULL; static xQueueHandle rx_queue = NULL; static uint8_t rx_data_buf[64]; @@ -31,10 +33,26 @@ static intr_handle_t intr_handle = NULL; static volatile bool initial_empty = false; static xSemaphoreHandle tx_lock = NULL; static uint32_t tx_timeout_ms = 200; +static esp_event_loop_handle_t arduino_hw_cdc_event_loop_handle = NULL; + +static esp_err_t arduino_hw_cdc_event_post(esp_event_base_t event_base, int32_t event_id, void *event_data, size_t event_data_size, BaseType_t *task_unblocked){ + if(arduino_hw_cdc_event_loop_handle == NULL){ + return ESP_FAIL; + } + return esp_event_isr_post_to(arduino_hw_cdc_event_loop_handle, event_base, event_id, event_data, event_data_size, task_unblocked); +} + +static esp_err_t arduino_hw_cdc_event_handler_register_with(esp_event_base_t event_base, int32_t event_id, esp_event_handler_t event_handler, void *event_handler_arg){ + if(arduino_hw_cdc_event_loop_handle == NULL){ + return ESP_FAIL; + } + return esp_event_handler_register_with(arduino_hw_cdc_event_loop_handle, event_base, event_id, event_handler, event_handler_arg); +} static void hw_cdc_isr_handler(void *arg) { portBASE_TYPE xTaskWoken = 0; uint32_t usbjtag_intr_status = 0; + arduino_hw_cdc_event_data_t event = {0}; usbjtag_intr_status = usb_serial_jtag_ll_get_intsts_mask(); if (usbjtag_intr_status & USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY) { @@ -47,6 +65,7 @@ static void hw_cdc_isr_handler(void *arg) { initial_empty = true; //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); @@ -60,6 +79,8 @@ static void hw_cdc_isr_handler(void *arg) { 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); @@ -79,6 +100,8 @@ static void hw_cdc_isr_handler(void *arg) { } //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) { @@ -86,6 +109,7 @@ static void hw_cdc_isr_handler(void *arg) { 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 (xTaskWoken == pdTRUE) { @@ -103,11 +127,26 @@ static void ARDUINO_ISR_ATTR cdc0_write_char(char c) { } HWCDC::HWCDC() { - + if (!arduino_hw_cdc_event_loop_handle) { + esp_event_loop_args_t event_task_args = { + .queue_size = 5, + .task_name = "arduino_hw_cdc_events", + .task_priority = 5, + .task_stack_size = 2048, + .task_core_id = tskNO_AFFINITY + }; + if (esp_event_loop_create(&event_task_args, &arduino_hw_cdc_event_loop_handle) != ESP_OK) { + log_e("esp_event_loop_create failed"); + } + } } HWCDC::~HWCDC(){ end(); + if (arduino_hw_cdc_event_loop_handle) { + esp_event_loop_delete(arduino_hw_cdc_event_loop_handle); + arduino_hw_cdc_event_loop_handle = NULL; + } } HWCDC::operator bool() const @@ -115,7 +154,13 @@ HWCDC::operator bool() const return initial_empty; } -static void dummy_putc(char c){} +void HWCDC::onEvent(esp_event_handler_t callback){ + onEvent(ARDUINO_HW_CDC_ANY_EVENT, callback); +} + +void HWCDC::onEvent(arduino_hw_cdc_event_t event, esp_event_handler_t callback){ + arduino_hw_cdc_event_handler_register_with(ARDUINO_HW_CDC_EVENTS, event, callback, this); +} void HWCDC::begin(unsigned long baud) { diff --git a/cores/esp32/HWCDC.h b/cores/esp32/HWCDC.h index e8fe6c70cea..81a334b76b9 100644 --- a/cores/esp32/HWCDC.h +++ b/cores/esp32/HWCDC.h @@ -17,14 +17,38 @@ #if CONFIG_IDF_TARGET_ESP32C3 #include +#include "esp_event.h" #include "Stream.h" +ESP_EVENT_DECLARE_BASE(ARDUINO_HW_CDC_EVENTS); + +typedef enum { + ARDUINO_HW_CDC_ANY_EVENT = ESP_EVENT_ANY_ID, + ARDUINO_HW_CDC_CONNECTED_EVENT = 0, + ARDUINO_HW_CDC_BUS_RESET_EVENT, + ARDUINO_HW_CDC_RX_EVENT, + ARDUINO_HW_CDC_TX_EVENT, + ARDUINO_HW_CDC_MAX_EVENT, +} arduino_hw_cdc_event_t; + +typedef union { + struct { + size_t len; + } rx; + struct { + size_t len; + } tx; +} arduino_hw_cdc_event_data_t; + class HWCDC: public Stream { public: HWCDC(); ~HWCDC(); + void onEvent(esp_event_handler_t callback); + void onEvent(arduino_hw_cdc_event_t event, esp_event_handler_t callback); + size_t setRxBufferSize(size_t); size_t setTxBufferSize(size_t); void setTxTimeoutMs(uint32_t timeout); From c37e77d7fe8960f4d103f16e110af4484495e180 Mon Sep 17 00:00:00 2001 From: me-no-dev Date: Wed, 15 Sep 2021 19:18:52 +0300 Subject: [PATCH 6/6] Init event loop only if necessary --- cores/esp32/HWCDC.cpp | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/cores/esp32/HWCDC.cpp b/cores/esp32/HWCDC.cpp index 1baf0f49f4a..f2639a1f008 100644 --- a/cores/esp32/HWCDC.cpp +++ b/cores/esp32/HWCDC.cpp @@ -43,6 +43,18 @@ static esp_err_t arduino_hw_cdc_event_post(esp_event_base_t event_base, int32_t } static esp_err_t arduino_hw_cdc_event_handler_register_with(esp_event_base_t event_base, int32_t event_id, esp_event_handler_t event_handler, void *event_handler_arg){ + if (!arduino_hw_cdc_event_loop_handle) { + esp_event_loop_args_t event_task_args = { + .queue_size = 5, + .task_name = "arduino_hw_cdc_events", + .task_priority = 5, + .task_stack_size = 2048, + .task_core_id = tskNO_AFFINITY + }; + if (esp_event_loop_create(&event_task_args, &arduino_hw_cdc_event_loop_handle) != ESP_OK) { + log_e("esp_event_loop_create failed"); + } + } if(arduino_hw_cdc_event_loop_handle == NULL){ return ESP_FAIL; } @@ -127,26 +139,11 @@ static void ARDUINO_ISR_ATTR cdc0_write_char(char c) { } HWCDC::HWCDC() { - if (!arduino_hw_cdc_event_loop_handle) { - esp_event_loop_args_t event_task_args = { - .queue_size = 5, - .task_name = "arduino_hw_cdc_events", - .task_priority = 5, - .task_stack_size = 2048, - .task_core_id = tskNO_AFFINITY - }; - if (esp_event_loop_create(&event_task_args, &arduino_hw_cdc_event_loop_handle) != ESP_OK) { - log_e("esp_event_loop_create failed"); - } - } + } HWCDC::~HWCDC(){ end(); - if (arduino_hw_cdc_event_loop_handle) { - esp_event_loop_delete(arduino_hw_cdc_event_loop_handle); - arduino_hw_cdc_event_loop_handle = NULL; - } } HWCDC::operator bool() const @@ -190,6 +187,10 @@ void HWCDC::end() } setRxBufferSize(0); setTxBufferSize(0); + if (arduino_hw_cdc_event_loop_handle) { + esp_event_loop_delete(arduino_hw_cdc_event_loop_handle); + arduino_hw_cdc_event_loop_handle = NULL; + } } void HWCDC::setTxTimeoutMs(uint32_t timeout){