Skip to content

pyboard: Fix espxx boards hanging in bootloader after reset button. #11076

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Apr 4, 2023
Merged

pyboard: Fix espxx boards hanging in bootloader after reset button. #11076

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions tools/pyboard.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,11 +287,15 @@ def __init__(
for attempt in range(wait + 1):
try:
if os.name == "nt":
# Windows does not set DTR or RTS by default
self.serial = serial.Serial(**serial_kwargs)
self.serial.dtr = True
self.serial.rts = False
self.serial.port = device
portinfo = list(serial.tools.list_ports.grep(device)) # type: ignore
if portinfo and portinfo[0].manufacturer != "Microsoft":
# ESP8266/ESP32 boards use RTS/CTS for flashing and boot mode selection.
# DTR False: to avoid using the reset button will hang the MCU in bootloader mode
# RTS False: to prevent pulses on rts on serial.close() that would POWERON_RESET an ESPxx
self.serial.dtr = False # DTR False = gpio0 High = Normal boot
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure why this was previously True and is now False?

Also, if Windows doesn't set DTR/RTS by default, wouldn't it be good to set them for all drivers? Why can't it just set them both False for all drivers? Or at least True/False for Microsoft drivers?

Copy link
Contributor Author

@Josverl Josverl Mar 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I added the reasoning in the Issue , perhaps not the best place.
let me copy it here :

I'm not sure why this was previously True and is now False?
as that solve one problem (Reset on disconnect) , but now I found it created a 2nd one (Hang in bootloader )

to solve both False ,+ False is needed for espxx , but that would then block stm32 and others

any unconditional combination ends up blocking normal operation on some type of board or another.
9 see below truth table)
as to the why: I suspect that the non-Microsoft drivers may be less strict in the de-initialization , causing the spikes, but that is just speculation based on my professional bias.

A simple overview (for windows only) :

DTR CTS ESP8266 ESP32 STM32 SAMD51 RP2040
unspecified unspecified ❌ Win32:Reset on disconnect ❌ Win32:Reset on disconnect ✅ OK ✅ OK ✅ OK
True False ❌ Win32:Hang in bootloader ❌ Win32:Hang in bootloader ✅ OK ✅ OK ✅ OK
False False ✅ OK ✅ OK ❌ No Repl ❌ No Repl ❌ No Repl
True True ❌ Win32:Reset on disconnect ❌ Win32:Reset on disconnect ❌ No Repl ❌ No Repl ❌ No Repl
False True ❌ Win32 : Reset on disconnect ❌ Win32 : Reset on disconnect ❌ No Repl ❌ no Repl ❌ No Repl
serial.manufacturer wch.cn Silicon Labs Microsoft Microsoft Microsoft
serial.description USB-SERIAL CH340 Silicon Labs CP210x USB to UART Bridge USB Serial Device USB Serial Device USB Serial Device

Updated Logic

The updated logic will only set the dtr/rts signals for boards that do not use standard Microsoft drivers( based on the manufacturer)
It would also be possible to check against a list of known driver manufactures (like wch.cn or Silicon Labs ) but this would require a list of known drivers for all ports. ( which I do not have)

serial.manufacturer DTR CTS ESP8266 ESP32 STM32 SAMD51 RP2040
== "Microsoft" unspecified unspecified - - ✅ OK ✅ OK ✅ OK
!= "Microsoft" (wch.cn,Silicon Labs) False False ✅ OK ✅ OK - - -

Copy link
Member

Choose a reason for hiding this comment

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

OK, thanks for the details. That makes sense then.

self.serial.rts = False # RTS False = EN High = MCU enabled
self.serial.open()
else:
self.serial = serial.Serial(device, **serial_kwargs)
Expand Down