HandyTech: Interpret reading position packets from Active Tactile Control#20353
HandyTech: Interpret reading position packets from Active Tactile Control#20353codeofdusk wants to merge 2 commits into
Conversation
This commit surfaces no user behaviour; to be done in future work.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds Active Tactile Control (ATC) reading-position handling for Handy Tech braille displays to filter transient/invalid touch reports after a refresh.
Changes:
- Introduces ATC reading position tracking with a post-refresh “settling” suppression window.
- Hooks
HT_EXTPKT_READING_POSITIONpackets into the input stream handler. - Updates file header copyright attribution.
|
Interesting approach. I prototyped something that didn't save the reading position on the mixin, but executed a new AtcGesture that contained the reading position. Of course this is still something that can be done. Also there is no handling of old display reading position handling (Modular Evolution), but that can easily be added later. First of all thoug, I think we need to know how far we can go with implementing ATC parsing directly in NVDA without violating the ATC patents held by Handy Tech/Helptech. I assume they are only patenting the hardware, not the software that uses it. That said, this pr is doing nothing extra besides what BRLTTY already has. |
VoiceOver (iOS/MacOS) also implements its own support for this feature, and there's another implementation in #4775 (comment). I feel strongly that we're fine. |
seanbudd
added
the
conceptApproved
| f"Unexpected ATC reading position packet with length {len(packet)}: {packet!r}", | ||
| ) | ||
| return | ||
| reading_position = packet[1] |
There was a problem hiding this comment.
please make sure to use lowerCamelCase for variable names throught these changes
|
|
||
| def __init__(self, display): | ||
| super().__init__(display) | ||
| self._readingPosition: Optional[int] = None |
There was a problem hiding this comment.
please make sure to use modern type hints such as | None or list throughout these changes
|
|
||
| def __init__(self, display): | ||
| super().__init__(display) | ||
| self._readingPosition: Optional[int] = None |
There was a problem hiding this comment.
| self._readingPosition: Optional[int] = None | |
| self._readingPosition: int | None = None |
| def __init__(self, display): | ||
| super().__init__(display) | ||
| self._readingPosition: Optional[int] = None | ||
| self._lastRefreshTime = 0.0 |
There was a problem hiding this comment.
| self._lastRefreshTime = 0.0 | |
| self._lastRefreshTime: float = 0.0 |
| log.debug("Enabling ATC") | ||
| self._display.atc = True | ||
|
|
||
| def display(self, cells: List[int]): |
There was a problem hiding this comment.
| def display(self, cells: List[int]): | |
| def display(self, cells: list[int]): |
| f"Unexpected ATC reading position packet with length {len(packet)}: {packet!r}", | ||
| ) | ||
| return | ||
| reading_position = packet[1] |
There was a problem hiding this comment.
| reading_position = packet[1] | |
| readingPosition = packet[1] |
| ) | ||
| return | ||
| reading_position = packet[1] | ||
| elapsed_since_refresh = time.monotonic() - self._lastRefreshTime |
There was a problem hiding this comment.
| elapsed_since_refresh = time.monotonic() - self._lastRefreshTime | |
| elapsedSinceRefresh = time.monotonic() - self._lastRefreshTime |
| return | ||
| reading_position = packet[1] | ||
| elapsed_since_refresh = time.monotonic() - self._lastRefreshTime | ||
| is_settling_after_refresh = elapsed_since_refresh < self.READ_SUPPRESS_AFTER_REFRESH_SECONDS |
There was a problem hiding this comment.
| is_settling_after_refresh = elapsed_since_refresh < self.READ_SUPPRESS_AFTER_REFRESH_SECONDS | |
| isSettlingAfterRefresh = elapsed_since_refresh < self.READ_SUPPRESS_AFTER_REFRESH_SECONDS |
Link to issue number:
Re #4775.
Summary of the issue:
Some HandyTech Braille displays, such as the Activator, can communicate the user's reading position back to the controlling device. This allows, for instance, the display to advance when the user finishes reading a line (instead of on a timer or with a button press), to speak the current word or line (or perform some other action) if the user dwells, etc. NVDA lacks support for interpreting HandyTech control packets containing this information.
Description of how this pull request fixes the issue:
This PR adds support for parsing reading position packets to the HandyTech driver and stores the current reading position in a private field. No user-facing behaviour changes have been made as the API will likely require some discussion (do we want to expose reading position as events, an extension point, something else?) and that work is better done in a follow-up.
Testing strategy:
Tested with an Activator over USB-C. Reported reading positions are accurate, including in text fields with a rapidly refreshing blinking cursor.
Known issues with pull request:
The ATC protocol is undocumented as far as I can tell. I've made a best effort and added logging in case of unexpected packet shapes. NVDA was already receiving (and throwing away) these reading position packets, so these changes have no other impact on the HandyTech driver.
Code Review Checklist: