Skip to content

shared/tinyusb: Add back runtime CDC config for older TinyUSB. - #19653

Open
dpgeorge wants to merge 1 commit into
micropython:masterfrom
dpgeorge:shared-tinyusb-add-back-cdc-config
Open

shared/tinyusb: Add back runtime CDC config for older TinyUSB.#19653
dpgeorge wants to merge 1 commit into
micropython:masterfrom
dpgeorge:shared-tinyusb-add-back-cdc-config

Conversation

@dpgeorge

Copy link
Copy Markdown
Member

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.

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>
@dpgeorge dpgeorge added the shared Relates to shared/ directory in source label Aug 26, 2026
@dpgeorge
dpgeorge requested a review from projectgus August 26, 2026 06:41
@codecov

codecov Bot commented Aug 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.58%. Comparing base (4d80ff0) to head (af8c50b).

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           
Flag Coverage Δ
unix-coverage-32bit 98.59% <ø> (ø)
unix-coverage-64bit 98.52% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

Copy link
Copy Markdown

Code size report:

Reference:  tools/ci: Restore esp32 lockfiles when changing branches. [4d80ff0]
Comparison: shared/tinyusb: Add back runtime CDC config for older TinyUSB. [merge of af8c50b]
  mpy-cross:    +0 +0.000% 
   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
      esp32:    +0 +0.000% ESP32_GENERIC
     mimxrt:    +0 +0.000% TEENSY40
        rp2:    +0 +0.000% RPI_PICO_W
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS
  qemu rv32:    +0 +0.000% VIRT_RV32

@projectgus projectgus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

@dpgeorge

Copy link
Copy Markdown
Member Author

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!

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.

@dpgeorge

Copy link
Copy Markdown
Member Author

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.

@projectgus

Copy link
Copy Markdown
Contributor

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!

Remember that TinyUSB stock at 0.21.0 broke the TX persistence, which is why we needed to fork TinyUSB!

Oh yeah, of course. Too many bugs to keep track of!

So I guess we could update our Espressif TinyUSB fork, and add the same patch we added to our TinyUSB fork to fix it.

Agree, that does sound like a good approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

shared Relates to shared/ directory in source

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants