espressif / esp-idf

Espressif IoT Development Framework. Official development framework for Espressif SoCs.
Apache License 2.0
13.62k stars 7.28k forks source link

idf.py monitor --no-reset --port <port> is resetting the chip on Windows (IDFGH-13869) #14714

Open fhrbata opened 2 weeks ago

fhrbata commented 2 weeks ago

Answers checklist.

General issue report

This problem was originally reported by @mjgc in https://github.com/espressif/esp-idf/issues/14040, but it's not directly related to it.

esp-idf-monitor is setting RTS and DTR lines(functions) to LOW/1/ON prior calling pyserial open. On Windows pyserial is storing these values in DCB device-control block by calling SetCommState. The default values prior port opening, IIUC, should be HIGH/0/OFF, meaning the RTS/DTR function lines are disabled.

Interchange state table from EIA/TIA-232, which helps me to better see that ON is 0 and OFF is 1 Notation Negative Voltage Positive Voltage
Binary State 1 0
Signal Condition Marking Spacing
Function OFF ON

I wrote a simple script which reads the DCB and prints the state of RTS and DTR.

# usage: python dcb.py <port(e.g. com7)>
from serial import win32
import sys
import ctypes

handle = win32.CreateFile(
        sys.argv[1],
        win32.GENERIC_READ | win32.GENERIC_WRITE,
        0,
    None,
    win32.OPEN_EXISTING,
        win32.FILE_ATTRIBUTE_NORMAL | win32.FILE_FLAG_OVERLAPPED,
        0)

if handle == win32.INVALID_HANDLE_VALUE:
    sys.exit('invalid handle')

dcb = win32.DCB()
win32.GetCommState(handle, ctypes.byref(dcb))
for k in dir(dcb):
    if k not in ('fRtsControl', 'fDtrControl'):
        continue
    print(k, ':', getattr(dcb, k))
win32.CloseHandle(handle)

With its help it can be seen how the states change after esp-idf-monitor exit. Prior invoking esp-idf-monitor

> python dcb.py com9
fDtrControl : 0
fRtsControl : 0

After running python -m esp_idf_monitor --no-reset -p com9

> python dcb.py com9
fDtrControl : 1
fRtsControl : 1

Now when the com9 is opened, it will cause the chip to go to reset, because fRtsControl is set to ON in DCB and RTS will be set to LOW. After the file is closed RTS goes to OFF again causing the reset. It can be seen with following simple python code and logical analyzer. I tried with UMFT234XF USB->UART.

>>> f = open('com9')
>>> f.close()

The fDtrControl and fRtsControl stay set to 1. So each operation opening/closing com9 will cause chip reset, but opening it e.g. in powershell will change the values back to 0

$ port= new-Object System.IO.Ports.SerialPort com9,9600,None,8,one
$ port.open()
$ port.close()

As it turned out click is doing parameter expansion(glob) on Windows, which calls stat, meaning opening/closing the com9 port and causing the reset before esp-idf-monitor is invoked. This can be partially mitigated by using windows_expand_args in click, but not in general. This probably needs to be fixed on the esp-idf-monitor side. The following patch seems to be working for me, but this will need some further investigation and testing.

diff --git a/esp_idf_monitor/base/serial_reader.py b/esp_idf_monitor/base/serial_reader.py
index 8b7bdd3..0e945d1 100644
--- a/esp_idf_monitor/base/serial_reader.py
+++ b/esp_idf_monitor/base/serial_reader.py
@@ -99,15 +99,12 @@ class SerialReader(Reader):
             self.close_serial()

     def open_serial(self, reset: bool) -> None:
-        # set the DTR/RTS into LOW prior open
-        self.reset_strategy._setRTS(LOW)
-        self.reset_strategy._setDTR(LOW)
-
-        self.serial.open()
-
         # set DTR/RTS into expected HIGH state, but set the RTS first to avoid reset
         self.reset_strategy._setRTS(HIGH)
         self.reset_strategy._setDTR(HIGH)
+
+        self.serial.open()
+
         if reset:
             self.reset_strategy.hard()
atanisoft commented 2 weeks ago

same can also be seen on Linux, thanks for a simple possible fix.

fhrbata commented 1 week ago

Hello @atanisoft , thank you very much for your feedback! I just tried and even though the mentioned fix seems to be working on windows, it's not working on linux for me and the monitor resets the chip every time. Unfortunately I don't have USB->UART converter with DTR line exposed(just RTS), so I cannot verify it's state, but here is what I think might be happening. It looks like that on windows the common setting for DTR/RTS is to be set OFF(HIGH), while on Linux they seem to be ON(LOW). The pyserial is setting DTR first, after the port is opened with os.open, so setting DTR OFF(HIGH) before opening the port with pyserial's open causes the chip to reset. Maybe it should be possible just to drop the initial settings in monitor.

diff --git a/esp_idf_monitor/base/serial_reader.py b/esp_idf_monitor/base/serial_reader.py                                           
index 8b7bdd38815c..afe5647b233b 100644                                                                                              
--- a/esp_idf_monitor/base/serial_reader.py                                                                                          
+++ b/esp_idf_monitor/base/serial_reader.py                                                                                          
@@ -99,10 +99,6 @@ class SerialReader(Reader):                                                                                       
             self.close_serial()                                                                                                     

     def open_serial(self, reset: bool) -> None:                                                                                     
-        # set the DTR/RTS into LOW prior open                                                                                       
-        self.reset_strategy._setRTS(LOW)                                                                                            
-        self.reset_strategy._setDTR(LOW)                                                                                            
-                                                                                                                                    
         self.serial.open()                                                                                                          

         # set DTR/RTS into expected HIGH state, but set the RTS first to avoid reset                         

I haven't tested this though. Anyway I believe it's not possible to get this 100% right, because we cannot be sure about the actual state of the lines(driver settings, system settings, how the lines were set in DCB), but I for sure can be missing something and other will have better ideas. Thank you

fhrbata commented 1 week ago

Maybe it should be possible just to drop the initial settings in monitor.

This will not work because pyserial is by default activating(ON) DTR/RTS and this gets also stored in DCB. Probably best would be to deactivate(OFF) DTR/RTS prior calling pyserial open on windows.