Add Bluetooth Low Energy support for DotPad braille displays #19122
Conversation
| log.debug("Starting BLE scanner for background scan") | ||
| hwIo.ble.scanner.start() | ||
| # Give the scanner some time to get initial results | ||
| time.sleep(0.2) |
There was a problem hiding this comment.
Can we somehow wait non-blocking here?
There was a problem hiding this comment.
I was assuming that a background scan was always running on the hwIo thread and that a blocking sleep wouldn't matter that much. In theory, Windows is already scanning and caching results, but depending on hardware and BLE stack implementation we might end up with 0 results without a wait. So I found it more acceptable to have a small delay in a background scan then having no results and only having a chance to find BLE devices in the next scanning round. Especially when having Bluetooth classic devices that can match in a background scan and having Windows take 10-20 seconds to get a timeout when trying to connect a Bluetooth serial port, I think a 0.2 sec delay is acceptable.
If my assumptions are invalid or you see a way to wait non-blocking and still get results in the same scanning round, I would like to hear your thoughts.
| @param bluetooth: Whether the handler is expected to yield USB devices. | ||
| @param bluetooth: Whether the handler is expected to yield Bluetooth devices. | ||
| @type bluetooth: bool | ||
| @param ble: Whether the handler is expected to yield BLE devices. |
There was a problem hiding this comment.
Have you considered reusing the bluetooth parameter? Can't you just handle BLE in case a driver is enabled that supports BLE?
I assume scanning for BLE can be a bit costly. It makes not that much sense to scan for these devices when I don't have a device that supports it.
There was a problem hiding this comment.
I considered it, but chose a separate ble parameter because:
- BLE can connect to devices without Windows pairing them first and there might be a wish to disable the feature for example in an environment with any BLE braille displays, where Bluetooth classic pairing would still be acceptable for automatic detection. Note that we do not expose that in the UI at the moment
- Windows' documentation is unclear about the cost of BLE scanning. It seems Windows is doing some kind of scanning and caching anyway, but this might also be dependent on the hardware and Bluetooth drivers/stack and is hard to test without clear documentation and not much Bluetooth adapters to test with
However, if there is no wish to disable BLE separately, I'd be fine with merging the ble/bluetooth parameters.
| ports[portKey] = portName | ||
| except Exception: | ||
| # If BLE scanning fails, continue without BLE devices | ||
| pass |
There was a problem hiding this comment.
Silently discarding a bare except feels like bad practice. Could you either make the exception more specific or log a debug warning or the like?
| if isinstance(port, bdDetect.DeviceMatch): | ||
| yield port | ||
| elif isinstance(port, str): | ||
| # Check if this is a specific BLE device port (format: "ble:DeviceName@Address" or legacy "ble:DeviceName") |
There was a problem hiding this comment.
Can you split this out into a helper function?
| try: | ||
| self._processPacket(packet[4:]) | ||
| except Exception: | ||
| log.error("Error processing packet", exc_info=True) |
There was a problem hiding this comment.
This will re-enter the loop, right? Can't this create an infinite loop we'll never break from?
There was a problem hiding this comment.
Before processing the packet, it's removed from the buffer. So in most cases we would end up with an empty buffer and the loop would exit. If there somehow is a few bytes left in the buffer it will either be a valid packet and processed as such, or the loop will exit because there are not enough bytes. Where/how do you see the risk of the loop not being exited?
| else: | ||
| # No ports available - show helpful message | ||
| # Translators: Message shown when no devices are available for a braille display | ||
| noDevicesMessage = _("(No devices found - switch to another display and back to refresh)") |
There was a problem hiding this comment.
Could there be a better UX for this? Can't you just refocus or something?
There was a problem hiding this comment.
Can you clarify this? Do you want the ports list to be enabled so people can read this message better or do you want that the ports list dynamically updates in realtime if devices are discovered?
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.
DotPad braille displays support both USB Serial and Bluetooth Low Energy (BLE) connections. NVDA previously only supported USB connections. - DotPad displays can now be automatically detected and connected via BLE - Users can manually select specific BLE devices from the braille settings dialog - BLE device scanning starts automatically when opening braille settings - Device list can be refreshed by switching to another display and back - When no BLE devices are found, a helpful message guides users to refresh - Add Bleak 1.1.0 dependency for BLE support - Implement asyncio event loop integration for async BLE operations - Create hwIo.ble module with Scanner and Ble classes - Extend bdDetect to support BLE device detection alongside USB/Bluetooth - Add BLE devices to getPossiblePorts() for manual selection in settings dialog - Update driverHasPossibleDevices() to include BLE devices - Implemented real-time device detection using extension points DotPad Driver: - Registered BLE devices via driverRegistrar.addBleDevices() with name-based matching - Serialized BLE devices as "name@address" for unique identification - Overrode check() method to show driver when BLE adapter available - Added BLE connection support in _tryConnect() method GUI Integration: - Start BLE scanner when braille settings dialog opens - Stop scanner on dialog close (unless background detection active) - Show "(No devices found - switch to another display and back to refresh)" when no ports available - Format BLE devices as "Bluetooth: DeviceName" in port dropdown
Co-authored-by: Copilot <[email protected]> Co-authored-by: Leonard de Ruijter <[email protected]>
…routine, wait for the result and either return that result or any raised exceptions
SaschaCowley
added
the
conceptApproved
|
@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. |
4 participants
DotPad braille displays support both USB Serial and Bluetooth Low Energy (BLE) connections. NVDA previously only supported USB connections.
DotPad Driver:
GUI Integration:
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:
Description of development approach:
Added Bleak for BLE communication. Since Bleak is an asyncio library, I had to implement an asyncio event loop as well. I chose to run this on it's separate thread and not integrate it in NVDA's WX main loop in any way.
bdDetect has been modified to scan for and match BLE devices with their drivers. Since with BLE in Windows the app is responsible for all the scanning and connecting, the scanner starts as soon as the braille display selection is opened. Otherwise, we never would have devices to connect to in the port selection list. If there are no ports, the ports list shows a message to help the user that they can switch to another display and back to refresh the list.
The DotPad driver has been modified to accept BLE devices to connect to. BLE devices are stored in the configuration as
ble:name@address, since we need the address to connect if there is no scanner result. If there is a scanner result, we can match on name as well.The onReceive method of the driver has been altered so it accepts both input one byte at a time (for serial) or as whole packets (BLE).
The BLE scanner implements a new Action (extension point) that fires when a BLE device is discovered. Therefore, connection after turning on the device should be very quickly established.
Testing strategy:
Wrote extensive unit tests and tested with actual hardware.
Things to test:
Known issues with pull request:
Since BLE does not require any pairing, the bdDetect system now connects to any DotPad it discovers. Since DotPad is disabled for automatic detection by default and users can set a specific BLE device to connect to, I think this is acceptable. If not, we would have to implement a GUI for selecting devices and keep a list of selected devices that are allowed to be discovered automatically.
Code Review Checklist: