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

USBCDC::flush is blocking #7554

Closed
1 task done
tablatronix opened this issue Dec 6, 2022 · 30 comments · Fixed by #9275
Closed
1 task done

USBCDC::flush is blocking #7554

tablatronix opened this issue Dec 6, 2022 · 30 comments · Fixed by #9275
Assignees
Labels
Status: Needs investigation We need to do some research before taking next steps on this issue Type: Question Only question

Comments

@tablatronix
Copy link
Contributor

tablatronix commented Dec 6, 2022

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

  • I confirm I have checked existing issues, online documentation and Troubleshooting guide.
@tablatronix tablatronix added the Status: Awaiting triage Issue is waiting for triage label Dec 6, 2022
@SuGlider SuGlider self-assigned this Dec 14, 2022
@SuGlider SuGlider added Type: For reference Common questions & problems Type: Question Only question and removed Status: Awaiting triage Issue is waiting for triage labels Dec 14, 2022
@SuGlider
Copy link
Collaborator

SuGlider commented Dec 14, 2022

@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
Copy link
Contributor Author

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
Copy link
Contributor Author

tablatronix commented Dec 15, 2022

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
Copy link
Contributor Author

tablatronix commented Dec 15, 2022

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
Copy link
Contributor Author

tablatronix commented Dec 15, 2022

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
Copy link
Collaborator

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
Copy link
Contributor Author

tablatronix commented Dec 17, 2022

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
Copy link
Collaborator

SuGlider commented Jan 22, 2023

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
Copy link

Hi @tablatronix

I'm having the same problem and I proposed a PR #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
Copy link
Contributor Author

tablatronix commented Feb 1, 2023

@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
Copy link
Collaborator

SuGlider commented Feb 1, 2023

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
Copy link
Collaborator

SuGlider commented Feb 1, 2023

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.

#7779

@Xylopyrographer
Copy link
Contributor

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
Copy link
Contributor

is there a workaround to make it work currently?

@sblantipodi
Copy link
Contributor

I have workarounded the problem like this

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

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

@imwhocodes
Copy link

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
Copy link
Contributor

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
Copy link
Contributor Author

This is still an issue, testing on S3

ESP-IDF version: 4.4.5.230614

@SuGlider
Copy link
Collaborator

SuGlider commented Aug 9, 2023

I'll test it with the latest Arduino Core.

@SuGlider SuGlider added Status: Needs investigation We need to do some research before taking next steps on this issue and removed Type: For reference Common questions & problems labels Aug 9, 2023
@beachmiles
Copy link

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
Copy link

evansgl commented Sep 19, 2023

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
Copy link
Contributor Author

tablatronix commented Sep 19, 2023

@evansgl no this is not an RTS issue, afaik

@ck2510
Copy link

ck2510 commented Jan 7, 2024

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
Copy link
Collaborator

@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
Copy link

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

@ChrGri
Copy link

ChrGri commented May 1, 2024

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
Copy link

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
Copy link

ChrGri commented May 1, 2024

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
Copy link

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
Copy link

ChrGri commented May 2, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Needs investigation We need to do some research before taking next steps on this issue Type: Question Only question
Projects
None yet
10 participants