8000 esp32: Fix ARDUINO_NANO_ESP32 build configuration. by projectgus · Pull Request #15782 · micropython/micropython · GitHub
[go: up one dir, main page]
Skip to content

esp32: Fix ARDUINO_NANO_ESP32 build configuration. #15782

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 4, 2024
Merged

esp32: Fix ARDUINO_NANO_ESP32 build configuration. #15782

merged 1 commit into from
Sep 4, 2024

Conversation

projectgus
Copy link
Contributor
@projectgus projectgus commented Sep 4, 2024 •
edited
Loading

Summary

Regression introduced by #15727 (comment) now that MICROPY_HW_USB_CDC is set.

The ARDUINO_NANO_ESP32 specifically builds shared/tinyusb/mp_usb_cdc.c for the 1200bps reset behaviour. However MicroPython esp32 doesn't yet use the rest of the shared/tinyusb functionality.

Testing

I've verified this fixes the build for this board, and have eyeballed mp_usb_cdc.c to see that it should compile an equivalent binary to before MICROPY_HW_USB_CDC was defined. However I don't have a suitable board to test this on.

@projectgus
esp32: Fix ARDUINO_NANO_ESP32 build configuration.
a6c35ae 
Regression introduced by 5e692d0 now at MICROPY_HW_USB_CDC is set.

The ARDUINO_NANO_ESP32 specifically builds shared/tinyusb/mp_usb_cdc.c
for the 1200bps reset behaviour. However MicroPython esp32 doesn't yet
use the rest of the shared/tinyusb functionality.

Signed-off-by: Angus Gratton <angus@redyak.com.au>
@projectgus
Copy link
Contributor Author

@andrewleech Heads-up about this board for your work in #15108, unless you've already looked into it. I think most of the board-specific changes (i.e. explicitly adding ${MICROPY_DIR}/shared/tinyusb/mp_usbd_cdc.c from mpconfigboard.cmake) can be cleaned up when this is added, plus this commit can be reverted - so it should end up a lot tidier.

@dpgeorge
Copy link
Member
dpgeorge commented Sep 4, 2024

Thanks, this looks good to me. The change is confined to the ARDUINO_NANO_ESP32 board so that limits its scope, which is good.

For reference: commit 84a8f7e added MICROPY_EXCLUDE_SHARED_TINYUSB_USBD_CDC for the nrf port, but that was recently no longer used by the nrf port in 1907569 . So this option can be deleted once esp32 moves to the shared TinyUSB code.

dpgeorge
dpgeorge approved these changes Sep 4, 2024
View reviewed changes
Copy link
Member
@dpgeorge dpgeorge left a comment

Choose a reason for hiding this comment

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

I verified that this compiled successfully with IDF 5.2.2.

@dpgeorge dpgeorge merged commit a6c35ae into micropython:master Sep 4, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dpgeorge dpgeorge dpgeorge approved these changes

@andrewleech andrewleech Awaiting requested review from andrewleech

Assignees
No one assigned
Labels
bug port-esp32
Projects
None yet
Milestone
No milestone
398B Development

Successfully merging this pull request may close these issues.
2 participants
@projectgus @dpgeorge
0