Correctly reconnect to Focus Blue displays on resume#13330
Merged
Conversation
Fixes nvaccess#9830 After control of the session is returned to the user from the post-resume lock screen, I/O directed at some Focus Blue braille displays would raise an error about the operation being cancelled, causing the I/O thread to crash but not triggering a restart of the display driver, leading the display to not function correctly until NVDA was restarted. This commit adds a callback that allows the I/O exception thrown in hwio to be supressed and uses it to terminate and reinitialize the Braille display after resume. One flaw with the current strategy is that the termination can cause more I/O requests to be cancelled, leading to repeated renitilization.
seanbudd
approved these changes
Feb 11, 2022
See test results for failed build of commit aff0999b2b |
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
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:
Fixes #9830
Summary of the issue:
NVDA doesn't correctly regain control of some Focus Blue displays after the user has left the post-resume secure desktop. When in this state, Braille is only sent to the display when on a secure desktop. To regain normal functionality, the user must restart NVDA, possibly after unplugging the display and plugging it back in again.
From a technical perspective, the failure mode shows up as an error 995 (referring to cancelled I/O operations) in the background reader thread, which then leads to all the ack packets timing out.
Description of how this pull request fixes the issue:
This PR adds an
onReadErrorcallback tohwIo, which allowshwIousers to handle I/O errors that can't be caught with a try/catch (since the exception happens on a different thread).It then uses this callback to detect the interrupted I/O characteristic of the post-resume failure state. When the issue occurs, the driver is terminated and reinitialized.
Testing strategy:
I suspended and resumed my computer while running a development build that included this fix and observed that, unlike with the latest release, the Braille display reflected changes to what I'd typed in a terminal after resume, instead of consistently showing "Secure Desktop".
I also checked the logs to ensure that the driver was only initialized once and that there weren't other exceptions being thrown as a result of this change.
Known issues with pull request:
None
Change log entries:
New features
Changes
Bug fixes
For Developers
Code Review Checklist: