Braille Sense display driver #1266

Closed
nvaccessAuto opened this Issue Dec 8, 2010 · 25 comments

3 participants

@nvaccessAuto

Reported by Casalino on 2010-12-08 13:45
I developped this driver that lets use the Braille Sense display device from Hims with NVDA. At this point you can use all scrolling keys and the main qwerty or key combination keys as tab, arrows, shift , enter and esc.
I can not completely support all the keyboard as keys are coded using dots rapresentation and I can not find a way to handle localization issues (for example various braille tables).
I implemented an auto scan mode to find the right com or usb port at wich Braille Sense is connected. It seems to be fast enough, but probably I could restrict the scan between the existing com port as indicated in the windows registry.

@nvaccessAuto

Attachment BrailleSenseNvdaDriver_v0.5.zip added by Casalino on 2010-12-08 13:46
Description:

@nvaccessAuto

Comment 1 by jteh on 2011-11-07 06:38
It seems this ticket was overlooked. Sorry.

I've only looked at the code briefly so far. A few points:

  • Where did you obtain !HanSoneConnect.dll? Are you certain it is legal for us to distribute this with NVDA?
  • Are you willing to update this driver soon and maintain it into the future?
  • In order to get this included into NVDA now, it needs to be updated to use the new input gesture framework. That is, all keys must be recognised and converted to input gestures. This way, a user can provide their own key bindings if desired.
@nvaccessAuto

Attachment braillesense_for_2011.2_and_next.zip added by drein on 2011-11-16 09:45
Description:

@nvaccessAuto

Comment 2 by drein on 2011-11-16 09:46
Hello,
to be onest, the develop of this driver was stopped for a while, since nobody was interested in it.
But, since bot Gianluca Casalino and me have a Braillesense, Gianluca decide to continue the driver. So:
1. here you can download a driver that works for NVDA 2011.2 and next version.
In this driver, there is an autoScan function, so don't worry about the Sense connection. If you are connected via USB or via bluetooth or serial port, the driver will understand by itself.
This function can be improved to make the scan only in ports that are in the system registry, but, for now it works...
Remember that you must pair bluetooth manually in order to use it, and you should install the USB Driver, but I think you have it.
2. You can use your Sense to write directly to the computer. This is a great advantage because you can for example connect via bluetooth and use your Braillesense away, and you can write using your qwerty or your perkins keyboard, depending of your braillesense model.
The problem is that we have mapped for Italy, we have no idea of what happen with other sense model. Please try and let us know, and if you need other key, we can add them.

  1. With the driver we put the HanSoneConnect.dll. This dll was given us by the Hims Engeneers, and they were really available to develop new feature and help us. We have no problem including the dll, because they are agree. Copy the dll in the nvda\brailledisplaydrivers folder, instead the .py file should be copied into the userconfig\brailledisplaydrivers folder if you are using a portable, or go to start menu, programs, nvda, explore user nvda settings, select brailledisplaydrivers and copy there if you are using the installer.

Remember that the driver is in a beta stage, and we hope that one day this will be included in main NVDA, like other display braille.

Best regards.

@nvaccessAuto

Comment 3 by drein on 2011-11-16 09:49
Sorry Jamie...
I assume you have a Braillesense, and I don't think it is so! Be patient... it is a strange day.
Anyway, I hope some user outside Italy make some tests.

@nvaccessAuto

Comment 4 by drein on 2012-01-23 10:35
The driver is working well, both with Braillesense standard both with Qwerty.

@nvaccessAuto

Comment 5 by jteh on 2012-01-23 23:10
Since you have to install a USB driver anyway, does it install HanSoneConnect.dll on the system as well? It'd make sense for the driver to include this, since it is needed to communicate with the display, rather than having to bundle it with the screen reader.

@nvaccessAuto

Comment 6 by drein on 2012-01-24 08:34
The driver installation is necessary when you use a USB connection.
If you use simply the comPort or the BlueTooth adapter, driver installation is not necessary. Since many users use the bluetooth, they don't install the driver and can use NVDA with Braillesense without problems.

Onestly, I don't remember if this file is installed when a user install the USB driver also for other screen readers.
I'm using always the bluetooth connection, since I like it and I hate the wires!

@nvaccessAuto

Comment 7 by orcauser on 2012-02-14 11:30
There is an arabic user with a 18 cell braille sense, because brltty assumes all braille sense displays are 32 cells this caused her problems.

Maybe we should consider louis_backtranslate, to insert the correct characters based on the current nvda braille table.

Thanks for your work on this.

@nvaccessAuto

Comment 8 by drein on 2012-03-29 07:27
A little update:
1. The dll is necessary also for usb, in fact this file is not present in other parts of my hard disk, so we must include it.
2. The driver works also with the new Braille Sense u2 model, without any modifications.

@nvaccessAuto

