Skip to content

Route incoming remote speech through the local SpeechManager instead of offsetting indexes - #73

Merged
LeonarddeR merged 2 commits into
mainfrom
speechIndexCallbacks
Jul 28, 2026
Merged

Route incoming remote speech through the local SpeechManager instead of offsetting indexes#73
LeonarddeR merged 2 commits into
mainfrom
speechIndexCallbacks

Conversation

@LeonarddeR

Copy link
Copy Markdown
Owner

Summary

The client previously added SPEECH_INDEX_OFFSET (10000) to every remote IndexCommand, spoke directly to the local synth, and demuxed global synthIndexReached notifications by index range. That was fragile: indexes up to 19999 violate the MAX_INDEX (9999) synth contract, offsets stack in chained sessions, and two concurrent remote sessions collide on the same offset.

Mirroring NVDA core _remoteClient, incoming remote speech now goes through the client's SpeechManager:

  • lib/protocol/speech.py: new RemoteIndexCallbackCommand (a BaseCallbackCommand carrying the raw remote index) and remapIndexesToCallbacks(), which converts every IndexCommand and appends an index 0 done speaking sentinel. SPEECH_INDEX_OFFSET is removed.
  • RemoteSpeechHandler keeps no index state and no longer registers synthIndexReached/synthDoneSpeaking: _speak converts the sequence and calls speech._manager.speak(); the manager allocates all synth indexes and runs the callbacks, which send INDEX n back. Server CANCEL cancels at manager level (speech._manager.cancel()), and a speech.extensions.speechCanceled handler plus _handleDriverChanged send INDEX 0 when local cancellation or a synth change drops in-flight callbacks.
  • The manager does no text processing (symbol pronunciation, dictionaries, Unicode normalization and filter_speechSequence live only in the high-level speech.speak(), which is not used), so remote speech content is unaffected.

The wire format is unchanged in both directions (raw server indexes; INDEX 0 = done speaking), so old and new peers interoperate on both protocol stacks. No server-side changes.

Testing

  • Unit tests now import the real speech.commands from the sibling NVDA checkout instead of hand-written stubs; only speech.languageHandling is stubbed (it pulls in the full speech subsystem).
  • New tests/test_speechIndexCallbacks.py covers the conversion helper (written first, red-green).
  • New tests/test_speechManagerIntegration.py drives the real speech.manager.SpeechManager with a fake synth: remote indexes are reported in order followed by the sentinel, the synth only receives manager-allocated indexes, cancel drops pending callbacks, and speech recovers after cancel.
  • Full suite: 202 tests green; ty check and prek run --all-files pass.
  • Manual verification against a real RDP session is still pending (say-all continuation, cancel/pause in session, local cancel during remote speech, client synth switch).

🤖 Generated with Claude Code

…of offsetting indexes

Remote IndexCommands are converted to callback commands (RemoteIndexCallbackCommand) that
report the raw remote index when speech reaches them, with index 0 appended as a done
speaking sentinel. The SpeechManager owns all synth index allocation, so the reserved
offset range (which exceeded the MAX_INDEX synth contract, stacked in chained sessions
and collided between concurrent sessions) is gone, along with the handler's
synthIndexReached/synthDoneSpeaking registrations and index bookkeeping. Server CANCEL
now cancels at manager level, and a speechCanceled handler sends a done speaking message
when local cancellation drops pending callbacks. The wire format is unchanged.

Unit tests now import the real speech.commands from the sibling NVDA checkout, and new
integration tests drive the real SpeechManager with converted sequences.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 28, 2026 07:20

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 PR reworks the client-side remote speech playback to route incoming remote speech through NVDA’s local SpeechManager (mirroring NVDA core _remoteClient) instead of using a fixed index offset and manually demuxing synthIndexReached events. This addresses fragility around synth index limits (MAX_INDEX), chained-session offset stacking, and collisions between concurrent remote sessions.

Changes:

  • Replace remote IndexCommand items with callback commands and append an INDEX 0 done-speaking sentinel via remapIndexesToCallbacks().
  • Update RemoteSpeechHandler to speak through speech.speech._manager and report remote indexes via callbacks, including sending INDEX 0 on cancellation / synth change.
  • Expand tests and stubs to import real NVDA speech.commands / speech.manager from the sibling NVDA checkout, and adjust serializer tests for EndUtteranceCommand equality semantics.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
addon/lib/protocol/speech.py Adds RemoteIndexCallbackCommand and remapIndexesToCallbacks(); removes index offset approach.
addon/globalPlugins/rdAccess/handlers/remoteSpeechHandler.py Routes remote speech through NVDA SpeechManager, sends indexes via callbacks, hooks cancel/driver-change behavior.
tests/test_speechManagerIntegration.py New integration coverage for SpeechManager callback/index behavior using a fake synth.
tests/test_speechIndexCallbacks.py New unit tests for index-command → callback-command remapping and sentinel behavior.
tests/_stubs.py Updates stubs to import real NVDA speech.commands (and support speech.manager) from sibling NVDA source; adds needed config/synth plumbing.
tests/test_serializer.py / tests/test_serializerConformance.py Adjusts sequence comparisons to handle EndUtteranceCommand lacking __eq__.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…g method

speechCanceled registers it directly and _handleDriverChanged calls it; both triggers
remain needed because setSynth cancels the synth without going through
speech.cancelSpeech, so speechCanceled does not fire on a synth change.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@LeonarddeR
LeonarddeR merged commit c1f8c49 into main Jul 28, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants