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

improve state management in speech.py #12395

Merged
merged 6 commits into from May 12, 2021
Merged

improve state management in speech.py #12395

merged 6 commits into from May 12, 2021

Conversation

seanbudd
Copy link
Member

@seanbudd seanbudd commented May 11, 2021

Link to issue number:

Fixes #12393, follow up of #12251

Summary of the issue:

Using shift to resume speech after using shift to pause speech does not resume speech.

This is because when isPaused is imported through speech\__init__.py it is does not reference the global variable used in speech.py

Description of how this pull request fixes the issue:

Removes the use of top level variables in speech.py in favour of a private dataclass to manage state, and an enum for certain modes. Constants (or rather, variables with names in all capitals) remain as is.

Testing strategy:

Using reproduction steps in #12393 to manually test the pausing and resuming of speech.

System tests for this may be useful, but require a change to the system test spy synth driver.

Known issues with pull request:

None

Change log entries:

Developer changes:

  • speech.curWordChars has been renamed speech._curWordChars
  • the following have been removed from speech and can be accessed through speech.getState(). These are readonly values now.
    • speechMode
    • speechMode_beeps_ms
    • beenCanceled
    • isPaused
  • to update speech.speechMode use speech.setSpeechMode
  • the following have been moved to speech.SpeechMode
    • speech.speechMode_off becomes speech.SpeechMode.off
    • speech.speechMode_beeps becomes speech.SpeechMode.beeps
    • speech.speechMode_talk becomes speech.SpeechMode.talk

Code Review Checklist:

  • Pull Request description is up to date.
  • Unit tests.
  • System (end to end) tests.
  • Manual tests.
  • User Documentation.
  • Change log entry.
  • Context sensitive help for GUI changes.

@seanbudd seanbudd changed the title Refactor speech remove use of global in speech.py May 11, 2021
@seanbudd seanbudd changed the title remove use of global in speech.py improve state management in speech.py May 11, 2021
@AppVeyorBot
Copy link

See test results for failed build of commit 46cab6a7b4

@seanbudd seanbudd marked this pull request as ready for review May 11, 2021 06:33
@seanbudd seanbudd requested a review from a team as a code owner May 11, 2021 06:33
@seanbudd seanbudd added this to the 2021.1 milestone May 11, 2021
@seanbudd seanbudd self-assigned this May 11, 2021
Copy link
Contributor

@feerrenrut feerrenrut left a comment

Choose a reason for hiding this comment

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

Generally looks good!



def initialize():
global _SpeechState
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be lower case: _speechState

@seanbudd seanbudd merged commit 8938ca4 into master May 12, 2021
@seanbudd seanbudd deleted the refactor-speech branch May 12, 2021 01:21
aaclause added a commit to aaclause/BrailleExtender that referenced this pull request May 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Resuming speech with shift no longer works
3 participants