From 1ce0d107c87f2d897e71fa3b7ee6bcfa7c754794 Mon Sep 17 00:00:00 2001 From: Victor Malai <78157734+vikmal-sw@users.noreply.github.com> Date: Mon, 21 Mar 2022 15:11:14 +0100 Subject: [PATCH 1/3] tools/pyboard.py: ProcessToSerial cross-platform fix 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. --- tools/pyboard.py | 119 +++++++++++++++++++++++++++++++++++------------ 1 file changed, 88 insertions(+), 31 deletions(-) diff --git a/tools/pyboard.py b/tools/pyboard.py index 15ddfd745d589..76bb83538f1d6 100755 --- a/tools/pyboard.py +++ b/tools/pyboard.py @@ -153,57 +153,114 @@ 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 +def read_stream(stream, q): + data_prec = b"" + while True: + data = stream.read(1) + if (data_prec == data) and (data_prec == b'\r'): + continue + else: + q.put(data) + data_prec = data class ProcessToSerial: "Execute a process and emulate serial connection using its stdin/stdout." def __init__(self, cmd): + # Verify if we are on Windows + import platform + if (platform.system() == 'Windows'): + self.ON_WINDOWS = True + else: + self.ON_WINDOWS = False + import subprocess + + #If on Windows, preexec_fn parameter should be replaced by start_new_session + if (self.ON_WINDOWS): + self.subp = subprocess.Popen( + cmd, + bufsize=0, + shell=True, + start_new_session=True, + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + ) + else: + self.subp = subprocess.Popen( + cmd, + bufsize=0, + shell=True, + preexec_fn=os.setsid, + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + ) + + if (self.ON_WINDOWS): + #If on Windows, select can not be used, a thread is used instead + # that constantly reads from subp.stdout + import threading + import queue + self.q = queue.Queue() + self.t = threading.Thread(target=read_stream, args=(self.subp.stdout, self.q)) + self.t.daemon=True # Makes thread exit when main script exits + self.t.start() + else: + # Initially was implemented with selectors, but that adds Python3 + # dependency. However, there can be race conditions communicating + # with a particular child process (like QEMU), and selectors may + # still work better in that case, so left inplace for now. + # + # import selectors + # self.sel = selectors.DefaultSelector() + # self.sel.register(self.subp.stdout, selectors.EVENT_READ) - self.subp = subprocess.Popen( - cmd, - bufsize=0, - shell=True, - preexec_fn=os.setsid, - stdin=subprocess.PIPE, - stdout=subprocess.PIPE, - ) - - # Initially was implemented with selectors, but that adds Python3 - # dependency. However, there can be race conditions communicating - # with a particular child process (like QEMU), and selectors may - # still work better in that case, so left inplace for now. - # - # import selectors - # self.sel = selectors.DefaultSelector() - # self.sel.register(self.subp.stdout, selectors.EVENT_READ) - - import select + import select - self.poll = select.poll() - self.poll.register(self.subp.stdout.fileno()) + self.poll = select.poll() + self.poll.register(self.subp.stdout.fileno()) def close(self): import signal - - os.killpg(os.getpgid(self.subp.pid), signal.SIGTERM) + #os.killpg can not be used on Windows, os.kill is used instead + if (self.ON_WINDOWS): + os.kill(self.subp.pid, signal.SIGTERM) + else: + os.killpg(os.getpgid(self.subp.pid), signal.SIGTERM) def read(self, size=1): data = b"" - while len(data) < size: - data += self.subp.stdout.read(size - len(data)) - return data + # If on Windows, data will be read from the queue that is completed by the thread + if (self.ON_WINDOWS): + i = 0 + while i != size: + data += self.q.get() + i += 1 + return data + else: + while len(data) < size: + data += self.subp.stdout.read(size - len(data)) + return data def write(self, data): self.subp.stdin.write(data) return len(data) def inWaiting(self): - # res = self.sel.select(0) - res = self.poll.poll(0) - if res: - return 1 - return 0 + # If on Windows, inWaiting will return (1 or 0) based on the emptiness of the queue + if (self.ON_WINDOWS): + if self.q.empty(): + return 0 + else: + return 1 + else: + # res = self.sel.select(0) + res = self.poll.poll(0) + if res: + return 1 + return 0 class ProcessPtyToTerminal: From 0e16b2e33be655800eea437a2713618daa650190 Mon Sep 17 00:00:00 2001 From: Victor Malai <78157734+vikmal-sw@users.noreply.github.com> Date: Thu, 24 Mar 2022 14:46:55 +0100 Subject: [PATCH 2/3] tools/pyboard.py: Creating 2 separate classes for ProcessToSerial 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 --- tools/pyboard.py | 162 ++++++++++++++++++++++++----------------------- 1 file changed, 84 insertions(+), 78 deletions(-) diff --git a/tools/pyboard.py b/tools/pyboard.py index 76bb83538f1d6..27192a28f5f3d 100755 --- a/tools/pyboard.py +++ b/tools/pyboard.py @@ -165,102 +165,103 @@ def read_stream(stream, q): q.put(data) data_prec = data -class ProcessToSerial: - "Execute a process and emulate serial connection using its stdin/stdout." +class ProcessToSerialThreading: + "Execute a process and emulate serial connection using its stdin/stdout. Communication with stdout is done through the read_stream thread" - def __init__(self, cmd): - # Verify if we are on Windows - import platform - if (platform.system() == 'Windows'): - self.ON_WINDOWS = True - else: - self.ON_WINDOWS = False - + def __init__(self, cmd): import subprocess - #If on Windows, preexec_fn parameter should be replaced by start_new_session - if (self.ON_WINDOWS): - self.subp = subprocess.Popen( - cmd, - bufsize=0, - shell=True, - start_new_session=True, - stdin=subprocess.PIPE, - stdout=subprocess.PIPE, - ) + # On Windows, preexec_fn parameter should be replaced by start_new_session + self.subp = subprocess.Popen( + cmd, + bufsize=0, + shell=True, + start_new_session=True, + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + ) + + # On Windows, select can not be used, a thread is used instead + # that constantly reads from subp.stdout and completes a queue + import threading + import queue + self.q = queue.Queue() + self.t = threading.Thread(target=read_stream, args=(self.subp.stdout, self.q)) + self.t.daemon=True # Makes thread exit when main script exits + self.t.start() + + def close(self): + import signal + os.kill(self.subp.pid, signal.SIGTERM) + + def read(self, size=1): + data = b"" + # On Windows, data will be read from the queue that is completed by the thread + i = 0 + while i != size: + data += self.q.get() + i += 1 + return data + + def write(self, data): + self.subp.stdin.write(data) + return len(data) + + def inWaiting(self): + if self.q.empty(): + return 0 else: - self.subp = subprocess.Popen( - cmd, - bufsize=0, - shell=True, - preexec_fn=os.setsid, - stdin=subprocess.PIPE, - stdout=subprocess.PIPE, - ) + return 1 + +class ProcessToSerialPosix: + "Execute a process and emulate serial connection using its stdin/stdout. Using select to check if any data available on stdout" + + def __init__(self, cmd): + import subprocess - if (self.ON_WINDOWS): - #If on Windows, select can not be used, a thread is used instead - # that constantly reads from subp.stdout - import threading - import queue - self.q = queue.Queue() - self.t = threading.Thread(target=read_stream, args=(self.subp.stdout, self.q)) - self.t.daemon=True # Makes thread exit when main script exits - self.t.start() - else: - # Initially was implemented with selectors, but that adds Python3 - # dependency. However, there can be race conditions communicating - # with a particular child process (like QEMU), and selectors may - # still work better in that case, so left inplace for now. - # - # import selectors - # self.sel = selectors.DefaultSelector() - # self.sel.register(self.subp.stdout, selectors.EVENT_READ) + self.subp = subprocess.Popen( + cmd, + bufsize=0, + shell=True, + preexec_fn=os.setsid, + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + ) + + # Initially was implemented with selectors, but that adds Python3 + # dependency. However, there can be race conditions communicating + # with a particular child process (like QEMU), and selectors may + # still work better in that case, so left inplace for now. + # + # import selectors + # self.sel = selectors.DefaultSelector() + # self.sel.register(self.subp.stdout, selectors.EVENT_READ) - import select + import select - self.poll = select.poll() - self.poll.register(self.subp.stdout.fileno()) + self.poll = select.poll() + self.poll.register(self.subp.stdout.fileno()) def close(self): import signal - #os.killpg can not be used on Windows, os.kill is used instead - if (self.ON_WINDOWS): - os.kill(self.subp.pid, signal.SIGTERM) - else: - os.killpg(os.getpgid(self.subp.pid), signal.SIGTERM) + os.killpg(os.getpgid(self.subp.pid), signal.SIGTERM) def read(self, size=1): data = b"" - # If on Windows, data will be read from the queue that is completed by the thread - if (self.ON_WINDOWS): - i = 0 - while i != size: - data += self.q.get() - i += 1 - return data - else: - while len(data) < size: - data += self.subp.stdout.read(size - len(data)) - return data + while len(data) < size: + data += self.subp.stdout.read(size - len(data)) + return data def write(self, data): self.subp.stdin.write(data) return len(data) def inWaiting(self): - # If on Windows, inWaiting will return (1 or 0) based on the emptiness of the queue - if (self.ON_WINDOWS): - if self.q.empty(): - return 0 - else: - return 1 - else: - # res = self.sel.select(0) - res = self.poll.poll(0) - if res: - return 1 - return 0 + # res = self.sel.select(0) + res = self.poll.poll(0) + if res: + return 1 + return 0 class ProcessPtyToTerminal: @@ -315,7 +316,12 @@ def __init__( self.in_raw_repl = False self.use_raw_paste = True if device.startswith("exec:"): - self.serial = ProcessToSerial(device[len("exec:") :]) + # Class selector based on select module platform dependancy (available on Unix, but not on Windows) + import select + if hasattr(select, "poll"): + self.serial = ProcessToSerialPosix(device[len("exec:") :]) + else: + self.serial = ProcessToSerialThreading(device[len("exec:") :]) elif device.startswith("execpty:"): self.serial = ProcessPtyToTerminal(device[len("qemupty:") :]) elif device and device[0].isdigit() and device[-1].isdigit() and device.count(".") == 3: From 6889dde7f5bc8c4f615c4c079fba73ead594a965 Mon Sep 17 00:00:00 2001 From: Victor Malai <78157734+vikmal-sw@users.noreply.github.com> Date: Mon, 4 Apr 2022 11:12:36 +0200 Subject: [PATCH 3/3] tools/pyboard.py: Keep ProcessToSerial class in global scope 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. --- tools/pyboard.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tools/pyboard.py b/tools/pyboard.py index 27192a28f5f3d..c752557d0cc8c 100755 --- a/tools/pyboard.py +++ b/tools/pyboard.py @@ -263,6 +263,12 @@ def inWaiting(self): return 1 return 0 +# Class selector based on select module platform dependancy (available on Unix, but not on Windows) +import select +if hasattr(select, "poll"): + ProcessToSerial = ProcessToSerialPosix +else: + ProcessToSerial = ProcessToSerialThreading class ProcessPtyToTerminal: """Execute a process which creates a PTY and prints slave PTY as @@ -316,12 +322,7 @@ def __init__( self.in_raw_repl = False self.use_raw_paste = True if device.startswith("exec:"): - # Class selector based on select module platform dependancy (available on Unix, but not on Windows) - import select - if hasattr(select, "poll"): - self.serial = ProcessToSerialPosix(device[len("exec:") :]) - else: - self.serial = ProcessToSerialThreading(device[len("exec:") :]) + self.serial = ProcessToSerial(device[len("exec:") :]) elif device.startswith("execpty:"): self.serial = ProcessPtyToTerminal(device[len("qemupty:") :]) elif device and device[0].isdigit() and device[-1].isdigit() and device.count(".") == 3: