From f4e468c90f449cd6e79335d091edaa6ddcab0581 Mon Sep 17 00:00:00 2001
From: Dan Smith <dsmith@danplanet.com>
Date: Fri, 22 Dec 2023 09:47:47 -0800
Subject: [PATCH] Fix up port sorting for strange windows ports

After the previous change, if any  port string ends up without a COMn
port label we would compare an int to a str while sorting and break.
Fix this up by sorting the devices themselves so we can do a better
job and then format the label after.

Also log all the ports we get from the manifest for better forensics.

Fixes #11032
---
 chirp/wxui/clone.py                 | 21 ++++++++-----
 tests/unit/test_wxui_radiothread.py | 46 +++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+), 8 deletions(-)

diff --git a/chirp/wxui/clone.py b/chirp/wxui/clone.py
index 59f89ed32..fe3c2ee1a 100644
--- a/chirp/wxui/clone.py
+++ b/chirp/wxui/clone.py
@@ -220,12 +220,16 @@ def port_label(port):
 
 
 def port_sort_key(port):
-    _dev, label = port
-    m = re.match('^COM([0-9]+):.*', label)
-    if m:
-        return int(m.group(1))
-    else:
-        return label
+    key = port.device
+    if platform.system() == 'Windows':
+        try:
+            m = re.match('^COM([0-9]+)$', port.device)
+            if m:
+                key = 'COM%08i' % int(m.group(1))
+        except Exception as e:
+            LOG.warning('Failed to stable format %s: %s', port.device, e)
+
+    return key
 
 
 # Make this global so it sticks for a session
@@ -322,10 +326,11 @@ def set_ports(self, system_ports=None, select=None):
             '/dev/cu.Bluetooth-Incoming-Port',
         ]
 
+        LOG.debug('All system ports: %s', [x.__dict__ for x in system_ports])
         self.ports = [(port.device, port_label(port))
-                      for port in system_ports
+                      for port in sorted(system_ports,
+                                         key=port_sort_key)
                       if port.device not in filter_ports]
-        self.ports.sort(key=port_sort_key)
 
         favorite_ports = CONF.get('favorite_ports', 'state') or ''
         for port in favorite_ports.split(','):
diff --git a/tests/unit/test_wxui_radiothread.py b/tests/unit/test_wxui_radiothread.py
index 99f94380d..1088b8186 100644
--- a/tests/unit/test_wxui_radiothread.py
+++ b/tests/unit/test_wxui_radiothread.py
@@ -3,10 +3,17 @@
 from unittest import mock
 
 sys.modules['wx'] = wx = mock.MagicMock()
+sys.modules['wx.lib'] = mock.MagicMock()
+sys.modules['wx.lib.scrolledpanel'] = mock.MagicMock()
+sys.modules['wx.lib.sized_controls'] = mock.MagicMock()
+sys.modules['wx.richtext'] = mock.MagicMock()
+wx.lib.newevent.NewCommandEvent.return_value = None, None
+sys.modules['chirp.wxui.developer'] = mock.MagicMock()
 
 # These need to be imported after the above mock so that we don't require
 # wx to be present for these tests
 from tests.unit import base  # noqa
+from chirp.wxui import clone  # noqa
 from chirp.wxui import radiothread  # noqa
 
 
@@ -105,3 +112,42 @@ def test_thread_abort_priority(self):
         radio.set_memory.assert_not_called()
         radio.get_features.assert_not_called()
         wx.PostEvent.assert_not_called()
+
+
+class TestClone(base.BaseTest):
+    @mock.patch('platform.system', return_value='Linux')
+    def test_sort_ports_unix(self, system):
+        ports = [
+            mock.MagicMock(device='/dev/cu.zed',
+                           description='My Zed'),
+            mock.MagicMock(device='/dev/cu.abc',
+                           description='Some device'),
+            mock.MagicMock(device='/dev/cu.serial',
+                           description='')
+            ]
+        self.assertEqual(
+            ['Some device (cu.abc)',
+             'cu.serial',
+             'My Zed (cu.zed)'],
+            [clone.port_label(p)
+                for p in sorted(ports, key=clone.port_sort_key)])
+
+    @mock.patch('platform.system', return_value='Windows')
+    def test_sort_ports_windows(self, system):
+        ports = [
+            mock.MagicMock(device='COM7',
+                           description='Some serial device'),
+            mock.MagicMock(device='COM17',
+                           description='Some other device'),
+            mock.MagicMock(device='CNC0',
+                           description='Some weird device'),
+            mock.MagicMock(device='COM4',
+                           description=''),
+            ]
+        self.assertEqual(
+            ['CNC0: Some weird device',
+             'COM4',
+             'COM7: Some serial device',
+             'COM17: Some other device'],
+            [clone.port_label(p)
+                for p in sorted(ports, key=clone.port_sort_key)])