Support for serial braille displays

[nvaccessAuto]

Reported by drein on 2009-09-24 09:45
NVDA should have a dialog box in which an user could select port of the Display Braille and BaudRate.
Blocking #2012, #2679

[nvaccessAuto]

Comment 1 by jteh on 2009-12-08 05:24
Note that this won't work for BRLTTY, which is what is used to support all serial displays at present, as all display configuration has to be done using the BRLTTY configuration. It will only be useful for native NVDA drivers such as the MB408 driver proposed in #241.

[nvaccessAuto]

Comment 2 by jteh on 2009-12-08 05:25
Changes:
Milestone changed from 2010.1 to 2010.2

[nvaccessAuto]

Comment 3 by jteh on 2010-05-18 04:39
Changes:
Milestone changed from 2010.2 to 2010.3

[nvaccessAuto]

Comment by jteh on 2010-09-24 07:26
(In #937) Requires support for configuration of com port, though we may also be able to auto detect this in the case of USB and perhaps even bluetooth.

[nvaccessAuto]

Comment 5 by jteh on 2011-01-23 05:10
Moving this to near-term, as it isn't needed for 2011.1. However, I'm seriously considering not supporting serial displays at all due to the difficulty in configuring them for many users. Anyone who really needs this can still use BRLTTY.
Changes:
Milestone changed from 2011.1 to near-term

[nvaccessAuto]

Comment 6 by jteh on 2011-06-09 03:25
Changes:
Milestone changed from near-term to None

[nvaccessAuto]

Comment 8 by aliminator on 2012-08-28 08:48
Configuring the com port manually should not be difficult for most users. As long as it is a simple text file where nothing else resides in except of the com port setting,
editing this file is nothing else than editing another file. It is even more difficult to create a table in MS Word.
Surely, a combo box with some com ports where the user simply selects is more intuitive. This can be e.g. implemented in an additional Devices dialogue in the settings where all braille and speech devices are grouped.

Furthermore a user has to change (eventually) the com port only once.
The probability of changing com ports frequently is quite low.

There is only BRLTTY as an alternative if serial braille devices are not supported natively.
But configuring BRLTTY is much more difficult.

[nvaccessAuto]

Comment 9 by jteh (in reply to comment 8) on 2012-08-28 09:08
Replying to aliminator:

Configuring the com port manually should not be difficult for most users.

From past experience, this causes a lot of confusion for novice users. However, given the high cost of new braille displays, I agree we should eventually support this.

As long as it is a simple text file where nothing else resides in except of the com port setting,

editing this file is nothing else than editing another file.

Requiring the user to edit a file is unacceptable for this in the core, as is having a separate config file just for the port specification. Of course, add-ons can do what they like.

Surely, a combo box with some com ports where the user simply selects is more intuitive.

For sure. Ultimately, this is what we will need.

This can be e.g. implemented in an additional Devices dialogue in the settings where all braille and speech devices are grouped.

How do you anticipate this working as far as the UI is concerned? Is it per device or global to all devices? What happens to this setting when the display is changed?

[nvaccessAuto]

Comment 10 by ragb on 2012-08-28 10:45
My 2 cents on this:

Given the still many available displays that support only RS232 communication, I truly believe NVDA must support these natively (Brltty is orders of magnitude harder to configure then a COM port, which users of these displays are somehow used to). However, as Jamie states, there are many issues that come up, most of them related with user experience and usability.

1. COM port setting. I think that for this displays, either in NVDA or an add-on the user is required to set the COM port explicitly. I'm tempted to say that probing serials is dangerous (shame on me, I did that on the braillenote driver). So, we most find a way to provide the user a way to set that port. If people create add-ons for all serials braille displays, surely we will have duplication of eforts, and a bunch of different ways to do the same. So the idial would be to do that in NVDA itself, if possible and desirable.
2. Editing an ini file by hand is not acceptable, on a today screen reader product, just to connect a braille display. Obvious if the brailledisplay is packaged as an add-on, the add-on author can do what he/she wants, but even that way I believe it is not acceptable at all, for a good user interaction. So, some sort of GUI to select the COM port (and possibly other parameters, which I honestly can't think of any) should be provided, either by NVDA or the add-on.

If it is provided by NVDA, I can think of two possible designs:

• On the braille display settings dialog, provide two settings: a check box that enables serial communication on braille displays and, when that is checked, a list box with the available COM port choices. (A small variant could be a "None" choice on the COM port list, but I dislike this solution).
• Have an "advanced braille settings dialog", accessible for the usual braille settings dialog, where on can change COM port settings and possible other future additions.

For the add-on interface, I think adding a dialog to the preferences menu (as it's done on the OCR add-on) is the best option.

1. This is an even more personal opinion: I tend to think that we should integrate as many braille display and synth drivers in NVDA itself as possible. At least if they met minimum quality standards and such, and don't colide with anything else within NVDA. Braille display driver support increase NVDA user base, I believe. Obvious that increases the code NVDA developers most maintain. In the case that a driver is not maintained and stops working, I do think that we should drop it, if anyone is able to maintain it.

For the Papenmeier case (#1265) we could for instance integrate the driver with only USB and bluetooth support (does it support bluetooth at all?) but... then someone would create an add-on, with similar code, just to support serial displays. That doesn't seem very ... clever I think (can't get a better word).

Further more, I think NV Access and the NVDA community should clearly state if they want or not to natively support braille devices, so this issue does not come up again. My opinion is yes, thay/we shoud, but has consequences (more support load and code maintenance). It would also be interesting to have an idea of howmany serial braille devices are still in utilization, if pople use real RS232 ports or are using USB to serial adaptors (which tend to change COM ports randomly), etc...

[nvaccessAuto]

Comment 11 by jteh (in reply to comment 10) on 2012-08-28 11:49
I agree with most of your sentiments, Rui. A few comments:

Replying to ragb:

I'm tempted to say that probing serials is dangerous

I'm okay with this in the longrun as long as it is an action that the user must take; e.g. press a button which pops up a warning explaining that this could impact other devices on the system. However, manual port selection still needs to be an option.

If it is provided by NVDA, I can think of two possible designs:

• On the braille display settings dialog, provide two settings: a check box that enables serial communication on braille displays and, when that is checked, a list box with the available COM port choices. (A small variant could be a "None" choice on the COM port list, but I dislike this solution).

I'd prefer to generalise this and make it a "port" choice. This way, it can contain "default" or "USB/bluetooth" or similar.

The question is: how does a display specify the types of ports it supports? It doesn't make sense to show ports a display can't support. Also, how is this stored in the configuration? If it's stored in a display specific section, that means the author has to take this into account in their config spec if they want to add their own display specific settings in the future. How is the display initialised with this port?

1. This is an even more personal opinion: I tend to think that we should integrate as many braille display and synth drivers in NVDA itself as possible.

In general, I agree. However, there are cases where this isn't realistic; e.g. too many dependencies, excessive size, not fitting nicely within NVDA's architecture, etc.
Changes:
Changed title from "NVDA should allow select comport and baudrate in DisplayBraille" to "Support for serial braille displays"

[nvaccessAuto]

Comment 12 by ragb (in reply to comment 11) on 2012-08-28 13:03
Comments to your comments :):
Replying to jteh:

Replying to ragb:

I'm tempted to say that probing serials is dangerous

I'm okay with this in the longrun as long as it is an action that the user must take; e.g. press a button which pops up a warning explaining that this could impact other devices on the system. However, manual port selection still needs to be an option.

This seems to be a good solution. At least we can say "you've been warned." wen probing serials ports. Manual selection is also a must-have, I agree.

If it is provided by NVDA, I can think of two possible designs:

• On the braille display settings dialog, provide two settings: a check box that enables serial communication on braille displays and, when that is checked, a list box with the available COM port choices. (A small variant could be a "None" choice on the COM port list, but I dislike this solution).

I'd prefer to generalise this and make it a "port" choice. This way, it can contain "default" or "USB/bluetooth" or similar.

Seems an interesting possibility. But:

The question is: how does a display specify the types of ports it supports? It doesn't make sense to show ports a display can't support. Also, how is this stored in the configuration? If it's stored in a display specific section, that means the author has to take this into account in their config spec if they want to add their own display specific settings in the future. How is the display initialised with this port?

I thing this needs heavy refactoring :). Firstly the display driver must explicitly state what commuinications protocol it supports. Probing capabilities and specific should also be stated on the driver, then the GUI can act accordingly. Something like the supportedSettings mechagnism on synth drivers. I have some dobts about the user interface though, and this is not something I've beeing thinking that much so there might be other dificulties, but there is something that really concerns me:
If we change braille display drivers (and I think twhat I propose needs changing of all drivers) we may create regressions and such. Testing it is not that easy because we don't have quick access to many of the supported displays, and the most of the driver were coded by external devs. It's something I think should be kept in mind.

Side note: if we do a refactoring on braille displays we can thing about automatic display detection.

1. This is an even more personal opinion: I tend to think that we should integrate as many braille display and synth drivers in NVDA itself as possible.

In general, I agree. However, there are cases where this isn't realistic; e.g. too many dependencies, excessive size, not fitting nicely within NVDA's architecture, etc.

I agree. The drivers (braille, or synth drivers) must be integrated into NVDA, not external code that for some reason is distributed within NVDA.

[nvaccessAuto]

Comment 13 by jteh (in reply to comment 12) on 2012-08-28 23:23
Replying to ragb:

The question is: how does a display specify the types of ports it supports? It doesn't make sense to show ports a display can't support. Also, how is this stored in the configuration? If it's stored in a display specific section, that means the author has to take this into account in their config spec if they want to add their own display specific settings in the future. How is the display initialised with this port?

I thing this needs heavy refactoring :).

Actually, on further thought (and I need feedback here), I'm not convinced this requires total refactor. Note that this is different to synth settings, as those are only relevant once the synth is initialised. The port setting has to be available before initialisation.

My idea is this:

• !BrailleDisplayDriver classes can implement a getPossiblePorts() classmethod.
• The base implementation will raise !NotImplementedError, in which case no port selection will be possible and NVDA will not try to initialise with a port.
• Otherwise, it must return an !OrderedDict of {port: displayName}; e.g. {"com1": _("serial: com1")}
• There can be helper functions in hwPortUtils which return, for example, all serial ports in this format, but a driver is also able to include its own ports.
• The !BrailleDisplayDriver constructor is then called with a port keyword argument specifying the selected port.

Question:

• How do we store this in the configuration? I guess we can store it in a display specific section.
  As to handling probing (but I think this should be done later, not in the initial implementation):
• !BrailleDisplayDriver classes would have a canProbe bool attribute and a probe() classmethod.
• probe() would take a port argument and return true if found and false if not.
• Alternatively, probe() might take no arguments and the display determines what ports to probe. I'm not sure whether this is needed.

Side note: if we do a refactoring on braille displays we can thing about automatic display detection.

This is a bit different to probing, as it should be non-dangerous; probing is active, whereas auto-detection is passive. I guess we can have an autoDetect() method or similar which passively scans for USB/bluetooth. Again, I don't think we should do this for the initial implementation.

It's good to have someone to brainstorm this with. Frankly, one of the other reasons I haven't been willing to do this is that I need an advanced dev who actually cares about this/is affected by it and can test it.

[nvaccessAuto]

Comment 14 by nvdakor on 2012-08-28 23:52
Mostly for James:
Here are possible issues you may have (from users' perspective):

• Display name: some users would change their display name (say, on a BrailleNote), so if we probe for a display, using static name would not work (this is the method that Apple might be using). In other situations, one display may may emulate protocol of another, so it could complicate things.
• When you probe, you may need to make sure a given port is opened (particularly for Bluetooth, as some people use dongles). For BT displays, you need a way to automatically pair with the desired display (some displays do allow this) and choose the serial port profile (mostly concerns PDA devices such as Braille Sense which may expose multiple profiles).
• For probing order, I think we should give USB or BT higher priority, as they are faster than serial. In my thinking, the probing algorithm should be:
  If NvDA uses one thread for probing:
• Check to see if BT is installed. This may involve looking up device entries.
• If BT is found, check to see if BT is on.
• If BT is not installed or BT is off, move onto USB.
• If BT is on, start searching for devices; but then we may run into timing problem, as we might need to check from various static device name entries to see if we get a response from one of the "correct" devices. Once the device is found, try establishing auto pair; if not, we may have to stop and move onto USB.
• If USB is chosen, poll USB ports to see if a device is connected and if the desired display device (again using a static display name entry) is found and is ready to receive braille output from NVDA (via Liblouis). we may run into possible problems, as many use USB 2.0 ports and if a hub is involved.
• If USB probing fails, then resort to serial probing.
• If serial probing fails, then we have no display for the current session.
  If NVDA may use multiple threads for probing:
• Search for BT and USB at the same time.
• Whatever returns with the display early - that connection would be used. However, we would have race condition issues and USB hub problem.
• If either fails, probe serial.
  The single thread would suit all cases, but it may lead to longer init time (I think). Multiple threads would work (if written right), but it is not optimal for many and it would be wasting resources (if threads fail to return with a display).
  For auto detection, could be possible if:
• A braille display uses static display name.
• if the probing subroutine looks for the display on specific ports.
  I agree with James on this matter.
  For actual implementation, I think we should ask users, gather feedback and ask people interested in this to perhaps create a branch, with braille display users and driver devs working together to come up with a solution in the long term (due to number of braille displays out there).

[nvaccessAuto]

Comment 15 by jteh (in reply to comment 14) on 2012-08-29 01:16
Replying to nvdakor:

• Display name: some users would change their display name (say, on a BrailleNote)

I meant display name as in the name of the port; e.g. "serial: com1". The issue you mention is relevant to bluetooth detection, though.

• When you probe, you may need to make sure a given port is opened (particularly for Bluetooth, as some people use dongles). For BT displays, you need a way to automatically pair with the desired display (some displays do allow this) and choose the serial port profile (mostly concerns PDA devices such as Braille Sense which may expose multiple profiles).

Imo, we shouldn't handle pairing and should instead leave it up to the user and OS, at least for now. That is what we currently expect for bluetooth displays and many other applications expect this also.

[nvaccessAuto]

Comment 16 by aliminator (in reply to comment 13) on 2012-08-29 10:58
Replying to jteh:

My idea is this:

• !BrailleDisplayDriver classes can implement a getPossiblePorts() classmethod.
• The base implementation will raise !NotImplementedError, in which case no port selection will be possible and NVDA will not try to initialise with a port.

In case of an exception, NVDA can fall back to the standard method of querying braille drivers so that no existing driver needs to be modified.

Question:

• How do we store this in the configuration? I guess we can store it in a display specific section.

Like the subsection in the speech's one.
Does the developer has to take care for the port settings or can NVDA handle it when !true is returned from probing?

[nvaccessAuto]

Comment 17 by ragb (in reply to comment 13) on 2012-08-29 13:51
Replying to jteh:

Actually, on further thought (and I need feedback here), I'm not convinced this requires total refactor. Note that this is different to synth settings, as those are only relevant once the synth is initialised. The port setting has to be available before initialisation.

True.

My idea is this:

• !BrailleDisplayDriver classes can implement a getPossiblePorts() classmethod.

This method returns USB, bluetooth and/or serial ports, (or those that are supported). Is that right?

• The base implementation will raise !NotImplementedError, in which case no port selection will be possible and NVDA will not try to initialise with a port.
• Otherwise, it must return an !OrderedDict of {port: displayName}; e.g. {"com1": _("serial: com1")}
• There can be helper functions in hwPortUtils which return, for example, all serial ports in this format, but a driver is also able to include its own ports.

Yes. For instance, those drivers that are based on vendor libraries (Freedom scientific, for instance) could report better display names and such (I don't know if the fs library supports display listing, this was for example purposes only).

• The !BrailleDisplayDriver constructor is then called with a port keyword argument specifying the selected port.

Question:

• How do we store this in the configuration? I guess we can store it in a display specific section.

I think so. If there are other specific display settings to store (can you think of any?) we could also store them on that section. Maybe we should generate the config spec based on this, as in synth drivers. However I can't imagine other specific display settings.

As to handling probing (but I think this should be done later, not in the initial implementation):

• !BrailleDisplayDriver classes would have a canProbe bool attribute and a probe() classmethod.

Agreed. And not for the first implementation phase.

• probe() would take a port argument and return true if found and false if not.
• Alternatively, probe() might take no arguments and the display determines what ports to probe. I'm not sure whether this is needed.

If the first alternative is called with all ports reported be !etPossiblePorts (the method you mensioned above) this seems to work. However the 2nd alternative makes posible the driver to check on a different order or something. I'm for the first one though.

Side note: if we do a refactoring on braille displays we can thing about automatic display detection.

This is a bit different to probing, as it should be non-dangerous; probing is active, whereas auto-detection is passive. I guess we can have an autoDetect() method or similar which passively scans for USB/bluetooth. Again, I don't think we should do this for the initial implementation.

That makes sence. For USB devices we can use device IDS, as it's done on most drivers now. For bluetooth we can have name checks, but (when possible) bluetooth addresses (those have some manofacture part which might be useful. Just a thought though).

It's good to have someone to brainstorm this with. Frankly, one of the other reasons I haven't been willing to do this is that I need an advanced dev who actually cares about this/is affected by it and can test it.

That is good to know :).

[nvaccessAuto]

Comment 18 by ragb (in reply to comment 14) on 2012-08-29 14:03
Replying to nvdakor:

• Display name: some users would change their display name (say, on a BrailleNote), so if we probe for a display, using static name would not work (this is the method that Apple might be using). In other situations, one display may may emulate protocol of another, so it could complicate things.

If the manofactures colaborate maybe we can find a better wy to identify bluetooth devices, using bluetooth address, for instance. But I dobt this.

[...]

• For probing order, I think we should give USB or BT higher priority, as they are faster than serial.

I'd say USB, bluetooth, serial. If one connects a display via USB he/she wants to use it that way, I supose.

In my thinking, the probing algorithm should be:

If NvDA uses one thread for probing:

1. Check to see if BT is installed. This may involve looking up device entries.
2. If BT is found, check to see if BT is on.
3. If BT is not installed or BT is off, move onto USB.
4. If BT is on, start searching for devices; but then we may run into timing problem, as we might need to check from various static device name entries to see if we get a response from one of the "correct" devices. Once the device is found, try establishing auto pair; if not, we may have to stop and move onto USB.

As Jamie has stated, I think we should only consider already paired devices. Handling various bluetooth stacks is a mess, or at least it was before...

1. If USB is chosen, poll USB ports to see if a device is connected and if the desired display device (again using a static display name entry) is found and is ready to receive braille output from NVDA (via Liblouis). we may run into possible problems, as many use USB 2.0 ports and if a hub is involved.
2. If USB probing fails, then resort to serial probing.
3. If serial probing fails, then we have no display for the current session.

If NVDA may use multiple threads for probing:

I don't think we need the complexity of multi-threading for this. The only real advantage I see is checking various ports at the same time, because checking for the presence of various displays implies hard synchronization problems. It's doable but I don't think we need that, and no determinism in display choice, etc. However it is possible to do, I believe.

1. Search for BT and USB at the same time.
2. Whatever returns with the display early - that connection would be used. However, we would have race condition issues and USB hub problem.

Note that this is non-deterministic. That is, the returned display can be different on different runs, even with the same connected hardware. I don't think this is good for the user experience.

1. If either fails, probe serial.

The single thread would suit all cases, but it may lead to longer init time (I think). Multiple threads would work (if written right), but it is not optimal for many and it would be wasting resources (if threads fail to return with a display).

Don't think the performance enhancement woud be that high so I'm for te single thread aproach. This is not that important, though.

[nvaccessAuto]

Comment 19 by ragb (in reply to comment 15) on 2012-08-29 14:06
Replying to jteh:

Imo, we shouldn't handle pairing and should instead leave it up to the user and OS, at least for now. That is what we currently expect for bluetooth displays and many other applications expect this also.

+1. Handling bluetooth stacks is not something I'd like to do, whenever possible.

[nvaccessAuto]

Comment 20 by jteh (in reply to comment 17) on 2012-08-30 08:47
Replying to ragb:

• !BrailleDisplayDriver classes can implement a getPossiblePorts() classmethod.

This method returns USB, bluetooth and/or serial ports, (or those that are supported). Is that right?

Technically, it can return whatever ports it likes, but yes, this will usually return USB, bluetooth and serial ports.

[nvaccessAuto]

Comment 21 by ragb on 2012-08-30 12:26
So...
The discussion on this ticket is a bit more than serial braille display support (although it is included too). I can implement it. As it requires time (for testing and coding) i propose this feature for the release after 2012.3. It could be done for 2012.3 I believe but I would'nt able to help that much in september.

[nvaccessAuto]

Comment 23 by jteh on 2012-10-19 07:45
Changes:
Milestone changed from None to 2012.3

[nvaccessAuto]

Comment 24 by jteh on 2012-10-23 02:38
Oops. Wrong milestone.
Changes:
Milestone changed from 2012.3 to 2013.1

[nvaccessAuto]

Comment 25 by ragb on 2012-11-02 14:26
As this requires Colaboration from braille display drivers' developers, I'll start coding support in NVDA port setting ports and such (initial design). active Probing and more advanced features I think we can postphone, and implement if needed.

[nvaccessAuto]

Comment 26 by ragb on 2012-11-02 15:44
New branch for this here:

http://bzr.nvaccess.org/nvda/braillePorts/

• Added port selection for the freedom scientific driver.
• Add port choice to the brailleSettingsDialog and to the config (one port for each display in a display-specific section).
• Drivers that don't support only have, as port, the "automatic", which is the current behavoiour of trying usb/bluetooth connections.

Question: Waht to do for drivers that can't support automatic detection, such as serial-only displays? Don't show the automatic setting? So, in that case, what would be the default for those displays? (we have nothing like that in NVDA of course, but will possibly have, since we want to support serial-only displays).

[nvaccessAuto]

Comment by ragb on 2012-11-03 12:02
(In #2012) If #426 gets implemented, I think the driver can be included in NVDA core (instead of beeing an add-on). It would be also good supporting newer models of the braillenote, but I need testers or a device to hack on, which seems both dificult to find :).

Adding blocking information ad reassigning to me 8since I have a device here to text with).

[nvaccessAuto]

Comment 28 by jteh (in reply to comment 26) on 2012-11-05 03:57
Replying to ragb:

New branch for this here:

Nice work! Looks pretty good!

• Added port selection for the freedom scientific driver.

I'm not sure i follow the need for this. I guess i see the point in having USB and bluetooth options, but why list the bluetooth ports separately? I guess a user might have multiple FS displays paired via bluetooth, but that seems extremely rare.

• Add port choice to the brailleSettingsDialog and to the config (one port for each display in a display-specific section).

It'd be great if the port choice could be disabled if only the auto port is available.

• Drivers that don't support only have, as port, the "automatic", which is the current behavoiour of trying usb/bluetooth connections.

Makes sense.

Question: Waht to do for drivers that can't support automatic detection, such as serial-only displays? Don't show the automatic setting?

Yes.

So, in that case, what would be the default for those displays?

Hmm. I thought of this a few days ago too. I think the best we can do is default to the first port. This is entirely arbitrary, but the only other option is to have a "none" option, which is just silly.

I also wonder whether we should introduce some functions in hwPortUtils to get lists of port names and descriptions in the right format given certain criteria. This way, anyone wanting to support serial ports, bluetooth ports, etc. can just call those functions. An added benefit is that the descriptions will be localised consistently; e.g. "serial: com1", "bluetooth", etc.

[nvaccessAuto]

Comment 29 by ragb (in reply to comment 28) on 2012-11-05 11:32
Replying to jteh:

Replying to ragb:

New branch for this here:

Nice work! Looks pretty good!

:)

• Added port selection for the freedom scientific driver.

I'm not sure i follow the need for this. I guess i see the point in having USB and bluetooth options, but why list the bluetooth ports separately? I guess a user might have multiple FS displays paired via bluetooth, but that seems extremely rare.

My point was more wards to show the bluetooth name of the device, so the user is sure of what device he will connect to. If it is to be automatic, he can simply choose the automatic option. Having two paired displays is more of a side benefit. But it was just a momentary decision, I'm happy to put just bluetooth there and have the driver choose the first bluetooth device, which, unless you happen to work for some FS dealer or something, would be always the right decision (the only one)..

• Add port choice to the brailleSettingsDialog and to the config (one port for each display in a display-specific section).

It'd be great if the port choice could be disabled if only the auto port is available.

Yes, let me see how one does that with wx Python. Probably we can hide that, but I guess making it disabled would be better.

• Drivers that don't support only have, as port, the "automatic", which is the current behavoiour of trying usb/bluetooth connections.

Makes sense.

Question: Waht to do for drivers that can't support automatic detection, such as serial-only displays? Don't show the automatic setting?

Yes.

Ok. My idea is to add another property to the base BrailleDisplay driver class, such as canDoAutomaticPortSelecction, which will be true by default.

So, in that case, what would be the default for those displays?

Hmm. I thought of this a few days ago too. I think the best we can do is default to the first port. This is entirely arbitrary, but the only other option is to have a "none" option, which is just silly.

Agreed.

I also wonder whether we should introduce some functions in hwPortUtils to get lists of port names and descriptions in the right format given certain criteria. This way, anyone wanting to support serial ports, bluetooth ports, etc. can just call those functions. An added benefit is that the descriptions

will be localised consistently; e.g. "serial: com1", "bluetooth", etc.

Makes all sence. Many of the same logic is already duplicated on many drivers, this will help on that too.

[nvaccessAuto]

Comment 30 by ragb (in reply to comment 28) on 2012-11-06 09:36
Replying to jteh:

I also wonder whether we should introduce some functions in hwPortUtils to get lists of port names and descriptions in the right format given certain criteria. This way, anyone wanting to support serial ports, bluetooth ports, etc. can just call those functions. An added benefit is that the descriptions will be localised consistently; e.g. "serial: com1", "bluetooth", etc.

I've got some dobts here. For serial ports, you simply list COM ports and return (in the description) something like "serial: COM1". But for bluetooth ports, what to do? Returning simply bluetooth? This way we may have more than one port with the description "bluetooth". Same question for USB ports: We should just return USB in the description? What to do if more than one USB found? This is a rare case of course. One idea is simply to only return the first USB or bluetooth port found.

BTW, these listing functions must receive some sort of predicate to filter the returned ports: i.g. list of bluetooth ports with blutooth names on smoe list, or starting with something (the driver must know those details).

[nvaccessAuto]

Comment 31 by jteh (in reply to comment 30) on 2012-11-07 09:17
Replying to ragb:

• Added port selection for the freedom scientific driver.

I'm not sure i follow the need for this. I guess i see the point in having USB and bluetooth options, but why list the bluetooth ports separately?

My point was more wards to show the bluetooth name of the device, so the user is sure of what device he will connect to.

That doesn't help for Freedom Scientific because the bluetooth name is always the same.

If it is to be automatic, he can simply choose the automatic option.

True, though that will always prefer USB if present.

I'm happy to put just bluetooth there and have the driver choose the first bluetooth device, which, unless you happen to work for some FS dealer or something, would be always the right decision (the only one)..

Personally, that'd be my vote. I'm not convinced the fs driver needs port selection at all, since it can all be done automatically, but I'm assuming you wanted this for a reason.

Question: Waht to do for drivers that can't support automatic detection, such as serial-only displays? Don't show the automatic setting?

My idea is to add another property to the base BrailleDisplay driver class, such as canDoAutomaticPortSelecction, which will be true by default.

I see you've done that. Looks fine to me.

My original idea was that the driver itself would provide the automatic option. By default, this means that drivers that don't need port selection have no ports at all. This avoids the need to specially support an automatic option. I'm happy with your approach, though I'm curious as to what you think of this simpler approach.

Replying to ragb:

I also wonder whether we should introduce some functions in hwPortUtils to get lists of port names and descriptions in the right format given certain criteria.

I've got some dobts here. For serial ports, you simply list COM ports and return (in the description) something like "serial: COM1". But for bluetooth ports, what to do? Returning simply bluetooth?

Hmm. You're right. This idea doesn't really work in practice, does it? :)

