Braille Display Driver for Seika40/80

[nvaccessAuto]

Reported by Ulf on 2012-06-15 10:26
Braille Display Driver for Seika Version 3, 4, 5 (40 cell) and Seika 80.
The Seika displays are connected via USB, it has 8 buttons and 80/80 cells with cursor routing keys.

[nvaccessAuto]

Attachment Seika.py added by Ulf on 2012-06-15 10:28
Description:
braille display driver sourcecode

[nvaccessAuto]

Attachment userGuide.en.html added by Ulf on 2012-06-15 10:30
Description:
snipple html code für english userguide

[nvaccessAuto]

Attachment userGuide.de.html added by Ulf on 2012-06-15 10:31
Description:
html code for german userguide

[nvaccessAuto]

Comment 1 by jteh on 2012-06-15 10:40
Thanks! We'll take a look at this when we can.

If possible, please provide the User Guide snippets in t2t format, as this is the format we use for our User Guide. The format is fairly simple. You can find the t2t file for the English User Guide here.

[nvaccessAuto]

Attachment userGuide.de.t2t added by Ulf on 2012-06-15 11:03
Description:
Userguide snippet for english in t2t

[nvaccessAuto]

Attachment userGuide.de.2.t2t added by Ulf on 2012-06-15 11:04
Description:
Userguide snippet for german in t2t

[nvaccessAuto]

Attachment userGuide.en.t2t added by Ulf on 2012-06-15 11:06
Description:
Userguide snippet for english in t2t

[nvaccessAuto]

Comment 2 by jteh on 2012-06-18 03:01
Thanks for your great work. Here's some code review:

The module should start with a lower case letter; i.e. seika.py instead of Seika.py. This also means BrailleDisplayDriver.name should be "seika" as well.

CELL_COUNT80 = 80
CELL_COUNT40 = 40
CELL_COUNT16 = 16

Is there any need for these constants? IN this case, they don't seem to serve any purpose, since they include the number anyway.

SEIKA_QUERY  = chr(0xFF) + chr(0xFF) + chr(0x1C)

This could be better written as:

SEIKA_QUERY = "\xFF\xFF\x1C"

Similar for the other constants just below it.

SEIKA_SENDHEADER = SEIKA_SENDHEADER40

This is never used, so should be removed.

SEIKA_KEYMAP = [   "LEFT",

This map doesn't seem to correspond in any way with the bytes received from the display, which might make the code cleaner. I'll cover this more below. Also a cosmetic issue: the entries are indented with a tab and a space instead of just a tab.

USB_IDS = frozenset((
  "USB\\Vid_10c4&Pid_ea60", # Seika USB to Ser
        "BTHENUM\\{00001101-0000-1000-8000-00805f9b34fb}"

I assume the BTHENUM entry is debugging? Also, the USB\ part seems redundant if this is for USB ids only.

class BrailleDisplayDriver(braille.BrailleDisplayDriver):
  name = "Seika"

Should start with a lower case letter as above.

  description = "Seika Braille"

Perhaps _("Seika braille displays")

  numCells = CELL_COUNT80

Defining this here seems redundant, as it's always set in __init__().

In __init__():

              usbID = hwID.split("&R", 1)[0](
)

I don't understand what this code does. It doesn't seem like it will ever succeed. Can you explain?

          cells = SEIKA_QUERY
          self._ser.write(cells)

No need for the cells variable; better written simply as:

          self._ser.write(SEIKA_QUERY)

          if versionS.startswith(r"seika80"):
...
              SEIKA_SENDHEADER = SEIKA_SENDHEADER80

This won't have the intended effect because you haven't declared it as a global at the top of the function. Anyway, it's never used, so should be removed.

In display():

      else:
          line = SEIKA_SENDHEADER40 + "".join(chr(0)+chr(cell) for cell in cells)

I'd probably write chr(0) as "\0" to avoid an unnecessary function call.

      expectedLength = SEIKA_SENDHEADER_COUNT + 80
      line += chr(0) * (expectedLength - len(line))

I'm possibly misunderstanding this, but I don't think it's necessary. Braille will always send exactly numCells to the display, padding with 0 if necessary.

In handleResponses():

I know this stuff is always nearly impossible to write elegantly. However, there are a few important problems here:

1. There is a sleep after reading each byte. While small, this will cause lag every time a key is pressed. I'm hoping there must be some way to know how many bytes need to be read in advance. Does the first byte provide information about this perhaps?
2. There is quite a lot of hard-coding here. The determination of keys is done in a big if/elif block. In addition, the indexes into SEIKA_KEYMAP are hard-coded, which makes the key map pointless as a constant.
   One option might be to have a list of the keys corresponding to each bit, combine the ordinal values for all the bytes and then enumerate through the list of keys checking for each bit. You don't even need to combine the ordinal values if you prefer; you could just have a list of keys for each byte. This is how I handle it in the baum driver. At the very least, if the rest of the if/elif block stays the same, I would remove SEIKA_KEYMAP, as it doesn't seem to serve any real purpose.

Btw, another cosmetic thing: j += 1 is probably nicer than j = j + 1. You can do similarly with other operators.

Please let me know if you need further explanation on any of these points. Thanks once again.

[nvaccessAuto]

Comment 3 by Ulf on 2012-06-19 15:05
Hello, I have modifyed a lot. One problem is the key input with the small sleep-time. If the user press a 'normal' key on the braille display, it sends 2 bytes, and if he press a routing key it sends 12 (by the 40 cell) or 22 bytes (for the 80cell display) In a test I have use a slleptime of 0.001 and it works too. But without this sleep I have sometimes lost a key.
And yes, normaly I write "i++;" or "for(i=0;i<x;++i) ...;". This is my first python stuff.
Ulf

[nvaccessAuto]

Comment 4 by jteh (in reply to comment 3) on 2012-06-19 23:14
Replying to Ulf:

One problem is the key input with the small sleep-time. If the user press a 'normal' key on the braille display, it sends 2 bytes, and if he press a routing key it sends 12 (by the 40 cell) or 22 bytes (for the 80cell display) ... without this sleep I have sometimes lost a key.

Do the first 2 bytes sent by the display tell you what kind of message it will be (routing or normal key)? If so, you can read 2 bytes, examine the data and then read 10 more bytes or 20 more bytes as required.

And yes, normaly I write "i++;" or "for(i=0;i<x;++i) ...;". This is my first python stuff.

No problem. My code review is not meant to be accusatory. Hopefully, it's helpful and will ensure the best possible code quality.

[nvaccessAuto]

Attachment seika.py added by Ulf on 2012-06-20 15:22
Description:

[nvaccessAuto]

Comment 5 by Ulf on 2012-06-20 15:38
2 small changes:

• in line 42 I have add a upper()
  if not hwID.upper().startswith(r"USB\VID_10C4&PID_EA60"):
  In Win XP the Vendor and Product ID was lowercase, in Windows 7 it is uppercase, so I check all in uppercase letters
• in handleResponses(self) I have tried same other ways without the sleep-loop
  This way seems to run stable without the sleep.
  Can anyone test it? I don't know why brltty is faster, it sends the same characters??
  Ulf

[nvaccessAuto]

Comment 6 by drein on 2012-06-27 12:43
it is very strange, because the driver is working, but when I write quite fast, the display braille takes some seconds to show all charachters, instead if using with Brltty this problem doesn't exist.
I have no solutions for this, but I'm reporting only...

[nvaccessAuto]

Comment 7 by jteh (in reply to comment 2) on 2012-07-18 08:32
Replying to jteh:

1. There is quite a lot of hard-coding here. The determination of keys is done in a big if/elif block.

If you did want to simplify this and avoid the big if/elif block, you could have a list of keys like this:

KEY_NAMES = [Byte 1
    "left", "b6", "right", "b5", "b4", None, None, None,
    # Byte 2
    None, "b1", None, "b2", "b3", None, None, None,
](
    #)

Then you can grab the bytes, make them into a 16 bit number and check for each bit to know whether that key is pressed. If you're not comfortable doing this, it's probably best to leave it as is, since you'll be the maintainer of the driver.

[nvaccessAuto]

Comment 8 by jteh (in reply to comment 6) on 2012-07-18 10:15
Replying to drein:

it is very strange, because the driver is working, but when I write quite fast, the display braille takes some seconds to show all charachters

This is because the Seika displays connect at 9600bps, so every single update to the display takes almost a tenth of a second: 88 chars / (9600 bps / 10 bits per char) = 0.092 sec. So, if you type 10 characters, it'll take 1 second to update, excluding cursor blinking, etc. The solution is to only allow one update in this interval. This can be done by using a timer. If the timer is not running, send to the display and set a timer for this delay which does a delayed update. If the timer is running, set a variable for the delayed update and return. If you're not sure how to code this, I can do it, though I'll need someone with a display to test it.

[nvaccessAuto]

Comment 9 by Ulf on 2012-07-18 11:26
Hello,

1. the big if-block
   Ok, you are gight it is a block with 8 if statements, but thats all, 8 if - then statements
   if I transfor the bitfield to a 16 bit value and make a loop to check each bit, there are more statements and the part will became slower. There is the complex statement to shift the bits in the 16 bit value and then the same 'if ...' only in a loop.
2. the speed problem
   a lot of brailledisplay are working with 9600 bps and uses nearly same 'def display(self, cells):' methode (buam, hedo ...) Si I think this is a problem of the most displays. My problem: I've tried to reproduce this problem on 2 machines, but I think I am to slow, there was a delay, but not in seconds.
   Is it possible in NVDA that the 'display' methode will be called at one time more than once? I'll test it.

[nvaccessAuto]

Comment 10 by jteh (in reply to comment 9) on 2012-07-18 12:22
Replying to Ulf:

a lot of brailledisplay are working with 9600 bps and uses nearly same 'def display(self, cells):' methode (buam, hedo ...)

baum uses 19200, which is twice as fast. Also, for a 40 cell display, it sends about half the characters (no interspersed nulls), so it's four times as fast. I can't speak for hedo, as I don't own one.

Is it possible in NVDA that the 'display' methode will be called at one time more than once?

It's probably possible in some cases if there are very fast updates.

[nvaccessAuto]

Comment 11 by jteh (in reply to comment 8) on 2012-08-03 07:09
Replying to jteh:

The solution is to only allow one update in this interval. This can be done by using a timer. If the timer is not running, send to the display and set a timer for this delay which does a delayed update. If the timer is running, set a variable for the delayed update and return.

Actually, this solution won't work because braille runs in the main thread and these display calls block, thus blocking the rest of NVDA. I need to think more about a workable solution.

[nvaccessAuto]

Comment 12 by Ulf on 2012-08-03 11:42
I think there is no need for changing to a timer or so. I have make same tests and have measured times.

Is it possible in NVDA that the 'display' methode will be called at one time more than once?

It's probably possible in some cases if there are very fast updates

This was never held in my tests. I have try to write very fast, but in this time I can not read ;) And if I put a finger on a key, ok in this case it takes a little bit, but not in seconds, only in parts of seconds. I have test it with 40 and 80 cell displays.

[nvaccessAuto]

Comment 13 by jteh (in reply to comment 6) on 2012-08-06 07:10
Replying to drein:

it is very strange, because the driver is working, but when I write quite fast, the display braille takes some seconds to show all charachters

Can you be more precise? What application/control were you testing this in? How fast were you writing (roughly how many characters per second or what did you type and how long did it take)? How long did it take before all characters were displayed? Did it really take several seconds or do you just mean there was a noticeable delay?

@ulf: Any reason for using upper case key names rather than lower case as is the case elsewhere in NVDA?

[nvaccessAuto]

Comment 14 by jteh on 2012-08-06 07:11
Changes:
Milestone changed from None to 2012.3

[nvaccessAuto]

Comment 15 by jteh on 2012-08-08 08:21
Merged in e8a6e8e with several linguistic changes by me (such as lower casing the key names). This shouldn't have broken anything, but please test and reopen if it did. Thanks for your work!

Further issues (such as the speed issue) should be filed as separate tickets if they need to be addressed.
Changes:
State: closed