search

espressif / arduino-esp32

Arduino core for the ESP32
GNU Lesser General Public License v2.1
13.44k stars 7.38k forks source link

USBCDC::flush is blocking #7554

Closed tablatronix closed 7 months ago

tablatronix commented 1 year ago

Board

ESP32 S3

Device Description

S3 mini devkit

Hardware Configuration

Nothing special S3 mini 

build_flags = -DDEBUG_ESP_WIFI -DESP32 -DCORE_DEBUG_LEVEL=5 -DWM_DEBUG_LEVEL=3 -DARDUINO_USB_MODE=1 -DARDUINO_USB_CDC_ON_BOOT=1 -DARDUINO_USB_DFU_ON_BOOT=1

Version

v2.0.5

IDE Name

pio

Operating System

osx:latest

Flash frequency

40

PSRAM enabled

no

Upload speed

921600

Description

USBCDC flush() will block until you connect to it and it can flush.. seems odd,

!tud_cdc_n_connected is reporting wrong?

void USBCDC::flush(void)
{
    if(itf >= MAX_USB_CDC_DEVICES || tx_lock == NULL || !tud_cdc_n_connected(itf)){
        return;
    }
    if(xSemaphoreTake(tx_lock, tx_timeout_ms / portTICK_PERIOD_MS) != pdPASS){
        return;
    }
    tud_cdc_n_write_flush(itf);
    xSemaphoreGive(tx_lock);
}

Sketch

void setup(){
Serial.begin(115200);
Serial.println("Booting....");
delay(200);
Serial.flush();
// we never get here
}

Debug Message

I will have to attach a physical uart and check if there is any notice

Other Steps to Reproduce

No response

I have checked existing issues, online documentation and the Troubleshooting Guide

SuGlider commented 1 year ago

@tablatronix The issue is -DARDUINO_USB_MODE=1 that is used with Hardware USB CDC. It uses HWCDC.h/.cpp

TinyUSB requires -DARDUINO_USB_MODE=0. It uses USBCDC.h/.cpp

ESP32-S3 can only use one or the other. Not both at the same time.

tablatronix commented 1 year ago

Ok so I posted the wrong method, I will take a look at HWCDC.*, But I don't see how that affect the issue, It is still an unexpected issue/bug

tablatronix commented 1 year ago

I am working around this now by checking initial_empty not sure its purpose, but it seems to be useful enough for bus state so flush doesn't sit there waiting for USB_SERIAL_JTAG_INTR_SERIAL_IN_EMPTYor whatever indefinitely

I tried to use tx_timeout_ms to break xSemaphoreTake, but coudln't get it to fail.

void HWCDC::flush(void)
{
    if(tx_ring_buf == NULL || tx_lock == NULL || !initial_empty){
        return;
    }
    if(xSemaphoreTake(tx_lock, tx_timeout_ms / portTICK_PERIOD_MS) != pdPASS){
        return;
    }
tablatronix commented 1 year ago

There also is something wrong with HWCDC ISRs and wifi..

To get wifi working with HWCDC, I have to setRxBufferSize + to get wifi to work ( blocking ISR??) . or setTxTimeoutMs low, probably very similar issue but in write blocking

And there is some delay proportional to the size of the buffer, so something is waiting until it's full ?

tablatronix commented 1 year ago

yup same issue, fixed the same way , well kludged. But yeah if we can make HWCDC not get stuck blocking/waiting when no client is present or when no async event will ever be reached.

I see that write - xRingbufferSend has non blocking, but I have not looked into what going on there or if its using it

SuGlider commented 1 year ago

I have tested it and I can't reproduce the issue.

When you run the ESP32S3, the USB cable is unplugged? Please provide more details on how to reproduce the issues you are experimenting.

tablatronix commented 1 year ago

Usb is plugged but no serial connected. Could this be an osx issue with dtr? Behavior differs with and without a serial terminal on the socket

SuGlider commented 1 year ago

I have Win11 here. I don't know if this is an osx issue only.

But, yes, when a Serial Terminal is initiated, there is a signal to USBCDC that tells to Arduino that the connection has been started. Before opening the Terminal Window using /dev/ttyUSBxx, Arduino CDC will not work (read/write), therefore, USBCDC::flush() may block, once it is never able to send the data through the USB end point, and then never return.

https://github.com/espressif/arduino-esp32/blob/master/cores/esp32/USBCDC.cpp#L238-L240

The sketch can test if it has been connected already by using

  if(Serial) {
    log_i("USB Serial Terminal has started and connected to CDC.");
  } else {
    log_i("USB Serial Terminal in not connected to CDC.");
  }

Given that flush() should only return if data is sent, I think that blocking when there is no CDC connection may be the right behavior.

imwhocodes commented 1 year ago

Hi @tablatronix

I'm having the same problem and I proposed a PR https://github.com/espressif/arduino-esp32/pull/7721

If you can (super easy with PlatformIO) you can test using this branch: https://github.com/imwhocodes/arduino-esp32/tree/usb-unconnected-stall-fix

platform_packages = 
    platformio/framework-arduinoespressif32 @ https://github.com/imwhocodes/arduino-esp32#usb-unconnected-stall-fix

@SuGlider

Given that flush() should only return if data is sent, I think that blocking when there is no CDC connection may be the right behavior.

Yes this is True but in the UART case there is no concept of "connection", Stream::flush() on UART will take maximum (pending_bytes * 10) / baudrate time (10 is for converting bytes to bauds) and never stall indefinitely

I would argue that if no usb is connected all write should be NO-OP and not even try to actually write on the medium And there is still operator HWCDC::bool() const if you want to wait for the usb to be connected

tablatronix commented 1 year ago

@imwhocodes Thank you , YES exactly I stumbled on this trying to use stream redirection based on various things, cdc when USB, when no cdc uart tx to an led for activity status ) and netlogging etc. As well as toggling HWCDC during runtime, all strange things, but thats what I do break stuff lol

SuGlider commented 1 year ago

I would argue that if no usb is connected all write should be NO-OP and not even try to actually write on the medium And there is still operator HWCDC::bool() const if you want to wait for the usb to be connected

OK, this is an important change.

In the current implementation if USB isn't connected, the sent data is buffered into a Ringbuffer. In this case as soon as it is connected to the USB Host, the buffered data will flow - therefore, dependening on the buffer size, no data would be lost.

Anyway, this Ringbuffer may go full before USB is connected to the Host. But, in that case, the semaphore timing should pass though, when writing to the CDC.

By other hand, based on the PR, the bytes sent before the user opens the connection or after the connection is closed are lost - just like it would be with a UART.

SuGlider commented 1 year ago

Adding to this disussion, it seems that there is another user requesting that while CDC isn't connected, it shouldn't buffer any sent data.

https://github.com/espressif/arduino-esp32/issues/7779

Xylopyrographer commented 1 year ago

Just ran into this problem porting code to a XIAO S3 board which has only a single S3-native USB port. The project works OK when connected to an open port the computer however, when deployed, it is designed to be powered by an external supply connected to the USB port. Since there is obviously no way a serial port can be available this way, the sketch hangs forever.

Therefore a simple API that detects the presence of a USB CDC connection is needed. Let the developer test if there is a connection and decided what to do. Either wait for one to appear, or ignore it and carry on. Please.

Ref also #7721 and #7779

sblantipodi commented 1 year ago

is there a workaround to make it work currently?

sblantipodi commented 1 year ago

I have workarounded the problem like this

while (Serial.available()) {
  Serial.read();
}

it's not the same but it works for me.

imwhocodes commented 1 year ago

is there a workaround to make it work currently?

@sblantipodi If you are using Platformio you can add this:

platform_packages = 
    platformio/framework-arduinoespressif32 @ https://github.com/imwhocodes/arduino-esp32#master-with-personal-pullrequest

To your platformio.ini file This is my fork of this repo where I tried to fix this problem, and my case it worked

sblantipodi commented 1 year ago

Thanks @imwhocodes I'll give it a try.

... do you see some issues in using the workaround I described above instead of using Serial.flush() ?

tablatronix commented 1 year ago

This is still an issue, testing on S3

ESP-IDF version: 4.4.5.230614

SuGlider commented 1 year ago

I'll test it with the latest Arduino Core.

beachmiles commented 1 year ago

Looks like Im seeing this bug in Arduino v2.0.11 based on ESP-IDF v4.4.5 on my esp32-c3 AirM2M_CORE_ESP32C3. For the USB CDC serial port running the Serial.flush() command after outputting text from the wifi scan causes the micro to freeze when the COM port is not connected. Removing the Serial.flush() command fixes the freeing issue.

evansgl commented 1 year ago

Can this issue also be related to the fact that an ESP32-C3-MINI CDC resets the board when you try to access the serial via Windows OS?

tablatronix commented 1 year ago

@evansgl no this is not an RTS issue, afaik

ck2510 commented 9 months ago

I ran into this issue with a Seeed Xiao ESP32S3 when using platformio with both frameworks: framework = arduino, espidf

My quick and dirty workaround was to connect a separate USB serial adapter to UART1 which was configured against GPIO43 and GPIO44 via menuconfig. Instead of "Serial" i then used "Serial1" in my program. I assume Serial suffers from this issue as this seems to be an instance of HWCDC.

PLATFORM: Espressif 32 (6.5.0)

SuGlider commented 7 months ago

@tablatronix @imwhocodes @Xylopyrographer @sblantipodi @beachmiles @evansgl @ck2510 The original issue is related to using this settings: "-DARDUINO_USB_MODE=1 -DARDUINO_USB_CDC_ON_BOOT=1", which is related to the JTAG/HW CDC Serial peripheral from ESP32-S3, ESP32-C3, ESP32-C6 and ESP32-H2. The issue is that when USB cable is unplugged, flush() will block and write() will also block as soon as the HWCDC Ringbuffer gets full.

It also won't work properly when the sketch waits for the CDC to be connected using a Serial Monitor, using a code like while(!Serial) delay(100);

These and some other issue were fixed by the PR #9275 When USB is unplugged, nothing will block any HW CDC writing or flushing. When the buffer get full, its content will be discarted and it will show the latest written data. By this way, when the sketch connects to a serial monitor, it will display the data flow.

It has been fixed for arduino core 3.0.0-RC1 and it is available in the master branch.

Check the PR examples.

BCsabaEngine commented 5 months ago

3.0 changes are backpatched to 2.x. I have solved with setTxTimeout(0) and remove flush() commands.

ChrGri commented 5 months ago

3.0 changes are backpatched to 2.x. I have solved with setTxTimeout(0) and remove flush() commands.

As I'm fairly new to platformio, may I ask what to do, to include the backpatched changes? Cannot use >core 3.0 unfortunately.

Thank you and best regards Chris

BCsabaEngine commented 5 months ago

3.0 changes are backpatched to 2.x. I have solved with setTxTimeout(0) and remove flush() commands.

As I'm fairly new to platformio, may I ask what to do, to include the backpatched changes? Cannot use >core 3.0 unfortunately.

Thank you and best regards Chris

v2.x contains bugfixes than implemented in 3.x, so you need to upgrade to latest v2 only.

ChrGri commented 5 months ago

3.0 changes are backpatched to 2.x. I have solved with setTxTimeout(0) and remove flush() commands.

As I'm fairly new to platformio, may I ask what to do, to include the backpatched changes? Cannot use >core 3.0 unfortunately. Thank you and best regards Chris

v2.x contains bugfixes than implemented in 3.x, so you need to upgrade to latest v2 only.

Thank you. Could you please quickly check if I'm doing that right in my platformio.ini file? https://github.com/ChrGri/DIY-Sim-Racing-FFB-Pedal/blob/develop/ESP32/platformio.ini

It's a project where USB HID joystick and serial output are provided by the ESP32 S3. Without serial monitor opened, the joystick output is reliable. When the serial monitor is opened, joystick and serial output block after some time.

Thank you and best regards. Chris

Edit: I've tried deactivating HID joystick output. Serial communication was fine then, no blocking. Seems like USBHID gamecontroller output and serial together are still an issue and can result in blocking.

BCsabaEngine commented 5 months ago

3.0 changes are backpatched to 2.x. I have solved with setTxTimeout(0) and remove flush() commands.

As I'm fairly new to platformio, may I ask what to do, to include the backpatched changes? Cannot use >core 3.0 unfortunately. Thank you and best regards Chris

v2.x contains bugfixes than implemented in 3.x, so you need to upgrade to latest v2 only.

Thank you. Could you please quickly check if I'm doing that right in my platformio.ini file? https://github.com/ChrGri/DIY-Sim-Racing-FFB-Pedal/blob/develop/ESP32/platformio.ini

It's a project where USB HID joystick and serial output are provided by the ESP32 S3. Without serial monitor opened, the joystick output is reliable. When the serial monitor is opened, joystick and serial output block after some time.

Thank you and best regards. Chris

Edit: I've tried deactivating HID joystick output. Serial communication was fine then, no blocking. Seems like USBHID gamecontroller output and serial together are still an issue and can result in blocking.

I use the default ExpressIF

platform = espressif32
board = lolin_s3_mini
framework = arduino
board_build.partitions = min_spiffs.csv
upload_port = /dev/ttyACM0
upload_speed = 921600
monitor_speed = 115200
build_flags = 
    -DARDUINO_RUNNING_CORE=1
    -DARDUINO_EVENT_RUNNING_CORE=1
    -DARDUINO_USB_MODE=1
    -DARDUINO_USB_CDC_ON_BOOT=1
    -DARDUINO_USB_MSC_ON_BOOT=0
    -DARDUINO_USB_DFU_ON_BOOT=0
    -DCORE_DEBUG_LEVEL=2
lib_deps = 
...
ChrGri commented 5 months ago

3.0 changes are backpatched to 2.x. I have solved with setTxTimeout(0) and remove flush() commands.

As I'm fairly new to platformio, may I ask what to do, to include the backpatched changes? Cannot use >core 3.0 unfortunately. Thank you and best regards Chris

v2.x contains bugfixes than implemented in 3.x, so you need to upgrade to latest v2 only.

Thank you. Could you please quickly check if I'm doing that right in my platformio.ini file? https://github.com/ChrGri/DIY-Sim-Racing-FFB-Pedal/blob/develop/ESP32/platformio.ini It's a project where USB HID joystick and serial output are provided by the ESP32 S3. Without serial monitor opened, the joystick output is reliable. When the serial monitor is opened, joystick and serial output block after some time. Thank you and best regards. Chris Edit: I've tried deactivating HID joystick output. Serial communication was fine then, no blocking. Seems like USBHID gamecontroller output and serial together are still an issue and can result in blocking.

I use the default ExpressIF

platform = espressif32
board = lolin_s3_mini
framework = arduino
board_build.partitions = min_spiffs.csv
upload_port = /dev/ttyACM0
upload_speed = 921600
monitor_speed = 115200
build_flags = 
  -DARDUINO_RUNNING_CORE=1
  -DARDUINO_EVENT_RUNNING_CORE=1
  -DARDUINO_USB_MODE=1
  -DARDUINO_USB_CDC_ON_BOOT=1
  -DARDUINO_USB_MSC_ON_BOOT=0
  -DARDUINO_USB_DFU_ON_BOOT=0
  -DCORE_DEBUG_LEVEL=2
lib_deps = 
...

Thank you. I'd say something changed till yesterday. Combining the build flags to:

build_flags =
  -DARDUINO_RUNNING_CORE=1
  -DARDUINO_EVENT_RUNNING_CORE=1
  -DCORE_DEBUG_LEVEL=1
    -DARDUINO_USB_MODE=0
    -DARDUINO_USB_CDC_ON_BOOT=1
  -DARDUINO_USB_MSC_ON_BOOT=0
    -DARDUINO_USB_DFU_ON_BOOT=0
  -DPCB_VERSION=6
  -DUSB_VID=0xF011
  -DUSB_PID=0xF011
  '-DUSB_PRODUCT="DiyFfbPedal"'
  '-DUSB_MANUFACTURER="OpenSource"'

The serial output doesn't seem to stall. The USB HID output does though .

BR Chris