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.

Sign up for GitHub

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

Jump to bottom

Native driver for Freedom Scientific displays #8853

Merged
merged 59 commits into from May 21, 2019
Merged

Native driver for Freedom Scientific displays #8853

merged 59 commits into from May 21, 2019

Conversation

bramd
Copy link
Contributor

@bramd bramd commented Oct 17, 2018 •
edited by LeonarddeR

Edited by @LeonarddeR

Link to issue number:

Fixes #7727
fixes #4464
fixes #8849
fixes #8729

Summary of the issue:

The current Freedom Scientific braille display driver requires a binary component from Freedom Scientific. This prevents the driver from integrating fully with NVDA features like braille display auto detection. Also, braille input is not supported by the current driver.

Description of how this pull request fixes the issue:

  1. This is a rewrite of the driver in pure Python. It still requires the USB driver from Freedom Scientific for working with displays over USB, but the communication with the display is now handled in Python.

    It also supports braille input and auto detection.

  2. This PR adds a new feature to braille auto detection as well. It keeps searching for a USB display when a Bluetooth connection is active, it will only connect to USB devices for the same driver. This is because a Focus display switches to USB when a cable is plugged in, even if a Bluetooth connection is active. The old binary driver does auto switching to USB as well, so not supporting this would be a regression for users. This also works for other display brands.

Testing performed:

  • Tested with a Focus 40 blue 4th generation over USB and Bluetooth by me

  • Tested with a Focus 40 fifth generation by @MarcoZehe over USB and Bluetooth

  • Other users have done tests as well during development of this driver, see Native driver for Freedom Scientific braille displays #7727

  • @LeonarddeR tested the new bdDetect behavior as follows:

    1. Connect a Handy Tech Basic Braille via Bluetooth. The display is detected
    2. Connect a Handy Tech Easy Braille via USB. The Basic Braille is automatically disconnected in favour of the USB device.
    3. Disconnect the Easy Braille. NVDA switches back to the Basic Braille over bluetooth automatically.
    4. Connect the Handy Tech Easy Braille via USB again. The Basic Braille is automatically disconnected in favour of the USB device.
    5. Again, disconnect the Easy Braille. NVDA switches back to the Basic Braille over bluetooth automatically.
    6. Connect an Eurobraille Esys over USB. NVDA keeps the connection to the Basic Braille over Bluetooth, since the Esys doesn't belong to the Handy Tech driver

Known issues with pull request:

  • None

Change log entry:

Leonard de Ruijter and others added 30 commits September 25, 2017 11:27
Attempt to create a new hwIo class for USB bulk devices
c1784f0
@bramd
Merge branch 'bulk' into freedomscientific-native
dd6fae5
@bramd
Native driver for Freedom Scientific braille displays, work in progre…
20da169 
…ss. Braille output and wizWheels are working. Tested on a Focus 40 blue
@bramd
Key and extended key handling
882a307 
 * Use a writeSize of 0 to dynamically allocate a write buffer
 * Enable extended keys for newer Focus models
 * Handle keys and extended keys
@bramd
Merge remote-tracking branch 'upstream/master' into freedomscientific…
52f041e 
…-native
@bramd
Add support for routing and second row of buttons on older Focus/Pacm…
bff76e8 
…ate displays (hotkeys)
@bramd
Merge remote-tracking branch 'upstream/master' into freedomscientific…
2bddae8 
…-native
@bramd
Properly implement Freedom Scientific displays connected via Bluetooth
f17a225 
 * Queue cells when an ACK is pending, so we don't overflow the display
   with new output
 * Set the correct serial parameters for the Bluetoot hconnection
 * Handle serial communication in _onReceive
 * Fix bug in the _getBluetoothPorts method
@bramd
Merge remote-tracking branch 'upstream/master' into freedomscientific…
a4b647a 
…-native
@bramd
Fix output on Focus series 1 (44/70/84), improve initialization
685d360 
 * Implement a custom translation table for Focus 1 models. This is
   untested for now, since I don't have such a display
 * Better initialization logic to better handle failing ports and
   correctly raise an exception if no display is found
 * Better error handling in hwIo.IoBase and hwIo.Bulk if close is called
   when the device is not initialized or failed to open. This might happen
   if __init__ fails and close is called by __dell_
@bramd
Add USB VID/PID for Focus/PAC mate displays
fd7e914
@bramd
Code documentation and cleanup
e37038a
@bramd
Merge remote-tracking branch 'upstream/master' into freedomscientific…
d49a493 
…-native
@bramd
Fix typo in br(breedomscientific]:leftBumerbarUp gesture. Fix merged …
d60639f 
…incorrectly
@bramd
Mark brailleDisplayDrivers/freedomScientific.py as UTF-8
331e13d
@bramd
Fixed reference from numCells to self.numCells
2249037
@bramd
Merge branch 'master' into freedomscientific-native
f54e54e
@bramd
Enable auto detection of Freedom Scientific braille displays
cef92d2
@bramd
Some code cleanup
d783b49
@bramd
Fix mistake in display initialization introduced in the previous comm…
8364932 
…it and use generic ACK handling
@bramd
Fix key handling if multiple keys are released at the same time
4a50975
@bramd
Don't set _awaitingAck manually in display method
5ad6818
@bramd
Merge branch 'master' into freedomscientific-native
d0f6e6e
@bramd
Merge remote-tracking branch 'upstream/master' into freedomscientific…
a3fb229 
…-native
@bramd
Merge remote-tracking branch 'upstream/master' into freedomscientific…
4f4d6ce 
…-native
@bramd
Code cleanup
363ea52
@bramd
The Focus displays seem to switch to USB as soon as a USB cable is co…
224bdca 
…nnected, even if it's still connected via Bluetooth. If there is a Bluetooth connection, we keep the scan for USB devices active for the current driver, so if a Focus is plugged in it will automatically reconnect via USB and drop the Bluetoot h connection.
@bramd
The Focus displays seem to switch to USB as soon as a USB cable is co…
2194af1 
…nnected, even if it's still connected via Bluetooth. If there is a Bluetooth connection, we keep the scan for USB devices active for the current driver, so if a Focus is plugged in it will automatically reconnect via USB and drop the Bluetoot h connection.
@bramd
Update UserGuide:Freedom Scientific displays support auto detection
4125f0f
@bramd
Add key bindings for win+tab and alt+shift+tab
318439b
jcsteh
jcsteh previously requested changes Apr 9, 2019
View reviewed changes
source/brailleDisplayDrivers/freedomScientific.py Outdated Show resolved Hide resolved
@bramd
Copy link
Contributor Author

bramd commented Apr 10, 2019

@jcsteh You are right, I didn't have a test device while making the last changes. Now I have and I reproduced and fixed the issue. Thanks for testing.

@bramd
Copy link
Contributor Author

bramd commented Apr 13, 2019

Just fixed the last issues and found a problem in the custom translation code for the very first Focus displays. If anyone here has such an old display, please test it with this driver to ensure it now works as expected.

@bramd
Copy link
Contributor Author

bramd commented Apr 15, 2019

After extended use I encountered the following issue two times:

After pressing an unassigned key on the display, I get the following traceback:

error executing script: <function <lambda> at 0x0546F9F0> with gesture 'rightShiftKey'
Traceback (most recent call last):
  File "scriptHandler.pyc", line 187, in executeScript
  File "inputCore.pyc", line 467, in <lambda>
  File "inputCore.pyc", line 153, in send
NotImplementedError

I didn't find a way to reproduce this reliably. Any thoughts @jcsteh, @michaelDCurran, @LeonarddeR?

@lukaszgo1
Copy link
Contributor

Any chance for combining the routing keys with other keys on the display or is it simply impossible with these models?

@LeonarddeR
Copy link
Collaborator

I didn't find a way to reproduce this reliably. Any thoughts @jcsteh, @michaelDCurran, @LeonarddeR?

This looks unrelated to this pr. May be you could file a separate issue?

@LeonarddeR
Copy link
Collaborator

@feerrenrut: Requested another round of review as @bramd noted that this is ready. This is most likely going to change a bit as soon as Python 3 arrives, however it is as good as finished and there's some demand for this being included soon.

@feerrenrut feerrenrut dismissed jcsteh’s stale review May 2, 2019 14:33

The requested changes were addressed.

@feerrenrut
Copy link
Contributor

We would really like to get this PR merged before we get into the python3 migration. I will take a look at clarifying some of the remaining numbers and adding comments, I'll submit a PR to you against this branch. Hopefully this process will illuminate whether or not I have understood how this works.

@feerrenrut feerrenrut mentioned this pull request May 6, 2019
Merged
feerrenrut and others added 3 commits May 20, 2019 11:32
@feerrenrut feerrenrut merged commit 327b2dd into nvaccess:master May 21, 2019
@nvaccessAuto nvaccessAuto added this to the 2019.2 milestone May 21, 2019
feerrenrut added a commit that referenced this pull request May 21, 2019
@feerrenrut
Update changes file for PR #8853
aa280b6
@bramd bramd deleted the freedomscientific-native branch June 8, 2019 21:36
@Adriani90 Adriani90 mentioned this pull request Sep 5, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LeonarddeR LeonarddeR LeonarddeR approved these changes

@feerrenrut feerrenrut feerrenrut approved these changes

@jcsteh jcsteh jcsteh left review comments

@michaelDCurran michaelDCurran Awaiting requested review from michaelDCurran

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
2019.2
7 participants
@bramd @LeonarddeR @MarcoZehe @lukaszgo1 @feerrenrut @jcsteh @nvaccessAuto