Add support for new braille display from Nattiq by m-noman · Pull Request #10778 · nvaccess/nvda

After further checking and testing, it seems the driver is stable and good to merge,
so that NVDA can officially have the support for the Nattiq nBraille systems.
@LeonarddeR @feerrenrut @Adriani90
if someone can review,
Thanks.

LeonarddeR

Overall, this looks pretty nice.

Could you say anything about how widely this display is used?

AppVeyorBot · 2020-02-15T07:11:24Z

PR introduces Flake8 errors
Build (for testing PR)

See test results for failed build of commit e0c10840c5

m-noman · 2020-02-15T07:15:11Z

@LeonarddeR
Thank you for the feedback,
This Braille display is actually New.
It was released around December last year, so normally it is rather new and does not yet have a huge user-base, but I expect it may get more exposure due to its low price comparing to other brands.
It also have support for other screen readers like HAL/Supernova, and so this is how I created this driver. All testing I am doing is on the braille display unit.

I edited out the comments, I am not sure if it is now better, or need more comments/explanations?
I will keep testing and will await the feedback for the changes.

Thank you.

LeonarddeR · 2020-02-17T11:37:26Z

Are there actually any docs about this display, i.e. this driver is implemented using serial communication, but is this a usb to serial converter? If so, you might want to add the vid and pid of the converter to bdDetect in order for automatic braille display detection to work.

m-noman · 2020-02-17T12:28:33Z

Are there actually any docs about this display, i.e. this driver is implemented using serial communication, but is this a usb to serial converter? If so, you might want to add the vid and pid of the converter to bdDetect in order for automatic braille display detection to work.

I don't think there are things that are publicly available.
but the Braille display is not using a usb to serial converter, rather it is using a hardware based usb controller. It is based on Atmel micro-controllers. Though its PID/VID will be kind of generic as they are for the Atmel micro-chip, but I checked the bdDetect.py file and it is possible to add them there for auto-detection. Though manually, the braille display seems to works fine, on starts/reboots/ and even re-installs ( not sure how? or this might be normal )

AppVeyorBot · 2020-02-17T12:42:37Z

PR introduces Flake8 errors
Build (for testing PR)

See test results for failed build of commit 049ea2751c

m-noman · 2020-02-17T13:28:11Z

@LeonarddeR
I just noticed that now NVDA uses python 3 instead of 2.
I guess that was the reason why it worked for me with either bytes or strings.
Now when I updated to python 3 and VS2019, I can not write strings like before,
and because the unit expect a string encoded value, so I will just use the dash and cell data as strings then encode the output for serial.write().
I tried it out and it seems to works fine. I will await your review.
Also the auto-detect works now.

Thank you.

Co-Authored-By: Leonard de Ruijter <leonardder@users.noreply.github.com>

AppVeyorBot · 2020-02-17T13:59:43Z

PR introduces Flake8 errors
Build (for testing PR)

See test results for failed build of commit 93a1d731a1

LeonarddeR

I think the code is nice as it is now, thanks for your contribution.

Next steps would be updating the user guide

Add a paragraph about the display to chapter 15 (supported braille displays) of the user guide: userDocs\en\userGuide.t2t
Add the driver to the Displays supporting automatic detection in the background section

added Nattiq nBraille displays description.

m-noman · 2020-02-19T09:24:47Z

I think the code is nice as it is now, thanks for your contribution.

Next steps would be updating the user guide
* Add a paragraph about the display to chapter 15 (supported braille displays) of the user guide: userDocs\en\userGuide.t2t

* Add the driver to the `Displays supporting automatic detection in the background` section

I added the display to the list of automatic supported displays, and also a small paragraph with the keys for the display.
I put it above BRLTTY, I do not think this needs to be ordered by alphabetic? because the others are not in order. I only choose it to be above BRLTTY, as that is not a display but rather a protocol/program, so technically, it is now the last ( if the order was meant to be from older to newer )
@LeonarddeR if there are any problems, or it needs to have more information, I will try to add more. but mostly I do not think we need to have more than this, as the link I attached ( in the very first post ) have all the technical details etc.

Thanks.

LeonarddeR

Three small things:

Please start a new line after every sentence in the user guide
manufacturer website > manufaturer's website
Also, see my comment about missing assignments.

added the previous/back assignment, and edited the first line spelling error.

feerrenrut

This looks fine to me, though I am unable to test it. I will wait for a review from @michaelDCurran

The release process has already started for 2020.1 I propose this goes into the 2020.2 release.

michaelDCurran · 2020-03-09T19:26:57Z