Same question for USB ports: We should just return USB in the description? What to do if more than one USB found?

USB doesn't fit into hwPortUtils anyway because there are no standard USB port IDs.

I'm starting to think we should just leave it alone for now. :)

[nvaccessAuto]

Comment 32 by ragb (in reply to comment 31) on 2012-11-09 13:33
Replying to jteh:

Replying to ragb:

My point was more wards to show the bluetooth name of the device, so the user is sure of what device he will connect to.

That doesn't help for Freedom Scientific because the bluetooth name is always the same.

For the same moel yes. But it seems that now there are the focus 14 blue, which we don't have the name for. I'll add another ticket to solve that. But yes, for each model the name is the same.

If it is to be automatic, he can simply choose the automatic option.

True, though that will always prefer USB if present.

Yes.

I'm happy to put just bluetooth there and have the driver choose the first bluetooth device, which, unless you happen to work for some FS dealer or something, would be always the right decision (the only one)..

Personally, that'd be my vote. I'm not convinced the fs driver needs port selection at all, since it can all be done automatically, but I'm assuming you wanted this for a reason.

I wanted this so I can choose bluetooth, even if the driver is connected via USB. On one hand I can be charging it and disconnect without loosing braille and, when using virtual machines, it's easier to switch the display from host to guest if they are connected in different ways. But this is a very peculiar usecase. And that is the display I have here for testing so I coded it to easily test the port selection. I'm happy to revert it and just use automatic port selection, if you and others think would be better to.

Replying to ragb:

I also wonder whether we should introduce some functions in hwPortUtils to get lists of port names and descriptions in the right format given certain criteria.

I've got some dobts here. For serial ports, you simply list COM ports and return (in the description) something like "serial: COM1". But for bluetooth ports, what to do? Returning simply bluetooth?

Hmm. You're right. This idea doesn't really work in practice, does it? :)

Probably not. But we can have soe more utility methods on hwPortUtils to get bluetooth ports and such, to easy on the implementation of getPossiblePorts. That would be always passing some filter to hwPortUtils,ustComPorts

Same question for USB ports: We should just return USB in the description? What to do if more than one USB found?

USB doesn't fit into hwPortUtils anyway because there are no standard USB port IDs.

True.

I'm starting to think we should just leave it alone for now. :)

If we find many duplication o implementing drivers we can simply refactor the code and create some utility methods there.

[nvaccessAuto]

Comment 38 by jteh (in reply to comment 37) on 2012-11-22 21:07
Replying to ragb:

My original idea was that the driver itself would provide the automatic option. By default, this means that drivers that don't need port selection have no ports at all. This avoids the need to specially support an automatic option.

Maybe I'm not getting your point. Actual drivers (i.g., those I didn't modify) only suport the automatic port selection, because getPossiblePorts is not implemented, so there are no ports for these displays.

My idea was that the driver would be responsible for returning an automatic option in getPossiblePorts if required. If getPossiblePorts is not implemented, there are no ports; it isn't applicable. For displays which have only one connection possibility, I'm not sure the name "automatic" makes sense. This also avoids the need for the extra canDoAutomaticPortSelection attribute.

Replying to ragb:

in getPossiblePorts I report COM that are not bluetooth ports (as the commit message states, the filter seems quite weak). However, I beleive this (or a better check) can be refactored to be on hwPortUtils.

What sort of check? We can only be sure of bluetooth ports, as the user might want to use a USB serial converter or similar and there's no standard way to detect those.

One dobt I have is what to report for the serail port name, that is, what is shown to the user on the GUI. Now the driver reports something like "Serial port: COM1" (this can be translated). However this might not be a friendly name. Many ports have friendly names that are quite readable (although most not translated) and this might be useful information to the users.

Why not just do "serial: {friendlyName}"? From what I've seen, the friendly name usually includes the port as well, and if it doesn't, the user probably renamed it. Also, I prefer "serial: " instead of "serial port: ", as port is redundant here.

[nvaccessAuto]

Comment 39 by ragb (in reply to comment 38) on 2012-11-23 16:11
Replying to jteh:

Replying to ragb:

My original idea was that the driver itself would provide the automatic option. By default, this means that drivers that don't need port selection have no ports at all. This avoids the need to specially support an automatic option.

Maybe I'm not getting your point. Actual drivers (i.g., those I didn't modify) only suport the automatic port selection, because getPossiblePorts is not implemented, so there are no ports for these displays.

My idea was that the driver would be responsible for returning an automatic option in getPossiblePorts if required. If getPossiblePorts is not implemented, there are no ports; it isn't applicable. For displays which have only one connection possibility, I'm not sure the name "automatic" makes sense. This also avoids the need for the extra canDoAutomaticPortSelection attribute.

I understand. in braillePorts,5602 I removed the attributed and returned automatic as another port in the FS driver. The problem I found is on what to store on the config when no port selection is possible (or not applicable). I'm storing auto, but maybe None would be a better option. What do you think?

Replying to ragb:

in getPossiblePorts I report COM that are not bluetooth ports (as the commit message states, the filter seems quite weak). However, I beleive this (or a better check) can be refactored to be on hwPortUtils.

What sort of check? We can only be sure of bluetooth ports, as the user might want to use a USB serial converter or similar and there's no standard way to detect those.

I was talking about the check on the braillenote driver to filter out bluetooth ports (as we only want serial ports, either native or using a converter).

        ports = OrderedDict()
        for p in hwPortUtils.listComPorts():
            if p[and p.get("bluetoothAddress") is None:
                ports[p["port"]('port'].startswith("COM"))] = _("Serial: {portName}").format(portName=p["friendlyName"])
        return ports

It seems a bit week IMO, but I can't get with a better implementation.

One dobt I have is what to report for the serail port name, that is, what is shown to the user on the GUI. Now the driver reports something like "Serial port: COM1" (this can be translated). However this might not be a friendly name. Many ports have friendly names that are quite readable (although most not translated) and this might be useful information to the users.

Why not just do "serial: {friendlyName}"? From what I've seen, the friendly name usually includes the port as well, and if it doesn't, the user probably renamed it. Also, I prefer "serial: " instead of "serial port: ", as port is redundant here.

Agreed. Done in change set:braillePorts,5603

[nvaccessAuto]

Comment 40 by jteh (in reply to comment 39) on 2012-11-24 06:51
Replying to ragb:

I understand. in braillePorts,5602 I removed the attributed and returned automatic as another port in the FS driver.

Just to be sure, do you think this is actually a good idea? I wasn't entirely certain myself, which is why I was curious about your thoughts.

The problem I found is on what to store on the config when no port selection is possible (or not applicable). I'm storing auto, but maybe None would be a better option. What do you think?

Perhaps None or an empty value? I'm not sure how configobj handles None and empty.

I was talking about the check on the braillenote driver to filter out bluetooth ports (as we only want serial ports, either native or using a converter).

          if p['port'].startswith("COM") and p.get("bluetoothAddress") is None:

I'm not sure if the COM startswith is necessary. These should all be COM ports anyway. Is there something else you're trying to filter out? I don't see how you could possibly do any better or why you would want to.

[nvaccessAuto]

Comment 41 by ragb (in reply to comment 40) on 2012-11-25 13:16
Replying to jteh:

Replying to ragb:

I understand. in braillePorts,5602 I removed the attributed and returned automatic as another port in the FS driver.

Just to be sure, do you think this is actually a good idea? I wasn't entirely certain myself, which is why I was curious about your thoughts.

I think so. This way we can diferentiate between displays that can use automatic port selection and those which can't do port selection at all.
What is needed is to change the default on the config for None or something that is not a port, since now "auto" is like any other port. Note that as "auto" must be the first item on the port list in the settings dialog, when the user saves braille settings without changing the port, it should be chosen as default and stored.
One thing I'm not sure is if if we need to find if a display supports port setting, should we catch the NotImplementedError, or have an attribute. On the GUI there is now problem, since we need the port list anyway, but we may need to check that on brailleDisplayDriver.setDisplayName. Now it isn't checked because I'm trusting in the port default value on the driver constructor, but, for drivers that doesn't support automatic setting (or any reasonable default), that doesn't work. This is something like a chicken and eg problem: if the driver needs a port to be constructed and the user didn't choose any on the gui and there is no reasonable default, what to choose? I'd say throw an exception. In practice I only see this happening when updating NVDA and restarting with some braille display set, which, in the newer version, needs a defined port. Maybe not probable but I may forget it so it's writen here.

I was talking about the check on the braillenote driver to filter out bluetooth ports (as we only want serial ports, either native or using a converter).

            if p['port'].startswith("COM") and p.get("bluetoothAddress") is None:

I'm not sure if the COM startswith is necessary. These should all be COM ports anyway. Is there something else you're trying to filter out? I don't see how you could possibly do any better or why you would want to.

Does this not return also USB ports? Say for mobile phones, or something connected. Usually those have different names from "COMx", unless acting as a modem or something.

[nvaccessAuto]

Comment 42 by jteh (in reply to comment 41) on 2012-11-25 22:09
Replying to ragb:

Note that as "auto" must be the first item on the port list in the settings dialog, when the user saves braille settings without changing the port, it should be chosen as default and stored.

I guess we can just always select the first port if there's no port in the config.

One thing I'm not sure is if if we need to find if a display supports port setting, should we catch the NotImplementedError, or have an attribute.

I think catch the exception. This avoids the need for another attribute.

we may need to check that on brailleDisplayDriver.setDisplayName. Now it isn't checked because I'm trusting in the port default value on the driver constructor, but, for drivers that doesn't support automatic setting (or any reasonable default), that doesn't work.

This won't ever happen for built-in drivers. All of the current built-in drivers support automatic port detection, so they won't suddenly be unable to . Unfortunately, the situation isn't as clear for add-on drivers and I guess we'll need to throw an exception for those as you suggest.

This does raise another issue, though. I guess the port name for automatic as returned by drivers should always be None. Otherwise, initialising from a previous config won't work.

I'm not sure if the COM startswith is necessary. These should all be COM ports anyway.

Does this not return also USB ports? Say for mobile phones, or something connected.

Well, USB emulated serial ports, yes.

Usually those have different names from "COMx", unless acting as a modem or something.

I've never seen a USB serial port with a different name, but that's not to say there aren't any. Can you give an example?

[nvaccessAuto]

Comment 43 by jteh (in reply to comment 42) on 2012-11-25 22:12
Replying to jteh:

This does raise another issue, though. I guess the port name for automatic as returned by drivers should always be None. Otherwise, initialising from a previous config won't work.

I guess this was one advantage of your previous implementation. Auto was always None; the author didn't have to explicitly know that. Tricky. Now I'm not sure which implementation we should go for. :) I'm not keen on making displays support both "auto" and None for auto port detection; seems rather inelegant.

[nvaccessAuto]

Comment 44 by ragb (in reply to comment 42) on 2012-11-26 20:11
Replying to jteh:

Replying to ragb:

Note that as "auto" must be the first item on the port list in the settings dialog, when the user saves braille settings without changing the port, it should be chosen as default and stored.

I guess we can just always select the first port if there's no port in the config.

Agreed. I implemented it that way in change set:braillePorts,5604 and simplified that code a bit.

we may need to check that on brailleDisplayDriver.setDisplayName. Now it isn't checked because I'm trusting in the port default value on the driver constructor, but, for drivers that doesn't support automatic setting (or any reasonable default), that doesn't work.

This won't ever happen for built-in drivers. All of the current built-in drivers support automatic port detection, so they won't suddenly be unable to . Unfortunately, the situation isn't as clear for add-on drivers and I guess we'll need to throw an exception for those as you suggest.

Agreed. Now it throws a type error because drivers that don't support automatic setting (hipoteticaly) require a port argument. If no port is selected code will invoke the driver constructor with no arguments, so the type error. (probably a bit confusing, easy to write in code).

This does raise another issue, though. I guess the port name for automatic as returned by drivers should always be None. Otherwise, initialising from a previous config won't work.

For now, I'm storing "auto" if automatic port is select (on the GUI) and "" if nothing was selected (None doesn't work due to configobj implementation - see brailePorts,5605). What happens is that (at least the fs driver which is the only one supporting explicit automatic port selection) as the port argument defaultin to "auto", which makes it work as old driver (when passing no arguments).

I'm not sure if the COM startswith is necessary. These should all be COM ports anyway.

Does this not return also USB ports? Say for mobile phones, or something connected.

Well, USB emulated serial ports, yes.

Usually those have different names from "COMx", unless acting as a modem or something.

I've never seen a USB serial port with a different name, but that's not to say there aren't any. Can you give an example?

I think I've seen those, but can't find one now. Lets ignore it until I find :).

[nvaccessAuto]

Comment 45 by ragb (in reply to comment 43) on 2012-11-26 20:16
Replying to jteh:

Replying to jteh:

This does raise another issue, though. I guess the port name for automatic as returned by drivers should always be None. Otherwise, initialising from a previous config won't work.

I guess this was one advantage of your previous implementation. Auto was always None; the author didn't have to explicitly know that. Tricky. Now I'm not sure which implementation we should go for. :) I'm not keen on making displays support both "auto" and None for auto port detection; seems rather inelegant.

The choices are "auto" or "" (None doesn't work for storing). As it is implemented now, we can have both stored, however they mean slightly different things: "" means nothing was selected on the GUI before (old drivers and drivers that weren't configured at least once) and "auto", which means "auto" was selected on the GUI for the display. Practically speaking they mean the same, because all builtin-drivers fallback to automatic port selection which is the current behavior for main and previous versions. However, if we add i.g. the braillenote driver, "auto" doesn't make sence for it (at least while bluetooth support is not implemented).

[nvaccessAuto]

Comment 46 by jteh on 2012-11-28 05:08
Okay. I'm happy with this. So, let's try to get this merged sooner rather than later.

A tiny bit of code review:

In braille.py:

@@ -1489,6 +1499,20 @@

+ AUTOMATIC_PORT = ("auto", _("Automatic"))

Needs translators comment.

