tools/pyboard.py: ProcessToSerial cross-platform fix #8435
Conversation
Using ProcessToSerial class from pyboard.py on Windows was not possible due to some Unix-only code (for example, select.poll or selectors.select()). A solution to get ProcessToSerial working on Windows is by creating a thread that constantly reads from the subp.stdout and completes a queue. Then, the read() method will get the characters from the queue. This solution also works on Unix platforms, but I have implemented it only for Windows platforms, leaving the original code for Unix platforms.
Codecov Report
@@ Coverage Diff @@
## master #8435 +/- ##
=======================================
Coverage 98.25% 98.25%
=======================================
Files 154 154
Lines 20284 20284
=======================================
Hits 19931 19931
Misses 353 353 Continue to review full report at Codecov.
|
tools/pyboard.py
Outdated
| continue | ||
| else: | ||
| q.put(data) | ||
| data_prec = data | ||
|
|
||
| class ProcessToSerial: |
There was a problem hiding this comment.
I think it'd be much cleaner to create separate classes, such as: ProcessToSerialPosix and ProcessToSerialThreaded. Then some code like:
import select
if hasattr(select, "poll"):
ProcessToSerial = ProcessToSerialPosix
else:
ProcessToSerial = ProcessToSerialThreading|
Thanks for the contribution! It's also possible to use |
In order to make the code cleaner, there are 2 separate classes instead of ProcessToSerial : - ProcessToSerialPosix which uses select module to check if any data available on the subprocess stdout (working well on Unix) - ProcessToSerialThreading which uses a thread that completes a queue with data from the subprocess stdout (working well on Windows as an alternative to select) The choice is made by verifying if select has the attribute "poll" on the host platform
|
Thank you very much for your review and answer dpgeorge ! |
tools/pyboard.py
Outdated
| if hasattr(select, "poll"): | ||
| self.serial = ProcessToSerialPosix(device[len("exec:") :]) | ||
| else: | ||
| self.serial = ProcessToSerialThreading(device[len("exec:") :]) |
There was a problem hiding this comment.
The ProcessToSerial class is really part of the public API of this file, so I suggest to put this code in the global scope, so that ProcessToSerial is defined in the global scope. Then the code here doesn't need to change at all.
There was a problem hiding this comment.
Excuse me, I don't have much experience with Python. If I keep the ProcessToSerial class, how and where I could declare ProcessToSerialPosix and ProcessToSerialThreading ? I appreciate very much your help and indications.
Thank you in advance.
Best regards,
Victor
There was a problem hiding this comment.
class ProcessToSerialPosix:
...
class ProcessToSerialThreading:
...
if hasattr(select, "poll"):
ProcessToSerial = ProcessToSerialPosix
else:
ProcessToSerial = ProcessToSerialThreadingThere was a problem hiding this comment.
Thank you very much for your time and help ! I didn't know you can do that in Python. I made a commit with these changes.
OK, in that case let's stick with Queue. |
| @@ -153,13 +153,72 @@ def inWaiting(self): | |||
| else: | |||
| return n_waiting | |||
|
|
|||
| # A thread that constantly reads from subp stdout and puts data into a queue (FIFO list) | |||
| # When tested, it reads double '\r' which messes up the further code, so if double '\r' detected, skip one | |||
There was a problem hiding this comment.
Do you understand why it reads double \r?
There was a problem hiding this comment.
I have tried to investigate the reason, but unfortunately I am still not able to understand why it reads double \r
Considering ProcessToSerial is a part of the public API of this file, it is defined in the global scope. This leaves the Pyboard class code unchanged.
Update ADC and I2S APIs for 5.1
Using ProcessToSerial class from pyboard.py on Windows was not possible due to some Unix-only code (for example, select.poll or selectors.select()). A solution to get ProcessToSerial working on Windows is by creating a thread that constantly reads from the subp.stdout and completes a queue. Then, the read() method will get the characters from the queue. This solution also works on Unix platforms, but I have implemented it only for Windows platforms, leaving the original code for Unix platforms.