Skip to content

Conversation

projectgus
Copy link
Contributor

@projectgus projectgus commented Aug 28, 2025

Summary

The DTR quirk workaround from #10347 is needed for the Espressif Serial/JTAG device, but not for TinyUSB - in fact DTR must be set for TinyUSB to correctly determine if the serial port is open (and leads to issues with lost bytes otherwise).

  • On the current master branch and v1.26 release with Espressif TinyUSB >=v0.18.1, using mpremote with ESP32-S3 native serial port can result in a hang due to the Windows workaround. Specifically: if the FIFO is full, this logic in mp_usb_cdc.c determines the port is not connected and busy-loops indefinitely waiting for the FIFO to empty).
  • With upstream TinyUSB or TinyUSB <v0.18, this problem can instead cause lost bytes (as the FIFO will drop old bytes if full and it thinks it is disconnected).

The fix is to not apply the quirk in this case - it's needed to avoid a spurious reset under Windows with the Espressif Serial/JTAG device, but not needed for TinyUSB (no spurious reset observed with mpremote v1.25, pre-#10347).

This fix may help with issues discussed in https://github.com/orgs/micropython/discussions/17465 and in comments of #17865, but only on Windows (I think), as the other hosts will set DTR when they open the port (I believe...).

Testing

  • Verified that on Windows 11 mpremote v1.26 would hang an ESP32-S3 + Octal SPIRAM board running current master (using steps provided by @gohai here, thanks @gohai!)
  • Verified that on Windows 11 mpremote v1.25 would not hang that board, and mpremote connect COMx doesn't trigger a spurious reset.
  • Verified that on Windows 11 mpremote v1.25 could trigger a spurious reset connecting to the Espressif Serial/JTAG interface of ESP32-C6. This is one of the issues fixed in tools/pyboard.py : Set DTR on Windows to avoid ESPxx hard reset. #10347.
  • Verified that using mpremote built from this branch, neither the hang nor the spurious reset occurs.

Trade-offs and Alternatives

  • We could try and find some other heuristic for "port is open", but it's hard to think of one that might not sometimes cause unnecessary long delays waiting for the host to read data over USB (when the port is actually closed).
  • I'm hopeful there might be a better way to manage DTR/RTS on Windows, but it's so fiddly that there may not be...

This work was funded through GitHub Sponsors.

The DTR quirk workaround from dea949e is needed for the Espressif
Serial/JTAG device, but not for TinyUSB - in fact DTR must be set for
TinyUSB to correctly determine if the serial port is open (and leads to
issues with lost bytes otherwise).

This work was funded through GitHub Sponsors.

Signed-off-by: Angus Gratton <angus@redyak.com.au>
@projectgus projectgus added the tools Relates to tools/ directory in source, or other tooling label Aug 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 Aug 28, 2025

Codecov Report

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

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #17999   +/-   ##
=======================================
  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.

@Josverl
Copy link
Contributor

Josverl commented Aug 28, 2025

@projectgus , I have been working on an alternative way to ID the serial ports based on VID/PID : #17800

While that guarantees that the dtr won't be set for usb-cdc, there is a risk of missing out on some serial devices.
We can bake in the known ones, and load additional ones from the Mpremote config.py now that it works across platforms

@projectgus
Copy link
Contributor Author

@projectgus , I have been working on an alternative way to ID the serial ports based on VID/PID : #17800

While that guarantees that the dtr won't be set for usb-cdc, there is a risk of missing out on some serial devices. We can bake in the known ones, and load additional ones from the Mpremote config.py now that it works across platforms

Thanks @Josverl. I did see this, but I have some concern that the list of possible ESP* USB/serial chips will trend in the limit towards the current logic (i.e. someone somewhere has made an ESP board with approximately every USB/serial converter on the market).

I actually have another idea for how to manage this, I'll try to put up a draft PR soon and see what you think.

@projectgus
Copy link
Contributor Author

projectgus commented Aug 29, 2025

Whether or not we merge #18001, I suggest we still consider this PR for a v1.26.1 release (as it's much lower risk of regression).

@dpgeorge
Copy link
Member

Yes, I think we'll need a v1.26.1.

@Josverl
Copy link
Contributor

Josverl commented Aug 29, 2025

someone somewhere has made an ESP board with approximately every USB/serial converter on the market.

I agree that the list is likely endless , and unobtainable - but I do not think that we can truncate the list to just one VID/PID either.
While I do not have any Pycom device, I think that would not be detected , while it could be detected.

So if we built the logic to add :

  • a, known but incomplete, list of VID/PIDs ,
  • a way to change the behaviour by either :
    • parameters : --dtr high|low|none --rts high|low|none ( careful of inversion logic on ESPxx boards)
    • or config ( VID / PID list )

I think we can offer both simple reliability for most boards , and ability to work with any boards.

( bit HIL testing takes time- and timeframes matter - so its OK to smash the Big bugs first, and worry about the little critters later if you want to release 1.26.1 )

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