Notify the user when connecting as the controlled computer fails#20544
Open
danielw97 wants to merge 2 commits into
Open
Notify the user when connecting as the controlled computer fails#20544danielw97 wants to merge 2 commits into
danielw97 wants to merge 2 commits into
Conversation
…ccess#19103) On the first failed attempt of a follower connection which has never succeeded, announce the failure via ui.delayedMessage and keep retrying in the background. Unlike the reverted nvaccess#19477, the reconnector thread is left untouched, so unattended autoconnect (nvaccess#20122) keeps working.
…e counter reset NVDA keeps retrying the connection in the background, so the failure is not an error NVDA has given up on; logging it as an error also plays the error sound on test versions of NVDA. Also adds a unit test covering that starting a new follower connection resets the announcement state, so a failure of the new connection is announced again.
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 #19103
Summary of the issue:
When an error occurs while connecting as the controlled computer, NVDA only logs the failure; the user receives no feedback. This is inconsistent with leader connections, where an error dialog is shown.
A previous fix (#19477) was reverted in #20131: its failure handler called
disconnectAsFollower(), which stopped the transport'sConnectorThread, so a single failed attempt at startup permanently gave up the connection. This broke users who rely on follower autoconnect as their primary means of accessing a headless or otherwise physically inaccessible machine (#20122).This PR follows the approach discussed and agreed with @SaschaCowley in #19103: notify without giving up.
Description of user facing changes:
On the first failed attempt of a follower connection that has never succeeded, NVDA now reports in speech and braille: "Unable to connect to the Remote Access server. Retrying".
Connection attempts continue in the background exactly as before (roughly every 5 seconds, indefinitely), so when the server becomes reachable the connection completes and the existing "Connected as controlled computer" cue confirms it. Subsequent failed retries are not announced, to avoid repeating the message every few seconds while the network is down.
Description of developer facing changes:
None.
Description of development approach:
transportConnectionFailedhandler for the follower transport inconnectAsFollower, mirroring the leader flow.RemoteClient.onConnectAsFollowerFailed, which announces the failure once viaui.delayedMessage, consistent with how_remoteClient/cues.pyreports other connection events. A transient spoken/braille message does not demand dismissal and does not imply NVDA has given up, so it works both for the interactive case and for unattended autoconnect.successfulConnects == 0(so reconnection attempts of a previously working session are not announced, matching the leader handler) plus a_followerConnectFailurescounter reset when a new follower connection is initiated. A counter was used rather than a boolean to leave a hook for possible future escalation (see below).Possible future work, suggested by @SaschaCowley in #19103: after several consecutive failures (e.g. 6 failures / 30 seconds), escalate to a non-modal dialog with an indeterminate progress bar and an option to cancel the connection. Left out of this PR to keep the change minimal; the failure counter provides a natural hook for it.
Testing strategy:
tests/unit/test_remote/test_remoteClient.py:Failed to connectmessage, no repeats).runlint(ruff + pyright) and the fulltest_remoteunit test suite pass.Known issues with pull request:
None known.
Code Review Checklist:
Note: I used AI assistance (Claude) while developing this PR. I have reviewed, tested, and understand all changes.