Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added ability for braille to show what NVDA is speaking #15956

Merged
merged 167 commits into from Apr 16, 2024

Conversation

Emil-18
Copy link
Contributor

@Emil-18 Emil-18 commented Dec 22, 2023

Link to issue number:

fixes #15898

Summary of the issue:

Both in NVDA core, as well as some add-ons, have features that work with speech but don't work with braille

Description of user facing changes

Braille only users will be able to read NVDA speech in braille, thus get access to a lot more features then they would otherwise

Description of development approach

A new braille mode has been added, called "speech emulation". In this mode, NVDA will send all text that has been spoken since the last time speech was canceled, to the braille display. If the speech was canceled do to the user scrolling the braille display back or fourth, the speech output mode will treat it as if the speech was not canceled, so the user can read the text without erasing it accidentally. If braille.handler.rawText is grater then 100000 characters, it will not be sent, because in my experience, NVDA will freeze when that much text is sent to the braille display at once. The braille settings panel has been rearranged so that all settings not aplickable to speech emulation is grouped together. A new combo box has been added to the braille settings panel, and a new script with the gesture NVDA+alt+t have been added, that allows the user to change braille mode.

Testing strategy:

tested with a Focus braille display. To test that it ignored speech being canceled when scrolling the braille display back or fourth, I turned off "Speech interrupt for typed characters", then opened the run dialog, scrolled the braille display to the end of the text while in speech output mode with the "Interrupt speech while scrolling" setting turned on, and then typed some text. I then scrolled the braille display back and fourth, and continued typing text while doing this to ensure that it worked as it should. Everything worked as expected

Known issues with pull request:

None so far

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.

@AppVeyorBot

This comment was marked as resolved.

@Emil-18 Emil-18 changed the title Added abillity for braille to show what NVDA is speaking Added ability for braille to show what NVDA is speaking Dec 22, 2023
@XLTechie
Copy link
Contributor

XLTechie commented Dec 22, 2023 via email

@lukaszgo1
Copy link
Contributor

Both in NVDA core, as well as some add-ons, have features that work with speech but don't work with braille

Would you be able to provide some examples. Especially for core features if they don't work well in Braille it is useful to have an issue to track the problem.

@burmancomp
Copy link
Contributor

I tested and noticed that something about new setting should be added to configSpec.py to get nvda from source running.

If braille is tethered to review, changing setting from settings dialog braille category from "speech emulation" to "follow cursors" causes attribute error and displaying braille is stopped until changed to what braille is tethered to.

I did not get nvda+alt+t to work with laptop layout when I used caps lock as nvda key although it was shown in braille category of gestures but this is something what is not problem of this pr.

@AppVeyorBot

This comment was marked as resolved.

@Emil-18
Copy link
Contributor Author

Emil-18 commented Dec 23, 2023

@lukaszgo1 As of the latest alpha, braille does not show:

  • The object under the mouse when mouse tracking is enabled.
  • Formatting information when it is turned on in document formatting settings.
  • How many list items it is in a list on the web when entered, in a situation where speech would report list with x items.
  • How many rows and colums a web/word table have when entered.
  • Information about objects that are higher up in the object hierarchy, such as lists or groupings, in legacy edge UIA browse mode documents when browse mode is turned on. Same for open/libre office documents.
  • On screen text on web pages that you can review with the review cursor commands in focus mode that isn't in edit boxes. I know that you aren't actualy supposed to review this text, but it is still information that a speech user can get and a braille only user can't.
    It is impossible to use the f9 selection mode with braille when the current review position is in an object that don't has useful text according to the braille.NVDAObjectHasUsefulText function.
    When ui.message is called in a fiew milliseconds after a previous call, something I have seen a lot of add-ons do, it is impossible to read what was sent to the braille display in the first call, as this text is overwritten by the text in the second.
    When ui.message is called inside a thread, and a braille display is connected, and show messages is set to use timeout, it gives the following error
    ERROR - stderr (19:11:34.275) - Thread-2 (test) (10760):
    C++ assertion "wxThread::IsMain()" failed at ....\src\common\timerimpl.cpp(57) in wxTimerImpl::Start(): timer can only be started from the main thread
    resulting in the rest of the function never executing.

@Emil-18
Copy link
Contributor Author

Emil-18 commented Dec 23, 2023

@XLTechie I don't know.

@XLTechie
Copy link
Contributor

XLTechie commented Dec 23, 2023 via email

@AppVeyorBot

This comment was marked as resolved.

@AppVeyorBot

This comment was marked as resolved.

@AppVeyorBot
Copy link

See test results for failed build of commit 672d8524ed

@Emil-18
Copy link
Contributor Author

Emil-18 commented Mar 16, 2024

@seanbudd I committed your changes and tested them. It works as it should now.
You wrote

I don't believe script is a tuple here? I don't believe this is the correct type hint, please see comment
#15956

But in that comment, you suggested that exact change?

@AppVeyorBot
Copy link

See test results for failed build of commit 29373466ff

@AppVeyorBot
Copy link

See test results for failed build of commit ea8be83f53

@AppVeyorBot
Copy link

See test results for failed build of commit d28da2d23b

@AppVeyorBot
Copy link

See test results for failed build of commit c12f4224ba

@Emil-18
Copy link
Contributor Author

Emil-18 commented Mar 31, 2024

@seanbudd I think I have fixed the type hint on the suppressClearBrailleRegions function now

@Emil-18 Emil-18 marked this pull request as ready for review April 6, 2024 16:37
Copy link
Member

@seanbudd seanbudd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks almost ready

projectDocs/dev/developerGuide/developerGuide.t2t Outdated Show resolved Hide resolved
filter_speechSequence = Filter[SpeechSequence]()
"""
Filters speech sequence before it passes to synthDriver.

:param value: the speech sequence to be filtered.
:type value: SpeechSequence

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you remove this junk new line added

@@ -6,7 +6,6 @@

"""High-level functions to speak information.
"""

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please restore this new line

user_docs/en/changes.t2t Outdated Show resolved Hide resolved
Copy link
Member

@Qchristensen Qchristensen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reads well. Agree with Sean's change.

Emil-18 and others added 4 commits April 15, 2024 14:23
@seanbudd seanbudd closed this Apr 16, 2024
@seanbudd seanbudd reopened this Apr 16, 2024
@seanbudd seanbudd merged commit 3d4a0a8 into nvaccess:master Apr 16, 2024
1 check passed
@nvaccessAuto nvaccessAuto added this to the 2024.2 milestone Apr 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add abillity for braille to be tethered to speech