Added a Braille display driver for the SuperBraille device used primarily in Taiwan.#7352
Conversation
| # Translators: Names of braille displays. | ||
| description = _("SuperBraille") | ||
| isThreadSafe=True | ||
| _dev=None |
There was a problem hiding this comment.
I don't think this needs to be initialised here; it should be set in the constructor. There's a risk that this might hide a logic error somewhere.
| for portInfo in comPorts: | ||
| # Translators: Name of a serial communications port. | ||
| ports[portInfo["port"]] = _("Serial: {portName}").format(portName=portInfo["friendlyName"]) | ||
| print "ports: %s"%ports |
| log.debug("Checking port %s for a SuperBraille", port) | ||
| try: | ||
| self._dev = hwIo.Serial(port, baudrate=BAUD_RATE, stopbits=serial.STOPBITS_ONE, parity=serial.PARITY_NONE, timeout=TIMEOUT, writeTimeout=TIMEOUT, onReceive=self._onReceive) | ||
| log.debug("Port opened.") |
There was a problem hiding this comment.
I don't think these debug messages are needed because we can get this info (and more) from the hwIo debug logging.
| found = True | ||
| break | ||
| else: | ||
| self._dev.close() |
There was a problem hiding this comment.
Each port needs to be closed after a failed attempt, not just the last one. So, this else block needs to be indented one more so it's inside the loop iteration.
| else: | ||
| self._dev.close() | ||
| if not found: | ||
| raise RuntimeError, "No SuperBralle found" |
| return | ||
| data=self._dev.read(1) | ||
| if data!='\x05': | ||
| log.info("Unknown second byte") |
There was a problem hiding this comment.
I'd log these as debugWarning, rather than info, as they are meaningless to users and probably harmless unless the user is seeing a problem.
| self._dev.write(DISPLAY_TAG + "".join(out)) | ||
| except EnvironmentError as e: | ||
| self._closeComPort() | ||
| raise e |
There was a problem hiding this comment.
You shouldn't need to catch exceptions here and close the device; braille core now handles that for you. If the display method throws an exception, it switches to noBraille, which calls terminate on the display. This also means you can get rid of the checks for _dev being None and probably merge _closeComPort into terminate.
| | Toggle braille tethered to | A | | ||
| %kc:endInclude | ||
|
|
||
| ++ Super Braille ++ |
There was a problem hiding this comment.
This heading calls it "Super Braille" (with a space), whereas elsewhere, it's "SuperBraille" (camel case). Which one? :)
| %kc:endInclude | ||
|
|
||
| ++ Super Braille ++ | ||
| The SuperBraille device, mostly available in Taiwan, can be connected to by either USB or serial. |
There was a problem hiding this comment.
Are there any links we need to/can provide here for USB drivers or the like? I get this thing isn't overly well documented (at least in English), so we can always add that later if you have no idea and someone asks for it.
…ile to superBrl,matching its name attribute. It can't internally be called superBraille as clashing with the existing add-on would make debugging tricky.
jcsteh
left a comment
There was a problem hiding this comment.
Looks good barring one nit below. Just a question: I understand why you wouldn't want them to be the same name, since the add-on would override, but how will the users tell the difference between the two? Do they have a different description or something?
| self._dev.close() | ||
| if not found: | ||
| raise RuntimeError, "No SuperBralle found" | ||
| raise RuntimeError, "No SuperBraille found" |
There was a problem hiding this comment.
Sorry, missed this. This is old Python syntax. Please change to:
raise RuntimeError("No SuperBraille found")
That is, pass the message to the exception's constructor.
jcsteh
left a comment
There was a problem hiding this comment.
Bah. Forgot to hit approve.
Summary of the issue:
In Taiwan, the primary braille display that is used is the SuperBraille. this is Taiwan-made and is very popular in Government and education etc, despite its large size and lack of typing and scroll buttons.
Due to other Taiwan-specific screen readers now no longer being maintained, NVDA is fast becoming a very popular screen reader. However, for NVDA uptake it is essential that the SuperBraille is supported as individuals and governments have spent significant finances on these displays.
Description of how this pull request fixes the issue:
There was a 3rd party add-on written for this display. However, it had issues when restarting NVDA, and was not using the most modern braile display driver techniques in NVDA.
This pull request introduces that code, at the same time addressing the restart bug, and modernizing the driver.
This new driver also stops probing all serial ports, rather only allowing automatic for one particular USB hardware ID, and then allowing manual selection for other serial ports.
Testing performed:
A SuperBraille v3.2 was used to test while updating this driver.
Known issues with pull request: