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

addon/globalPlugins/rdAccess/handlers/remoteSpeechHandler.py

@@ -10,7 +10,9 @@
 import tones
 from hwIo.ioThread import IoThread
 from logHandler import log
-from speech.commands import IndexCommand, PitchCommand
+from speech.commands import PitchCommand
+from speech.extensions import speechCanceled
+from speech.priorities import Spri
 from speech.types import SpeechSequence
 
 from ._remoteHandler import RemoteHandler
@@ -29,16 +31,13 @@ class RemoteSpeechHandler(RemoteHandler[synthDriverHandler.SynthDriver]):
 	driverType = protocol.DriverType.SPEECH
 
 	def __init__(self, ioThread: IoThread, pipeName: str):
-		self._indexesSpeaking = []
 		super().__init__(ioThread, pipeName)
-		synthDriverHandler.synthIndexReached.register(self._onSynthIndexReached)
-		synthDriverHandler.synthDoneSpeaking.register(self._onSynthDoneSpeaking)
+		speechCanceled.register(self._notifyDoneSpeaking)
 		synthDriverHandler.synthChanged.register(self._handleDriverChanged)
 
 	def terminate(self):
 		synthDriverHandler.synthChanged.unregister(self._handleDriverChanged)
-		synthDriverHandler.synthDoneSpeaking.unregister(self._onSynthDoneSpeaking)
-		synthDriverHandler.synthIndexReached.unregister(self._onSynthIndexReached)
+		speechCanceled.unregister(self._notifyDoneSpeaking)
 		super().terminate()
 
 	def _get__driver(self):
@@ -66,24 +65,27 @@ def _speak(self, sequence: SpeechSequence):
 		pitchChange = configuration.getIncomingSpeechPitchChange(fromCache=True)
 		if pitchChange != 0 and PitchCommand in self._driver.supportedCommands:
 			sequence = [PitchCommand(offset=pitchChange), *sequence, PitchCommand()]
-		for item in sequence:
-			if isinstance(item, IndexCommand):
-				item.index += protocol.speech.SPEECH_INDEX_OFFSET
-				self._indexesSpeaking.append(item.index)
-		# Send speech to the current synth directly because we don't want unnecessary processing to happen.
-		# We need to change speech state accordingly.
+		sequence = protocol.speech.remapIndexesToCallbacks(sequence, self._sendIndex)
 		assert speech.speech._speechState is not None
 		speech.speech._speechState.isPaused = False
 		speech.speech._speechState.beenCanceled = False
-		self._driver.speak(sequence)
+		speech.speech._manager.speak(sequence, priority=Spri.NORMAL)
+
+	def _sendIndex(self, index: int):
+		try:
+			self.sendMessage(protocol.RdMessageType.INDEX, index=index)
+		except OSError:
+			log.warning("Error sending index", exc_info=True)
+
+	def _notifyDoneSpeaking(self):
+		self._sendIndex(0)
 
 	@protocol.commandHandler(protocol.RdMessageType.CANCEL)
 	def _command_cancel(self):
-		self._indexesSpeaking.clear()
 		self._queueFunctionOnMainThread(self._cancel, _immediate=True)
 
 	def _cancel(self):
-		self._driver.cancel()
+		speech.speech._manager.cancel()
 		assert speech.speech._speechState is not None
 		speech.speech._speechState.beenCanceled = True
 		speech.speech._speechState.isPaused = False
@@ -108,31 +110,8 @@ def _command_playWaveFile(self, **kwargs):
 		kwargs["asynchronous"] = True
 		nvwave.playWaveFile(**kwargs)
 
-	def _onSynthIndexReached(
-		self,
-		synth: synthDriverHandler.SynthDriver | None = None,
-		index: int | None = None,
-	):
-		assert synth == self._driver
-		if index in self._indexesSpeaking:
-			subtractedIndex = index - protocol.speech.SPEECH_INDEX_OFFSET
-			try:
-				self.sendMessage(protocol.RdMessageType.INDEX, index=subtractedIndex)
-			except OSError:
-				log.warning("Error calling _onSynthIndexReached", exc_info=True)
-			self._indexesSpeaking.remove(index)
-
-	def _onSynthDoneSpeaking(self, synth: synthDriverHandler.SynthDriver | None = None):
-		assert synth == self._driver
-		if len(self._indexesSpeaking) > 0:
-			self._indexesSpeaking.clear()
-			try:
-				self.sendMessage(protocol.RdMessageType.INDEX, index=0)
-			except OSError:
-				log.warning("Error calling _onSynthDoneSpeaking", exc_info=True)
-
 	def _handleDriverChanged(self, synth: synthDriverHandler.SynthDriver):
-		self._indexesSpeaking.clear()
+		self._notifyDoneSpeaking()
 		super()._handleDriverChanged(synth)
 		self._attributeSenderStore(
 			protocol.SpeechAttribute.SUPPORTED_COMMANDS,

addon/lib/protocol/speech.py

@@ -2,11 +2,17 @@
 # Copyright 2023 Leonard de Ruijter <alderuijter@gmail.com>
 # License: GNU General Public License version 2.0 or later
 
+from __future__ import annotations
+
+import typing
 from enum import IntEnum, StrEnum
 
-import speech.manager
+from speech.commands import BaseCallbackCommand, IndexCommand
+
+if typing.TYPE_CHECKING:
+	from collections.abc import Callable
 
-SPEECH_INDEX_OFFSET = speech.manager.SpeechManager.MAX_INDEX + 1
+	from speech.types import SpeechSequence
 
 
 class SpeechCommand(IntEnum):
@@ -21,3 +27,36 @@ class SpeechCommand(IntEnum):
 class SpeechAttribute(StrEnum):
 	SUPPORTED_COMMANDS = "supportedCommands"
 	LANGUAGE = "language"
+
+
+class RemoteIndexCallbackCommand(BaseCallbackCommand):
+	"""Calls ``onIndexReached`` with a remote index when speech reaches this command.
+
+	Index 0 doubles as the done speaking sentinel.
+	"""
+
+	def __init__(self, index: int, onIndexReached: Callable[[int], None]):
+		self.index = index
+		self._onIndexReached = onIndexReached
+
+	def run(self):
+		self._onIndexReached(self.index)
+
+	def __repr__(self):
+		return f"RemoteIndexCallbackCommand({self.index!r})"
+
+
+def remapIndexesToCallbacks(
+	sequence: SpeechSequence,
+	onIndexReached: Callable[[int], None],
+) -> SpeechSequence:
+	"""Return a new sequence with every IndexCommand replaced by a
+	:class:`RemoteIndexCallbackCommand` and a done speaking sentinel (index 0) appended.
+	"""
+	return [
+		*(
+			RemoteIndexCallbackCommand(item.index, onIndexReached) if isinstance(item, IndexCommand) else item
+			for item in sequence
+		),
+		RemoteIndexCallbackCommand(0, onIndexReached),
+	]

tests/_stubs.py

@@ -4,9 +4,10 @@
 
 """Stand-ins for NVDA runtime modules, installed into ``sys.modules``.
 
-Only leaf modules are stubbed. ``baseObject``, ``extensionPoints``, ``winKernel`` and the
-``hwIo`` submodules are imported for real from the sibling NVDA source checkout; their
-dependencies (``logHandler``, ``garbageHandler``, ``NVDAState``, ``config``) are covered here.
+Only leaf modules are stubbed. ``baseObject``, ``extensionPoints``, ``winKernel``, the
+``hwIo`` submodules and ``speech.commands`` are imported for real from the sibling NVDA source
+checkout; their dependencies (``logHandler``, ``garbageHandler``, ``NVDAState``, ``config``,
+``synthDriverHandler.getSynth``) are covered here.
 """
 
 from __future__ import annotations
@@ -17,13 +18,21 @@
 from pathlib import Path
 from typing import Any
 
+_NVDA_SOURCE = Path(__file__).resolve().parent.parent.parent / "nvda" / "source"
+
 
 class FakeLogger:
 	"""Collects log records so tests can optionally assert on them."""
 
+	DEBUG = 10
+	INFO = 20
+
 	def __init__(self):
 		self.records: list[tuple[str, str]] = []
 
+	def isEnabledFor(self, level: int) -> bool:
+		return False
+
 	def _log(self, level: str, msg: Any, *args: Any, **kwargs: Any):
 		self.records.append((level, str(msg)))
 
@@ -69,7 +78,10 @@ def getFormattedStacksForAllThreads() -> str:
 	logHandler.getFormattedStacksForAllThreads = getFormattedStacksForAllThreads
 
 	config = _module("config")
-	config.conf = {"debugLog": {"hwIo": False}}
+	config.conf = {
+		"debugLog": {"hwIo": False, "speechManager": False},
+		"featureFlag": {"cancelExpiredFocusSpeech": 0},
+	}
 
 	garbageHandler = _module("garbageHandler")
 
@@ -165,16 +177,8 @@ class BrailleInputGesture:
 	brailleInput.BrailleInputGesture = BrailleInputGesture
 
 	speech = _module("speech")
-	speechManager = _module("speech.manager")
-	speech.manager = speechManager
-
-	class SpeechManager:
-		MAX_INDEX = 9999
-
-	speechManager.SpeechManager = SpeechManager
-
-	_installSpeechCommandsStub(speech)
 	_installDriverSettingAndSynthVoiceStubs()
+	_installSpeechCommands(speech)
 	_installInputCoreStub()
 
 
@@ -186,9 +190,8 @@ def _installHwIo() -> None:
 	directory. Submodule imports then resolve against the real sources without that import
 	ever running.
 	"""
-	hwIoPath = Path(__file__).resolve().parent.parent.parent / "nvda" / "source" / "hwIo"
 	hwIo = _module("hwIo")
-	hwIo.__path__ = [str(hwIoPath)]
+	hwIo.__path__ = [str(_NVDA_SOURCE / "hwIo")]
 	hwIo.base = importlib.import_module("hwIo.base")
 	hwIo.ioThread = importlib.import_module("hwIo.ioThread")
 
@@ -205,66 +208,35 @@ def _setStubIdentity(cls: type, module: str) -> None:
 	cls.__qualname__ = cls.__name__
 
 
-def _installSpeechCommandsStub(speech: types.ModuleType) -> None:
-	speechCommands = _module("speech.commands")
-	speech.commands = speechCommands
-
-	class SpeechCommand:
-		pass
-
-	class SynthCommand(SpeechCommand):
-		pass
+def _installSpeechCommands(speech: types.ModuleType) -> None:
+	"""Make the real ``speech.commands`` (and ``speech.manager`` and its dependencies) importable
+	from the NVDA checkout.
 
-	class IndexCommand(SynthCommand):
-		def __init__(self, index: int):
-			self.index = index
-
-		def __eq__(self, other: Any) -> bool:
-			return type(self) is type(other) and self.index == other.index
-
-	class SynthParamCommand(SynthCommand):
-		pass
-
-	class BaseProsodyCommand(SynthParamCommand):
-		pass
+	``speech/__init__.py`` pulls in the full speech subsystem, so like ``hwIo`` the package is
+	registered with ``__path__`` pointing straight at the real sources. ``speech.commands``
+	itself only needs ``config`` and ``synthDriverHandler.getSynth``, both installed above.
+	``speech.languageHandling`` pulls in the full ``speech.speech`` module, so the one function
+	``speech.manager`` needs from it is stubbed instead.
+	"""
+	speech.__path__ = [str(_NVDA_SOURCE / "speech")]
+	speechCommands = importlib.import_module("speech.commands")
+	speech.commands = speechCommands
 
-	class PitchCommand(BaseProsodyCommand):
-		def __init__(self, offset: int = 0):
-			self.offset = offset
+	class NotASpeechCommand:
+		"""Injected into speech.commands without being a SpeechCommand subclass; used to test
+		that the dynamic find_class rule for speech.commands rejects it.
+		"""
 
-		def __eq__(self, other: Any) -> bool:
-			return type(self) is type(other) and self.offset == other.offset
+	_setStubIdentity(NotASpeechCommand, "speech.commands")
+	speechCommands.NotASpeechCommand = NotASpeechCommand
 
-	class BreakCommand(SynthCommand):
-		def __init__(self, time: int = 0):
-			self.time = time
+	speechLanguageHandling = _module("speech.languageHandling")
 
-		def __eq__(self, other: Any) -> bool:
-			return type(self) is type(other) and self.time == other.time
+	def shouldSwitchVoice() -> bool:
+		return True
 
-	class EndUtteranceCommand(SpeechCommand):
-		def __eq__(self, other: Any) -> bool:
-			return type(self) is type(other)
-
-	class NotASpeechCommand:
-		"""Exists in speech.commands but is not a SpeechCommand subclass; used to test that the
-		dynamic find_class rule for speech.commands rejects it.
-		"""
-
-	stubClasses = (
-		SpeechCommand,
-		SynthCommand,
-		IndexCommand,
-		SynthParamCommand,
-		BaseProsodyCommand,
-		PitchCommand,
-		BreakCommand,
-		EndUtteranceCommand,
-		NotASpeechCommand,
-	)
-	for cls in stubClasses:
-		_setStubIdentity(cls, "speech.commands")
-		setattr(speechCommands, cls.__name__, cls)
+	speechLanguageHandling.shouldSwitchVoice = shouldSwitchVoice
+	speech.languageHandling = speechLanguageHandling
 
 
 def _installDriverSettingAndSynthVoiceStubs() -> None:
@@ -316,7 +288,24 @@ class BooleanDriverSetting(DriverSetting):
 	autoSettingsDriverSetting.NumericDriverSetting = NumericDriverSetting
 	autoSettingsDriverSetting.BooleanDriverSetting = BooleanDriverSetting
 
+	from extensionPoints import Action
+
 	synthDriverHandler = _module("synthDriverHandler")
+	synthDriverHandler._currentSynth = None
+
+	def getSynth() -> Any:
+		return synthDriverHandler._currentSynth
+
+	synthDriverHandler.getSynth = getSynth
+	synthDriverHandler.synthIndexReached = Action()
+	synthDriverHandler.synthDoneSpeaking = Action()
+	synthDriverHandler.pre_synthSpeak = Action()
+
+	class SynthDriver:
+		pass
+
+	_setStubIdentity(SynthDriver, "synthDriverHandler")
+	synthDriverHandler.SynthDriver = SynthDriver
 
 	class VoiceInfo(StringParameterInfo):
 		def __init__(self, id: str, displayName: str, language: str | None = None):

tests/test_serializer.py

@@ -46,7 +46,9 @@ def test_roundTrip(self):
 			speech.commands.EndUtteranceCommand(),
 		]
 		obj = self.roundTrip(RdMessageType.SPEAK, sequence=sequence)
-		self.assertEqual(obj["sequence"], sequence)
+		# EndUtteranceCommand defines no __eq__, so compare it by type.
+		self.assertEqual(obj["sequence"][:-1], sequence[:-1])
+		self.assertIs(type(obj["sequence"][-1]), speech.commands.EndUtteranceCommand)
 
 	def test_unknownClassSkipped(self):
 		data = b'{"sequence": ["hi", ["NoSuchCommand", {}]], "type": "speak"}\n'

tests/test_serializerConformance.py

@@ -98,9 +98,14 @@ def test_serializeByteEquality(self):
 					),
 				)
 
+	def assertSequenceRoundTrips(self, decoded: list, sequence: list):
+		# EndUtteranceCommand defines no __eq__, so compare it by type.
+		self.assertEqual(decoded[:-1], sequence[:-1])
+		self.assertIs(type(decoded[-1]), speech.commands.EndUtteranceCommand)
+
 	def test_crossDeserializeSpeak(self):
 		sequence = self._sampleSequence()
 		fromTheirs = self.ours.deserialize(self.theirs.serialize(type="speak", sequence=sequence))
-		self.assertEqual(fromTheirs["sequence"], sequence)
+		self.assertSequenceRoundTrips(fromTheirs["sequence"], sequence)
 		fromOurs = self.theirs.deserialize(self.ours.serialize(type="speak", sequence=sequence))
-		self.assertEqual(fromOurs["sequence"], sequence)
+		self.assertSequenceRoundTrips(fromOurs["sequence"], sequence)