New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Give NVDA the ability to automatically detect braille displays. #1271
Comments
Comment 1 by ragb on 2013-09-02 10:33 Recently, I have been encountering some users with difficulties understanding what braille display driver they should use, either because they don't really know the display they are using, or don't know the manufacture or something else. So, I'll research something about automatically detecting USB / bluetooth displays (brltty may be a good start) Some questions I can think of:
Both options are great of course, but implies more complexity and changes to the braille subsystem and display drivers. #426 has some interesting discussion about this. |
Comment 2 by jteh on 2013-09-03 01:05 Bluetooth is a tricky one. You can't passively detect Bluetooth displays. However, probing Bluetooth is safer than serial because Bluetooth ports are paired to a specific device. Still, it seems ugly to constantly probe for any paired Bluetooth displays. |
Comment 3 by ragb (in reply to comment 2) on 2013-09-05 10:24
Agreed.
Unless we can find a way to by notified about bluetooth displays becoming available (that is without constant pooling) I guess probing is the best we can get. As most drivers already implement some sort of automatic connection, I believe probing won't be very hard to do. Basic design (based on already discussed stuff on #426 and others):
Regarding the interface: I believe a "probe" button on the braille settings dialog, next to the display selection, which pops a "probing" dialog would be the least confusing solution for now. This dialog can alos be accessible from the wellcome screen. As regarding autodetection, I'll have to research a bit more about windows plug and play and so on to be able to propose something detailed. |
Comment 4 by ragb on 2013-09-24 10:29 Should probing be used only for serial ports or we try also to connect to other available ports (USB and bluetooth, usually)?. With autodetection implemented, probing those may be useless, but we may not be able to implement auto detection for all drivers. |
Comment 5 by jteh on 2013-09-24 10:34 |
Comment 6 by leonarddr on 2013-09-24 11:11 |
Comment 7 by ragb on 2013-09-24 11:37 At first I was convinced that the best implementation was to have probing at the driver level. Here I think of two alternatives: Drivers have a However, a much simpler alternative is to implement probing at the braille subsystem module, taking advantage of the (possibly implemented)
In theory, this solution makes all drivers "probing-ready", but we loose priorities and driver specific probing methods, which, at second thought, are not so so important due to probing being a rare procedure, I believe. Comments? |
Comment 8 by ragb (in reply to comment 6) on 2013-09-24 11:42
This won't be called at startup at all. It must be always requested by the user. |
Comment 9 by jteh on 2013-09-24 12:46 There are two problems with doing as you suggest:
|
Comment 10 by ragb (in reply to comment 9) on 2013-09-24 14:01
My idea was more of trying faster ports from all drivers first, and then slower ones from all drivers, so the priorities thing. But honestly it is kind of over-complicating.
I forgot to mention this. Probably we spawn an
It is ugly indeed, but honestly I can't find a cleaner solution :$.
Maybe... We could restrict probing for bluetooth ports and having a background thread polling them from time to time, when no braille display is selected. |
Comment 11 by ragb (in reply to comment 9) on 2013-09-24 17:02 Replying to jteh:
We could do bluetooth pooling this way. Moreover ,we could also do USB pooling in the background, avoiding dealing with plug and play, USB ids, etc (has I believe OS X and brltty in linux are doing). It's lesse eficient but way simpler. |
Comment 12 by jteh (in reply to comment 11) on 2013-09-24 21:57
Pretty sure BRLtty uses udev to handle plug and play for USB. It's certainly simpler, but it has the disadvantage that it can only be done periodically to avoid performance issues. If I plug in a USB display, I would expect it to be detected immediately, rather than having to wait. With Bluetooth, you don't expect this so much because Bluetooth is wireless. |
Comment 13 by jteh on 2013-09-25 05:42 For auto detection, we can periodically try to create all displays with the default port in a background thread. Windows notifies about device connection via the WM_DEVICECHANGE message. Eventually, we could detect this and trigger a search instead of waiting. The only problem is that there are some drivers which succeed even if a device isn't connected such as brltty and handyTech. Brltty only succeeds if brltty is running, but that could still be an issue. I guess we could have an attribute which disables automatic detection. We probably need to fix handyTech so that it doesn't do this; it really shouldn't. |
Comment 14 by ragb (in reply to comment 13) on 2013-10-06 17:36
I've been playing with this lately (see branch 51271, for a start).
Agreed. See |
Comment 15 by jteh (in reply to comment 14) on 2013-10-07 23:39
That's a very nice start. :)
I wonder whether we need to have something that manages a window for this and other things in future. This probably needs to be handled in a separate ticket.
Nor one of mine. :)
I just realised those drivers return 0 for numCells when they didn't detect a display, so we could make those drivers throw an exception if this occurs. That way, they can still support probing.
Yeah. It might be time to follow this up. They definitely need to release the code to honour the license and it has been several months now. Once you get to background automatic detection, we need to think about UI and persistence.
|
Comment 16 by ragb (in reply to comment 15) on 2013-10-09 21:54
Agreed. We may need to register various handlers from many places in NVDA's code, so a central window where one registers handlers for various messages may be useful. I'll submit a ticket for this.
I like this option. It makes all drivers more consistent.
I like the checkbox option. Do we need to have autodetection always running or only when nobraille is selected?
I'm not really sure about this. If we don't set this, ton the next restart, the display may not be detected right away and the user will have to wait for probbing (if probbing is fast enough it may not be a problem). However, if we set it in the config, the user will have an error next time NVDA starts and the display is disconnected. If probing is not that fast, I'd say that setting detected displays in the config would be my choice.
As you say, it depends on 2 :).
Yes. If a display starts throwing errors it should be disconnected and auto detection put to work. Regards, Rui Batista |
Comment 17 by jteh (in reply to comment 16) on 2013-10-15 04:30
On further thought, I'm not sure it makes sense to allow the user to manually select a display while automatic detection is enabled; see below.
Are you saying that automatic detection would resume if the previously configured display threw an error? To me, this seems odd and difficult to explain. It's one of the reasons I think manual and auto should be mutually exclusive. My feeling is that plug-and-play detection should be mutually exclusive with manual selection to avoid confusion. For example, if the user manually selects a display but then plugs in another one, what should NVDA do? If they are mutually exclusive, then it probably makes most sense for this to be a choice in the braille display list. Of course, the user might want to probe for/detect their display just once; e.g. if they want to save the selection but don't know which display they have or which driver/port to use. My only concern is that manual probing will intrusively probe arbitrary serial ports, which the user might not want. This suggests we need an option to just probe USB and Bluetooth, but then this starts to get confusing for the user. Arrrg! |
Comment 19 by jteh on 2013-11-19 05:23 |
Comment 20 by ragb (in reply to comment 19) on 2013-11-24 12:18
Hi, Regards, Rui Batista |
Comment 22 by jteh on 2013-12-04 08:28 |
Comment 23 by jteh on 2013-12-05 05:25 |
Comment 24 by ragb (in reply to comment 23) on 2013-12-05 11:28
Plug-and-play is far more important, IMO. Serial ports are an older technology, although I believe we should support it, in this case I don't see it as a huge priority. BTW, thanks for taking this, I'm really struggling with IOS development these days... Regards, Rui Batista |
Comment 25 by ragb on 2013-12-11 11:39 Have just read the last commits. Here is some feedback:
The overall code seems very very good to me :). Manual probing, if gets to be done, is very straight forward with this. |
Comment 26 by jteh on 2013-12-11 12:29 Stupidly, now that I've pretty much got all of the core code working, I've realised there is one major problem I'd never really considered. This means that all drivers will be imported whenever automatic detection is enabled. Since we want to do this by default, this is probably pretty bad. It seems to only increase memory footprint by a few mb, but this is still not nice. So, believe it or not, I'm considering throwing all of this code away and starting from scratch. My current thinking is to have a central module which maps all relevant USB IDs and Bluetooth names to their corresponding drivers. This way, the driver only needs to be imported if a matching device is detected. To make things easier, the driver will contain this info in a specific format and the build system will scan all drivers and generate this central module. This should also be more efficient than constantly calling drivers. If we do go with this, it pretty much eliminates the need for most of my current changes. The ports stuff is nice, but it probably doesn't serve any actual purpose with this proposed approach. This also avoids issues with strange drivers such as handyTech. Of course, this doesn't take manual probing into account. We'll probably go back to something more like your original code for that. Thoughts? |
Comment 27 by ragb (in reply to comment 26) on 2013-12-11 13:55
Ok, nice.
It depends on how many are "few MB" :). As far as I know one can't properly unload modules in Python....
In theory it is more efficient and avoids the unneeded imports. However, can we know all the device ids for all drivers somehow? Won't it be kind of a maintenance nightmare?
Yes. Note that I like your approach, as far as I know bratty uses something similar in Linux, but I'm afraid of complexities like manufactures don't providing proper USB ids (and we having no access to devices or windows device drivers to get them).
Above :). Regards, Rui Batista |
Comment 29 by bramd on 2014-12-21 09:42 I use a Brailliant display on the road and a Handytech display at the office. The Handytech driver probes quite aggressively (might have something to do with my current driver settings as well...). So, a system that only initializes that driver if a Handytech device is connected and switches to my brailliant automagically is very welcome. |
Comment 30 by jteh on 2014-12-21 23:36 To be honest, I actually can't quite remember what problems still exist, though I know there was still work to do. I think one major problem is that there are some displays which stupidly use generic USB ids which might also be used by other displays, so the code needs to support trying more than one driver per USB id, which it doesn't currently support. |
Comment 31 by jteh on 2014-12-21 23:38 |
Comment 32 by dkager on 2015-08-09 11:09 I find this most interesting for the use case of unplugging your display, say to move your laptop somewhere, then re-attaching it and being able to continue working without restarting NVDA. Thus even if you were to turn off auto-detection it would be nice if individual drivers supported re-connecting. Or of course you could leave auto-detection enabled and that should take care of things. This leads to the suggestion that auto-detection always starts with the previously active display. Keeping the above in mind I think the configuration file should save the last detected display, if only to avoid probing for other ones. It is likely that the previous display is still the current one, e.g. on a desktop PC with a braille display. Another way to avoid importing all drivers (and maybe also some confusion) is to allow users to check the displays they use and only probe for those. The default would be to probe all displays. Just a few initial thoughts. :) |
After some thinking, I'm not sure whether this fixes our issue. When initializing a display from the background thread, that thread is currently busy with initializing, and in the initializing process, it calls waitForRead, which makes the background thread itself block. I assume that when queueing an overlapped completion routine when initializing, the initialization (which is the current apc) should have returned in order for the overlapped completion routine to be executed. However, we want to execute that routine during initialization, and not after it, as at that point, initialization has already failed. Or, and probably I''ve just missed this, will the completion routine just interrupt the current running APC? I should study computer science one day. |
@leonardder commented on 7 Sep 2017, 19:26 GMT+10:
That's the only way I can think of. @leonardder commented on 7 Sep 2017, 20:14 GMT+10:
That's a good question. I actually had to go look this up myself. From the documentation for QueueUserAPC:
I think the stack overflow bit is only relevant if you have too many levels of this, but we should only ever go two APCs deep, so we should be fine. There's no reason we should ever run another init function within the first init function. Btw, the documentation confirms that this also applies to IO completion functions:
This probably wouldn't be taught in computer science. :) |
Thanks for elaborating! I've successfully ported probing to the bg thread, only one nut left to crack:
This is the wx.CallLater that executes a back ground scan every 10 seconds. Although it should only be started from the main thread, it seems to run just fine, but it shows this debug warning every 10 seconds, which is well, not what we want. Do we want to have the timer run in the background thread? If not, which is the easiest method, we can just wrap the wx.CallLater in a wx.CallAfter in such a way that we can still stop the CallLater if desired. It executes _startBgScan which itself queues the actual probing apc in the background thread anyway. However, if we have a special background thread, why not have the timer run on that thread. I'm not sure how to do this though, probably using a waitable timer, which executes apc functions. |
Oo. Yes, this is bad and definitely needs to be fixed. While starting timers from background threads does seem to work, the fact that it causes wx assertions suggests it could have nasty side effects. We fixed several instances of this last year and suspect this may have led to unexplained crashes in the past.
Not sure if you mean it's only ever started from the main thread or that wx requires that it only be started from the main thread. This part of the stack confirms it's definitely being started in the background thread:
As you suggest, I think having the timer in the background thread makes the most sense, since the thread is already there. I think using a waitable timer makes sense. Note that you'll need to set the timer in the background thread in all cases so that it gets executed there. Also, ensure it is a one shot timer (lPeriod 0) to avoid the timer accidentally running during a |
I consider the code in the @BabbageCom i1271 branch to be feature complete, but before I file a pr, here is an overview of the changes since @jcsteh worked on this. Several things have changed, as @jcsteh's implementation was pre hwIo. Most notably:
Additional ideas are welcome, though.
|
@michaelDCurran: I forgot to mention that your feedback on the above comment would also be appreciated. Also @Qchristensen, @josephsl and others, it would be great if you could share your opinion on how you think braille display detection would fit into the current documentation (e.g. should we just list the supported displays in a separate paragraph, or should we add a note to every specific display paragraph)? |
hi,
i think, that the separate paragraph " supported autodetection braille
displays" will be a better option
W dniu 07.11.2017 o 10:58, Leonard de Ruijter pisze:
…
@michaelDCurran <https://github.com/michaeldcurran>: I forgot to
mention that your feedback on the above comment would also be
appreciated. Also @Qchristensen <https://github.com/qchristensen>,
@josephsl <https://github.com/josephsl> and others, it would be great
if you could share your opinion on how you think braille display
detection would fit into the current documentation (e.g. should we
just list the supported displays in a separate paragraph, or should we
add a note to every specific display paragraph)?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#1271 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AKohk7DPGG79Pxc1FzKucK6gLMe9xjkrks5s0CnXgaJpZM4JVEoT>.
|
For what it's worth, I'd suggest:
1. Having a general paragraph noting that NVDA can auto detect many displays and that the display specific section will note if detection is not supported for that display; and
2. Having a sentence in a display section if auto detection is not supported.
Reasons:
1. This means a user doesn't need to read through a huge list to work out whether their display is supported for detection;
2. The eventual aim is for as many displays as possible to support detection, so a list is just going to get longer and longer; and
3. Users can just read the section about their display for display specific info.
Having said that, you should totally ignore my opinion here in favour of NV Access staff. :)
Also, I'd like to publicly thank Leonard for taking over this work, adopting my somewhat incomplete code. I'm really glad to see it live on and finally come to fruition. :)
|
…sm can be activated by choosing the Automatic option from NVDA's braille display settings. See nvaccess#1271 for an in depth discussion of funcionality.
…sm can be activated by choosing the Automatic option from NVDA's braille display settings. See nvaccess#1271 for an in depth discussion of funcionality. * brailliantB, use generic implementation for USB serial devices * Use generic check function for braille display drivers supporting bdDetect * Make auto detection the default * Set a timeout for joining the bgThread when auto detection was on * Poll for bluetooth devices on app switches * Support bluetooth HID in bdDetect
…sm can be activated by choosing the Automatic option from NVDA's braille display settings. See nvaccess#1271 for an in depth discussion of funcionality. * brailliantB, use generic implementation for USB serial devices * Use generic check function for braille display drivers supporting bdDetect * Make auto detection the default * Set a timeout for joining the bgThread when auto detection was on * Poll for bluetooth devices on app switches * Support bluetooth HID in bdDetect
* Implement braille display auto detection. This new background mechanism can be activated by choosing the Automatic option from NVDA's braille display settings. See #1271 for an in depth discussion of funcionality. * brailliantB, use generic implementation for USB serial devices * Use generic check function for braille display drivers supporting bdDetect * Make auto detection the default * Set a timeout for joining the bgThread when auto detection was on * Poll for bluetooth devices on app switches * Support bluetooth HID in bdDetect * Use a separate thread for background scanning. * Make the bdDetect thread a daemon thread * Disable auto detection within the unit test framework. If we don't do this, detection will occur and unit tests will fail. * in braille.handler.handleGainFocus, check whether the focused object has an active tree interceptor. If so, focus the tree interceptor instead * Revert the use a separate thread for background scanning and make sure recursion does not occur when using an APC This reverts commit 5b97f39. * Created Detector._scanQueuedSafe which wraps changing the state of _scanQueued within a lock * Fix malformed VID and PID for Brailliant, add an extra check * NO longer filter serial ports for Brailliant * Updated changes.t2t
Reported by jkenn337 on 2010-12-11 15:50
When a braille display is connected to the computer and brltty is installed, NVDA should scan for and automatically detect and start using the connected USB or bluetooth display. This way the user does not have to mess about with choosing their display and things. Orca and the mac already do this. This enhancement should be across all operating systems.
Blocking #1555, #3648
The text was updated successfully, but these errors were encountered: