Route incoming remote speech through the local SpeechManager instead of offsetting indexes - #73
Merged
Merged
Conversation
…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>
Contributor
There was a problem hiding this comment.
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
IndexCommanditems with callback commands and append anINDEX 0done-speaking sentinel viaremapIndexesToCallbacks(). - Update
RemoteSpeechHandlerto speak throughspeech.speech._managerand report remote indexes via callbacks, including sendingINDEX 0on cancellation / synth change. - Expand tests and stubs to import real NVDA
speech.commands/speech.managerfrom the sibling NVDA checkout, and adjust serializer tests forEndUtteranceCommandequality 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>
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.
Summary
The client previously added
SPEECH_INDEX_OFFSET(10000) to every remoteIndexCommand, spoke directly to the local synth, and demuxed globalsynthIndexReachednotifications by index range. That was fragile: indexes up to 19999 violate theMAX_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'sSpeechManager:lib/protocol/speech.py: newRemoteIndexCallbackCommand(aBaseCallbackCommandcarrying the raw remote index) andremapIndexesToCallbacks(), which converts everyIndexCommandand appends an index 0 done speaking sentinel.SPEECH_INDEX_OFFSETis removed.RemoteSpeechHandlerkeeps no index state and no longer registerssynthIndexReached/synthDoneSpeaking:_speakconverts the sequence and callsspeech._manager.speak(); the manager allocates all synth indexes and runs the callbacks, which sendINDEX nback. ServerCANCELcancels at manager level (speech._manager.cancel()), and aspeech.extensions.speechCanceledhandler plus_handleDriverChangedsendINDEX 0when local cancellation or a synth change drops in-flight callbacks.filter_speechSequencelive only in the high-levelspeech.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
speech.commandsfrom the sibling NVDA checkout instead of hand-written stubs; onlyspeech.languageHandlingis stubbed (it pulls in the full speech subsystem).tests/test_speechIndexCallbacks.pycovers the conversion helper (written first, red-green).tests/test_speechManagerIntegration.pydrives the realspeech.manager.SpeechManagerwith 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.ty checkandprek run --all-filespass.🤖 Generated with Claude Code