Make automatic braille display detection more pythonic

source/bdDetect.py

@@ -15,11 +15,10 @@
 import itertools
 from collections import namedtuple, defaultdict, OrderedDict
 import threading
-
+from concurrent.futures import ThreadPoolExecutor, Future
 import typing
 import hwPortUtils
 import braille
-import winKernel
 import winUser
 from logHandler import log
 import config
@@ -238,121 +237,119 @@ class Detector(object):
 	This should only be used by the L{braille} module.
 	"""
 
-	def __init__(self, usb=True, bluetooth=True, limitToDevices=None):
+	def __init__(
+			self,
+			usb: bool = True,
+			bluetooth: bool = True,
+			limitToDevices: typing.Optional[typing.List[str]] = None
+	):
 		"""Constructor.
 		The keyword arguments initialize the detector in a particular state.
-		On an initialized instance, these initial arguments can be overridden by calling L{_startBgScan} or L{rescan}.
+		On an initialized instance, these initial arguments can be overridden by calling
+		L{_queueBgScan} or L{rescan}.
 		@param usb: Whether this instance should detect USB devices initially.
-		@type usb: bool
 		@param bluetooth: Whether this instance should detect Bluetooth devices initially.
-		@type bluetooth: bool
 		@param limitToDevices: Drivers to which detection should be limited initially.
 			C{None} if no driver filtering should occur.
 		"""
-		self._BgScanApc = winKernel.PAPCFUNC(self._bgScan)
+		self._executor = ThreadPoolExecutor(1)
 		self._btDevsLock = threading.Lock()
-		self._btDevs = None
+		self._btDevs: typing.Optional[typing.Tuple[str, DeviceMatch]] = None
+		self._queuedFuture: typing.Optional[Future] = None
 		messageWindow.pre_handleWindowMessage.register(self.handleWindowMessage)
 		appModuleHandler.post_appSwitch.register(self.pollBluetoothDevices)
 		self._stopEvent = threading.Event()
-		self._queuedScanLock = threading.Lock()
-		self._scanQueued = False
 		self._detectUsb = usb
 		self._detectBluetooth = bluetooth
 		self._limitToDevices = limitToDevices
-		self._runningApcLock = threading.Lock()
 		# Perform initial scan.
-		self._startBgScan(usb=usb, bluetooth=bluetooth, limitToDevices=limitToDevices)
-
-	@property
-	def _scanQueuedSafe(self):
-		"""Returns L{_scanQueued} in a thread safe way by using L{_queuedScanLock}."""
-		with self._queuedScanLock:
-			return self._scanQueued
-
-	@_scanQueuedSafe.setter
-	def _scanQueuedSafe(self, state):
-		"""Sets L{_scanQueued} in a thread safe way by using L{_queuedScanLock}."""
-		with self._queuedScanLock:
-			self._scanQueued = state
-
-	def _startBgScan(self, usb=False, bluetooth=False, limitToDevices=None):
-		"""Starts a scan for devices.
+		self._queueBgScan(usb=usb, bluetooth=bluetooth, limitToDevices=limitToDevices)
+
+	def _queueBgScan(
+			self,
+			usb: bool = False,
+			bluetooth: bool = False,
+			limitToDevices: typing.Optional[typing.List[str]] = None
+	):
+		"""Queues a scan for devices.
 		If a scan is already in progress, a new scan will be queued after the current scan.
 		To explicitely cancel a scan in progress, use L{rescan}.
 		@param usb: Whether USB devices should be detected for this and subsequent scans.
-		@type usb: bool
 		@param bluetooth: Whether Bluetooth devices should be detected for this and subsequent scans.
-		@type bluetooth: bool
 		@param limitToDevices: Drivers to which detection should be limited for this and subsequent scans.
 			C{None} if no driver filtering should occur.
 		"""
-		with self._queuedScanLock:
-			self._detectUsb = usb
-			self._detectBluetooth = bluetooth
-			self._limitToDevices = limitToDevices
-			if not self._scanQueued:
-				self._scanQueued = True
-				if self._runningApcLock.locked():
-					# There's currently a scan in progress.
-					# Since the scan is embeded in a loop, it will automatically do another scan,
-					# unless a display has been found.
-					return
-				braille._BgThread.queueApc(self._BgScanApc)
+		self._detectUsb = usb
+		self._detectBluetooth = bluetooth
+		self._limitToDevices = limitToDevices
+		if self._queuedFuture:
+			# This will cancel a queued scan (i.e. not the currently running scan, if any)
+			# If this future belongs to a scan that is currently running or finished, this does nothing.
+			self._queuedFuture.cancel()
+		self._queuedFuture = self._executor.submit(self._bgScan, usb, bluetooth, limitToDevices)
 
 	def _stopBgScan(self):
 		"""Stops the current scan as soon as possible and prevents a queued scan to start."""
-		if not self._runningApcLock.locked():
-			# No scan to stop
-			return
 		self._stopEvent.set()
-		self._scanQueuedSafe = False
-
-	def _bgScan(self, param):
-		if self._runningApcLock.locked():
-			log.debugWarning("Braille display detection background scan APC executed while one is already running")
-			return
-		with self._runningApcLock:
-			while self._scanQueuedSafe:
-				# Clear the stop event before a scan is started.
-				# Since a scan can take some time to complete, another thread can set the stop event to cancel it.
-				self._stopEvent.clear()
-				with self._queuedScanLock:
-					self._scanQueued = False
-					detectUsb = self._detectUsb
-					detectBluetooth = self._detectBluetooth
-					limitToDevices = self._limitToDevices
-				if detectUsb:
-					if self._stopEvent.isSet():
-						continue
-					for driver, match in getDriversForConnectedUsbDevices():
-						if self._stopEvent.isSet() or (self._limitToDevices and driver not in self._limitToDevices):
-							continue
-						if braille.handler.setDisplayByName(driver, detected=match):
-							return
-				if detectBluetooth:
-					if self._stopEvent.isSet():
-						continue
-					with self._btDevsLock:
-						if self._btDevs is None:
-							btDevs = list(getDriversForPossibleBluetoothDevices())
-							# Cache Bluetooth devices for next time.
-							btDevsCache = []
-						else:
-							btDevs = self._btDevs
-							btDevsCache = btDevs
-					for driver, match in btDevs:
-						if self._stopEvent.isSet() or (self._limitToDevices and driver not in self._limitToDevices):
-							continue
-						if btDevsCache is not btDevs:
-							btDevsCache.append((driver, match))
-						if braille.handler.setDisplayByName(driver, detected=match):
-							return
-					if self._stopEvent.isSet():
-						continue
-					if btDevsCache is not btDevs:
-						with self._btDevsLock:
-							self._btDevs = btDevsCache
+		if self._queuedFuture:
+			# This will cancel a queued scan (i.e. not the currently running scan, if any)
+			# If this future belongs to a scan that is currently running or finished, this does nothing.
+			self._queuedFuture.cancel()
+
+	# C901 '_bgScan' is too complex
+	# Note: when working on _bgScan, look for opportunities to simplify
+	# and move logic out into smaller helper functions.
+	def _bgScan(  # noqa: C901
+			self,
+			detectUsb: bool,
+			detectBluetooth: bool,
+			limitToDevices: typing.Optional[typing.List[str]]
+	):
+		"""Performs the actual background scan.
+		this function should be run on a background thread.
+		@param usb: Whether USB devices should be detected for this particular scan.
+		@param bluetooth: Whether Bluetooth devices should be detected for this particular scan.
+		@param limitToDevices: Drivers to which detection should be limited for this scan.
+			C{None} if no driver filtering should occur.
+		"""
+		# Clear the stop event before a scan is started.
+		# Since a scan can take some time to complete, another thread can set the stop event to cancel it.
+		self._stopEvent.clear()
+		if detectUsb:
+			if self._stopEvent.isSet():
+				return
+			for driver, match in getDriversForConnectedUsbDevices():
+				if self._stopEvent.isSet():
+					return
+				if (limitToDevices and driver not in limitToDevices):
+					continue
+				if braille.handler.setDisplayByName(driver, detected=match):
+					return
+		if detectBluetooth:
+			if self._stopEvent.isSet():
+				return
+			with self._btDevsLock:
+				if self._btDevs is None:
+					btDevs = list(getDriversForPossibleBluetoothDevices())
+					# Cache Bluetooth devices for next time.
+					btDevsCache = []
+				else:
+					btDevs = self._btDevs
+					btDevsCache = btDevs
+			for driver, match in btDevs:
+				if self._stopEvent.isSet():
+					return
+				if (self._limitToDevices and driver not in limitToDevices):
+					continue
+				if btDevsCache is not btDevs:
+					btDevsCache.append((driver, match))
+				if braille.handler.setDisplayByName(driver, detected=match):
+					return
+			if self._stopEvent.isSet():
+				return
+			if btDevsCache is not btDevs:
+				with self._btDevsLock:
+					self._btDevs = btDevsCache
 
 	def rescan(self, usb=True, bluetooth=True, limitToDevices=None):
 		"""Stop a current scan when in progress, and start scanning from scratch.
@@ -367,7 +364,7 @@ def rescan(self, usb=True, bluetooth=True, limitToDevices=None):
 		with self._btDevsLock:
 			# A Bluetooth com port or HID device might have been added.
 			self._btDevs = None
-		self._startBgScan(usb=usb, bluetooth=bluetooth, limitToDevices=limitToDevices)
+		self._queueBgScan(usb=usb, bluetooth=bluetooth, limitToDevices=limitToDevices)
 
 	def handleWindowMessage(self, msg=None, wParam=None):
 		if msg == winUser.WM_DEVICECHANGE and wParam == DBT_DEVNODES_CHANGED:
@@ -382,12 +379,13 @@ def pollBluetoothDevices(self):
 		with self._btDevsLock:
 			if not self._btDevs:
 				return
-		self._startBgScan(bluetooth=self._detectBluetooth, limitToDevices=self._limitToDevices)
+		self._queueBgScan(bluetooth=self._detectBluetooth, limitToDevices=self._limitToDevices)
 
 	def terminate(self):
 		appModuleHandler.post_appSwitch.unregister(self.pollBluetoothDevices)
 		messageWindow.pre_handleWindowMessage.unregister(self.handleWindowMessage)
 		self._stopBgScan()
+		self._executor.shutdown(wait=False)
 
 
 def getConnectedUsbDevicesForDriver(driver) -> typing.Iterator[DeviceMatch]:

source/braille.py

@@ -1788,7 +1788,6 @@ def __init__(self):
 		brailleViewer.postBrailleViewerToolToggledAction.register(self._onBrailleViewerChangedState)
 
 	def terminate(self):
-		bgThreadStopTimeout = 2.5 if self._detectionEnabled else None
 		self._disableDetection()
 		if self._messageCallLater:
 			self._messageCallLater.Stop()
@@ -1800,7 +1799,7 @@ def terminate(self):
 		if self.display:
 			self.display.terminate()
 			self.display = None
-		_BgThread.stop(timeout=bgThreadStopTimeout)
+		_BgThread.stop()
 		louisHelper.terminate()
 
 	def getTether(self):
@@ -1890,9 +1889,8 @@ def setDisplayByName(  # noqa: C901
 					# Re-initialize with supported kwargs.
 					extensionPoints.callWithSupportedKwargs(newDisplay.__init__, **kwargs)
 			else:
-				if newDisplay.isThreadSafe and not detected:
+				if newDisplay.isThreadSafe:
 					# Start the thread if it wasn't already.
-					# Auto detection implies the thread is already started.
 					_BgThread.start()
 				try:
 					newDisplay = newDisplay(**kwargs)
@@ -1950,7 +1948,7 @@ def _updateDisplay(self):
 		blinkRate = config.conf["braille"]["cursorBlinkRate"]
 		if cursorShouldBlink and blinkRate:
 			self._cursorBlinkTimer = gui.NonReEntrantTimer(self._blink)
-			# This is called from the background thread when a display is auto detected.
+			# This is called from another thread when a display is auto detected.
 			# Make sure we start the blink timer from the main thread to avoid wx assertions
 			wx.CallAfter(self._cursorBlinkTimer.Start,blinkRate)
 
@@ -2265,7 +2263,6 @@ def _enableDetection(self, usb=True, bluetooth=True, keepCurrentDisplay=False, l
 		if self._detectionEnabled and self._detector:
 			self._detector.rescan(usb=usb, bluetooth=bluetooth, limitToDevices=limitToDevices)
 			return
-		_BgThread.start()
 		config.conf["braille"]["display"] = AUTO_DISPLAY_NAME
 		if not keepCurrentDisplay:
 			self.setDisplayByName("noBraille", isFallback=True)
@@ -2305,10 +2302,12 @@ def start(cls):
 
 	@classmethod
 	def queueApc(cls, func, param=0):
+		# Ensure the thread is running
+		cls.start()
 		ctypes.windll.kernel32.QueueUserAPC(func, cls.handle, param)
 
 	@classmethod
-	def stop(cls, timeout=None):
+	def stop(cls):
 		if not cls.thread:
 			return
 		cls.exit = True
@@ -2318,7 +2317,7 @@ def stop(cls, timeout=None):
 		cls.ackTimerHandle = None
 		# Wake up the thread. It will exit when it sees exit is True.
 		cls.queueApc(cls.executor)
-		cls.thread.join(timeout)
+		cls.thread.join()
 		cls.exit = False
 		winKernel.closeHandle(cls.handle)
 		cls.handle = None

source/setup.py

@@ -202,6 +202,9 @@ def getRecursiveDataFiles(dest,source,excludes=()):
 			"winxptheme",
 			# numpy is an optional dependency of comtypes but we don't require it.
 			"numpy",
+			# multiprocessing isn't going to work in a frozen environment
+			"multiprocessing",
+			"concurrent.futures.process",
 		],
 		"packages": [
 			"NVDAObjects",