-
-
Notifications
You must be signed in to change notification settings - Fork 8.4k
mpremote: fix esp usb reset #17776
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
mpremote: fix esp usb reset #17776
Conversation
42b8129
to
538c9e2
Compare
Code size report:
|
Codecov Report✅ All modified and coverable lines are covered by tests. 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. 🚀 New features to boost your workflow:
|
There was a problem hiding this 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>
538c9e2
to
dea949e
Compare
Thank you! |
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
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.