+     If the driver supports automatic port setting it should return as the first port C{brailleDisplayDriver.AUTOMATIC_PORT}

Change the C{blah} to L{blah}, as this entity will be in the documentation, so using L{blah} will create a link to it.

In brailleDisplayDrivers/brailleNote.py:

+ description = _("BrailleNote")

Needs translators comment.

+ def _getBluetoothPorts(cls):
+     return (p[for p in hwPortUtils.listComPorts() \
+     if "bluetoothName" in p and any(p["bluetoothName"]("port"]).startswith(prefix) for prefix in bluetoothName_prefixes))

This is really minor cosmetic stuff and is more preference, but thought I'd mention it anyway. I prefer to avoid the \ line continuation sign and instead use brackets, which seem more natural. In this case, you already have brackets, so it is redundant. Also, it might be helpful to indent the continued line by one more tab to make it clear it's a continuation.

The User Guide needs to be updated for these enhancements, including adding documentation for the !BrailleNote driver. The !BrailleNote driver also needs a What's New entry.

Aside from these points, I'm happy for this to be merged.

[nvaccessAuto]

Comment 47 by ragb (in reply to comment 46) on 2012-11-28 14:25
Replying to jteh:

Okay. I'm happy with this. So, let's try to get this merged sooner rather than later.

A tiny bit of code review:

[...]
Done. Forgot the doxygen stuff buut I'll push it with the next commits.

The User Guide needs to be updated for these enhancements, including adding documentation for the !BrailleNote driver. The !BrailleNote driver also needs a What's New entry.

Ok. I'll probably you to review the gramar and such but I'll do it.

Aside from these points, I'm happy for this to be merged.

Nice :)

[nvaccessAuto]

Comment 48 by jteh on 2012-12-03 08:04
Sorry, one more nit. In gui/settingsDialogs.py, in BrailleSettingsDialog.updatePossiblePorts:

      self.possiblePorts = [      self.portSelectionPossible = True
      try:
          for k, v in displayCls.getPossiblePorts().iteritems():
              self.possiblePorts.append((k,v),)
      except NotImplementedError:
          self.portSelectionPossible = False

I'd probably simplify this to:

        try:
            self.possiblePorts = list(displayCls.getPossiblePorts().iteritems())
            self.portSelectionPossible = True
        except NotImplementedError:
            self.possiblePorts = [](]
>)

            self.portSelectionPossible = False

I'd also consider removing the portSelectionPossible check altogether and instead just doing a boolean check on possiblePorts. The condition self.possiblePorts will evaluate to True if the list is non-empty and False if it's empty. This has the added bonus of protecting against a stupid driver returning no ports, though that's just a side effect, since that would be an authoring error. :)

Also, Joseph's recent log on #2012 highlights the case where a port was previously configured but is not present on the system when the dialog is opened. You can probably fix this and simplify the code like this:

            try:
                selectedPort = config.conf[= [p[0]("braille"][displayName]["port"]
                portNames) for p in self.possiblePorts]
                selection = portNames.index(selectedPort)
            except (KeyError, ValueError):
                # Display not in config or configured port not valid.
                selection = 0

[nvaccessAuto]

Comment 49 by ragb (in reply to comment 48) on 2012-12-03 11:56
Replying to jteh:

I'd probably simplify this to:

      try:
          self.possiblePorts = list(displayCls.getPossiblePorts().iteritems())
          self.portSelectionPossible = True
      except NotImplementedError:
          self.possiblePorts = []
          self.portSelectionPossible = False

I'd also consider removing the portSelectionPossible check altogether and instead just doing a boolean check on possiblePorts. The condition self.possiblePorts will evaluate to True if the list is non-empty and False if it's empty. This has the added bonus of protecting against a stupid driver returning no ports, though that's just a side effect, since that would be an authoring error. :)

Yes, that variable is not needed, unless for readability or something. Done in change set:braillePorts,5615

Also, Joseph's recent log on #2012 highlights the case where a port was previously configured but is not present on the system when the dialog is opened. You can probably fix this and simplify the code like this:

Actually I considered this case but I had implemented it wrong :$. Thanks. It's commited too.

[nvaccessAuto]

Comment 50 by aliminator on 2012-12-05 09:51
Is it possible to merge those changes into main so we could test it and try to adopt changes to the serial Papenmeier driver (#2679)

[nvaccessAuto]

Comment 51 by jteh on 2012-12-05 11:26
This isn't ready for merging just yet due to the !BrailleNote driver. I can build snapshots for this branch in the interim if needed.

[nvaccessAuto]

Comment 52 by aliminator on 2012-12-05 11:35
That would be great.

[nvaccessAuto]

Comment 53 by jteh on 2012-12-05 11:45
I've just configured the server to build snapshots for braillePorts. The first should hopefully appear in about 15 minutes.

[nvaccessAuto]

Comment 54 by ragb on 2012-12-05 16:24
Hi,

I pushed my changes (mostly documentation). It probably needs some language corrections before merging into main. I'm not sure if the changes entries are right (please confirm).

[nvaccessAuto]

Comment 55 by jteh on 2012-12-05 22:53
I've made a few minor changes. There's also some minor code review in #2012. Once that's addressed, I'm happy for you to merge this.

[nvaccessAuto]

Comment 56 by aliminator on 2012-12-06 10:21
We have updated the driver in 2679 so far.
Please provide some feedback there.

[nvaccessAuto]

Comment 57 by jteh on 2012-12-07 06:04
Rui, I see you've made the brailleNote driver changes. Are you happy for this to be merged now? I'm happy for you to merge it unless you would prefer me to do it.

[nvaccessAuto]

Comment 58 by ragb (in reply to comment 57) on 2012-12-07 10:55
Replying to jteh:

Rui, I see you've made the brailleNote driver changes. Are you happy for this to be merged now? I'm happy for you to merge it unless you would prefer me to do it.

I prefer you to merge it, if you don't mind. I'm still not confortable with bzr merges, and I don't want to screw up main :).

[nvaccessAuto]

Comment 59 by jteh on 2012-12-07 11:05
Merged in b41bcdf. Thanks once again for your great work!
Changes:
State: closed