NLS eReader Zoomax braille driver by florin-trutiu · Pull Request #17509 · nvaccess/nvda

florin-trutiu · 2024-12-12T11:23:58Z

Link to issue number:

Summary of the issue:

This is the NVDA driver for the NLS eReader Zoomax braille display.
It supports both USB and Bluetooth automatic detection.

Description of user facing changes

With this driver the user can use directly the NLS eReader Zoomax display without the need to manually install it as an addon.

Description of development approach

The driver is similar with the existing braille display drivers for NVDA.

Testing strategy:

Known issues with pull request:

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.

@coderabbitai summary

seanbudd · 2024-12-13T02:10:05Z

Thanks for opening this PR @florin-trutiu !

Please fill out the PR template
Please remove this line from the userGuide:

nvda/user_docs/en/userGuide.md

Line 4284 in c17b423

* Note that the Zoomax is currently not supported without external drivers
Please add a changelog entry in changes.md noting that support for this braille device has been added

seanbudd · 2024-12-13T02:14:08Z

pre-commit.ci autofix

florin-trutiu · 2024-12-13T14:06:27Z

Thanks for opening this PR @florin-trutiu !

Please fill out the PR template

Please remove this line from the userGuide:

nvda/user_docs/en/userGuide.md

Line 4284 in c17b423

* Note that the Zoomax is currently not supported without external drivers

Please add a changelog entry in changes.md noting that support for this braille device has been added

@seanbudd
Removed the mentioned line.
I noticed that the line was in the Huwanware section because of a confusion, there are two NLS eReader braille displays. one is manufactured by Humanware and one by Zoomax.
So to avoid confusion I appended the Humanware name to the NLS eReader device mentioned in the Humanware displays section.

In the user guide I have now added a section for the NLS eReader Zoomax braille display.

CyrilleB79 · 2024-12-13T15:03:43Z

@florin-trutiu, you have added the template, but for completeness, could you please writing the required information in its sections rather than writing before it? Thanks!

SaschaCowley

Thanks for submitting this, @florin-trutiu! A lot of questions and stylistic questions on this to begin with

SaschaCowley · 2025-01-10T05:03:43Z

+# -*- coding: UTF-8 -*-
+# brailleDisplayDrivers/nlseReaderZoomax.py
+# Description:
+# NLS eReader Zoomax driver for NVDA.


Please update this to be in line with the copyright headers in the rest of NVDA's source.

Suggested change

# -*- coding: UTF-8 -*-

# brailleDisplayDrivers/nlseReaderZoomax.py

# Description:

# NLS eReader Zoomax driver for NVDA.

# A part of NonVisual Desktop Access (NVDA)

# Copyright (C) 2025 NV Access Limited, Florin Trutiu

# This file is covered by the GNU General Public License.

# See the file COPYING for more details.

# NLS eReader Zoomax driver for NVDA.

SaschaCowley · 2025-01-10T05:09:46Z

+# NLS eReader Zoomax driver for NVDA.
+
+import time
+from typing import Union, List, Optional


Union type expressions are preferred over typing.Union. Likewise, T | None is preferred over typing.Optional[T]. And typing.List is a deprecated alias of list.

SaschaCowley · 2025-01-10T05:11:47Z

+from typing import Union, List, Optional
+
+import braille
+from hwIo import intToByte, boolToByte


You import hwIo at line 14.

SaschaCowley · 2025-01-10T05:12:26Z

+import bdDetect
+import serial
+
+TIMEOUT = 0.2


Please add units.

Suggested change

TIMEOUT = 0.2

TIMEOUT_SEC = 0.2

SaschaCowley · 2025-01-10T05:12:49Z

+TIMEOUT = 0.2
+BAUD_RATE = 19200
+CONNECT_RETRIES = 5
+TIMEOUT_BETWEEN_RETRIES = 2


Please add units

Suggested change

TIMEOUT_BETWEEN_RETRIES = 2

TIMEOUT_BETWEEN_RETRIES_SEC = 2

SaschaCowley · 2025-01-10T06:02:05Z

+		if not isinstance(command, bytes):
+			raise TypeError(typeErrorString.format("command", "bytes", type(command).__name__))
+
+		arg = arg.replace(ESCAPE, ESCAPE * 2)


Can you please explain why you are doing this?

SaschaCowley · 2025-01-10T06:02:34Z

+
+	def _onReceive(self, data: bytes):
+		if data != ESCAPE:
+			log.debugWarning("Ignoring byte before escape: %r" % data)


Please use an f-string instead

SaschaCowley · 2025-01-10T06:19:27Z

+			# If not, we wish to know about it, allow decode to raise.
+			self._deviceID = arg.decode("latin-1", errors="strict")
+		elif command in KEY_NAMES:
+			arg = sum(byte << offset * 8 for offset, byte in enumerate(arg))


Why not

Suggested change

arg = sum(byte << offset * 8 for offset, byte in enumerate(arg))

arg = int.from_bytes(reversed(arg))

SaschaCowley · 2025-01-10T06:23:12Z

+			if arg < self._keysDown.get(command, 0):
+				# Release.
+				if not self._ignoreKeyReleases:
+					# The first key released executes the key combination.
+					try:
+						inputCore.manager.executeGesture(InputGesture(self._deviceID, self._keysDown))
+					except inputCore.NoInputGestureAction:
+						pass
+					# Any further releases are just the rest of the keys in the combination being released,
+					# so they should be ignored.
+					self._ignoreKeyReleases = True
+			else:
+				# Press.
+				# This begins a new key combination.
+				self._ignoreKeyReleases = False
+			if arg > 0:
+				self._keysDown[command] = arg
+			elif command in self._keysDown:
+				# All keys in this group have been released.
+				# #3541: Remove this group so it doesn't count as a group with keys down.
+				del self._keysDown[command]


I'm having a lot of trouble following what's going on here. Could you please comment a bit more on what's going on?

SaschaCowley · 2025-01-10T06:30:45Z

+
+		self.keyNames = names = []
+		for group, groupKeysDown in keysDown.items():
+			if group == LOC_BRAILLE_KEYS and len(keysDown) == 1 and not groupKeysDown & 0xF8:


Why 0xf8?

SaschaCowley · 2025-01-10T06:34:22Z

 * Automatic language switching is now supported when using Microsoft Speech API version 5 (SAPI5) and Microsoft Speech Platform voices. (#17146, @gexgd0419)
 * NVDA can now be configured to speak the current line or paragraph when navigating with braille navigation keys. (#17053, @nvdaes)
 * In Word, the selection update is now reported when using Word commands to extend or reduce the selection (`f8` or `shift+f8`). (#3293, @CyrilleB79)
+* NLS eReader Zoomax driver has been added. Previously it was available only as a separate addon.


Suggested change

* NLS eReader Zoomax driver has been added. Previously it was available only as a separate addon.

* Support for the NLS eReader Zoomax braille display has been added. (#17509, @florin-trutiu)

SaschaCowley · 2025-01-10T06:41:37Z

+
+The NLS eReader Zoomax device supports USB or bluetooth connections.
+The Windows 10 and Windows 11 operating systems will automatically detect and install the necessary drivers for this display.
+For computers where the Internet connection is disabled or not available, for the USB connection, a driver can be download from the USB to serial CH340 chip manufacturer  (https://www.wch-ic.com/downloads/CH341SER_EXE.html)


Suggested change

For computers where the Internet connection is disabled or not available, for the USB connection, a driver can be download from the USB to serial CH340 chip manufacturer (https://www.wch-ic.com/downloads/CH341SER_EXE.html)

For computers where the Internet connection is disabled or not available, you can manually [download and install the USB to serial CH340 chip driver](https://www.wch-ic.com/downloads/CH341SER_EXE.html) to support this display over USB.

seanbudd · 2025-03-03T00:30:50Z

@florin-trutiu - Is this ready for re-review? I noticed you pushed some changes in January, but this was never marked as ready for review again

Adriani90 · 2025-05-22T22:57:01Z

it seems @florin-trutiu is not quite active anymore on this repo. Should we mark this as abandoned? Or should we ask within the community whether someone else wants to take it over?

florin-trutiu · 2025-05-23T05:04:48Z

@Adriani90
The NLS eReader Zoomax braille driver is/was similar with the rest of the Braille display drivers already present in the NVDA sources.
The problem appeared when I was requested by @SaschaCowley to make multiple changes to the driver, although, like I mentioned, the driver was similar with the rest of the drivers and those drivers work and do not have the changes I was requested to make.
At that time I replied to @SaschaCowley and told him that I talked with my manager and that I will make the changes, but as it required major changes, I could not find the time to do it..
I will discuss again the issue with those in charge to see if I can move it higher in my priorities list.

florin-trutiu requested a review from a team as a code owner December 12, 2024 11:23

florin-trutiu requested a review from SaschaCowley December 12, 2024 11:23

seanbudd self-requested a review December 12, 2024 22:56

florin-trutiu requested a review from a team as a code owner December 13, 2024 11:34

florin-trutiu requested a review from Qchristensen December 13, 2024 11:34

SaschaCowley added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Dec 17, 2024

SaschaCowley reviewed Jan 10, 2025

View reviewed changes

SaschaCowley requested changes Jan 10, 2025

View reviewed changes

SaschaCowley marked this pull request as draft January 13, 2025 00:31

florin-trutiu closed this Jun 16, 2025

florin-trutiu force-pushed the master branch from d1d3f6c to 8c9efe8 Compare June 16, 2025 09:01

florin-trutiu mentioned this pull request Jun 16, 2025

NLS eReader Zoomax braille driver #18260

Closed

5 tasks

	arg = sum(byte << offset * 8 for offset, byte in enumerate(arg))
	arg = int.from_bytes(reversed(arg))

	* NLS eReader Zoomax driver has been added. Previously it was available only as a separate addon.
	* Support for the NLS eReader Zoomax braille display has been added. (#17509, @florin-trutiu)

	For computers where the Internet connection is disabled or not available, for the USB connection, a driver can be download from the USB to serial CH340 chip manufacturer (https://www.wch-ic.com/downloads/CH341SER_EXE.html)
	For computers where the Internet connection is disabled or not available, you can manually [download and install the USB to serial CH340 chip driver](https://www.wch-ic.com/downloads/CH341SER_EXE.html) to support this display over USB.

Uh oh!

Conversation

florin-trutiu commented Dec 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Link to issue number:

Summary of the issue:

Description of user facing changes

Description of development approach

Testing strategy:

Known issues with pull request:

Code Review Checklist:

Uh oh!

seanbudd commented Dec 13, 2024

Uh oh!

seanbudd commented Dec 13, 2024

Uh oh!

florin-trutiu commented Dec 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CyrilleB79 commented Dec 13, 2024

Uh oh!

SaschaCowley left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seanbudd commented Mar 3, 2025

Uh oh!

Adriani90 commented May 22, 2025

Uh oh!

florin-trutiu commented May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

florin-trutiu commented Dec 12, 2024 •

edited

Loading

florin-trutiu commented Dec 13, 2024 •

edited

Loading

florin-trutiu commented May 23, 2025 •

edited

Loading