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

ESP32-S3 and ESP-IDF don't play well with USB_CDC and need USB_SERIAL_JTAG #5929

Merged
merged 5 commits into from
Dec 15, 2023

Conversation

clydebarrow
Copy link
Contributor

instead.

What does this implement/fix?

Fixes compile errors when compiling for ESP-IDF on ESP32-S3 (and probably S2) due to change introduced with
#4853.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Other

Related issue or feature (if applicable): fixes

Pull request in esphome-docs with documentation (if applicable): esphome/esphome-docs#

Test Environment

  • ESP32
  • ESP32 IDF
  • ESP8266
  • RP2040
  • BK72xx
  • RTL87xx

Example entry for config.yaml:

# Example config.yaml

Checklist:

  • The code change is tested and works locally.
  • Tests have been added to verify that the new code works (under tests/ folder).

If user exposed functionality or configuration variables are added/changed:

@probot-esphome
Copy link

Hey there @esphome/core, mind taking a look at this pull request as it has been labeled with an integration (logger) you are listed as a code owner for? Thanks!
(message by CodeOwnersMention)

Co-authored-by: Keith Burzinski <kbx81x@gmail.com>
kbx81
kbx81 previously approved these changes Dec 14, 2023
kbx81
kbx81 previously approved these changes Dec 14, 2023
@clydebarrow clydebarrow marked this pull request as draft December 14, 2023 10:53
@clydebarrow
Copy link
Contributor Author

gonna add the C3 as well.

@clydebarrow clydebarrow marked this pull request as ready for review December 14, 2023 11:35
@probot-esphome
Copy link

Hey there @esphome/core, mind taking a look at this pull request as it has been labeled with an integration (logger) you are listed as a code owner for? Thanks!
(message by CodeOwnersMention)

@clydebarrow clydebarrow changed the title ESP32-S3 and S2 don't play well with USB_CDC and need USB_SERIAL_JTAG ESP32-S3 and ESP-IDF don't play well with USB_CDC and need USB_SERIAL_JTAG Dec 14, 2023
@clydebarrow
Copy link
Contributor Author

Tested on S3, C3 and plain ESP32. Also tested on RP2040.

@clydebarrow
Copy link
Contributor Author

Arduino on ESP32-C3 is still not right. The current setup results in a default of UART0, which means no output when using the USB_CDC port. If I tweak the config to allow USB_CDC for C3 then it fails at compile time because Serial is not defined by Arduino - it's Serial0. If I replace Serial with Serial0 then another compile error in the platformio main.cpp blocks it. If I work-around that I can get it to build and flash, but then it bootloops :-(

So for now I think it's best left as is and note that USB serial is not supported on the C3 under Arduino.

@kbx81
Copy link
Member

kbx81 commented Dec 15, 2023

Arduino on ESP32-C3 is still not right. The current setup results in a default of UART0, which means no output when using the USB_CDC port. If I tweak the config to allow USB_CDC for C3 then it fails at compile time because Serial is not defined by Arduino - it's Serial0. If I replace Serial with Serial0 then another compile error in the platformio main.cpp blocks it. If I work-around that I can get it to build and flash, but then it bootloops :-(

It behaves this way even with this added? 🤔

@clydebarrow
Copy link
Contributor Author

It behaves this way even with this added? 🤔

Yes. That works fine with S3, but on C3 Serial (without a 0) is undefined, not only in ESPHome code but also in platformio code. And fixing that leads to the bootloop. I just don't think the Arduino support for the C3 is mature.

/Users/clyde/.platformio/packages/framework-arduinoespressif32@3.20005.220925/cores/esp32/main.cpp: In function 'void app_main()':
/Users/clyde/.platformio/packages/framework-arduinoespressif32@3.20005.220925/cores/esp32/main.cpp:58:5: error: 'Serial' was not declared in this scope
     Serial.begin();
     ^~~~~~

@clydebarrow clydebarrow merged commit 300343a into esphome:dev Dec 15, 2023
49 checks passed
@clydebarrow
Copy link
Contributor Author

Oh, well this is weird, it's now working with Arduino on C3, with the default as UART0.

@github-actions github-actions bot locked and limited conversation to collaborators Dec 17, 2023
@jesserockz jesserockz added this to the 2023.12.2 milestone Dec 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants