Add Bluetooth Low Energy support for DotPad braille displays - #19122
Add Bluetooth Low Energy support for DotPad braille displays#19122bramd wants to merge 20 commits into
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds Bluetooth Low Energy (BLE) support for DotPad braille displays, enabling automatic detection and manual selection of BLE-connected devices alongside existing USB support.
Key changes:
- Introduced Bleak 1.1.0 dependency and asyncio event loop infrastructure for BLE communication
- Extended bdDetect system to handle BLE device scanning and matching in parallel with USB/Bluetooth
- Updated GUI to automatically start BLE scanning when braille settings dialog opens, with user guidance for device refresh
Reviewed Changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| user_docs/en/userGuide.md | Added BLE connection documentation and usage instructions for DotPad |
| user_docs/en/changes.md | Documented BLE support as a new feature in changelog |
| tests/unit/test_hwIo_ble.py | Added comprehensive unit tests for Scanner, Ble I/O, and device lookup |
| tests/unit/test_bdDetect.py | Extended tests to cover BLE device registration and matching |
| tests/unit/brailleDisplayDrivers/test_dotPad.py | Added buffered receive tests for serial and BLE packet handling |
| source/hwIo/ble/_scanner.py | Implemented BLE scanner wrapper around Bleak with extension point for device discovery |
| source/hwIo/ble/_io.py | Created Ble I/O class for BLE communication with MTU-aware writes and notification handling |
| source/hwIo/ble/init.py | Exposed scanner singleton and device lookup utility functions |
| source/gui/settingsDialogs.py | Modified braille settings dialog to start/stop BLE scanner and display helpful messages |
| source/core.py | Integrated asyncio event loop initialization and termination |
| source/brailleDisplayDrivers/dotPad/driver.py | Enhanced driver to support BLE connections with buffered packet processing |
| source/brailleDisplayDrivers/dotPad/defs.py | Added BLE service and characteristic UUIDs |
| source/braille.py | Extended device detection and port enumeration to include BLE devices |
| source/bdDetect.py | Added BLE scanning, real-time discovery notifications, and device matching |
| source/asyncioEventLoop.py | Created asyncio event loop module for running async operations |
| pyproject.toml | Added Bleak dependency and WinRT package exceptions |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
@SaschaCowley I see you fixed the Chrome system tests yesterday, but in this PR the startup/shutdown and installer tests are still failing. Is this expected or could it indicate a problem introduced in this PR? |
|
Hi @bramd, sometimes these tests do just fail. However the fact that the tests on Windows 2022 and 2025 failed in the same way is suspicious. I've triggered the tests to rerun to see if it was just a fluke |
Unfortunately, they still seem to fail. I'm trying to get a VM going with NVDA source to test properly without a Bluetooth adapter, but have some issues getting the build environment set up for now. Pending the failing tests, I think this is not ready for merge. However, I would appreciate it if you can at least test this on your hardware, since there are not much users with Dot Pad hardware to try this PR. |
I will try out the PR tomorrow and let you know how it goes. Just so you're aware, this PR may take longer for us to get to as it is quite large, so I want to set aside a while to sit down with it and properly digest it. We definitely have not forgotten about it though :) |
|
Hi @bramd, I have just tested this PR with our DotPad. I performed the current steps:
This caused the DotPad to emit two long buzzes, pause for a few seconds, then emit two more long buzzes, and NVDA to report an error connecting. |
|
it seems like this is causing serious system test errors around NVDA exiting safely |
No problem, I'm also still having weird issues (not related to this PR) building on my test machine without a Bluetooth adapter and would like to do a good test run on such a machine. That might also give some more insight on the failing system tests. For me this work is partly ported from what was in the Dot Pad add-on and partly rewritten to better fit NVDA core or because I saw room for improvement since the last iteration. Given the complexity of introducing asyncio and the Bleak library and touching quite some points in bdDetect I really appreciate a thorough code review on this. I've quite some time to work on this the coming weeks, so I should be able to address comments quickly. |
The first 2 buzzes are expected. The Dot Pad double buzzes every time a BLE connection is made or a connection is closed. So the second buzzes you see match the closing of the connection after the driver fails to initialize. Could you please try the following:
If you can't get it working, I'd be glad to help you out here or by email if we want to keep the PR comments relevant. |
There was a problem causing the asyncio loop to hang for 5 secs on shutdown. This has been fixed and now the tests pass as well. |
|
is this ready for re-review? |
Almost, I finally have a working VM with no Bluetooth adapter to do some more tests (took me some time due to #19189). It seems the scanner fails silently without an adapter (e.g. just does not return any results). So I will remove some checks for Bluetooth availability that will never trigger. The BLE APIs have been added in Windows 10 creator's update, which is no longer actively supported by NVDA. So we may consider the BLE stack to be always available, even without any Bluetooth hardware in the machine. |
243346f to
8085691
Compare
|
@seanbudd Just in case you missed the status change from draft to normal PR, this is ready for re-review. @SaschaCowley Have you been able to get this to work with your Dot Pad hardware? If not, I'd be glad to help you debug. |
|
@bramd - do you intend to continue to work on this PR? |
|
@seanbudd Sorry it took me a while. Yes, I intend to get back to this soon |
Integrate BLE braille display detection and connection on top of the hwIo.ble transport that now lives in master. This change contains only the integration layer; the Bleak/hwIo.ble stack is no longer part of this branch. - bdDetect: scan for and match BLE devices, register BLE match functions via DriverRegistrar.addBleDevices, and expose getBleDevicesForDriver. Guard all hwIo.ble.scanner access for the case where BLE is not initialized (e.g. unit tests), so device detection degrades gracefully instead of crashing. - braille: thread a `ble` scan flag through detection enable/rescan, match detected displays by the "ble" provider, and surface individual BLE devices as selectable "ble:Name@Address" ports. - DotPad driver: add check(), _isBleDotPad matcher, BLE automatic-detection registration, and a BLE branch in _tryConnect that opens an hwIo.ble.Ble device. Builds on the buffered-receive packet handling. - Braille settings dialog: start/stop the shared BLE scanner while the display selection dialog is open to populate the device list. - Enable the DotPad BLE unit tests (matcher, check, registration, _tryConnect) that were previously skeleton-skipped pending this integration. Stacked on the buffered-receive branch (nvaccess#19942); retarget to master once that merges.
- getPossiblePorts: move the BLE device enumeration into a _getBlePorts helper, and log the failure instead of silently swallowing it. The guard is kept because Scanner.results() reads a dict that the asyncio thread mutates, so a transient error must not cost the user the remaining ports. - _getTryPorts: move the "ble:DeviceName@Address" parsing into a _getBleTryPorts helper, keeping _getTryPorts readable. - Introduce a _BLE_PORT_PREFIX constant shared by both helpers, so the port format is written down in one place.
Cover _getBlePorts (port formatting, no devices, contained enumeration failure) and _getBleTryPorts (device found in the scan results, matched by address after a rename, fallback to the configured address, a name containing the separator, and a malformed port), plus that _getTryPorts routes a BLE port to the helper.
- bdDetect: _onBleDeviceDiscovered passed only ble=True to _queueBgScan, which stores its arguments as the state for subsequent scans. A single BLE advertisement therefore permanently disabled USB and Bluetooth detection, so a Bluetooth display switched on later was never found. Pass the current detection state through instead. - bdDetect: only start the BLE scanner when a driver in scope actually registered BLE devices. DotPad is the only such driver and is excluded from automatic detection by default, so every user with automatic detection on was running a continuous radio scan for nothing. The existing check moved into a shared _hasBleDrivers helper. - bdDetect: use devs.get() rather than subscripting the defaultdict, which inserted an empty entry for every driver without BLE support, and drop the resulting per-driver debug warning. - bdDetect: extract _bleDeviceToMatch, which was duplicated between _getBleDeviceMatch and getDriversForBleDevices. - display driver: match a configured BLE port on its address before its name. The address is in the port precisely to tell apart devices sharing a name, but the name could win depending on scan order. - display driver: let _getAutoPorts yield BLE devices, and ask for them for the automatic port. That port is offered as soon as BLE devices are known, but connecting through it always failed. - braille settings: leave the port list empty for the automatic display rather than showing the "no devices found" hint, which is meaningless when displays are detected automatically. - DotPad: name the port in the "no device found" error, which was reported even for an explicitly configured port. Adds unit tests for the address-over-name precedence and for the automatic port yielding BLE devices.
… open The BLE scanner was started in postInit, after makeSettings had already built the port list, so the list was always built against an empty scan and never showed a BLE device on first open. The dialog worked around this by telling the user to switch to another display and back. Start the scanner before the list is built, and check once a second while the dialog is open for devices discovered since. Newly found devices are appended rather than the list being rebuilt, so neither the existing entries nor the selection move under the user; a port that disappears meanwhile therefore stays listed, consistent with the list being a snapshot taken when the dialog opened. Only the selected driver's BLE ports are consulted per tick, as enumerating COM and Bluetooth ports is too slow to repeat at that rate. New devices are reported with ui.message, so the list does not change silently, and each device is reported only once. The port list hint becomes just "(No devices found)", the refresh it described being gone.
- Drop comments that restate the line below them, and keep the ones that record why: the cost of holding the Bluetooth radio, why the scan blocks briefly, why _queueBgScan must be passed the current detection state, why the driver dict is read with get(), and why an address beats a name when resolving a configured BLE port. - Convert the docstrings touched here to Sphinx, including the epytext left in scanForDevices, addDeviceScanner and addBleDevices. - Document the DotPad BLE UUIDs and the port refresh timer with docstrings rather than comments above them. - Bring bdDetect.py, test_bdDetect.py and test_brailleDisplayDrivers.py on to the current copyright header, and credit the contributors to the files this branch touches. - Import hwIo.ble at the top of settingsDialogs.py. It is free: bdDetect already imports it at module scope, so it is loaded before settingsDialogs finishes importing either way. Likewise hoist the dotPad import in test_bdDetect.py. - Rename the snake_case locals introduced in the new bdDetect tests.
Picking a BLE device in the braille display selection dialog always failed with "No DotPad device found for port ...". _tryConnect looked the device up with hwIo.ble.findDeviceByAddress, which is decorated with requiresBackgroundThread because it may start a scan and poll for up to five seconds. Connecting a manually selected display runs on the main thread, from onOk by way of setDisplayByName, so that lookup raised on every attempt. _tryConnect caught it, logged it at debug warning level where the default log level hides it, and returned False, leaving only the generic "no device found" error. Automatic detection was unaffected, as it connects from a background thread. The blocking behaviour is not wanted here in any case: _getBleTryPorts has already consulted the scan results by this point, so all _tryConnect needs is the discovered BLEDevice for the address, if there is one, to avoid implicit discovery. Add hwIo.ble.getDiscoveredDevice for that, which only reads what the scanner has already seen and therefore never blocks, and build findDeviceByAddress on top of it rather than repeating the lookup. The existing _tryConnect test patched findDeviceByAddress, so the mock hid the very call that failed. It now stubs the scanner instead and exercises the real lookup on the main thread, alongside a new test for the fallback to the configured address and tests for getDiscoveredDevice itself.
_stopBleScanner read braille.AUTO_DISPLAY_NAME, a deprecated alias, which logged a warning and a stack trace every time the dialog was confirmed or cancelled. The rest of the dialog already uses braille.constants.
Ble.__init__ waited on the connection coroutine with no timeout, so runCoroutineSync blocked until the Bluetooth stack gave up. Connecting a display that is switched off held the calling thread for tens of seconds, and that thread is NVDA's main thread when the display comes from the configuration at startup or from the braille settings dialog. Give the attempt LINK_TIMEOUT_SECONDS, matching Bleak own discovery timeout so a connection that would have succeeded is not cut short. The existing CONNECT_TIMEOUT_SECONDS keeps covering service discovery after the link is up, and now says so. A constructor that fails this way is never closed, as nothing owns the instance yet, so stop the reader thread it already started.
_bgScan slept for 200ms after starting the scanner so that the scan that followed had some results to match against. It does not need to: the detector subscribes to the scanner deviceDiscovered action, and a device that advertises after the scan has moved on queues a scan of its own with that device as the preferred one. The wait only ever covered the gap between starting the scanner and the first advertisement, which is exactly what the action reports. Devices already known to a scanner that was running beforehand are still matched inline, so the two paths cover each other. Adds tests for the discovery handler, as it is now the only route by which a device found during a background scan reaches the detector.
Scanner.start and stop handed their coroutine to runCoroutine and moved on without reading the result, so a refusal from Bleak ended up in a future nobody looked at. Bleak refuses on Windows when the machine has no Bluetooth adapter, when the adapter has no BLE central role, or when the radio is switched off. The scanner then recorded itself as scanning, which suppressed every later attempt and left callers waiting for results that could never arrive, while the caller try/except saw nothing. Wait for both, bounded by SCAN_CONTROL_TIMEOUT_SECONDS, and only record the scan as running once it has started. Waiting on stop also means the watcher has really stopped before a new scan is started, which Bleak would otherwise refuse. Callers now have to cope with the failure. findDeviceByAddress and the braille settings dialog already did. The background scan did not, and must: losing BLE there is no reason to abandon the USB and Bluetooth parts of the same scan. Stopping the scanner is guarded in the same way, so detection and shutdown do not depend on the Bluetooth stack behaving. Also corrects the DotPad check() docstring, which claimed the scanner runs without a Bluetooth adapter and simply returns nothing.
The new "Connecting to Dot Pad" section spelled out how port selection and automatic detection work, which is the same for every display and already documented once. Fold what is specific to this driver back into the existing paragraph: detection now covers Bluetooth as well as USB, a Bluetooth device can be picked as the port, and the port list fills in as devices are discovered. Drops the note about needing Windows 10 1703 for Bluetooth Low Energy, as NVDA already requires a later Windows than that.
- The DotPad driver imported both hwIo and hwIo.ble. Either alone binds hwIo, as its package imports Serial and ble, so keep only the one that names what the driver needs. - Move BLE above CUSTOM in ProtocolType, so the manufacturer specific entry stays last.
LeonarddeR
left a comment
There was a problem hiding this comment.
Note, I only reviewed the actual code, not the tests.
- Restore the parameter types on the scanForDevices documentation, which were lost when its epytext was converted and cannot be derived from a module level string. - Build the BLE DeviceMatch with keyword arguments. - Say concretely why the detector guards against a missing scanner. - Name the scanner rather than the radio in the comments about stopping it. - Record that BLE discoveries arrive on the asyncio event loop thread, and why the work is handed to the executor from there. - Document the port refresh interval with a docstring rather than a comment. - Read the discovered device with next() rather than a loop.
The message told the user to switch to another display and back to refresh the list. That refresh now happens on its own while the dialog is open, so the message lost its purpose, and shortening it only left a string saying what an empty list already conveys. Removing it takes the port list back to how every other display behaves, and takes a translatable string with it. The special case for the automatic display goes too, as it only existed to keep that message away from a display that has no ports to choose. What stays is clearing the list when there are no ports: this driver is offered even when nothing is connected, which makes it easy to arrive here with the previous display ports still on screen.
The braille display selection dialog disables the port control when the automatic port is the only entry. A BLE device discovered while the dialog is open therefore has to bring a second entry with it, or it would be listed but not selectable. That invariant lives in getPossiblePorts and had no test.
getDriversForBleDevices walked the whole driver registry for every device in the scan results, redoing the same filtering and lookup each time. Resolve the match functions of the drivers in scope once up front, so the loop over devices only calls them. Devices stay the outer loop, matching the USB and Bluetooth equivalents above it: the order decides which display is connected when several are in range, and it should be the one discovered first rather than whichever driver happens to be registered first. _hasBleDrivers now asks the same helper, rather than repeating the filter it used to duplicate. Adds tests for the ordering, the driver filter, and for leaving the scan results alone when no driver can match them.
check() returned True unconditionally, because a BLE device is only known once a scan has run and the braille display selection dialog is what starts one. That also offered the driver on machines that could never reach a device, which matters on hardware with no serial ports, where nothing else would have listed it. Bleak reports on the Bluetooth hardware, so ask it: hwIo.ble.isAvailable answers whether Windows has an adapter that can act as a central with its radio on. It is asked first, as it settles the question on its own where there is Bluetooth, and is cheaper than enumerating ports. The answer is deliberately optimistic. A machine may have Bluetooth and simply nothing switched on yet, and a question that cannot be answered reports available rather than hiding a driver that probably works. Gives the asyncio state its defaults, so the guard in runCoroutine raises the RuntimeError it intends rather than an AttributeError, and adds isRunning() for callers that would rather not ask than handle that.
Both sat on paths that only run once a driver in scope has registered BLE devices, by which point hwIo has been initialized: it comes up before bdDetect and goes down after it. Removing either leaves the whole unit suite passing, so neither was reachable from the tests they claimed to be for. The remaining guards, around registering and unregistering the discovery handler and around stopping the scanner, do carry the suite: detection is exercised there without hwIo being initialized.
It sat in the package __init__, which meant runCoroutine had to import it from there inside the function body: the package imports utils, so utils importing the package back would not resolve. Put it in utils with runCoroutine and runCoroutineSync, which it belongs with, and re-export it so callers keep asking _asyncioEventLoop.
|
@SaschaCowley Do you still have Dot Pad hardware? If so, I'd really appreciate if you could test this. |
|
@bramd yes, I still have the original Dot Pad, and I believe @michaelDCurran has the Dot Pad X. I will try to test this in the coming days. |
DotPad braille displays support both USB serial and Bluetooth Low Energy (BLE) connections. NVDA previously only supported USB.
This is the last part of a series. The BLE stack itself is already in master:
_asyncioEventLoopmodule.hwIo.ble(theScannersingleton and theBletransport) with unit tests.This PR is the integration layer that connects those to the braille display system, and unskips those tests.
Link to issue number:
Fixes #18584
Summary of the issue:
DotPad braille displays support both USB serial and Bluetooth Low Energy (BLE) connections. NVDA previously only supported USB connections.
Description of user facing changes:
Description of developer facing changes:
bdDetectgained ablescan flag, threaded throughrescan,_queueBgScanand_bgScan, plusDriverRegistrar.addBleDevicesfor drivers to register a BLE match function, andgetBleDevicesForDriver/getDriversForBleDevicesto query matches. The scanner is only started when a driver in scope has registered BLE devices, so the Bluetooth radio is left alone otherwise._Detectorsubscribes to the scanner'sdeviceDiscoveredextension point and queues a connection attempt as soon as a matching device advertises, rather than waiting for the next poll.BrailleDisplayDriveroffers each discovered BLE device as a port of the formble:DeviceName@Address, and resolves such a port back to a device, preferring the address over the name so that devices sharing a name can be told apart.hwIo.ble.getDiscoveredDevicewas added: a non blocking lookup of what the scanner has already seen, safe to call on the main thread.findDeviceByAddress, which may scan and therefore may not, is now built on it.LINK_TIMEOUT_SECONDS, so a display that is switched off cannot hold the calling thread.Description of development approach:
bdDetecttreats BLE as a communication type of its own rather than folding it into Bluetooth, so it can be scanned for and disabled independently. With BLE on Windows the application drives discovery, so nothing is known about a device until a scan has run. The braille display selection dialog therefore starts the shared scanner, and refreshes its port list while it is open so that devices found after it opened still appear.The DotPad driver matches on a device name starting with
DotPadand connects withhwIo.ble.Ble. A port is stored asble:Name@Address: the address is what a connection needs when no scan result is available, while the name keeps the entry recognisable, and matching prefers the address so that a rotating resolvable private address does not confuse two devices with the same name.Testing strategy:
Unit tests cover the bdDetect BLE registration and matching, the port formatting and resolution on the display driver, the DotPad BLE connection path, and the
hwIo.blelookup and connection timeout.Tested against DotPad 320A hardware:
Known issues with pull request:
BLE requires no pairing, so automatic detection will connect to any DotPad it discovers. DotPad is excluded from automatic detection by default and a specific device can be configured, so this seems acceptable. Restricting it further would mean keeping a list of devices allowed to be detected, and a way to manage it.
Two things found while testing are deliberately left out of scope:
BrailleDisplayDriver.getManualPortsoffers every serial port on the machine, so displays from other vendors appear as DotPad ports. Filtering by the DotPad USB ID interacts withcheck(), which currently reports DotPad as always available so that the display can be selected before any scan has run. The two need deciding together.Code Review Checklist: