Small BRLTTY driver improvements#15335
Merged
Merged
Conversation
* Mark the BRLTTY braille display driver as thread safe * Improve availability check for BRLTTY braille display driver: only report as available if a named pipe for BrlAPI is present * Add more BRLTTY key bindings (nvaccess#6483): * learn: toggle NVDA input help * prefmenu: open the NVDA menu * prefload/prefsave: Load/save NVDA configuration * time: Show time * say_line: Speak the current line where the review cursor is located * say_below: Say all using review cursor
See test results for failed build of commit 8285e215aa |
seanbudd
approved these changes
Aug 28, 2023
Member
|
Can you please add the new gestures to the BRLTTY userGuide section? |
LeonarddeR
reviewed
Aug 28, 2023
LeonarddeR
approved these changes
Aug 28, 2023
* Update URLs to brltty.app from brltty.com or mielke.cc * Add new key bindings
Member
|
is this ready for another review? |
Contributor
Author
|
@seanbudd Yes, unless @LeonarddeR has good reasons to change how we return the list of pipes (see his review at line 47). |
LeonarddeR
approved these changes
Aug 29, 2023
seanbudd
approved these changes
Aug 30, 2023
Qchristensen
approved these changes
Aug 30, 2023
Qchristensen
left a comment
Member
There was a problem hiding this comment.
All looks good in the user guide, thanks!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Link to issue number:
Partly related to issue #6483.
Summary of the issue:
The BRLTTY driver was available even when there was no BRLTTY instance with BrlAPI enabled running on the local computer (the only type of connection supported for now). Opening the actual connection has quite a long timeout, so this cannot be done as an availibility check. However, BrlAPI uses named pipes in a known location, so we can use that to check for availability. In the future we might also expose this as a port selection if multiple pipes are discovered and/or we add an interface to connect to a BrlAPI server on another host over TCP.
While I was working on this driver, I also marked it threadSafe and added a few more keybindings.
Description of user facing changes
The BRLTTY driver will not show up in the list of braille displays if no local BRLTTY with BrlAPI enabled is running.
More BRLTTY commands have been mapped to NVDA actions.
Description of development approach
Testing strategy:
Tested the availability of the BRLTTY driver in the following situations:
The BRLTTY driver only showed up in the last situation, which is the expected result.
Tested the new key bindings on a connected braille display through BRLTTY.
Known issues with pull request:
Change log entries:
New features
Changes
Bug fixes
For Developers
Code Review Checklist: