Skip to content
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.

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

monitor: make Ctrl-] work when running on WSL (v2) #1209

Closed
wants to merge 3 commits into from
Closed
Changes from 1 commit
Commits
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
49 changes: 44 additions & 5 deletions tools/idf_monitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import ctypes
import types
from distutils.version import StrictVersion
import signal

key_description = miniterm.key_description

Expand Down Expand Up @@ -130,12 +131,29 @@ class ConsoleReader(StoppableThread):
""" Read input keys from the console and push them to the queue,
until stopped.
"""
def __init__(self, console, event_queue):
def __init__(self, console, event_queue, running_on_wsl=False):
super(ConsoleReader, self).__init__()
self.console = console
self.event_queue = event_queue
self.tid = None
self.pthread_self = None
self.pthread_kill = None
if running_on_wsl:
libpthread = ctypes.CDLL("libpthread.so.0")
self.pthread_self = libpthread.pthread_self
self.pthread_self.argtypes = [ ]
self.pthread_self.restype = ctypes.c_void_p
self.pthread_kill = libpthread.pthread_kill
self.pthread_kill.argtypes = [ ctypes.c_void_p, ctypes.c_int ]
self.pthread_kill.restype = ctypes.c_int
signal.signal(signal.SIGRTMIN, self._handler)

def _handler(self, signum, frame):
pass

def run(self):
if self.pthread_self != None:
Copy link
Contributor

Choose a reason for hiding this comment

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

Typical Python style is to use "is not None" instead of "!= None".

In this case (and in the same check below) I'd be more comfortable using if running_on_wsl everywhere as this makes it easier to trace all the of the WSL workarounds (which can hopefully some day be removed!)

Copy link
Contributor

Choose a reason for hiding this comment

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

(Also if you don't need to test whether self.pthread_self is None, you can remove the self.pthread_self = None default value settings from the constructor)

self.tid = self.pthread_self()
self.console.setup()
try:
while self.alive:
Expand All @@ -153,13 +171,20 @@ def run(self):
c = self.console.getkey()
except KeyboardInterrupt:
c = '\x03'
except IOError:
if not self.alive:
# expected during _cancel when killed
break
Copy link
Contributor

Choose a reason for hiding this comment

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

This code swallows the IOError if self.alive is true. Does it make more sense to re-raise it?

if c is not None:
self.event_queue.put((TAG_KEY, c), False)
finally:
self.console.cleanup()

def _cancel(self):
if os.name == 'posix':
if self.pthread_kill != None:
if self.tid != None:
self.pthread_kill(self.tid, signal.SIGRTMIN)
elif os.name == 'posix':
# this is the way cancel() is implemented in pyserial 3.3 or newer,
# older pyserial (3.1+) has cancellation implemented via 'select',
# which does not work when console sends an escape sequence response
Expand All @@ -168,8 +193,8 @@ def _cancel(self):
#
# on Windows there is a different (also hacky) fix, applied above.
#
# note that TIOCSTI is not implemented in WSL / bash-on-Windows.
# TODO: introduce some workaround to make it work there.
# Note that TIOCSTI is not implemented in WSL / bash-on-Windows,
# see pthread_kill() above instead.
import fcntl, termios
fcntl.ioctl(self.console.fd, termios.TIOCSTI, b'\0')

Expand Down Expand Up @@ -220,6 +245,20 @@ class Monitor(object):
"""
def __init__(self, serial_instance, elf_file, make="make", toolchain_prefix=DEFAULT_TOOLCHAIN_PREFIX, eol="CRLF"):
super(Monitor, self).__init__()
posix_is_wsl = False
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about naming this running_on_wsl to match the field used in the Console instance? Another change that makes it easier to search for all of the WSL-related hacks at once. :)

if os.name == 'posix':
try:
with open('/proc/version', 'r') as pv:
if 'microsoft' in pv.read().lower():
posix_is_wsl = True
except:
pass
if posix_is_wsl:
# re-open stdin unbuffered to make reading interruptible
import io
enc = sys.stdin.encoding
sys.stdin = io.open(sys.stdin.fileno(), mode='rb', buffering=0, closefd = False)
sys.stdin.encoding = enc
self.event_queue = queue.Queue()
self.console = miniterm.Console()
if os.name == 'nt':
Expand All @@ -238,7 +277,7 @@ def getkey_patched(self):
self.console.getkey = types.MethodType(getkey_patched, self.console)

self.serial = serial_instance
self.console_reader = ConsoleReader(self.console, self.event_queue)
self.console_reader = ConsoleReader(self.console, self.event_queue, posix_is_wsl)
self.serial_reader = SerialReader(self.serial, self.event_queue)
self.elf_file = elf_file
self.make = make
Expand Down