show port description in port dropdown menu

[antistic]

Summary of changes

Add port description to the dropdown of available ports. E.g. instead of /dev/ttyACM0 it shows /dev/ttyACM0 - EcoSteno.

This was mentioned briefly in #789 but doesn't address the main issue (but might be useful anyway).

Pull Request Checklist

• Changes have tests
• News fragment added in news.d. See documentation for details

[sammdot]

• Can you give an example of what this would look like (either as an image or the text equivalent)? I'm trying port.description on my machine and I don't seem to get anything (either empty or n/a) for any of my serial ports.
• Is it safe to assume that the port names won't have spaces? Is there maybe a neater way of adding values to the individual items?

[benoit-pierre]

On which platform(s) was this tested? (I think detailed info is not available on all platform), see: https://pythonhosted.org/pyserial/tools.html#serial.tools.list_ports.comports.

[benoit-pierre]

* Is it safe to assume that the port names won't have spaces? Is there maybe a neater way of adding values to the individual items?

I agree.

[antistic]

Tested on Linux (I don't have access to another platform atm)

[benoit-pierre]

This approach is fundamentally at odds with the way the combo box is configured: it's specifically setup to be editable so (more knowledgeable) users can edit the field (e.g. to force the use of a by-id device link on Linux, or use a fake serial port for testing).

Instead, we could use a multi-lines tooltip, something along those lines:

 plover/gui_qt/machine_options.py | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git i/plover/gui_qt/machine_options.py w/plover/gui_qt/machine_options.py
index 61a7a60e..62f92bb5 100644
--- i/plover/gui_qt/machine_options.py
+++ w/plover/gui_qt/machine_options.py
@@ -1,6 +1,6 @@
 from copy import copy
 
-from PyQt5.QtCore import QVariant, pyqtSignal
+from PyQt5.QtCore import Qt, QVariant, pyqtSignal
 from PyQt5.QtWidgets import QWidget
 
 from serial import Serial
@@ -14,6 +14,25 @@
 _ = get_gettext()
 
 
+def port_info_tooltip(port_info):
+    parts = []
+    description = getattr(port_info, 'description')
+    if description not in {None, 'n/a'}:
+        tooltip.append(description)
+    for attr, fmt in (
+        ('manufacturer', _('- manufacturer: {value}')),
+        ('product', _('- product: {value}')),
+    ):
+        value = getattr(port_info, attr)
+        if value:
+            tooltip.append(fmt.format(value=value))
+    if not parts:
+        return None
+    if len(parts) > 1:
+        parts.insert(1, '')
+    return '\n'.join(parts)
+
+
 class SerialOption(QWidget, Ui_SerialWidget):
 
     valueChanged = pyqtSignal(QVariant)
@@ -63,7 +82,11 @@ def _update(self, field, value):
 
     def on_scan(self):
         self.port.clear()
-        self.port.addItems(sorted(x[0] for x in comports()))
+        for n, port_info in enumerate(sorted(comports())):
+            self.port.addItem(port_info.device)
+            tooltip = port_info_tooltip(port_info)
+            if tooltip is not None:
+                self.port.setItemData(n, tooltip, Qt.ToolTipRole)
 
     def on_port_changed(self, value):
         self._update('port', value)

(Patch against master)

[antistic]

This approach is fundamentally at odds with the way the combo box is configured: it's specifically setup to be editable so (more knowledgeable) users can edit the field (e.g. to force the use of a by-id device link on Linux, or use a fake serial port for testing).

That wasn't intended, maybe I should have set this pull request to WIP since I'm new to Qt and still figuring it out 😅 . I think in theory it should be okay to use the model approach since QComboBox does use a QStandardItem under the hood, but replacing it wasn't quite as straightforward as I hoped.

Instead, we could use a multi-lines tooltip, something along those lines:

I think that's much simpler code-wise, and contains more useful information, but I know that I personally wouldn't even think to try looking at the tool tip to see this information. I could imagine it being a more useful if the tooltip appeared quicker/instantly?

[benoit-pierre]

Instead, we could use a multi-lines tooltip, something along those lines:

I think that's much simpler code-wise, and contains more useful information, but I know that I personally wouldn't even think to try looking at the tool tip to see this information. I could imagine it being a more useful if the tooltip appeared quicker/instantly?

Agreed.

[petercpark]

When will this be merged? I think this is great.

[violet-fish]

At the very least, adding path completion to the text box would go a long way to making it easier to change out keyboards and use /dev/serial/by-id/ instead of the /dev/TTYAMC0 (which changes every time the board is unplugged)

[antistic]

I wanted to come back to this PR because it seems popular, but I'm not sure I understand what the current issues are.

I don't really like the tooltip idea — it seems really different UX-wise to other programs I've used, so I would prefer a different solution. But I don't think I can currently propose a better solution than the current implementation until I understand what the issues are.

My guesses:

• It doesn't give any extra information for Windows
• It doesn't give all the available information on other platforms (e.g. does not include manufacturer on Linux)
• The list item text and data are not the same, and this might be confusing
• It needs to be rebased on the current master

@benoit-pierre (or other reviewers), can you let me know if I've missed anything, the priority of the problems, and which problems are deal breakers?

[benoit-pierre]

• It doesn't give any extra information for Windows
• It doesn't give all the available information on other platforms (e.g. does not include manufacturer on Linux)
• The list item text and data are not the same, and this might be confusing
• It needs to be rebased on the current master

Yes, see #1510.

[antistic]

👍

Closing in favour of #1510