Comment 8 by jteh on 2014-04-21 22:12
Thanks for the code!

A few general questions:

Are these displays serial only or are there USB or Bluetooth displays as well?
If the dll was written specifically for the NVDA driver, is there any reason you opted for a dll rather than doing everything in Python? We prefer to avoid additional dlls if we can. This one is small enough that it's not a major problem, but if it's all serial, it seems it could all be done in Python fairly easily. If the dll was written before the NVDA driver, I certainly understand this.

For the most part, the code looks pretty good. In addition to comment:6, here is some code review:

log.warning(dllabspath)

I assume this was debugging code that was accidentally left. It should be removed.

      if (wParam == ECO_KEY_STATUS1 ) or (wParam == ECO_KEY_STATUS2) or (wParam == ECO_KEY_STATUS3 ) or (wParam == ECO_KEY_STATUS4) :

Rather than multiple equality checks, use the in operator; e.g. wParam in (foo, bar, bas)

      else:
          if (wParam > 0):

Unless I'm missing something, you can rewrite this as an elif and get rid of one indentation level. You can also get rid of the parentheses.

        elif wParam > 0:

  description = _("Lineas ecoBraille 20/40/80")

Any translatable strings need a translators comment above them. This will do:

    # Translators: The name of a braille display.

Does the display support pressing multiple keys at once? This doesn't seem to be supported by the driver. It's fine if you don't see a need to implement this. I mention it because most drivers do.

There is also some extraneous whitespace, but I can remove that easily enough.

Thanks again!

Uh oh!

Braille driver for Eco display #4078

Description

Activity

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions