fix(serial): list a COM port once, in the order a human reads it

An operator's keyer dropdown showed COM1 and COM3 twice, in the order
COM1 COM1 COM3 COM8 COM9 COM4 COM3, with "COM3COM3" as the selected value —
two entries sharing one value both counted as selected.

Not our bug to start with, but ours to absorb. Windows enumerates serial
ports from HARDWARE\DEVICEMAP\SERIALCOMM, which maps a DEVICE PATH to a COM
name, so the same name legitimately appears twice when two devices claim it:
a driver uninstalled without cleaning up after itself, or virtual-port
software. go.bug.st/serial returns one entry per device, faithfully.

Deduplicated and naturally sorted in the one binding every port dropdown in
the app calls, so CAT, rotators, amplifiers and the keyer all get it. COM4
now sits between COM3 and COM8 rather than after COM9, which is how a port
gets overlooked on a machine with a dozen.

A dropped duplicate is logged rather than swallowed: two devices holding one
COM name is also a good reason for one of them to report that port busy, and
that is not something an operator can work out from a list.

app.go

@@ -16538,7 +16538,72 @@ type WinkeyerSettings struct {
 
 // ListSerialPorts returns the available COM ports for the keyer dropdown.
 func (a *App) ListSerialPorts() ([]string, error) {
-	return winkeyer.ListPorts()
+	ports, err := winkeyer.ListPorts()
+	if err != nil {
+		return nil, err
+	}
+	return tidySerialPorts(ports), nil
+}
+
+// tidySerialPorts removes duplicates and puts the list in the order a human
+// reads it.
+//
+// Windows enumerates serial ports from HARDWARE\DEVICEMAP\SERIALCOMM, which maps
+// a DEVICE path to a COM name — so the same name legitimately appears twice when
+// two devices claim it. That happens with a driver uninstalled without cleaning
+// up after itself, and with virtual-port software. An operator saw COM1 and COM3
+// listed twice, and the dropdown showed "COM3COM3" as its value, because two
+// entries with the same value both counted as selected.
+//
+// The order is natural, not lexical: COM4 belongs between COM3 and COM8, and
+// listing it after COM9 is how a port gets overlooked on a machine with a dozen.
+// Anything not named COMn keeps its own alphabetical order, after them.
+//
+// A duplicate is worth a log line rather than silence: two devices holding one
+// COM name is also why one of them is "busy" when the other is opened, and that
+// is not something an operator can work out from a dropdown.
+func tidySerialPorts(ports []string) []string {
+	seen := make(map[string]bool, len(ports))
+	out := make([]string, 0, len(ports))
+	for _, p := range ports {
+		name := strings.TrimSpace(p)
+		if name == "" {
+			continue
+		}
+		key := strings.ToUpper(name)
+		if seen[key] {
+			applog.Printf("serial: %s is claimed by more than one device in the Windows port map — listing it once", name)
+			continue
+		}
+		seen[key] = true
+		out = append(out, name)
+	}
+	sort.SliceStable(out, func(i, j int) bool {
+		ni, oki := comPortNumber(out[i])
+		nj, okj := comPortNumber(out[j])
+		switch {
+		case oki && okj:
+			return ni < nj
+		case oki:
+			return true // COMn before anything else
+		case okj:
+			return false
+		}
+		return out[i] < out[j]
+	})
+	return out
+}
+
+// comPortNumber extracts n from "COMn", false for any other shape.
+func comPortNumber(s string) (int, bool) {
+	if len(s) <= 3 || !strings.EqualFold(s[:3], "COM") {
+		return 0, false
+	}
+	n, err := strconv.Atoi(s[3:])
+	if err != nil {
+		return 0, false
+	}
+	return n, true
 }
 
 // GetWinkeyerSettings returns the persisted keyer config (with sane defaults).

changelog.json

@@ -5,12 +5,14 @@
     "en": [
       "Send Spot: the comment now carries the award references after the mode — the ones you assigned (POTA, SOTA, IOTA…), not the DXCC, zone and prefix every reader works out from the callsign. A self-spot carries your OWN activation references instead.",
       "Modes: a fresh install now starts with SSB, CW, FT8, FT4, FT2, RTTY, PSK31 and FM. AM and DIGITALVOICE stay in the available list but are no longer selected by default.",
-      "Chase new: a panel listing the stations PSK Reporter is hearing within about 300 km of you that are new against your log — entity, band, mode, slot, prefix or square. Click one to put it in the entry and tune the rig. Digital modes only, and it shares the feed the band-opening watch and the locator store already use."
+      "Chase new: a panel listing the stations PSK Reporter is hearing within about 300 km of you that are new against your log — entity, band, mode, slot, prefix or square. Click one to put it in the entry and tune the rig. Digital modes only, and it shares the feed the band-opening watch and the locator store already use.",
+      "Serial ports: a port claimed by two devices in the Windows port map was listed twice in every port dropdown, and showed as “COM3COM3”. Listed once now, and in natural order — COM4 between COM3 and COM8, not after COM9."
     ],
     "fr": [
       "Envoi de spot : le commentaire porte désormais les références de diplôme après le mode — celles que vous avez attribuées (POTA, SOTA, IOTA…), pas le DXCC, la zone et le préfixe que chacun déduit de l’indicatif. Un auto-spot porte VOS références d’activation.",
       "Modes : une installation neuve démarre avec SSB, CW, FT8, FT4, FT2, RTTY, PSK31 et FM. AM et DIGITALVOICE restent dans la liste disponible mais ne sont plus sélectionnés par défaut.",
-      "Chasse au nouveau : un panneau listant les stations que PSK Reporter entend à moins de 300 km de chez vous et qui sont nouvelles par rapport à votre log — entité, bande, mode, créneau, préfixe ou carré. Un clic la met en saisie et accorde la radio. Modes numériques uniquement, et le flux est partagé avec la veille d’ouvertures et la base des locators."
+      "Chasse au nouveau : un panneau listant les stations que PSK Reporter entend à moins de 300 km de chez vous et qui sont nouvelles par rapport à votre log — entité, bande, mode, créneau, préfixe ou carré. Un clic la met en saisie et accorde la radio. Modes numériques uniquement, et le flux est partagé avec la veille d’ouvertures et la base des locators.",
+      "Ports série : un port revendiqué par deux périphériques dans la table Windows apparaissait en double dans toutes les listes, et s’affichait « COM3COM3 ». Une seule fois désormais, et dans l’ordre naturel — COM4 entre COM3 et COM8, pas après COM9."
     ]
   },
   {

serialports_test.go

@@ -0,0 +1,46 @@
+package main
+
+import (
+	"strings"
+	"testing"
+)
+
+// The exact list an operator's machine produced: COM1 and COM3 each claimed by
+// two devices in the Windows port map, and no order to speak of. The dropdown
+// showed every duplicate, and rendered its own value as "COM3COM3" because two
+// entries with the same value both counted as selected.
+func TestTidySerialPortsDeduplicatesAndOrders(t *testing.T) {
+	got := tidySerialPorts([]string{"COM1", "COM1", "COM3", "COM8", "COM9", "COM4", "COM3"})
+	want := []string{"COM1", "COM3", "COM4", "COM8", "COM9"}
+	if strings.Join(got, ",") != strings.Join(want, ",") {
+		t.Errorf("got %v, want %v", got, want)
+	}
+}
+
+// COM10 after COM9, not between COM1 and COM2 — lexical order is how a port
+// gets overlooked on a machine with a dozen of them.
+func TestTidySerialPortsSortsNaturally(t *testing.T) {
+	got := tidySerialPorts([]string{"COM10", "COM2", "COM1", "COM20", "COM3"})
+	want := []string{"COM1", "COM2", "COM3", "COM10", "COM20"}
+	if strings.Join(got, ",") != strings.Join(want, ",") {
+		t.Errorf("got %v, want %v", got, want)
+	}
+}
+
+// Not every port is a COMn: a device path or a Unix name must survive, after
+// the numbered ones, in its own order.
+func TestTidySerialPortsKeepsOtherNames(t *testing.T) {
+	got := tidySerialPorts([]string{"/dev/ttyUSB1", "COM3", "/dev/ttyUSB0", "COM1", "", "  "})
+	want := []string{"COM1", "COM3", "/dev/ttyUSB0", "/dev/ttyUSB1"}
+	if strings.Join(got, ",") != strings.Join(want, ",") {
+		t.Errorf("got %v, want %v", got, want)
+	}
+}
+
+// Case is not identity on Windows: "com3" and "COM3" are one port, and letting
+// both through would put the same duplicate back in the list.
+func TestTidySerialPortsIgnoresCase(t *testing.T) {
+	if got := tidySerialPorts([]string{"COM3", "com3"}); len(got) != 1 || got[0] != "COM3" {
+		t.Errorf("got %v, want [COM3]", got)
+	}
+}