Detect disconnected braille devices and close them immediately

bramd commented 6 years ago

Steps to reproduce:

Connect a braille display that uses a HwIO based driver
Focus some static information so no writes are send to the display. avoid a blinking cursor
Disconnect the display

Actual behavior:

Usually it takes until the next write for NVDA to see that the display is gone

Expected behavior:

NVDA immediately detects the disconnect and terminates the active braille driver

Additional information

This problem is also discussed in #7727 where it causes a switch from USB to Bluetooth to be delayed until the next write to the display

bramd commented 6 years ago

We might be able to do this by periodically checking if the handle is still valid, for example using kernel32.GetHandleInformation.

CC @leonardder

LeonarddeR commented 6 years ago

Hmm, I see your point, but I'm not sure whether checking the handle's status is the right thing to do. Note that when you turn off a Modular Evolution, for example, the handle is still valid. Now this display is probably an exceptional one.

I've also seen several unhandled exceptions thrown by hwIO, such as as part of the _ioDone callback, when the connection to a display was lost. These exceptions aren't handle by the braille module in such a way that the display driver is terminated.

bramd commented 6 years ago

Yes, the Modular Evolution is a strange case. The power switch disables the cells and input, but the USB-serial adapter seems to be still active, however it doesn't reply to the protocol anymore. I just tried it here and I see it's just not sending ACKs anymore, which is logged but there is no behavior for many failing ACKs. It would make sense to extend the generic ACK handling mechanism with a counter that counts the ACK timeouts in a row and terminates the driver at a certain threshold. This would detect the Modular Evolution poweroff and start searching other displays in a while. This might be a good solution that detects a few cases of an inactive display sooner and is less complicated than checking the handle.

That being said, I still think checking if the handle is valid at specific intervals is a good strategy to detect disconnected displays even if there is no communication to the display. The ACK threshold strategy would not work for USB bulk devices where there is no braille update at the time of disconnection.

If you have specific cases where we throw or log an exception now where we should actually terminate the driver, it seems a good idea to me to extend this issue with those cases as well as the ACK threshould described above.

LeonarddeR commented 6 years ago

The following is a snippet from @MarcoZehe's log:

ERROR - unhandled exception (07:49:57.563):
Traceback (most recent call last):
File "_ctypes/callbacks.c", line 315, in 'calling callback function'
File "hwIo.pyc", line 134, in _ioDone
WindowsError: [Error 31] Ein an das System angeschlossenes Gerät funktioniert nicht.

I believe that this is logged as soon as the connection with the display is destroyed, which makes sense, since there is always an async read in progress until either it finishes or it fails. Therefore, I believe we can catch almost every issue like this simply by forwarding errors raised by IoBase._ioDone to a method that calls braille.handler.handleDisplayUnavailable. I consider another reference from hwIo to the braille module a bit ugly, though, but it seems to be the quickest way to do.

LeonarddeR commented 6 years ago

@bramd commented on 15 Oct 2018, 15:33 CEST:

It would make sense to extend the generic ACK handling mechanism with a counter that counts the ACK timeouts in a row and terminates the driver at a certain threshold.

Agreed, as long as the threshold is a property on the driver.

This would detect the Modular Evolution poweroff and start searching other displays in a while.

Problem is that it won't detect a modular powering on again, so it might make the behavior even worse for the modular, unless we extend the on app change polling of bluetooth devices to usb devices as well.

dkager commented 6 years ago

Both braille display drivers I tried so far (Brailliant and ALVA BC6) throw errors immediately when unplugging the display. The rationale for not catching them was that in release builds nobody notices these errors.

Maybe for USB devices there is also a message/event generated by Windows upon removal?

Adriani90 commented 1 year ago

@leonardder I have the feeling there was a PR solving this but I don't remember exactly. Is this fixed now in NVDA 2023.2 RC?

dkager commented 8 months ago

@LeonarddeR we should either pick this up again or decide that it's good enough with the new auto-detect mechanism.

LeonarddeR commented 8 months ago

I think it might make sense to pick this up indeed, it should be a pretty trivial change.

bramd commented 8 months ago

I think there are two parts here that might be useful:

A way to close the driver when a certain number of unreceived ACKs is reached. Core should provide the generic mechanism to do this and specific drivers can set their treshhold if it is useful for them (e.g. Modular Evolution)
A way to indicate that a driver should autodetect USB devices on app change, since @LeonarddeR's comment still stands as far as I know. If you would turn a display off and it still has an active serial converter it will not be autodetected again

nvaccess / nvda