@feerrenrut to be clear, obviously I can't test this either as I don't have access to one of these displays.

m-noman added 2 commits February 11, 2020 16:21

Add files via upload

099ba1e

Nattiq Braille display support

Update nattiqbraille.py

28883c2

Update nattiqbraille.py

db2de89

Update nattiqbraille.py

879ca8e

Update nattiqbraille.py

4eb1e05

Update nattiqbraille.py

b281593

Update nattiqbraille.py

229ebde

Update nattiqbraille.py

2511fc7

Update nattiqbraille.py

b2ae21c

LeonarddeR suggested changes Feb 13, 2020

View reviewed changes

Update nattiqbraille.py

708a27a

Update nattiqbraille.py

0b257a0

m-noman requested a review from LeonarddeR February 15, 2020 07:52

LeonarddeR reviewed Feb 17, 2020

View reviewed changes

Update nattiqbraille.py

67265b0

Update bdDetect.py

3a0a934

Update bdDetect.py

4df03f1

Update nattiqbraille.py

a33daa0

LeonarddeR reviewed Feb 17, 2020

View reviewed changes

Comment thread source/brailleDisplayDrivers/nattiqbraille.py Outdated

Comment thread source/bdDetect.py

Comment thread source/brailleDisplayDrivers/nattiqbraille.py

m-noman and others added 2 commits February 17, 2020 05:49

Update source/brailleDisplayDrivers/nattiqbraille.py

d952ace

Co-Authored-By: Leonard de Ruijter <leonardder@users.noreply.github.com>

Update bdDetect.py

fc9ad9c

Update nattiqbraille.py

8579262

m-noman requested a review from LeonarddeR February 19, 2020 07:12

LeonarddeR reviewed Feb 19, 2020

View reviewed changes

Update userGuide.t2t

8c2bb2e

added Nattiq nBraille displays description.

m-noman requested a review from LeonarddeR February 23, 2020 09:19

LeonarddeR reviewed Feb 24, 2020

View reviewed changes

Comment thread user_docs/en/userGuide.t2t Outdated

Comment thread user_docs/en/userGuide.t2t

Update userGuide.t2t

9666b63

added the previous/back assignment, and edited the first line spelling error.

m-noman requested a review from LeonarddeR February 24, 2020 09:46

LeonarddeR approved these changes Mar 4, 2020

View reviewed changes

LeonarddeR requested a review from michaelDCurran March 4, 2020 12:59

feerrenrut approved these changes Mar 9, 2020

View reviewed changes

feerrenrut added this to the 2020.2 milestone Mar 9, 2020

michaelDCurran approved these changes Mar 9, 2020

View reviewed changes

feerrenrut merged commit 8ae6dca into nvaccess:master Mar 20, 2020

feerrenrut added a commit that referenced this pull request Mar 20, 2020

Update changes file for PR #10778

ced1faf

feerrenrut added component/braille-display-drivers enhancement labels Mar 20, 2020

Uh oh!

Conversation

m-noman commented Feb 11, 2020

Link to issue number: /

Summary of the issue: /

Description of how this pull request fixes the issue: /

Testing performed: Testing is done with the Nattiq Braille Display (USB)

Known issues with pull request: /

Change log entry:

Uh oh!

AppVeyorBot commented Feb 11, 2020

Uh oh!

AppVeyorBot commented Feb 11, 2020

Uh oh!

AppVeyorBot commented Feb 11, 2020

Uh oh!

AppVeyorBot commented Feb 11, 2020

Uh oh!

AppVeyorBot commented Feb 11, 2020

Uh oh!

AppVeyorBot commented Feb 12, 2020

Uh oh!

AppVeyorBot commented Feb 12, 2020

Uh oh!

m-noman commented Feb 13, 2020

Uh oh!

LeonarddeR left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AppVeyorBot commented Feb 15, 2020

Uh oh!

m-noman commented Feb 15, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

LeonarddeR commented Feb 17, 2020

Uh oh!

m-noman commented Feb 17, 2020

Uh oh!

AppVeyorBot commented Feb 17, 2020

Uh oh!

m-noman commented Feb 17, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AppVeyorBot commented Feb 17, 2020

Uh oh!

LeonarddeR left a comment

Choose a reason for hiding this comment

Uh oh!

m-noman commented Feb 19, 2020

Uh oh!

LeonarddeR left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

feerrenrut left a comment

Choose a reason for hiding this comment

Uh oh!

michaelDCurran commented Mar 9, 2020

Uh oh!

Reviewers

Assignees