Update NVDA Controller client to support speaking SSML sequences

[LeonarddeR]

Link to issue number:

Fixes #11028
Fixes #5638

Summary of the issue:

The NVDA Controller client has been stable for a long time, but it lacked support for modern speech features, such as priority and callbacks.

Description of user facing changes

None.

Description of development approach

Added the following functions to the controller client:

• nvdaController_getProcessId: To get the process id (PID) of the current instance of NVDA the controller client is using.
• nvdaController_speakSsml: To instruct NVDA to speak according to the given SSML. This function also supports:
  • Providing the symbol level.
  • Providing the priority of speech to be spoken.
  • speaking both synchronously (blockking) and asynchronously (instant return).
• nvdaController_setOnSsmlMarkReachedCallback: To register a callback of type onSsmlMarkReachedFuncType that is called in synchronous mode for every <mark /> tag encountered in the SSML sequence provided to nvdaController_speakSsml.

Testing strategy:

Updated python example. Note that this doesn't work very well with Espeak, but that's more of espeaks fault. This is best tested with OneCore.

Known issues with pull request:

• NVDA Helper Remote exports functions to behave like the NVDA Controller client. It looks like these exports are used nowhere in the wild. I kept this for now, but am tempted to remove it from NVDAHelperRemote.

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.

[lukaszgo1]

It is very likely my lack of understanding of how RPC rather than a real problem, but looking at this I cannot figure out how two applications wanting to provide their own callback for onSsmlMarkReached can do so. Since it seems to be necessary to assign the callback to the global pointer, it appears that, to be on the safe side, application developer should assign the old value of onSsmlMarkReached to a temporary variable before passing any speech to NVDA, assign their own callback, speak whatever has to be spoken, and restore the old value afterwards. Would it be possible to pass pointer to onSsmlMarkReached as an additional parameter to nvdaController_speakSsml instead of using the single global callback?

[LeonarddeR]

it appears that, to be on the safe side, application developer should assign the old value of onSsmlMarkReached to a temporary variable before passing any speech to NVDA, assign their own callback, speak whatever has to be spoken, and restore the old value afterwards.

That's not necessary. The old value is nullptr anyway and the internal nvdaController_onSsmlMarkReached only calls the exported pointer when it is set correctly.

Would it be possible to pass pointer to onSsmlMarkReached as an additional parameter to nvdaController_speakSsml instead of using the single global callback?

That would be a lot nicer indeed, though I don't think we'd be able to pass the function pointer over RPC.

I'll have a look into this.

[lukaszgo1]

it appears that, to be on the safe side, application developer should assign the old value of onSsmlMarkReached to a temporary variable before passing any speech to NVDA, assign their own callback, speak whatever has to be spoken, and restore the old value afterwards.

That's not necessary. The old value is nullptr anyway and the internal nvdaController_onSsmlMarkReached only calls the exported pointer when it is set correctly.

Backing up and restoring the old function pointer seems to still be necessary, without doing so I see no way in which two applications can provide their own callback. Without backing up and restoring, the last assigned callback will always be used.

[LeonarddeR]

Every application can provide its own callback because the callback is stored in the user space of the client. The server, NVDA in this case, doesn't know anything about what the callback does. A callback is just RPC in the opposite direction.

[lukaszgo1]

Note that this doesn't work very well with Espeak, but that's more of espeaks fault. This is best tested with OneCore.

Is this caused by issues in eSpeak, or our driver for it. In other words should this be reported against NVDA (if so is there an existing issue), or eSpeak (same question)?

Version 1.0 supports all past and present versions of NVDA, including 2024.1.

Would it be possible to create a system test bundling version 1.0 of controller client, which ensures that the current NVDA can indeed speak through it?

Version 1.1 is linked in the readme.

I assume you mend version 1.0 here?

[LeonarddeR]

Note that this doesn't work very well with Espeak, but that's more of espeaks fault. This is best tested with OneCore.

Is this caused by issues in eSpeak, or our driver for it. In other words should this be reported against NVDA (if so is there an existing issue), or eSpeak (same question)?

I recall that @jcsteh bumped into issues with Espeak indexing during speech refactor, but I"m not sure. I'm pretty sure it is an issue in ESpeak though.

Version 1.0 supports all past and present versions of NVDA, including 2024.1.

Would it be possible to create a system test bundling version 1.0 of controller client, which ensures that the current NVDA can indeed speak through it?

I'm not very comfortable with writing system tests honestly. I guess that this can be a unit test though.

Version 1.1 is linked in the readme.

I assume you mend version 1.0 here?

Correct, sorry.

[lukaszgo1]

Would it be possible to create a system test bundling version 1.0 of controller client, which ensures that the current NVDA can indeed speak through it?

I'm not very comfortable with writing system tests honestly. I guess that this can be a unit test though.

Doing this as an unit test seems tricky as you need to have NVDAHelper and speech initialized. In system tests the entire screen reader is running, so it appears to be much easier.

[LeonarddeR]

I found out a nice way to avoid the API breakage. I will update the description, changelog and docs accoordingly later this week.

[AppVeyorBot]

• Build (for testing PR): https://ci.appveyor.com/api/buildjobs/uxy46i4ji7st0a9d/artifacts/output/
• CI timing (mins):
  INIT 0.0,
  INSTALL_START 1.0,
  INSTALL_END 0.9,
  BUILD_START 0.0,
  FINISH_END 22.0

See test results for failed build of commit 420e50f977

[AppVeyorBot]

• Build (for testing PR): https://ci.appveyor.com/api/buildjobs/ikb2r9w8lajbdjcc/artifacts/output/nvda_snapshot_pr15734-29879,fd7a608a.exe
• PASS: Unit tests.
• PASS: System tests (tags: installer NVDA).
• FAIL: Lint check. See test results for more information.
• CI timing (mins):
  INIT 0.0,
  INSTALL_START 0.9,
  INSTALL_END 0.9,
  BUILD_START 0.0,
  BUILD_END 27.0,
  TESTSETUP_START 0.0,
  TESTSETUP_END 0.3,
  TEST_START 0.0,
  TEST_END 16.8,
  FINISH_END 0.2
• PASS: Translation comments check.

See test results for failed build of commit fd7a608ac8

[LeonarddeR]

I'm sorry, I had pending comments I forgot to post yesterday.

[LeonarddeR]

Thanks for approving @seanbudd. Is this still blocked by something?

[michaelDCurran]

Am I write in understanding that if the function is called with asynchronous as True, the mark callback will not be ever fired?

[LeonarddeR]

This is correct. As per the docs about callbacks:

The callback function ... lets the server query the client for information in the context of the original call. Callbacks are special cases of remote calls that execute as part of a single thread. A callback is issued in the context of a remote call. Any remote procedure defined as part of the same interface as the static callback function can call the callback function.

So in other words, a callback can only be called during a call. Calling the callback as part of an async call that was already completed wouldn't work.