Add support for Activator Pro models to Help Tech Braille driver. #16668
Conversation
|
@FelixGruetzmacher thanks for this work. This will also need:
|
See test results for failed build of commit abb1d082b1 |
…ted as it uses the blanket term `most Handy Tech displays` to define the set of supported devices. While this may suffice for the moment, a complete list will soon be added.
See test results for failed build of commit 3d86cfba42 |
seanbudd
added
the
conceptApproved
There was a problem hiding this comment.
Can you please update the user guide?
Additionally, this module is getting quite large. Would you consider splitting it up into a submodule like eurobraille and albatross in a separate PR?
…s the issue. Co-authored-by: Leonard de Ruijter <3049216+LeonarddeR@users.noreply.github.com>
…y reference to `object` class. Co-authored-by: Sean Budd <seanbudd123@gmail.com>
…d `super` syntax. Co-authored-by: Sean Budd <seanbudd123@gmail.com>
…common superclass.
|
Thank you for the comments. I cleaned up the code accordingly. Can we, just for the time being, leave this all in one module, and I'll split it up the next time it gets modified? |
WalkthroughThe recent changes introduce support for new Braille display models, specifically the Activator Pro 64 and Activator Pro 80, by adding corresponding constants, mixins, and classes to handle dynamic cell adjustments and enhance the handling of protocol properties. The automatic detection mechanism and input stream handling logic have also been updated to accommodate these new models. Documentation has been updated to reflect these enhancements. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Software
participant ActivatorPro
participant DisplayDriver
User->>Software: Connect ActivatorPro64/80
Software->>+ActivatorPro: Identify Model
ActivatorPro->>-Software: Return Model Info
Software->>+DisplayDriver: Initialize with Model Details
DisplayDriver-->>Software: Acknowledge
Software->>Software: Adjust Dynamic Cells
Software->>User: Display Braille
sequenceDiagram
participant User
participant NVDA
participant AddOnManager
User->>NVDA: Startup
NVDA->>AddOnManager: Check for updates
AddOnManager-->>NVDA: Notify for updates
NVDA->>User: Display Update Notifications
Recent review detailsConfiguration used: .coderabbit.yml Files selected for processing (2)
Additional context usedPath-based instructions (2)
Additional comments not posted (6)
Warning Review ran into problemsProblems (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
user_docs/en/changes.md (1)
24-25: Ensure consistency in bullet point formatting.Consider adding a bullet point for the "By default, after NVDA startup, you will be notified if any add-on updates are available." to match the formatting of the other entries.
| @@ -656,7 +709,7 @@ def registerAutomaticDetection(cls, driverRegistrar: bdDetect.DriverRegistrar): | |||
| "Braillino BL", | |||
| "Braille Wave BW", | |||
| "Easy Braille EBR", | |||
| "Activator AC", | |||
There was a problem hiding this comment.
Is this removal intentional?
There was a problem hiding this comment.
Yes. The intent is to match Activator AC4/serial, Activator Pro 64/serial and Activator Pro 80/serial with a single condition.
|
Might we get this into 2024.2? |
|
No, because NVDA2024.2 has entered the RC stage. Mergers of new features will not be accepted. |
Link to issue number:
There is no corresponding issue.
Summary of the issue:
Help Tech Activator Pro 80 and Activator Pro 64 are not supported.
Description of user facing changes
The two affected device types will be supported by NVDA and will also be autodetected.
Description of development approach
The driver was modified to include the new devices.
Testing strategy:
The driver was tested in the context of the current beta of NVDA.
Known issues with pull request:
There are no known issues.
Code Review Checklist:
Summary by CodeRabbit
New Features
Documentation