nvda icon indicating copy to clipboard operation
nvda copied to clipboard

Driver for Tivomatic Caiku Albatross 46/80 displays

Open burmancomp opened this issue 2 years ago • 49 comments

Link to issue number:

Summary of the issue:

No support for Tivomatic Caiku Albatross 46/80 braille display models.

Description of how this pull request fixes the issue:

General

Opposite to many other displays Albatross models do not wait that driver sends query of their presence to them. In stead, they send continuously init packets until driver sends to them quit that connection has been established.

These displays also require that they get some data within approximately 2 seconds, otherwise they fall back to "wait for connection state" and start again send their init packets until driver sends quit packet.

Similarly, if user enters device internal menu, then after exit init packets are continuously sent until driver sends quit packet.

Init packets are also sent when device is powered off and then on.

Display init packets consist of two bytes: the first one is \xff which tells that this is an init packet. The second one is settings byte which contains display settings like length of display and number of status cells. The most meaningful setting is the length of display, and other settings are ignored by this version. Other settings contained by settings byte can be regarded as notes to screenreader, and it is screenreader or driver job to use them when applicable. For example, there are no separate status cells in the device but if screenreader supports using status cells, it can be notified to use them by settings byte.

Settings byte can be anything between \x00 and \xff. Thus it could be the same as init byte.

It is possible that settings byte may have same value with any display buttons. Because init packets may be sent also during session (user exits display internal menu for example), it is essential to know if byte is button press or part of init packet.

Because display when it is in "wait for connection" state sends init packets continuously, there may be hundreds of bytes to handle. There are several rx buffers between device and driver which seems to cause situation that all data cannot be read with one read operation. It cannot be known when all data has been read. This is the case with init packets but also with key combinations.

There are other devices with same PID&VID. When automatic braille display detection is used, other displays with same PID&VID are tried before Albatross. Those drivers try to send queries to the port to detect their own displays. These queries may cause Albatross to send unexpected init packets which in turn could disturb this driver - it could get inappropriate settings byte. On the other hand, if Albatross settings byte is \xfe it causes problems to the other driver so that it causes infinite loop in its detection procedure.

To reduce complexity of data handling and to prevent disturbances to other drivers this driver accepts only settings bytes <= \xfd. From user perspective this means that with 80 model user can ask screenreader to use at most 13 status cells when without limitation user could ask 15. Limitation is applied only if user has switched all other settings to values that cause value of byte to be > \xfd. From settings byte for status cells there are reserved the most right 4 bits. Limitation does not affect on 46 cells model because the most left bit of byte defines the length of display (0 for 46 and 1 for 80 cells model).

Driver requirements

  • support for both 46 and 80 cells models
  • support for both automatic and manual detection
  • when connected allows device plugging out and in, and power switching off and on so that display content is up-to-date and buttons work as expected after these operations.

Design

Driver has modular structure:

  • init.py
  • constants.py
  • driver.py
  • gestures.py
  • _threading.py

Constants.py contains all the constant definitions, for example button values and names.

Driver.py is the main part of the code. It implements BrailleDisplayDriver class which is in response of all read and write operations. It also takes care to format data which is meant to be displayed on the braille line.

Important main functions of BrailleDisplayDriver are:

  • _readHandling; performs connecting/reconnecting to the device, and all read operations during connection
  • _somethingToWrite; performs all write operations to the display
  • display; prepares data to be displayed on the braille line

In gestures.py numeric values of pressed buttons are interpreted as gestures so that they can be forwarded to NVDA input system.

_threading.py defines two threads. Thread called albatross_read calls BrailleDisplayDriver _readHandling function when it gets signaled that port has data to be read. Idea is somewhat similar to hwIo onReceive function. For deeper read and write operations control own thread was implemented. In addition, it calls _readHandling if it detects port problems so that _readHandling can try to reconnect. Albatross_read thread sleeps most of the time because user does not press buttons continuously, and connection problems occur rarely.

The second thread is timer which checks periodically (after approximately 1.5 seconds) that some data has been sent to display so that it keeps connected. _SomethingToWrite function updates time of last write operation. If there is at least 1.5 seconds from last write operation, display is feeded data packet containing START_BYTE and END_BYTE which enclose data which is sent to be displayed on the braille line.

Testing strategy:

Daily use testing in Windows 7/10.

Known issues with pull request:

Braille Extender addon may not be compatible with this driver at the moment.

Change log entries:

New features "Added support for Tivomatic Caiku Albatross 46/80 braille displays. Changes Bug fixes For Developers

Code Review Checklist:

  • [x] Pull Request description:
    • description is up to date
    • change log entries
  • [x] Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • [x] API is compatible with existing add-ons.
  • [x] Documentation:
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • [x] UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English

burmancomp avatar Nov 09 '21 18:11 burmancomp

@burmancomp Please fill out the PR template and revert changes to the Finnish user guide edits to the translated documentation are handled separately via SVN.

lukaszgo1 avatar Nov 12 '21 13:11 lukaszgo1

  • PASS: Translation comments check.
  • PASS: Unit tests.
  • PASS: Lint check.
  • FAIL: System tests. See test results for more information.
  • Build (for testing PR): https://ci.appveyor.com/api/buildjobs/pksw80f9hcc6nc6w/artifacts/output/nvda_snapshot_pr13045-24206,1bc7e5c7.exe

See test results for failed build of commit 1bc7e5c706

AppVeyorBot avatar Nov 12 '21 18:11 AppVeyorBot

No idea how to modify information on pr template.

@burmancomp Please fill out the PR template and revert changes to the Finnish user guide edits to the translated documentation are handled separately via SVN.

burmancomp avatar Nov 12 '21 22:11 burmancomp

@burmancomp Click the context menu button (labelled by NVDA as "show options") above the first comment in this thread, then click "edit comment" from the revealed menu. In the edit field:

  • Delete "…ic detection. English and Finnish user guides, and bdDetect.py updated." on the first line.
  • Fill in the template according to the comments provided.

codeofdusk avatar Nov 12 '21 22:11 codeofdusk

Unfortunately there is no online resources anymore.

burmancomp avatar Nov 18 '21 04:11 burmancomp

@burmancomp Would you mint resolving conflicts here? @michaelDCurran Is this on your radar?

lukaszgo1 avatar Feb 11 '22 12:02 lukaszgo1

@feerrenrut What is the status. here?

lukaszgo1 avatar May 19 '22 12:05 lukaszgo1

  • PASS: Translation comments check.
  • PASS: Unit tests.
  • PASS: Lint check.
  • FAIL: System tests. See test results for more information.
  • Build (for testing PR): https://ci.appveyor.com/api/buildjobs/v3x9lintgb8h1lst/artifacts/output/nvda_snapshot_pr13045-25772,ba5d7b76.exe

See test results for failed build of commit ba5d7b7610

AppVeyorBot avatar Jul 05 '22 10:07 AppVeyorBot

I have now tested if SleepEx fails without Albatross driver code by running main branch code from source.

Plugging Albatross in and switching it on seems to be enough to cause following when automatic is selected as braille display:

DEBUGWARNING - brailleDisplayDrivers.alva.BrailleDisplayDriver.__init__ (11:32:55.627) - braille._BgThread (4272):
Traceback (most recent call last):
  File "brailleDisplayDrivers\alva.py", line 173, in __init__
    self._dev = hwIo.Serial(port, timeout=self.timeout, writeTimeout=self.timeout, onReceive=self._ser6OnReceive)
  File "hwIo\base.py", line 194, in __init__
    self._ser = serial.Serial(*args, **kwargs)
  File "C:\src\nvda\.venv\lib\site-packages\serial\serialwin32.py", line 33, in __init__
    super(Serial, self).__init__(*args, **kwargs)
  File "C:\src\nvda\.venv\lib\site-packages\serial\serialutil.py", line 244, in __init__
    self.open()
  File "C:\src\nvda\.venv\lib\site-packages\serial\serialwin32.py", line 64, in open
    raise SerialException("could not open port {!r}: {!r}".format(self.portstr, ctypes.WinError()))
serial.serialutil.SerialException: could not open port 'COM4': OSError(22, 'Semaforin aikakatkaisun määräaika on lopussa.', None, 121)
DEBUG - braille.BrailleHandler.setDisplayByName (11:32:55.642) - braille._BgThread (4272):
Reinitializing noBraille braille display
DEBUG - autoSettingsUtils.autoSettings.AutoSettings._registerConfigSaveAction (11:32:55.642) - braille._BgThread (4272):
registering pre_configSave action: <class 'brailleDisplayDrivers.noBraille.BrailleDisplayDriver'>
DEBUG - autoSettingsUtils.autoSettings.AutoSettings._loadSpecificSettings (11:32:55.642) - braille._BgThread (4272):
loading braille noBraille
INFO - braille.BrailleHandler.setDisplayByName (11:32:55.642) - braille._BgThread (4272):
Loaded braille display driver noBraille, current display has 0 cells.
ERROR - stderr (11:32:55.642) - braille._BgThread (4272):
Exception in thread braille._BgThread:
Traceback (most recent call last):
  File "C:\Users\teijo\AppData\Local\Programs\Python\Python37-32\lib\threading.py", line 926, in _bootstrap_inner
    self.run()
  File "C:\Users\teijo\AppData\Local\Programs\Python\Python37-32\lib\threading.py", line 870, in run
    self._target(*self._args, **self._kwargs)
  File "C:\src\nvda\source\braille.py", line 2322, in func
    ctypes.windll.kernel32.SleepEx(winKernel.INFINITE, True)
OSError: exception: access violation reading 0xFFFFFFFF

This is a problem when one is about to use Albatross.

By running with Albatross code I have encountered this problem occasionally/more or less frequently (no statistics unfortunately).

To solve this I have added try except blocks in braille.py. I do not think they would cause harm there.

burmancomp avatar Jul 11 '22 08:07 burmancomp

Thank you!

burmancomp avatar Jul 15 '22 04:07 burmancomp

@feerrenrut and @seanbudd what is realistic timetable to expect review? Asking because I have no plans for new commits.

burmancomp avatar Jul 26 '22 09:07 burmancomp

Plugging Albatross in and switching it on seems to be enough to cause following when automatic is selected as braille display: ... To solve this I have added try except blocks in braille.py. I do not think they would cause harm there.

Are you happy with this solution? What are the implications, are there trade-offs being made here?

feerrenrut avatar Jul 27 '22 00:07 feerrenrut

@burmancomp - Due to the amount of competing priorities (2700 issues, 50 PRs) it's hard for us to give specific timelines. This PR is too late for 2022.3beta1, so any investigation and review for it won't happen until 2022.3beta1 is out. It will take at least a week before we get to the PR.

Based on your latest comment, it appears that substantive investigation is still required on this PR to review it.

To solve this I have added try except blocks in braille.py. I do not think they would cause harm there.

Without evidence that this is the right development approach and won't cause harm, I don't think this PR should change braille.py in this manner. I think both the OSError and SerialException in the log provided in https://github.com/nvaccess/nvda/pull/13045#issuecomment-1180137642 need to be handled in a different manner.

seanbudd avatar Jul 27 '22 00:07 seanbudd

I think SerialException was there because Alva (BC680) was not present but port it uses with bluetooth was found in system.

As to OSError I don't know why SleepEx may fail when Albatross is plugged in and powered on. Maybe/likely it has something to do with Albatross sends continuously data until it receives connection establishment packet. But this is only a guess or consideration what might cause this.

Because sending data continuously when no connection to the driver is the builtin feature of Albatross devices, and because this causes SleepEx failure, I think practical solution could be try and except blocks. I could suppose that other displays do not suffer from this problem so using resources to find more sophisticated solution could be somewhat inefficient.

I have not noticed negative effects caused by try except blocks. Positive effect is that although SleepEx may fail I do not notice the failure from user perspective - display works as expected with NVDA. One point is also that OSERROR seems to occur only once near the initial connection. Because problem does not occur every time when launching NVDA, it increases challenges in testing if alternate solution would be developed. I am tend to think that try except blocks do not harm other displays. When Albatross is not present I suppose there should not be this exception.

If there are no try except blocks (or better solution is not found), typically there is connection between Albatross and driver but nothing is displayed because while loop where SleepEx is is broken by unhandled exception.

Unfortunately I don't have a better solution at this moment.

burmancomp avatar Jul 27 '22 10:07 burmancomp

@feerrenrut and @seanbudd here are three logs:

This one was when Albatross was powered on and plugged in, and Alva BC680 was powered off (run from source without albatross code). nvda-albatross_plugged_in.log

This one was when both displays were available (run from source without albatross code). nvda-alva_and_albatross.log

This one was when both devices were switched on although I think no matter was BC680 available or not because auto detect seems to try Albatross before Alva(run from source with Albatross code). nvda-albatross.log

In case 2 OSError is with WaitForSingleObjectEx.

When I started NVDA with albatross code (case 3) I have not noticed anything unusual although OSError occured.

burmancomp avatar Jul 27 '22 12:07 burmancomp

@feerrenrut and @seanbudd let me know if you have a better approach in mind for SleepEx crash than adding try and except blocks in to func in braille.py. Code I added there is as simple as:

		# Ensuring with try except that occasional OS errors with SleepEx
		# do not break loop
		try:
			ctypes.windll.kernel32.SleepEx(winKernel.INFINITE, True)
		except OSError:
			log.debug("", exc_info=True)

I do not know scenario where this would harm other drivers but do you think there are such ones?

burmancomp avatar Aug 17 '22 08:08 burmancomp

@burmancomp - is it not possible to catch these exceptions on a lower level (i.e. within your braille display driver specifically)?

seanbudd avatar Aug 23 '22 06:08 seanbudd

  • PASS: Translation comments check.
  • PASS: Unit tests.
  • PASS: Lint check.
  • FAIL: System tests. See test results for more information.
  • Build (for testing PR): https://ci.appveyor.com/api/buildjobs/40ljmb3jmne7w5g8/artifacts/output/nvda_snapshot_pr13045-26290,6d9c4926.exe

See test results for failed build of commit 6d9c4926c2

AppVeyorBot avatar Aug 23 '22 11:08 AppVeyorBot

  • PASS: Translation comments check.
  • PASS: Unit tests.
  • FAIL: Lint check. See test results for more information.
  • PASS: System tests.
  • Build (for testing PR): https://ci.appveyor.com/api/buildjobs/6yq1hb4xt2c2oeso/artifacts/output/nvda_snapshot_pr13045-26311,59f28359.exe

See test results for failed build of commit 59f28359da

AppVeyorBot avatar Aug 24 '22 10:08 AppVeyorBot

  • PASS: System tests.
  • FAIL: Lint check. See test results for more information.
  • Build (for testing PR): https://ci.appveyor.com/api/buildjobs/c9c7r69hhtd08sej/artifacts/output/nvda_snapshot_pr13045-26312,07dabcb3.exe
  • PASS: Unit tests.
  • PASS: Translation comments check.

See test results for failed build of commit 07dabcb347

AppVeyorBot avatar Aug 24 '22 13:08 AppVeyorBot

  • PASS: Translation comments check.
  • PASS: Unit tests.
  • PASS: Lint check.
  • Build execution time has reached the maximum allowed time for your plan (60 minutes).

See test results for failed build of commit 263b0ef436

AppVeyorBot avatar Aug 24 '22 20:08 AppVeyorBot

Hello @seanbudd and all,

As to try/except block (currently in threads.py from line 63), it is there to prevent whole thread from crashing in unexpected failures of functions called within try block. SleepEx in braille.py in func should not fail and cause exception but it does that. So I think it may be possible with other function calls as well.

No raising exceptions anymore within try block as you suggested.

Exception with SleepEx (in braille.py in func) seems to happen when using automatic display detection, and if it happens NVDA should be restarted to get NVDA pass information to display drivers. I think solution could not be not to use automatic display detection or require NVDA restarts. When exception has happened driver still works but because bg_thread does not send new data nothing is displayed.

Some reasons for driver complexity:

  • display sends continuously init byte and settings byte packets until driver asks it to stop; this happens when display is switched on, user exits display internal menu or when display had connection but received no appropriate data within approximately 2 seconds
  • displays with same VID&PID may cause init packets to be unpredictable, at least some of them write to port during their initialization process which might cause this; buffer reset seems to resolve this and it also reduces to some extend number of init packets which have to be handled
  • anyway there may be hundreds of bytes in rx buffers
  • when reading those bytes mostly one cannot get them to read at one time; this could be because there are several buffers between display and NVDA
  • settings byte can be anything between \x00 to \xff (\xff is not accepted in this driver because init byte is also \xff, settings with \xff are not realistic, and if \xff would be accepted I think driver could be more complex than current one)
  • settings byte value can be same with any of display's keys
  • only working solution seems to be handle all bytes somewhat
  • for example if NVDA is restarted quickly or connection needs to be established again due to read/write problems driver may need to wait init packets because display waits 2 seconds before it starts to send them
  • if display gets no appropriate data within approximately two seconds, continuous init packets sending starts; to prevent this there is timer which ensures that it gets something
  • when display is switched off virtual USB serial port disappears and when switched on it comes back; I wanted that driver can manage this and that display shows correct data when it is switched on again (display is updated also after exit from internal menu).

Hopefully code is easier to review as submodule.

burmancomp avatar Aug 26 '22 12:08 burmancomp

  • PASS: Translation comments check.
  • PASS: Unit tests.
  • PASS: Lint check.
  • FAIL: System tests. See test results for more information.
  • Build (for testing PR): https://ci.appveyor.com/api/buildjobs/ibhus3id7a8a1dc3/artifacts/output/nvda_snapshot_pr13045-26373,fb899231.exe

See test results for failed build of commit fb89923170

AppVeyorBot avatar Aug 28 '22 18:08 AppVeyorBot

I think solution could not be not to use automatic display detection or require NVDA restarts.

If this is the case, this should be noted in the user guide clearly. Users should know that they may need to restart NVDA or manually select the display.

seanbudd avatar Aug 31 '22 03:08 seanbudd

Thank you for the overall explanation @burmancomp, it has been very helpful.

Please make sure that this information is associated in relevant docstrings. For example, a short overview in a README.md file in the submodule would be extremely helpful. Also, adding relevant overviews to each file, class, and function docstrings.

seanbudd avatar Aug 31 '22 03:08 seanbudd

Is there any driver specifications or communication protocol documents available?

seanbudd avatar Aug 31 '22 05:08 seanbudd

I think solution could not be not to use automatic display detection or require NVDA restarts.

If this is the case, this should be noted in the user guide clearly. Users should know that they may need to restart NVDA or manually select the display.

I meant I think automatic detection should be supported, sorry that my English likely caused missunderstanding.

burmancomp avatar Aug 31 '22 08:08 burmancomp

Is there any driver specifications or communication protocol documents available?

I think there is likely no English documentation. I was involved to project when display was developed so my knowledge is based on that.

burmancomp avatar Aug 31 '22 09:08 burmancomp

@seanbudd,

I'm working with your requests but I have encountered some problems.

gui.messageBox problem

In the scenario where Albatross is selected directly (no automatic detection), and Albatross has unsuitable settings of which messageBox should notify log shows as follows:

ERROR - braille.BrailleHandler.setDisplayByName (11:05:36.147) - MainThread (428): Error initializing display driver albatross for kwargs {'port': 'auto'} Traceback (most recent call last): File "gui\message.py", line 70, in messageBox mainFrame.prePopup() AttributeError: 'NoneType' object has no attribute 'prePopup'

During handling of the above exception, another exception occurred:

Traceback (most recent call last): File "braille.py", line 1898, in setDisplayByName newDisplay = newDisplay(**kwargs) File "brailleDisplayDrivers\albatross_init_.py", line 103, in init self.searchPorts(port) File "brailleDisplayDrivers\albatross_init.py", line 139, in _searchPorts self.readHandling() File "brailleDisplayDrivers\albatross_init.py", line 362, in _readHandling if not data or not self.skipRedundantInitPackets(data): File "brailleDisplayDrivers\albatross_init.py", line 409, in _skipRedundantInitPackets wx.OK | wx.ICON_ERROR File "gui\message.py", line 74, in messageBox mainFrame.postPopup() AttributeError: 'NoneType' object has no attribute 'postPopup'

When using automatic detection gui.messageBox works as expected.

Problem with Handytech driver during automatic detection

When using automatic detection, and if Albatross settings byte is \xfe log often is filled with:

DEBUGWARNING - brailleDisplayDrivers.handyTech.BrailleDisplayDriver._handleInputStream (11:14:35.250) - braille._BgThread (4888): Unhandled packet of type b'\xff' DEBUGWARNING - brailleDisplayDrivers.handyTech.BrailleDisplayDriver._handleInputStream (11:14:35.250) - braille._BgThread (4888): Unknown model: b'\xff' ERROR - hwIo.base.IoBase._notifyReceive (11:14:35.250) - braille._BgThread (4888): Traceback (most recent call last): File "hwIo\base.py", line 169, in _notifyReceive self._onReceive(data) File "brailleDisplayDrivers\handyTech.py", line 854, in _serialOnReceive self._handleInputStream(data, self._dev) File "brailleDisplayDrivers\handyTech.py", line 863, in _handleInputStream "The model with ID %r is not supported by this driver" % modelId) RuntimeError: The model with ID b'\xff' is not supported by this driver

Only way seems to be switch Albatross off or at least entering internal menu would be required. Because Handytech displays are autodetected before Albatross there is no way notify user in the driver.

Strange input problem

I do not know how long time this has been present but it might have been long time there.

If Albatross is selected manually, and then NVDA launched, and if the first key which is pressed is any of the display keys, it is not recognized:

Calling function _readHandling for read DEBUG - brailleDisplayDrivers.albatross.BrailleDisplayDriver._somethingToRead (11:32:25.988) - albatross_read (5564): Read: b']]', length 2, in_waiting 0 DEBUG - brailleDisplayDrivers.albatross.BrailleDisplayDriver._skipRedundantInitPackets (11:32:25.988) - albatross_read (5564): Read: enqueued b']' DEBUG - brailleDisplayDrivers.albatross.BrailleDisplayDriver._skipRedundantInitPackets (11:32:25.988) - albatross_read (5564): Read: enqueued b']' DEBUG - brailleDisplayDrivers.albatross.BrailleDisplayDriver._handleReadQueue (11:32:25.988) - albatross_read (5564): _ReadQueue is: deque([b']', b']']), length 2 DEBUG - brailleDisplayDrivers.albatross.BrailleDisplayDriver._handleReadQueue (11:32:25.988) - albatross_read (5564): Read: dequeued b']', 1 items left DEBUG - brailleDisplayDrivers.albatross.BrailleDisplayDriver._handleKeyPresses (11:32:25.988) - albatross_read (5564): Partial ctrl packet b']' DEBUG - brailleDisplayDrivers.albatross.BrailleDisplayDriver._handleKeyPresses (11:32:25.988) - albatross_read (5564): Read: dequeued key b']', 0 items left DEBUG - brailleDisplayDrivers.albatross.BrailleDisplayDriver._handleKeyPresses (11:32:25.988) - albatross_read (5564): Keys for key press: b']]' DEBUG - brailleDisplayDrivers.albatross.BrailleDisplayDriver._handleKeyPresses (11:32:25.988) - albatross_read (5564): Pressed keys are {93} DEBUG - brailleDisplayDrivers.albatross.InputGestureKeys.init (11:32:25.988) - albatross_read (5564): Key is 93 and key name is eCursor1 DEBUG - config.featureFlag._validateConfig_featureFlag (11:32:26.004) - albatross_read (5564): Validating feature flag: DISABLED, optionsEnum: BoolFlag, behaviorOfDefault: ENABLED IO - inputCore.InputManager.executeGesture (11:32:26.004) - albatross_read (5564): Input: br(albatross):eCursor1 DEBUG - brailleDisplayDrivers.albatross.BrailleDisplayDriver.handleKeyPresses (11:32:26.004) - albatross_read (5564): Traceback (most recent call last): File "brailleDisplayDrivers\albatross_init.py", line 558, in _handleKeyPresses inputCore.manager.executeGesture(InputGestureKeys(pressedKeys)) File "inputCore.py", line 523, in executeGesture raise NoInputGestureAction inputCore.NoInputGestureAction

But if the first pressed key is any key of keyboard or if Albatross is automatically detected, this would not seem to occur. When this occurs, after first key press all key presses work as expected.

burmancomp avatar Sep 05 '22 08:09 burmancomp

In the scenario where Albatross is selected directly (no automatic detection), and Albatross has unsuitable settings of which messageBox should notify log shows as follows:

I suggest using ui.message instead, or at least if gui.mainFrame is None.

seanbudd avatar Sep 06 '22 04:09 seanbudd