shared/tinyusb: Add back runtime CDC config for older TinyUSB. - #19653
shared/tinyusb: Add back runtime CDC config for older TinyUSB.#19653dpgeorge wants to merge 1 commit into
Conversation
Commit 6bde1b5 removed this configuration because TinyUSB 0.21.0 no longer uses it. But the esp32 port still uses an older TinyUSB version/fork (based on TinyUSB 0.18.0) and therefore still needs this runtime CDC configuration. Signed-off-by: Damien George <damien@micropython.org>
Codecov Reportβ
All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #19653 +/- ##
=======================================
Coverage 98.58% 98.58%
=======================================
Files 182 182
Lines 23322 23322
Branches 5 5
=======================================
Hits 22993 22993
Misses 328 328
Partials 1 1
Flags with carried forward coverage won't be shown. Click here to find out more. β View full report in Codecov by Harness. π New features to boost your workflow:
|
|
Code size report: |
projectgus
left a comment
There was a problem hiding this comment.
LGTM. I noticed that Espressif has updated their TinyUSB component to v0.21.0~1, so I tried switching to this instead (which also let us drop our fork). However, for some reason the banner is corrupted with this version! (Some substrings of the full banner are sent, but not all of it.) Sigh.
Branch for future investigation is here: https://github.com/projectgus/micropython/tree/bugfix/esp32_tinyusb_version
Whereas this PR works fine for me.
Remember that TinyUSB stock at 0.21.0 broke the TX persistence, which is why we needed to fork TinyUSB! So I guess we could update our Espressif TinyUSB fork, and add the same patch we added to our TinyUSB fork to fix it. |
Actually, maybe that's a better fix than this PR? We need to update our Espressif TinyUSB at some point, so now might be a good idea. |
Oh yeah, of course. Too many bugs to keep track of!
Agree, that does sound like a good approach. |
Summary
Commit 6bde1b5 removed this configuration because TinyUSB 0.21.0 no longer uses it. But the esp32 port still uses an older TinyUSB version/fork (based on TinyUSB 0.18.0) and therefore still needs this runtime CDC configuration.
Without this setting, ESP32-S2 and -S3 boards do not display the initial REPL banner (that's mostly a cosmetic issue, but nevertheless nice to have).
Testing
Tested on ESP32_GENERIC_S3, the REPL banner is now there after a hard reset (prior to this fix it was not).
Tested RPI_PICO is not affected by this change.
Trade-offs and Alternatives
This is arguably a regression in MicroPython 1.29.0, but I don't think it's worth a patch release.
Generative AI
I did not use generative AI tools when creating this PR.