Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 30 commits into from
Feb 28, 2024
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
c5477c1
feat(hw_cdc):fixes the hardware cdc jtag plugged/unplugged status
SuGlider Feb 22, 2024
112e0c2
feat(usb): Creates HWSerial_Events.ino example
SuGlider Feb 22, 2024
742cbb4
feat: adds .skip.esp32
SuGlider Feb 22, 2024
ec05cd1
feat: adds .skip.esp32s2
SuGlider Feb 22, 2024
f7e8b9f
fix: fixes issues with Ubuntu CI
SuGlider Feb 22, 2024
0e168aa
Merge branch 'master' into SuGlider-patch-2
lucasssvaz Feb 23, 2024
52dec69
feat(serialcdc): non block functions
SuGlider Feb 25, 2024
c01e44a
Merge branch 'master' into SuGlider-patch-2
SuGlider Feb 25, 2024
2b1646d
Merge branch 'master' into SuGlider-patch-2
SuGlider Feb 27, 2024
1701a5a
Merge branch 'master' into SuGlider-patch-2
SuGlider Feb 27, 2024
b1d6108
fix(HWCDC): changes made demands testing for CDC ON BOOT
SuGlider Feb 27, 2024
a203d42
feat(hwcdc): Improves HWSerial_Events.ino
SuGlider Feb 28, 2024
774bb9a
feat(hwcdc): solves CDC connection issue
SuGlider Feb 28, 2024
98bad3e
Update cores/esp32/HWCDC.cpp
SuGlider Feb 28, 2024
06b8a08
Update cores/esp32/HWCDC.cpp
SuGlider Feb 28, 2024
c218f50
Update cores/esp32/HWCDC.cpp
SuGlider Feb 28, 2024
ca0a9f5
Update cores/esp32/HWCDC.cpp
SuGlider Feb 28, 2024
f7d66d2
Update cores/esp32/HWCDC.cpp
SuGlider Feb 28, 2024
980f7b1
Update cores/esp32/HWCDC.cpp
SuGlider Feb 28, 2024
8e6eda4
Update cores/esp32/HWCDC.cpp
SuGlider Feb 28, 2024
43075e4
Update cores/esp32/HWCDC.cpp
SuGlider Feb 28, 2024
3d2d18d
Update cores/esp32/HWCDC.cpp
SuGlider Feb 28, 2024
467fe9d
Update cores/esp32/HWCDC.cpp
SuGlider Feb 28, 2024
576d675
Update cores/esp32/HWCDC.cpp
SuGlider Feb 28, 2024
ab157cf
Update cores/esp32/HWCDC.cpp
SuGlider Feb 28, 2024
5d2cb69
Update cores/esp32/HWCDC.cpp
SuGlider Feb 28, 2024
5000939
Update cores/esp32/HWCDC.cpp
SuGlider Feb 28, 2024
a0c7b41
Update cores/esp32/HWCDC.cpp
SuGlider Feb 28, 2024
d4eca05
Update cores/esp32/HWCDC.cpp
SuGlider Feb 28, 2024
7b42a8f
Apply suggestions from code review
SuGlider Feb 28, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
144 changes: 106 additions & 38 deletions cores/esp32/HWCDC.cpp
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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
SuGlider marked this conversation as resolved.
Show resolved Hide resolved
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
SuGlider marked this conversation as resolved.
Show resolved Hide resolved
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.
Expand All @@ -102,7 +99,7 @@ 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;
Expand All @@ -124,18 +121,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!
SuGlider marked this conversation as resolved.
Show resolved Hide resolved
}

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)
SuGlider marked this conversation as resolved.
Show resolved Hide resolved
}

if (xTaskWoken == pdTRUE) {
Expand All @@ -144,12 +138,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
SuGlider marked this conversation as resolved.
Show resolved Hide resolved
if(usb_serial_jtag_is_connected()) {
tx_timeout_ms = requested_tx_timeout_ms; // once connected, restores the TX timeout
SuGlider marked this conversation as resolved.
Show resolved Hide resolved
}
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
SuGlider marked this conversation as resolved.
Show resolved Hide resolved
}

HWCDC::HWCDC() {
Expand All @@ -160,9 +158,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 function is called many times
SuGlider marked this conversation as resolved.
Show resolved Hide resolved
// typically with something like `while(!Serial) delay(10);`
SuGlider marked this conversation as resolved.
Show resolved Hide resolved
static bool running = false;

// USB may be unplugged
if (usb_serial_jtag_is_connected() == false) {
isConnected = false;
running = false;
return false;
}

if (isConnected) {
running = false;
return true;
}

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
SuGlider marked this conversation as resolved.
Show resolved Hide resolved
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){
Expand Down Expand Up @@ -206,9 +230,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");
}
}
Expand All @@ -227,8 +251,6 @@ 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();
}

void HWCDC::end()
Expand All @@ -248,13 +270,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;
}

/*
Expand All @@ -278,9 +298,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
SuGlider marked this conversation as resolved.
Show resolved Hide resolved
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
SuGlider marked this conversation as resolved.
Show resolved Hide resolved
}
if(xSemaphoreTake(tx_lock, tx_timeout_ms / portTICK_PERIOD_MS) != pdPASS){
return 0;
}
Expand All @@ -289,11 +313,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
SuGlider marked this conversation as resolved.
Show resolved Hide resolved
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
SuGlider marked this conversation as resolved.
Show resolved Hide resolved
} else {
isConnected = false;
}
if(xSemaphoreTake(tx_lock, tx_timeout_ms / portTICK_PERIOD_MS) != pdPASS){
return 0;
}
Expand All @@ -305,14 +350,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 just the lastest data buffered
SuGlider marked this conversation as resolved.
Show resolved Hide resolved
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();
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;
Expand All @@ -325,9 +372,15 @@ 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();
if(isConnected) usb_serial_jtag_ll_ena_intr_mask(USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTY);
}
}
// CDC is diconnected ==> flush all data from TX buffer
if(to_send && !usb_serial_jtag_ll_txfifo_writable()) {
isConnected = false;
flushTXBuffer(); // flush TX Ringbuffer
SuGlider marked this conversation as resolved.
Show resolved Hide resolved
}
xSemaphoreGive(tx_lock);
return size;
}
Expand All @@ -339,21 +392,36 @@ size_t HWCDC::write(uint8_t c)

void HWCDC::flush(void)
{
uint32_t tx_timeout_ms = 0; // if not connected, no timeout
SuGlider marked this conversation as resolved.
Show resolved Hide resolved
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
SuGlider marked this conversation as resolved.
Show resolved Hide resolved
} 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
SuGlider marked this conversation as resolved.
Show resolved Hide resolved
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();
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?
SuGlider marked this conversation as resolved.
Show resolved Hide resolved
vRingbufferGetInfo(tx_ring_buf, NULL, NULL, NULL, NULL, &uxItemsWaiting);
if (lastUxItemsWaiting == uxItemsWaiting) tries--; // avoids locking when USB is plugged, but CDC is not connected
SuGlider marked this conversation as resolved.
Show resolved Hide resolved
}
if (tries == 0) { // CDC isn't connected anymore...
isConnected = false;
flushTXBuffer(); // flush TX Ringbuffer
SuGlider marked this conversation as resolved.
Show resolved Hide resolved
}
xSemaphoreGive(tx_lock);
}
Expand Down
1 change: 1 addition & 0 deletions libraries/ESP32/examples/HWSerial_Events/.skip.esp32
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

1 change: 1 addition & 0 deletions libraries/ESP32/examples/HWSerial_Events/.skip.esp32s2
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

Loading