Pr7599 review actions

[michaelDCurran]

Link to issue number:

Addresses review actions in #7599

Summary of the issue:

Review comments were provided after #7599 was merged. these comments should be addressed where possible.

Description of how this pull request fixes the issue:

Addresses the following review actions:

• espeak: ensure that onIndexReached is set before espeak_setSynthCallback is called.
• speech.getSpeechForSpelling: use a named constant for the idiographic comma and rename char to speakCharAs.
• nvWave: use named constants for buffer size calculation.
• espeak: use named constants for converting ms to bytes.
• espeak: provide constants for callback return codes.
• Convert speech.py into a speech package, splitting out commands, manager and priorities into their own modules.
• SayAll: avoid error when reading to the end of a Microsoft word document where self.reader is set to None.

Testing performed:

Used NVDA with both eSpeak and Windows OneCore for several minutes at a time.
Ran through most testcases in #7599 including profile switches, beeps, playing sounds, and priorities.

Known issues with pull request:

Review actions not addressed:

• renaming of synthIndexReached action to something like post_synthIndexReached: I personally do not think this is necessary -- the name clearly states what it does. Though as @jcsteh was the one who originally came up with actions in the first place (for speechRefactor) perhaps he has an opinion as to whether we must be so specific.
• Splitting of sayAllhandler into its own package with modules for each of the large classes: This has not been done as these classes currently rely on some global variables. I feel there would need to be some careful redesign to get around this. I don't think it is worth the risk or time right now.
• Convertng the usage of yielding pitchCommands in places such as speakSpelling into some kind of raii pattern: I'm really not sure how you would do this with yield statements. If someone else wants to design something then be my guest.

Change log entry:

None.

[feerrenrut]

Looks good, thanks for taking the time to do this!

The commands file seems to be duplicated there is:

• speech./commands.py (notice the dot after speech
• speech/commands.py

[feerrenrut]

post_synthIndexReached: I personally do not think this is necessary -- the name clearly states what it does.

I think there is something to be said for consistency of these extension points, but I also agree that this usage doesn't fit the pattern. The conventions we came up with were to provide a prodictable naming scheme for the exension points, and to remove ambiguity about whether the thing was about to happen, or had just happend. The exention points we wrote the convention around acted like notifications of some pending or past behaviour in NVDA. These were obviously ambiguous. If I understand correctly, there is no inherent behaviour in NVDA associated with reaching an index, so there is no need to differentiate between before / after.

I'm ok with leaving this as it is, perhaps we should update the contributing wiki page.

[feerrenrut]

I've pushed these changes to this branch. If you are happy with this go ahead and mege.