Comment 9 by orcauser on 2012-04-17 14:54
@Jamie, Whats the best way forward?
@Simone, What do you think about moving the hard coded key mappings from the driver, to a gestures.ini that the user can include.
At least then they have a working display, with the basic functionality.

Thanks.

@nvaccessAuto

Comment 10 by drein on 2012-05-08 09:39
Hello,
I can make the necessary changes in the spare time, since I should take a lots of command and transform it in gesture. This is possible, but I want to know if Jamie is agree to include the dll, because it is not useful make a work for nothing. If Jamie gives the OK, then I start work on it.
I can also tell you that this driver works partially also with the new model of hims Display Braille, Braille Edge.
There are some additional keybindings to map, I should discuss with Gianluca about it, anyway, please let me know about the dll and then we can decide how to proceede.

@nvaccessAuto

Comment 11 by jteh on 2012-05-21 04:24
We will include the dll. It's not ideal, but we don't have another choice in this case (as per comment:6 and comment:8).

I'll try to review this properly soon. Sorry for the huge delay. However, there are two issues first off:

  1. NVDA doesn't currently support braille input. I don't want to hard-code support in one driver which is tied to specific languages or the like, so this would need to be removed for the driver to be included. When NVDA does get proper support for braille input, this driver will of course benefit.
  2. The auto probe functionality worries me. Probing could potentially confuse other devices. It should be possible to support USB and bluetooth without probing as we do for other drivers. Unfortunately, this means we can't support serial displays for now, but that's also true for other drivers. Changes: Milestone changed from None to near-term
@nvaccessAuto

Comment 12 by jteh on 2012-05-22 03:22
Thanks very much for this contribution. The code looks great for the most part.

Some code review:

cosmetic: There should be a blank line after the header comments and then another after the imports.

cosmetic: The BS_KEYS dict has the closing brace on the same line as the last entry. For readability, it probably makes sense to put it on the next line in this case:

    16384: 'scroll_right',
}

There's no need to import speech, wx or tones, as they aren't used.

The releasedKeys set doesn't seem to be used anywhere and should be removed.

def nvdaBsBrlWndProc(hwnd,msg,wParam,lParam):
...


  elif msg == nvdaBsBrlWm and wParam == BS_CURSORROUTING:

...


      except inputCore.NoInputGestureAction:

          pass        

cosmetic: Remove tabs at end of line.


      log.info(lParam)

Debugging code that should be removed.


  def check(cls):

      return True #bool(bsLib)

cosmetic: Comment at end of line should probably be removed.


  def __init__(self):

...


          for n in xrange(1,256):

              port=("com%d"%n)

As noted in comment:11, this auto probing worries me. The dll seems to be able to find USB displays itself, so we don't need to worry about those. Unless the bluetooth name can be changed in its entirety, we should be able to find bluetooth displays using the bluetooth name, though we'll need a list of bluetooth name patterns from HIMS. See the way i do this in the baum and brailliantB drivers for an example.


  def _get_numCells(self):

cosmetic: This line has an extra space at the start which should be removed.


  gestureMap = inputCore.GlobalGestureMap({

cosmetic: There should be a blank line before this.

As noted in comment:11, any braille key bindings that only involve dots should be removed, as they produce characters. Most of the others can stay. I'm less certain about chords that only involve space, as we may eventually want to have standard space+dot chords. For now, you can leave them, but be aware that they may be removed in a future release.

We need a User Guide section for this driver, including the key bindings. Please see the existing braille display sections in the User Guide for examples.

This final point is more an observation than anything you need to worry about. Normally, I'd prefer to keep all state (i.e. variables that aren't constants or global handles such as pressedKeys, _ignoreKeyReleases, etc.) in the class instance, rather than at module level, as they're technically instance specific, not global. In this case, it's harder because the callback is defined at module level. You can actually define it in the instance, but it's trickier. See the alvaBC6 driver for an example. However, this wouldn't stop us from including the driver.

@nvaccessAuto

Comment 13 by jteh on 2012-05-22 04:24
You mention that this works partially with the Braille Edge. What do you mean by partially; i.e. what doesn't work? Do you know if this works with any other HIMS displays? We should probably rename the driver accordingly once we know for sure.

@nvaccessAuto

Comment 14 by drein on 2012-05-22 08:22
The driver is able to connect with Braille Edge, and the two navigation keys at the left of the display works properly, so you can scroll right and left the text.
For what I remember, since now I have no Braille Edge here with me, also cursor routing works.
What for now is not working is:

  • function keys from f1 to f8. Perhaps one or two are ok, but it is a lucky event.
  • The two joysticks, they can't work until we assing bindings to them.

For what i remember, the input keyboard work, because it was the same configuration of BrailleSense.

the driver doesn't have official support for now for Braille Edge, and for now it works because some parts of the protocol and keybindings are the same of BrailleSense.

For what concern source code, I think Gianluca will look into it soon.

@nvaccessAuto

Comment 15 by drein on 2012-06-04 15:36
Just to give an update for this driver:
gianluca fixed some issues and rewrote some procedures in order to be fully compliant with NVDA policy.
The driver actually makes a port scan, only for USB and bluetooth.
Finally, I received Braille Edge and other HIMS products, so the test are started.
Only the standard gestures are taken, the user will be able to define its customed gesture for inputting text for example.

We have only one trouble, the driver works with all HIMS devices, except SyncBraille.
This because HIMS decided to not include SyncBraille support in the "handsoneconnect.dll", but they prefer to use another DLL for that display braille.
So, we are quite undecided, or Gianluca make one driver with 2 dll, or 2 drivers with 1 dll for each driver, or, eventually, we could consider to not include that display braille at all.

I asked to HIMS Engeneers if it was possible to unify the dll and have one for all Brailledisplays, but they prefer to rest with 2 dll.
It is a pitty because they are really similar and they differ in the "funnctionNames". SyncBraille works properly with the syncbraille.dll.

@nvaccessAuto

Comment 16 by jteh (in reply to comment 15) on 2012-06-04 22:59
Replying to drein:

We have only one trouble, the driver works with all HIMS devices, except SyncBraille.

This because HIMS decided to not include SyncBraille support in the "handsoneconnect.dll", but they prefer to use another DLL for that display braille.

So, we are quite undecided, or Gianluca make one driver with 2 dll, or 2 drivers with 1 dll for each driver, or, eventually, we could consider to not include that display braille at all.

I'd keep !SyncBraille as a separate driver. However, if you want to avoid code duplication, you could subclass the original HIMS driver and just override specific methods. This would require the code to be changed to not load hansoneConnect.dll at import time, though.

@nvaccessAuto

Comment 17 by drein on 2012-06-06 13:33
the driver is finished.
Now we have a complete support for BrailleSense PLUS, BrailleSense Qwerty, BrailleSense U2, Sense on hand and Braille Edge display braille.
all this display work both with bluetooth both USB.
Note that unfortunately Syncbraille has a separate driver due to the incompatibility with the main dll, so it is on a separate ticket #1267.
I attach the driver with the documentation in t2t format.
The user can define its gesture in order to implement keybinding to write with the builtin perkins keyboard.

Tested with all devices!

@nvaccessAuto

Attachment senseEdgeDrivers.zip added by drein on 2012-06-06 13:35
Description:

@nvaccessAuto

Comment 18 by jteh on 2012-06-07 02:05
Thanks! Just a couple of minor issues:

HIMS_CODE_DEVICES = {
      1: 'Braille Sense (2 scrools mode)',

Typo: scrools -> scrolls. Cosmetic: Also, this dict is indented with two tabs; it should be one. The closing brace has two unnecessary tabs after it.

For the key identifiers, is there any reason you've used underscores rather than camel case as in other NVDA gestures? For example, right_side_scroll_down instead of rightSideScrollDown.

In InputGesture.__init__:

      global HIMS_KEYS

Cosmetic: There's no need to declare this as global, as it's a constant and you're not assigning to it.

          try:
              if type(HIMS_KEYS[== dict: 
                  name = (HIMS_KEYS[value](value]))[if deviceFound in HIMS_KEYS[value](deviceFound]).keys() else HIMS_KEYS[               else:
                  name = HIMS_KEYS[value](value]['BrailleSense'])
)
              names.add(name)
          except:
              pass

You're fetching ```HIMS_KEYS[a lot here, which is suboptimal. keys() is deprecated and isinstance is more elegant than type() here. Finally, the except clause is open instead of catching a specific exception. I'd change this code to the following:

            try:
                name = HIMS_KEYS[value](value]```)
                if isinstance(name, dict):
                    try:
                        name = name[KeyError:
                        name = name["BrailleSense"](deviceFound]
                    except)
                names.add(name)
            except KeyError:
                pass

Once again, thanks for all of your great work.

@nvaccessAuto

Comment 19 by jteh on 2012-06-07 02:09
Changes:
Milestone changed from near-term to 2012.3

@nvaccessAuto

Comment 21 by jteh on 2012-06-07 07:48
I'll take care of the changes needed here and have already started working on them.

@nvaccessAuto

Comment 22 by jteh on 2012-06-07 08:19
Merged in ea9cd6f including several changes by me. If you're interested in the changes, please take a look at the merged revisions. Thanks for your great work.

I'm closing as fixed, but please test to make sure none of my changes broke anything. Please reopen if I broke something.
Changes:
State: closed

@jcsteh jcsteh was assigned by nvaccessAuto Nov 10, 2015
@nvaccessAuto nvaccessAuto added this to the 2012.3 milestone Nov 10, 2015
@leonardder

I've been testing with the current driver and the BrailleEdge. Is it true that f5-f8 still have no function yet? If so, I'd like to investigate whether people would benefit from adding these to the driver.

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