Skip to content

esp32: Bump esp_tinyusb component, add component version lock files to source control. #17960

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

projectgus
Copy link
Contributor

@projectgus projectgus commented Aug 20, 2025

Summary

Two changes related to ESP-IDF dependencies:

Testing

  • At this stage I haven't done any testing myself. In particular, I don't have easy access to a macOS host to reproduce the data corruption issue.
  • I pushed a temporary commit to this PR that contains a change to fail CI because the dependency lock files will mismatch. Failed job.

Trade-offs and Alternatives

  • @agatti has submitted esp32/esp32_common.cmake: Switch back to the vendored TinyUSB copy. #17913 which seems like it is a better short-term fix for all currently known TinyUSB issues. However, using upstream TinyUSB is a risk in the future because it's possible Espressif will make fixes that don't immediately go upstream. For example, upstream TinyUSB released 0.18.0 in December but Espressif have tagged 0.18.1, 0.18.2, 0.18.3, 0.18.4 since then... If we use upstream TinyUSB then it's likely we'll eventually run into issues that Espressif has already fixed in their fork, or that we should report to Espressif but can't report them because we're not using their supported TinyUSB.
  • There may be alternatives to adding all these lock files to Git. One alternative would be to specify all of the component versions exactly in idf_component.yml using == to get specific versions. However, this still doesn't provide any tracking of when the ESP-IDF version has changed from whatever the current recommended version is...
  • Paying more attention to exact component versions means from now that we'll need to also pay attention to updating components (and the lock file) when it's relevant to do so. However, as seen from this esp_tinyusb issue then that's probably something that we should do anyway.

This work was funded through GitHub Sponsors.

Reported to fix issues reported with serial corruption when interacting
with MicroPython from macOS hosts.

This work was funded through GitHub Sponsors.

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

codecov bot commented Aug 20, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.38%. Comparing base (b5fcb33) to head (6a61356).

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #17960   +/-   ##
=======================================
  Coverage   98.38%   98.38%           
=======================================
  Files         171      171           
  Lines       22296    22296           
=======================================
  Hits        21937    21937           
  Misses        359      359           

☔ View full report in Codecov by Sentry.
📢 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.

Copy link

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
     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

This is recommended by Espressif, and it's the only way to ensure
everyone builds the same set of component versions.

The awkward part is that updating the ESP-IDF version will churn a line
in each of these files (and possibly other changes).

This work was funded through GitHub Sponsors.

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

gohai commented Aug 20, 2025

@projectgus I built 6a61356, and will test when I have access to the boards tomorrow.

FWIW: my steps were

  • checking out 6a61356
  • cd ports/esp32
  • make submodules
  • make BOARD=...

If there is more to it, please kindly let me know, as I am not yet very familiar with the build process.

@agatti
Copy link
Contributor

agatti commented Aug 20, 2025

However, this still doesn't provide any tracking of when the ESP-IDF version has changed from whatever the current recommended version is... [...] Paying more attention to exact component versions means from now that we'll need to also pay attention to updating components (and the lock file) when it's relevant to do so.

Keep in mind that it's not an uncommon occurrence for Espressif to yank published component versions. That can make pro and con arguments both for keeping track of versions manually and for letting ESP-IDF resolve versions on its own so I won't weigh on this :)

@projectgus
Copy link
Contributor Author

Keep in mind that it's not an uncommon occurrence for Espressif to yank published component versions. That can make pro and con arguments both for keeping track of versions manually and for letting ESP-IDF resolve versions on its own

Although I agree there's pros and cons, I still think explicitly tracking dependencies is a win for this kind of scenario. We don't currently have any way to tell if someone is building with a local yanked component when they submit a bug report, but with this change they'd have to have explicitly chosen to do that.

Also, if we're building with a yanked component and this PR then CI should fail and we get the hint to go and update it.

The more complex case, as I see it, is that we won't automatically build MicroPython with bugfix releases of components, without manually running idf.py -B [...] update-components. In this aspect I agree it's a mixed bag, things won't quietly break but they won't quietly fix themselves either...

so I won't weigh on this :)

Overall I am very interested in your thoughts on this approach, both for this specific TinyUSB problem and generally - so feel free to weigh in if you think it's a step in the wrong direction.

@projectgus
Copy link
Contributor Author

If there is more to it, please kindly let me know, as I am not yet very familiar with the build process.

Thanks @gohai, those are the steps! To flash in the same step you can do make deploy BOARD=... PORT=... (more info in the README). Look forward to hearing if it changes anything (or not).

@dpgeorge
Copy link
Member

Although the lock files add complexity (at least, in the sense of adding more files) it sounds like it's worth it to keep proper track of which component versions are in use.

@gohai
Copy link

gohai commented Aug 21, 2025

@projectgus This works fine on my USB-OTG-connected ESP32-S3-Zero. (This one had issues copying files earlier.)

However, I am still seeing reproducible repl hangs on the ESP32S3-S3-DEV-KIT-NXR8. See e.g. below (just starting repl made it hang):

Screenshot 2025-08-21 at 9 02 06 AM

@projectgus
Copy link
Contributor Author

@gohai So you get this immediately from simply running mpremote?

@agatti
Copy link
Contributor

agatti commented Aug 21, 2025

@projectgus I'd say I don't have a preference on how the tracking is performed, as long as progress is made :)

I think it all boils down on how much of a maintenance burden it can be to track versions manually. From my past experiences I can sum it all up as this (virtually none of these can be MicroPython's concern, I'm dumping them here in the hope they may help make a more informed decision): how often dependencies need to be updated, how much time it takes to validate those changes, and what's the turnaround time to get an emergency fix from Espressif into MicroPython (eg. potential security implications, that's yet another Pandora's box that should better stay shut) or how quickly a yanked component can be gotten rid of in the published repo lockfiles.

For emergency changes, if people in charge for releasing the Espressif components you rely on are based out of Shanghai, then you folks being a couple hours ahead of them is not a big deal, otherwise you may want to keep that in mind too.

Finally, I wonder if this approach is compatible with https://github.com/agatti/micropython-idf-component. Granted, it's up to me to work around your choices in this case if the end result is not compatible so I'm just talking out loud here.

Incidentally, @dpgeorge if you ever plan to release MicroPython as an ESP-IDF component and put it up into Espressif's registry, feel free to take as much code/ideas/whatnot from that repo. I've licensed that as MIT in order to make the process easier on your end if an official component is an option you want to consider.

@agatti
Copy link
Contributor

agatti commented Aug 21, 2025

However, I am still seeing reproducible repl hangs on the ESP32S3-S3-DEV-KIT-NXR8. See e.g. below (just starting repl made it hang):

Just to be extra sure, does that board with the official 1.26 release firmware and with a firmware with updated IDF components work if you connect to it from Linux, Windows, or a virtualised Linux (so the full USB device is passed through)?

@gohai
Copy link

gohai commented Aug 21, 2025

@gohai So you get this immediately from simply running mpremote?

@projectgus It behaves identical to what I saw on 1.26 with that board... it sometimes hangs at startup, but if it does, running help() once or twice will definitely freeze it.

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

Successfully merging this pull request may close these issues.

ESP32-S3 mpremote sends corrupted fs hook when mounting local directory
4 participants