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

Native support for the Dot Pad tactile graphic Braille Display from Dot Inc #17007

Merged
merged 20 commits into from
Aug 28, 2024

Conversation

michaelDCurran
Copy link
Member

@michaelDCurran michaelDCurran commented Aug 15, 2024

Link to issue number:

None

Summary of the issue:

The Dot Pad from Dot Inc is a device that can display refreshable tactile graphics and braille. The A320 model can show tactile graphics of 120 by 80 dots (which could comfortably fit 8 lines of 20 cells each.
The device also has a dedicated line of 20 cells specifically for text.
Currently you can use the Dot Pad with NVDA via Brltty I believe, though I did not manage to get that working on my machine. There are also some NVDA add-ons to support Dot Pad in various forms, using the Dot Pad SDK.

NVDA should add native support for the Dot pad device, removing the requirement for Brltty or add-ons, at least for basic functionality.

Description of user facing changes

It is now possible to use the Dot Pad braille display with NVDA.
Features:

  • Can display braille either on the dedicated 20 cell text line, or on the tactile graphic area. This can be configured in NVDA's braille settings panel by changing the Braille Destination combo box.
  • Left and right panning keys are supported.

Description of development approach

Write a native braille display driver that communicates with the device over serial. Borrowed many of the constants and structures from BrlTty.
Also added a new tactile package, which contains untilities and classes for working with tactile graphics. So far just a TactileGraphicsBuffer which represents a buffer in which single dots can be set, and a drawBrailleCells function which can take bytes representing a string of standard braille cells, and draws them onto the graphics buffer.

Testing strategy:

Tested with a Dot pad A320 via USB. Used NVDA normally with braille over a period of time.

Known issues with pull request:

  • No auto detection
  • Dot pad will not refresh correctly if your hand is currently on the device. This is a hardware limitation.
  • NvDA's current multiline braille support is still very limited, in that it treats the graphics area as one long wrapped line. However, as NVDA internally does know the number of rows and columns, future improvemts such as per-line word wrapping are coming.

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.

Summary by CodeRabbit

  • New Features

    • Added native support for the Dot Pad tactile graphics device, enhancing braille output.
    • Introduced functionality for rendering Braille cells on a tactile graphics buffer.
    • Created an abstract class for managing tactile graphics buffers to support future developments.
  • Documentation

    • Updated user guide with a new section dedicated to the Dot Pad, detailing its capabilities and configuration options.

michaelDCurran and others added 3 commits August 15, 2024 13:54
…hat help with displaying tactile graphics. For now just a TactileGraphicsBuffer in which you can set (turn on) dots, and a drawBrailleCells function which takes standard braille cells and draws them onto the TactileGrapicsBuffer.
@AppVeyorBot
Copy link

  • Build execution time has reached the maximum allowed time for your plan (60 minutes).

See test results for failed build of commit 70e524017d

@bramd
Copy link
Contributor

bramd commented Aug 15, 2024

@michaelDCurran I would love to see better support for the Dot Pad in core. However, I've been working on an add-on for Dot to do this for a while now. We have lots of stuff that is in my opinion experimental and does not belong in core yet. However, if there is interest in adding tactile display support to core from NVAccess I would be glad to discuss which parts could be in core and how we can work together on that. Feel free to get in touch outside of Github as well for a more in depth discussion.

Unfortunately the addon code base is not open source right now, but will of course release it under GPL when we have a first public release.

@AppVeyorBot
Copy link

See test results for failed build of commit 70e524017d

@michaelDCurran
Copy link
Member Author

@bramd thanks for making contact. I've sent you an email about collaborating.

@michaelDCurran michaelDCurran marked this pull request as ready for review August 15, 2024 22:44
@michaelDCurran michaelDCurran requested review from a team as code owners August 15, 2024 22:44
Copy link
Contributor

coderabbitai bot commented Aug 15, 2024

Walkthrough

The recent updates to the NVDA project introduce native support for the Dot Pad tactile graphics device, significantly enhancing accessibility for visually impaired users. This includes comprehensive drivers for the DotPad display, a new abstract class for managing tactile graphics, and functions for rendering Braille on tactile buffers. User documentation has also been updated to guide users on configuring and utilizing the Dot Pad effectively.

Changes

Files Change Summary
source/brailleDisplayDrivers/dotPad.py Introduces a driver for the DotPad display, including command enums, display structures, and classes for managing graphics and device communication.
source/tactile/__init__.py Defines an abstract class TactileGraphicsBuffer for managing tactile graphics, including the setDot abstract method.
source/tactile/braille.py Implements drawBrailleCells function for rendering Braille cells onto a TactileGraphicsBuffer.
user_docs/en/changes.md Adds documentation for the integration of Dot Pad support, enhancing usability for tactile graphics.
user_docs/en/userGuide.md Includes a new section on the Dot Pad device, detailing its specifications, capabilities, and configuration options in NVDA.

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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Outside diff range, codebase verification and nitpick comments (2)
source/tactile/__init__.py (1)

31-34: Enhance the docstring for setDot.

Consider providing more details on the expected behavior or constraints for the setDot method to guide implementers.

		"""
		Sets a dot at the given coordinates.
		Implementations should ensure that coordinates are within the buffer's bounds.
		"""
source/brailleDisplayDrivers/dotPad.py (1)

243-260: Improve logging in _onReceive.

Enhance logging to include more details about the received data and potential errors for better traceability.

def _onReceive(self, header1):
	if ord(header1) != DP_PacketSyncByte.SYNC1:
		log.error(f"Unexpected header1: {header1}")
		raise RuntimeError(f"Bad {header1=}")
	header2 = self._dev.read(1)
	if ord(header2) != DP_PacketSyncByte.SYNC2:
		log.error(f"Unexpected header2: {header2}")
		raise RuntimeError(f"bad {header2=}")
	length = struct.unpack(">H", self._dev.read(2))[0]
	packetBody = self._dev.read(length)
	dest, cmdHigh, cmdLow, seqNum, *data, checksum = packetBody
	data = bytes(data)
	if checksum != functools.reduce(operator.xor, packetBody[:-1], DP_CHECKSUM_BASE):
		log.error("Checksum mismatch")
		raise RuntimeError("bad checksum")
	cmd = DP_Command(struct.unpack(">H", bytes([cmdHigh, cmdLow]))[0])
	log.debug(f"Received response {cmd.name}, {dest=}, {seqNum=}, data={bytes(data)}")
	if cmd.name.startswith("RSP_"):
		self._recordCommandResponse(cmd, data, dest, seqNum)
	elif cmd.name.startswith("NTF_"):
		self._handleNotification(cmd, data, dest, seqNum)

source/tactile/braille.py Outdated Show resolved Hide resolved
source/brailleDisplayDrivers/dotPad.py Outdated Show resolved Hide resolved
user_docs/en/userGuide.md Outdated Show resolved Hide resolved
user_docs/en/userGuide.md Outdated Show resolved Hide resolved
user_docs/en/userGuide.md Outdated Show resolved Hide resolved
user_docs/en/userGuide.md Outdated Show resolved Hide resolved
source/brailleDisplayDrivers/dotPad.py Outdated Show resolved Hide resolved
source/brailleDisplayDrivers/dotPad.py Outdated Show resolved Hide resolved
source/brailleDisplayDrivers/dotPad.py Outdated Show resolved Hide resolved
source/brailleDisplayDrivers/dotPad.py Outdated Show resolved Hide resolved
source/brailleDisplayDrivers/dotPad.py Outdated Show resolved Hide resolved
source/brailleDisplayDrivers/dotPad.py Outdated Show resolved Hide resolved
@AppVeyorBot
Copy link

See test results for failed build of commit ea25d5b0e6

@AppVeyorBot
Copy link

See test results for failed build of commit ea25d5b0e6

user_docs/en/userGuide.md Outdated Show resolved Hide resolved
source/tactile/braille.py Outdated Show resolved Hide resolved
@AppVeyorBot
Copy link

See test results for failed build of commit 44ef991013

@seanbudd seanbudd added this to the 2025.1 milestone Aug 20, 2024
@seanbudd seanbudd added merge-early Merge Early in a developer cycle conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. labels Aug 20, 2024
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.

Great work, will be a good asset to Dot Pad users :)

@SaschaCowley
Copy link
Member

Can you please resolve the merge conflicts?

@seanbudd seanbudd merged commit aa15949 into master Aug 28, 2024
4 checks passed
@seanbudd seanbudd deleted the dotPad branch August 28, 2024 06:11
@seanbudd seanbudd mentioned this pull request Aug 28, 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. merge-early Merge Early in a developer cycle
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants