Add possibility to cycle through output devices forward and backward

[beqabeqa473]

Link to issue number:

Fixes #8801

Summary of the issue:

After introducing an additional audio panel and moving output device selection out of synthesizer selection dialog, it became harder to recover from situation where audio device is loosing and speech doesn't output anymore. It is now possible to quickly switch to the next or previous output devices with shortcuts

Description of user facing changes

Added two unassigned shortcuts to switch to the next and previous output device.

Description of development approach

Added two scripts and a helper method to globalCommands.py moved out setting output device from gui.settingDialogs to a dedicated utils.synth module to eliminate code duplication

Testing strategy:

Tried to assign shortcuts to scripts listed above and confirmed that they switch output device forward and backward.

Known issues with pull request:

None found

Code Review Checklist:

• [x] 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
• [x] UX of all users considered:
  • Speech
  • Braille
  • Low Vision
  • Different web browsers
  • Localization in other languages / culture than English
• [x] API is compatible with existing add-ons.
• [ ] Security precautions taken.

[AppVeyorBot]

• PASS: Translation comments check.
• PASS: Unit tests.
• FAIL: Lint check. See test results for more information.
• PASS: System tests (tags: installer NVDA).
• Build (for testing PR): https://ci.appveyor.com/api/buildjobs/6dwi5jpsem8gvhy2/artifacts/output/nvda_snapshot_pr15493-29299,af326aeb.exe
• CI timing (mins): INIT 0.0, INSTALL_START 0.9, INSTALL_END 0.8, BUILD_START 0.0, BUILD_END 24.4, TESTSETUP_START 0.0, TESTSETUP_END 0.3, TEST_START 0.0, TEST_END 22.2, FINISH_END 0.1

See test results for failed build of commit af326aebcd

[LeonarddeR]

Is there a particular reason why we need two scripts for this? Shoudln't this just be a single cycler?

[beqabeqa473]

Yes. if there are lots of audio devices, it will be harder to find needed devices. I am sure lots of musicians are using loopback devices, virtual audio cables and for me for example, it is 15 audio devices available on my pc right now.

On 9/21/23, Leonard de Ruijter @.***> wrote:

Is there a particular reason why we need two scripts for this? Shoudln't this just be a single cycler?

-- Reply to this email directly or view it on GitHub: https://github.com/nvaccess/nvda/pull/15493#issuecomment-1729593724 You are receiving this because you authored the thread.

Message ID: @.***>

-- with best regards Beqa Gozalishvili Tell: +995593454005 Email: @.*** Web: https://gozaltech.org Skype: beqabeqa473 Telegram: https://t.me/gozaltech facebook: https://facebook.com/gozaltech twitter: https://twitter.com/beqabeqa473 Instagram: https://instagram.com/beqa.gozalishvili

[XLTechie]

I had the same question as @leonardder. I didn't ask it, because I realized that as long as both of the scripts wrap around, the default assignment (if any) only has to be to one of them.

Users with only two or three devices will only ever need a single script assigned. But users with absurd numbers of interfaces, can assign the second one as well, for increased flexibility.

[AppVeyorBot]

• PASS: Translation comments check.
• FAIL: Unit tests. See test results for more information.
• PASS: Lint check.
• FAIL: System tests (tags: installer NVDA). See test results for more information.
• Build (for testing PR): https://ci.appveyor.com/api/buildjobs/j5urs01ir4gmxcql/artifacts/output/nvda_snapshot_pr15493-29302,621b8a74.exe
• CI timing (mins): INIT 0.0, INSTALL_START 0.9, INSTALL_END 0.9, BUILD_START 0.0, BUILD_END 26.5, TESTSETUP_START 0.0, TESTSETUP_END 0.2, TEST_START 0.0, TEST_END 0.8, FINISH_END 0.1

See test results for failed build of commit 621b8a74e5

[seanbudd]

Is this ready for review?

[beqabeqa473]

Not yet.

On 9/22/23, Sean Budd @.***> wrote:

Is this ready for review?

-- Reply to this email directly or view it on GitHub: https://github.com/nvaccess/nvda/pull/15493#issuecomment-1730797080 You are receiving this because you authored the thread.

Message ID: @.***>

-- with best regards Beqa Gozalishvili Tell: +995593454005 Email: @.*** Web: https://gozaltech.org Skype: beqabeqa473 Telegram: https://t.me/gozaltech facebook: https://facebook.com/gozaltech twitter: https://twitter.com/beqabeqa473 Instagram: https://instagram.com/beqa.gozalishvili

[beqabeqa473]

Hello.

In fact, it doesn't contain logic for audio. Switching to another audio device involves reinitializing tts engine and tones module. I wanted to eliminate code duplication and decoupled this code. If you have a better alternative i am glad to implement it.

On 9/22/23, Leonard de Ruijter @.***> wrote:

@leonardder commented on this pull request.

@@ -66,6 +66,7 @@ from base64 import b16encode import vision from utils.security import objectBelowLockScreenAndWindowsIsLocked +from utils.synth import getOutputDeviceNames, setOutputDevice

I'm slightly puzzled why this module is called synth while in fact, it contains logic for audio. I'm also not sure why this can't just reside in the nvwave module.

-- Reply to this email directly or view it on GitHub: https://github.com/nvaccess/nvda/pull/15493#pullrequestreview-1639307586 You are receiving this because you authored the thread.

Message ID: @.***>

-- with best regards Beqa Gozalishvili Tell: +995593454005 Email: @.*** Web: https://gozaltech.org Skype: beqabeqa473 Telegram: https://t.me/gozaltech facebook: https://facebook.com/gozaltech twitter: https://twitter.com/beqabeqa473 Instagram: https://instagram.com/beqa.gozalishvili

[beqabeqa473]

Hello.

thanks for suggestions, I will be returning to this PR at these weekends to finish it.

On 11/10/23, Łukasz Golonka @.***> wrote:

@lukaszgo1 requested changes on this pull request.

@@ -4289,6 +4290,37 @@ def script_cycleParagraphStyle(self, gesture: "inputCore.InputGesture") -> None: config.conf["documentNavigation"]["paragraphStyle"] = newFlag.name ui.message(newFlag.displayString)

• def cycleOutputAudioDevices(self, direction: int):

Can this be made private (underscore prefixed), and converted to a static method, as far as I can tel it does not need access to the instance. Also the type annotation should reflect the fact that direction cannot be an arbitrary int, but only 1, or -1, I'd suggest using typing.Literal here.

•    	index = deviceNames.index(config.conf["speech"]["outputDevice"])
  

• 	newIndex = (index + direction) % len(deviceNames)
  

• except ValueError:
  

• 	newIndex = 0
  

• device = deviceNames[newIndex]
  

• setOutputDevice(device)
  

• ui.message(device)
  

• @script(

• description=_(
  

• 	# Translators: Describes the switch to next output device command.
  

• 	"switches to the next output device"
  

• ),
  

• category=SCRCAT_SPEECH,
  

• )
• def script_switchToNextOutputDevice(self, gesture: "inputCore.InputGesture") -> None:

No point in stringizing type hint here, same for the script below.

@@ -0,0 +1,31 @@ +# A part of NonVisual Desktop Access (NVDA) +# Copyright (C) 2023 Beka Gozalishvili +# This file may be used under the terms of the GNU General Public License, version 2 or later. +# For more details see: https://www.gnu.org/licenses/gpl-2.0.html

+import config +from gui.settingsDialogs import _synthWarningDialog +import nvwave +from synthDriverHandler import getSynth, setSynth +import tones + + +def getOutputDeviceNames():

I'd suggest this function should be moved to nvwave, with an appropriate name so that it is clear it returns friendly device names.

@@ -8,6 +8,7 @@ What's New in NVDA

== New Features ==

• Added support for Bluetooth Low Energy HID Braille displays. (#15470) +- Added unassigned gestures to to quickly cycle through output devices forward and backward. (#8801)

Double 'to':

- Added unassigned gestures to quickly cycle through output devices forward
and backward. (#8801)

@@ -1775,6 +1775,7 @@ The Audio category in the NVDA Settings dialog contains options that let you cha

==== Output device ====[SelectSynthesizerOutputDevice] This option allows you to choose the audio device that NVDA should instruct the selected synthesizer to speak through. +To switch to the next or previous output device, pleas assign custom shortcuts from [Input Gestures dialog #InputGestures].

Pleas -> please

+import tones

+def getOutputDeviceNames():

• deviceNames = nvwave.getOutputDeviceNames()

• #11349: On Windows 10 20H1 and 20H2, Microsoft Sound Mapper returns an

empty string.

• if deviceNames[0] in ("", "Microsoft Sound Mapper"):

• # Translators: name for default (Microsoft Sound Mapper) audio output
  

device.

• deviceNames[0] = _("Microsoft Sound Mapper")
  

• return deviceNames

+def setOutputDevice(device: str):

• config.conf["speech"]["outputDevice"] = device
• currentSynth = getSynth()
• if not setSynth(currentSynth.name):

I'm not sure I agree with the logic here. I understand you haven't written this code, but since we're revisiting it anyway let's try to make it nicer. Currently we set the configured device before checking if it can be used i.e. if the current synthesizer can speak through it. Also with the current design there is an import cycle, since you import gui here, yet gui imports from this new module. I'd move this to nvwave, but rewrite the code (in my view basic module for playing sound should not depend on other modules tones, synthesizer drivers, which depend on it), using extensionPoints.Decider in nvwave to which both tones and synthDriverHandler can subscribe (something like onMainAudioDeviceChanged). If both synthesizer and tones can work with the new device, only then it should be set in the config. Also in that case you can check what has been returned from the decider in the settings dialog, and display error in the gui there, and announce the new device, or failure in switching into it in the keyboard scripts. With this approach it would be necessary to modify tones and setSynth to accept the specified audio device, as until now they were always using the default one.

-- Reply to this email directly or view it on GitHub: https://github.com/nvaccess/nvda/pull/15493#pullrequestreview-1723545203 You are receiving this because you authored the thread.

Message ID: @.***>

-- with best regards Beqa Gozalishvili Tell: +995593454005 Email: @.*** Web: https://gozaltech.org Skype: beqabeqa473 Telegram: https://t.me/gozaltech facebook: https://facebook.com/gozaltech twitter: https://twitter.com/beqabeqa473 Instagram: https://instagram.com/beqa.gozalishvili

[cary-rowen]

Hi @beqabeqa473 Would you consider having this ready for 2024.1? Thanks

[XLTechie]

@beqabeqa473 What is the status of this? I would love to see it go to beta. I am in need of this feature. I'm going to have to implement it in an add-on if this PR is not proceeding soon. Life gets in the way, I very much understand, but please mention it if you need help bringing this forward.

[beqabeqa473]

Hello.

Sorry for delay. I have working prototype, i will push tomorrow and we will see how to proceed.

On 2/16/24, Luke Davis @.***> wrote:

@beqabeqa473 What is the status of this? I would love to see it go to beta. I am in need of this feature. I'm going to have to implement it in an add-on if this PR is not proceeding soon. Life gets in the way, I very much understand, but please mention it if you need help bringing this forward.

-- Reply to this email directly or view it on GitHub: https://github.com/nvaccess/nvda/pull/15493#issuecomment-1947985910 You are receiving this because you were mentioned.

Message ID: @.***>

-- with best regards Beqa Gozalishvili Tell: +995593454005 Email: @.*** Web: https://gozaltech.org Skype: beqabeqa473 Telegram: https://t.me/gozaltech facebook: https://facebook.com/gozaltech twitter: https://twitter.com/beqabeqa473 Instagram: https://instagram.com/beqa.gozalishvili

[beqabeqa473]

@XLTechie hi, i pushed latest changes, and i think it is ready to review.

[AppVeyorBot]

• FAIL: Translation comments check. Translation comments missing or unexpectedly included. See build log for more information.
• PASS: Unit tests.
• FAIL: Lint check. See test results for more information.
• PASS: System tests (tags: installer NVDA).
• Build (for testing PR): https://ci.appveyor.com/api/buildjobs/5w12s7p01fkrovfx/artifacts/output/nvda_snapshot_pr15493-31067,10ea7d96.exe
• CI timing (mins): INIT 0.0, INSTALL_START 1.0, INSTALL_END 0.9, BUILD_START 0.0, BUILD_END 29.4, TESTSETUP_START 0.0, TESTSETUP_END 0.4, TEST_START 0.0, TEST_END 2.4, FINISH_END 0.2

See test results for failed build of commit 10ea7d962c

[AppVeyorBot]

• FAIL: Translation comments check. Translation comments missing or unexpectedly included. See build log for more information.
• PASS: Unit tests.
• PASS: Lint check.
• PASS: System tests (tags: installer NVDA).
• Build (for testing PR): https://ci.appveyor.com/api/buildjobs/5t1692mame3srxvh/artifacts/output/nvda_snapshot_pr15493-31068,1bc860a9.exe
• CI timing (mins): INIT 0.0, INSTALL_START 1.0, INSTALL_END 0.9, BUILD_START 0.0, BUILD_END 28.3, TESTSETUP_START 0.0, TESTSETUP_END 0.4, TEST_START 0.0, TEST_END 2.3, FINISH_END 0.2

See test results for failed build of commit 1bc860a915

[beqabeqa473]

@seanbudd I've modified _synthWarningDialog to accept synth instance, while at it, i've made that synthDriverHandler.getSynthList now returns SynthDriver classes rather than names and descriptions and modified synth selection dialog to make it working with new layout.

[Adriani90]

@beqabeqa473 are you still working on this?

[beqabeqa473]

I will try to revisit this in the near feature.