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.

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 Freedom Scientific displays #8853

Merged
merged 59 commits into from May 21, 2019

Conversation

@bramd
Copy link
Contributor

commented Oct 17, 2018

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 #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:

  • New features

    • Freedom Scientific braille displays are now supported by braille display auto detection (#7727)
  • Changes

    • When an auto detected braille display is connected via Bluetooth, NVDA will keep searching for USB displays supported by the same driver and switch to a USB connection if it becomes available
  • Bugfixes

    • The bumper keys now work correctly on Freedom Scientific braille displays (#8849)

leonardder and others added some commits Sep 25, 2017

Native driver for Freedom Scientific braille displays, work in progre…
…ss. Braille output and wizWheels are working. Tested on a Focus 40 blue
Key and extended key handling
 * Use a writeSize of 0 to dynamically allocate a write buffer
 * Enable extended keys for newer Focus models
 * Handle keys and extended keys
Properly implement Freedom Scientific displays connected via Bluetooth
 * 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
Fix output on Focus series 1 (44/70/84), improve initialization
 * 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_
The Focus displays seem to switch to USB as soon as a USB cable is co…
…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.
The Focus displays seem to switch to USB as soon as a USB cable is co…
…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

This comment has been minimized.

Copy link
Contributor Author

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

This comment has been minimized.

Copy link
Contributor Author

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

This comment has been minimized.

Copy link
Contributor Author

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

This comment has been minimized.

Copy link
Contributor

commented Apr 25, 2019

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

@leonardder

This comment has been minimized.

Copy link
Collaborator

commented Apr 25, 2019

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 leonardder requested a review from feerrenrut Apr 25, 2019

@leonardder

This comment has been minimized.

Copy link
Collaborator

commented Apr 25, 2019

@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.

The requested changes were addressed.

@feerrenrut

This comment has been minimized.

Copy link
Contributor

commented May 3, 2019

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 and others added some commits May 20, 2019

Update source/brailleDisplayDrivers/freedomScientific.py
Co-Authored-By: Leonard de Ruijter <leonardder@users.noreply.github.com>
Merge pull request #3 from nvaccess/freedomscientific-native
Documentation and code clarification.

@feerrenrut feerrenrut merged commit 327b2dd into nvaccess:master May 21, 2019

1 check passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@nvaccessAuto nvaccessAuto added this to the 2019.2 milestone May 21, 2019

feerrenrut added a commit that referenced this pull request May 21, 2019

@bramd bramd deleted the bramd:freedomscientific-native branch Jun 8, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.