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

backports HWCDC to v2.0.15 #9462

Merged
merged 9 commits into from
Apr 9, 2024
Merged

Conversation

SuGlider
Copy link
Collaborator

@SuGlider SuGlider commented Apr 8, 2024

Description of Change

Backports all changes done in Core 3.0.0 to 2.0.15.
When selecting USB CDC On Boot: Enabled and USB Mode: Hardware CDC and JTAG, Serial will be the USB HW CDC port.
Changes/Fixes:

  • while(Serial); will only return when HW CDC is connected to a Serial Monitor.
  • Serial.end(); will stop HW CDC driver and disable USB Host enumeration
  • Serial.begin() will correctly enumerate the CDC port
  • Serial.isPlugged() will return true whenever USB port is plugged and Host has enumerate the CDC port
  • Serial.isConnected() wil return true whenever USB CDC is connected to a Serial Terminal and a session has stated
  • Fixed issues with writing long buffers of data. No data loss or failure
  • Serial.setTxTimeoutMs(time_ms) can be used to avoid failing writing within long data buffers due to Host delay

A new example is here provided.

Tests scenarios

Tested with ESP32-S3 and ESP32-C3 using the new example.
Tested using a modified version of code provided in #9401

Related links

N/A

@SuGlider SuGlider added this to the 2.0.15 milestone Apr 8, 2024
@SuGlider SuGlider self-assigned this Apr 8, 2024
@me-no-dev
Copy link
Member

HWSerial refers to UART, not to USB CDC. Please rename the example to HWCDC_Events or similar

cores/esp32/HWCDC.cpp Outdated Show resolved Hide resolved
cores/esp32/HWCDC.cpp Outdated Show resolved Hide resolved
cores/esp32/HWCDC.cpp Outdated Show resolved Hide resolved
@me-no-dev
Copy link
Member

Looks good to me. I do not see anything obvious that is bad, apart from the few nitpicks outlined above

cores/esp32/HWCDC.cpp Outdated Show resolved Hide resolved
cores/esp32/HWCDC.cpp Outdated Show resolved Hide resolved
cores/esp32/HWCDC.cpp Outdated Show resolved Hide resolved
@SuGlider
Copy link
Collaborator Author

SuGlider commented Apr 8, 2024

HWSerial refers to UART, not to USB CDC. Please rename the example to HWCDC_Events or similar

Done.

Copy link
Member

@P-R-O-C-H-Y P-R-O-C-H-Y left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SuGlider Looking good 👍
Please just fix the #endif comment.

cores/esp32/HWCDC.cpp Show resolved Hide resolved
Copy link
Collaborator

@lucasssvaz lucasssvaz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

@SuGlider
Copy link
Collaborator Author

SuGlider commented Apr 9, 2024

@me-no-dev / @VojtechBartoska - Ready for merging and releasing 2.0.15

@lucasssvaz lucasssvaz added the Area: Peripherals API Relates to peripheral's APIs. label Apr 9, 2024
@lucasssvaz lucasssvaz added the Status: Pending Merge Pull Request is ready to be merged label Apr 9, 2024
@me-no-dev me-no-dev merged commit 4465cac into espressif:release/v2.x Apr 9, 2024
49 checks passed
@def1149
Copy link

def1149 commented Apr 18, 2024

On an Adafruit Feather ESP32-S2 Serial.print() works WITHOUT a Serial.begin() ??!!

schlimmchen added a commit to helgeerbe/OpenDTU-OnBattery that referenced this pull request Jun 21, 2024
the "serial console" over USB would be garbled badly after switching to
"platform = espressif32@6.7.0". this did not happen to the upstream
version of MessageOutput. we used Serial.flush(), which seemed to be
good in the respective context. however, the changes in

github.com/espressif/arduino-esp32/pull/9462

made flush() detrimental. we remove the use of flush(), as it seems not
to be required (in particular, the upstream project does not use it).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Peripherals API Relates to peripheral's APIs. Status: Pending Merge Pull Request is ready to be merged
Projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants