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 driver for Eurobraille Esys, Esytime and newer Iris displays #7859

Merged
merged 14 commits into from Feb 9, 2018

Conversation

Projects
None yet
3 participants
@leonardder
Collaborator

leonardder commented Dec 15, 2017

Link to issue number:

Closes #7488.
Note that this also contains the commits from #7732, which is currently incubating. I'm sure git merging will deal with that properly.

Summary of the issue:

For Eurobraille displays, users currently rely on an add-on packaged driver that has some drawbacks for NVDA users, including:

  1. The inability to assign gestures for many keys using NVDA's input gesture framework
  2. The use of a custom braille input layer
  3. The add-on is quite huge to distribute and, due to its size, can't be integrated in NVDA core
  4. The add-on based driver won't support braille display auto detection (#1271)

Description of how this pull request fixes the issue:

This pr implements a native driver for Eurobraille displays, with support for automatic and manual port selection. Note that for Iris displays, port selection should always occur manually, as these are serial displays.

For Esytime, the driver supports switching between protocol based input (i.e. input handled by NVDA) and HID simulation input. Support for HID simulation might be added to Esys in the future.

The driver also contains key bindings as requested by Eurobraille. There ought to be proper support for several firmware versions, including newer braille displays which send acknowledgement packets. Due to the acknowledgement handling required, this driver requires #7732, and therefore this pr contains commits from that pr. All changes to braille.py are already incubating and thus don't need additional review.

Eurobraille wants us to provide other applications access to a device while NVDA is using it. Because of that, hwIo.Hid now takes an additional parameter exclusive, which defaults to True. If set to False as in the Eurobraille driver, access to the device handle is not restricted for other applications. As this is a specific manufacturer request, Any issues caused by this behaviour should be discussed with Eurobraille.

Testing performed:

I tested an Esys 12 and Esys 40 myself. I also owe many thanks to @clementb49 and @Andre9642, the former tested with an Esytime Evo I belief. Last but not least, many thanks go out to Eurobraille, as they performed tests with Esys 64 and 80, Esytime and Iris displays.

Known issues with pull request:

Eurobraille requested some key assignments which could not yet be made (e.g. toggling control or shift). This will be possible whenever #7843 is implemented.

Change log entry:

  • New Features

    • Support for Eurobraille Esys, Esytime and Iris braille displays. (#7488)
  • Changes for developers

    • hwIo.Hid now takes an additional parameter exclusive, which defaults to True. If set to False, other applications are allowed to communicate with a device while it is connected to NVDA. (#7859)

@leonardder leonardder requested a review from michaelDCurran Jan 16, 2018

@michaelDCurran

@leonardder: could you please merge master into this pr again now that acknowledgement packets has been merged to master? This will make the diff easier to read. Strangely git diff and github are still showing a lot of code being added which is already in master, such as CreateWaitableTimer.

| switch to next review mode | NVDA+numpad7 | NVDA+pageUp | 2-finger flick up | switches to the next available review mode |
| switch to previous review mode | NVDA+numpad1 | NVDA+pageDown | 2-finger flick down | switches to the previous available review mode |
| Switch to next review mode | NVDA+numpad7 | NVDA+pageUp | 2-finger flick up | switches to the next available review mode |
| Switch to previous review mode | NVDA+numpad1 | NVDA+pageDown | 2-finger flick down | switches to the previous available review mode |

This comment has been minimized.

@michaelDCurran

michaelDCurran Jan 18, 2018

Contributor

What changed here?

This comment has been minimized.

@leonardder

leonardder Jan 18, 2018

Collaborator

The initial command descriptions did not start with an upper case letter. Now they do. I actually had forgotten I had changed this. I can revert if you like, but it looked more consistent.

@@ -55,6 +56,58 @@ def CreateFile(fileName,desiredAccess,shareMode,securityAttributes,creationDispo
return res
def createWaitableTimer(securityAttributes=None, manualReset=False, name=None):

This comment has been minimized.

@michaelDCurran

michaelDCurran Jan 18, 2018

Contributor

I am confused with this diff. Isn't this all already in master? Did github get this wrong?

@@ -55,6 +56,58 @@ def CreateFile(fileName,desiredAccess,shareMode,securityAttributes,creationDispo
return res
def createWaitableTimer(securityAttributes=None, manualReset=False, name=None):

This comment has been minimized.

@michaelDCurran

michaelDCurran Jan 18, 2018

Contributor

I think github is not showing this right... but it looks like CreateWaitableTimer and SetWaitableTimer are being added here, yet they already exist in master I think.

@michaelDCurran

This comment has been minimized.

Contributor

michaelDCurran commented Jan 18, 2018

@leonardder: could you please merge master into this pr again now that acknowledgement packets has been merged to master? This will make the diff easier to read. Strangely git diff and github are still showing a lot of code being added which is already in master, such as CreateWaitableTimer.

@leonardder

This comment has been minimized.

Collaborator

leonardder commented Jan 18, 2018

Just did :)

@@ -46,6 +46,7 @@ def GetStdHandle(handleID):
GENERIC_WRITE=0x40000000
FILE_SHARE_READ=1
FILE_SHARE_WRITE=2
FILE_SHARE_DELETE=4

This comment has been minimized.

@leonardder

leonardder Jan 18, 2018

Collaborator

This one isn't used, but it was the only access right constant that was missing, so people who use winKernel might expect it should be there.

michaelDCurran added a commit that referenced this pull request Jan 18, 2018

leonardder added some commits Jan 26, 2018

Add esysuite and esybraille appModules on request of Eurobraille. The…
…se appModules simply set NVDA into sleep mode, as these applications are self-voicing/self-brailling

michaelDCurran added a commit that referenced this pull request Feb 1, 2018

@michaelDCurran michaelDCurran merged commit da5f33b into nvaccess:master Feb 9, 2018

@nvaccessAuto nvaccessAuto removed the incubating label Feb 9, 2018

@nvaccessAuto nvaccessAuto added this to the 2018.1 milestone Feb 9, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment