Skip to content

Improve Remote Access's consistency regarding connections#18600

Merged
SaschaCowley merged 8 commits into
masterfrom
connectionConsistency
Aug 1, 2025
Merged

Improve Remote Access's consistency regarding connections#18600
SaschaCowley merged 8 commits into
masterfrom
connectionConsistency

Conversation

@SaschaCowley

@SaschaCowley SaschaCowley commented Aug 1, 2025

Copy link
Copy Markdown
Member

Link to issue number:

Fixes #18476
Fixes #18383
Fixes #18106

Summary of the issue:

NVDA Remote Access currently:

  • Does not allow the user to abort a session while connecting to the server via the GUI or the toggle gesture.
  • Allows a user to create a new session while already connecting to a server.
  • Can get into an inconsistent state if a connection is aborted while connecting to the server.

Description of user facing changes:

Fixed the above issues.

Description of developer facing changes:

The connecting attribute of RemoteClient is now marked internal. It has been replaced by the isConnecting property, which synthesizes its return from more sources.

Description of development approach:

  • Set _connecting in more places.
  • Update the GUI in more places.
  • Replace RemoteClient._connecting with RemoteClient.isConnecting, as this reports whether RA is connecting or reconnecting.

Testing strategy:

Set up Remote Access to automatically connect on NVDA start-up. Tried starting NVDA with my remote access server running and not running. In both states, tried disconnecting and creating new connections etc.

Known issues with pull request:

The dialog presented to users when cancelling a session is the same regardless of whether they are in the connecting or connected state. I am not sure if this is a major issue.

Code Review Checklist:

  • Documentation:
    • Change log entry
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • API is compatible with existing add-ons.
  • Security precautions taken.

@coderabbitai summary

Copilot AI review requested due to automatic review settings August 1, 2025 05:55
@SaschaCowley SaschaCowley requested a review from a team as a code owner August 1, 2025 05:55
@SaschaCowley SaschaCowley requested a review from seanbudd August 1, 2025 05:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request improves the consistency of NVDA Remote Access's connection handling by fixing several issues related to connection state management. The changes ensure proper state tracking during connection attempts and prevent inconsistent behavior when users try to abort or create multiple sessions.

  • Replace the public connecting attribute with an internal _connecting attribute and a new isConnecting property that provides more accurate connection state information
  • Update connection state tracking in more places throughout the connection lifecycle
  • Prevent users from creating new sessions while already connecting and allow proper disconnection during connection attempts

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
user_docs/en/changes.md Adds changelog entry documenting the bug fix for interrupted sessions
tests/unit/test_remote/test_remoteClient.py Adds stub method for test compatibility
source/globalCommands.py Updates gesture handlers to use new isConnecting property instead of connecting attribute
source/_remoteClient/client.py Main implementation changes including state management improvements and new isConnecting property

Comment thread source/_remoteClient/client.py
Comment thread source/_remoteClient/client.py
@SaschaCowley SaschaCowley merged commit f3882ef into master Aug 1, 2025
54 of 57 checks passed
@SaschaCowley SaschaCowley deleted the connectionConsistency branch August 1, 2025 07:46
@SaschaCowley SaschaCowley added this to the 2025.3 milestone Aug 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants