Skip to content

Conversation

Josverl
Copy link
Contributor

@Josverl Josverl commented Jul 28, 2025

Summary

Detection of ESP-XX devices was based on just the names of the USB driver name.
and did not account for the switch of the newer ESP-xx devices to USB-CDC.
On Windows this caused unwanted/unneeded resets as the DTR/RTS signals are also used for automatic device reset over USB-CDC as reported by @crackwitz in #9659 (comment).

This PR uses the Espressif registered VID : 0x303A to detect USB-CDC ports ,
to enable the same DTR/RTS settings as used on the UART-USB connection.

Also improved the robustness of the code using getattr()

Depends on : #17775

Testing

Manual testing on Windows and WSL2

device platform USB-CDC UART
Win11 ESP32-C3
WSL2-Ubuntu ESP32-C3
Win11 ESP32-C2 n/a
WSL2-Ubuntu ESP32-C2 n/a
Win11 rp2040 n/a
Win11 ESP32-S3
WSL2-Ubuntu ESP32-S3

Trade-offs and Alternatives

Detection of the ESPxx devices via the USB-CDC VID is more deterministic than getattr(portinfo[0], "manufacturer", "") != "Microsoft", and will cover most of not all Espressif boards.

One possible drawback is that if/when other board manufacturers register their own VIDs for devices powered by an ESP32 - and access over USB-CDC that they will be exposed to unexpected resets.

@Josverl Josverl force-pushed the mpr/fix_esp_usb_reset branch from 42b8129 to 538c9e2 Compare July 28, 2025 15:34
@Josverl Josverl changed the title Mpr/fix esp usb reset mpremote: fix esp usb reset Jul 28, 2025
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

Copy link

codecov bot commented Jul 28, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.38%. Comparing base (4ba626a) to head (dea949e).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #17776   +/-   ##
=======================================
  Coverage   98.38%   98.38%           
=======================================
  Files         171      171           
  Lines       22283    22283           
=======================================
  Hits        21924    21924           
  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.

@dpgeorge dpgeorge added this to the release-1.26.0 milestone Jul 29, 2025
@dpgeorge dpgeorge added the tools Relates to tools/ directory in source, or other tooling label Jul 29, 2025
@projectgus projectgus self-requested a review July 30, 2025 23:35
Copy link
Contributor

@projectgus projectgus left a comment

Choose a reason for hiding this comment

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

I don't have a Windows setup to easily verify the fix but this seems very reasonable, thanks @Josverl!

Detection of ESP-XX devices was based on just the names of the USB driver
name, and did not account for the switch of the newer ESP-xx devices to
USB-CDC.  On Windows this caused unwanted/unneeded resets as the DTR/RTS
signals are also used for automatic device reset over USB-CDC.  See
micropython#9659 (comment)

This commit uses the Espressif registered VID 0x303A to detect USB-CDC
ports, to enable the same DTR/RTS settings as used on the UART-USB
connection.

Also improved the robustness of the code using `getattr()`.

Signed-off-by: Jos Verlinde <Jos_Verlinde@hotmail.com>
@dpgeorge dpgeorge force-pushed the mpr/fix_esp_usb_reset branch from 538c9e2 to dea949e Compare August 1, 2025 05:00
@dpgeorge dpgeorge merged commit dea949e into micropython:master Aug 1, 2025
71 checks passed
@dpgeorge
Copy link
Member

dpgeorge commented Aug 1, 2025

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tools Relates to tools/ directory in source, or other tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants