fix(amps): combined amplifiers are commanded together, power level included

Two faults in the combiner coupling, both reported from the operating position. THE POWER LEVEL WAS NEVER COUPLED. ON, OFF and OPERATE fanned out to the group; L/M/H did not — it was simply the command nobody had linked. Two combined amplifiers left at different power levels feed the combiner unevenly, which is the thing the coupling exists to prevent. THE COMMANDS DID NOT LEAVE TOGETHER. The second amplifier was commanded only once the first had answered, and an SPE answers over its own link in its own time. The combiner heard power appear on one input before the other and beeped about it, on every OFF and every ON. Each target now gets a goroutine, all parked on one channel until every one is ready; closing it releases them together. That is the difference between "start one, then start the other" and "both leave at once" — they have separate clients and separate connections, so nothing downstream re-serialises them. It matters most on the power level, which is not one command at all: an SPE has no "set level", so the driver taps the POWER key and waits for the amp to report the new one before tapping again — up to three taps, up to two seconds each. One after the other, the pair would sit at different levels for six seconds. A single amplifier still runs inline: no goroutine, no barrier, nothing new to go wrong for the operators who have one amp. The one that was clicked stays first, because its failure is the one worth reporting.
2026-08-17 10:42:19 +02:00
parent e5c9ca1a7d
commit dc898ce2af
3 changed files with 156 additions and 22 deletions
@@ -1,6 +1,11 @@
 package main

-import "testing"
+import (
+	"errors"
+	"sync"
+	"testing"
+	"time"
+)

 // The coupling is a SET, not a global switch.
 //
@@ -61,3 +66,73 @@ func TestLinkedAmpsNeedsTwo(t *testing.T) {
 		}
 	}
 }
+
+// Two combined amplifiers must be commanded AT THE SAME TIME, not one after the
+// other.
+//
+// Sequentially, the second was commanded only once the first had answered — and
+// an SPE answers over its own link, in its own time. The combiner heard power
+// appear on one input before the other and beeped about it, on every OFF and
+// every ON. This is what an operator hears, so it is worth a test that would
+// hear it too.
+func TestLinkedAmpCommandsLeaveTogether(t *testing.T) {
+	a := &App{}
+	const slow = 150 * time.Millisecond
+
+	var mu sync.Mutex
+	starts := map[string]time.Time{}
+	err := a.ampFanOut([]string{"one", "two"}, func(id string) error {
+		mu.Lock()
+		starts[id] = time.Now()
+		mu.Unlock()
+		time.Sleep(slow) // an amplifier taking its time to answer
+		return nil
+	})
+	if err != nil {
+		t.Fatalf("fan-out: %v", err)
+	}
+	if len(starts) != 2 {
+		t.Fatalf("%d amplifiers were commanded, want both", len(starts))
+	}
+	// Both goroutines wait on one channel and are released by closing it, so the
+	// gap is scheduling noise. Sequential execution would put a full command
+	// between them.
+	gap := starts["one"].Sub(starts["two"])
+	if gap < 0 {
+		gap = -gap
+	}
+	if gap > slow/3 {
+		t.Errorf("the two amplifiers were commanded %v apart — the combiner hears that as one input arriving late", gap)
+	}
+}
+
+// The amplifier the operator clicked comes first, and its failure is the one
+// reported: "the amp I pressed did not respond" beats the same message about
+// its silent partner.
+func TestLinkedAmpErrorNamesTheOneClicked(t *testing.T) {
+	a := &App{}
+	clicked := errors.New("the one clicked")
+	other := errors.New("the other one")
+	err := a.ampFanOut([]string{"clicked", "other"}, func(id string) error {
+		if id == "clicked" {
+			return clicked
+		}
+		return other
+	})
+	if !errors.Is(err, clicked) {
+		t.Errorf("fan-out reported %v, want the amplifier the operator pressed", err)
+	}
+}
+
+// One amplifier is the ordinary case and must not change: run inline, no
+// goroutine, no barrier, and the error straight back.
+func TestSingleAmpRunsInline(t *testing.T) {
+	a := &App{}
+	boom := errors.New("not running")
+	if err := a.ampFanOut([]string{"solo"}, func(string) error { return boom }); !errors.Is(err, boom) {
+		t.Errorf("a single amplifier reported %v, want the error itself", err)
+	}
+	if err := a.ampFanOut(nil, func(string) error { return boom }); err != nil {
+		t.Errorf("an empty group reported %v, want nothing to do", err)
+	}
+}
@@ -16740,13 +16740,55 @@ func (a *App) AmpOperate(id string, on bool) error {
 	// Fan out here rather than in the UI: the card and the docked widget both
 	// call this, and a coupling implemented in one of them would be missing from
 	// the other — which on a combiner means one amplifier keyed and one not.
-	var firstErr error
-	for _, tid := range a.ampTargets(id, a.GetLinkedAmps()) {
-		if err := a.ampOperateOne(tid, on); err != nil && firstErr == nil {
-			firstErr = err
+	return a.ampFanOut(a.ampTargets(id, a.GetLinkedAmps()), func(tid string) error {
+		return a.ampOperateOne(tid, on)
+	})
+}
+
+// ampFanOut runs one command against every amplifier AT THE SAME TIME.
+//
+// Sequentially, the second amplifier of a combined pair was commanded only once
+// the first had answered — and an SPE answers over its own link, in its own
+// time. The combiner heard power appear on one input before the other and
+// complained about it, on every OFF and every ON.
+//
+// So each target gets a goroutine, and they are all parked on the same channel
+// until every one of them is ready. Closing it releases them together: the
+// difference between "start one, then start the other" and "both leave at
+// once". They have separate clients and separate connections, so nothing
+// downstream re-serialises them.
+//
+// A single target — the ordinary case of one amplifier — runs inline. No
+// goroutine, no barrier, nothing to go wrong for the operators who have one amp.
+func (a *App) ampFanOut(targets []string, do func(id string) error) error {
+	if len(targets) == 0 {
+		return nil
+	}
+	if len(targets) == 1 {
+		return do(targets[0])
+	}
+	errs := make([]error, len(targets))
+	start := make(chan struct{})
+	var wg sync.WaitGroup
+	for i, id := range targets {
+		wg.Add(1)
+		go func(i int, id string) {
+			defer wg.Done()
+			<-start
+			errs[i] = do(id)
+		}(i, id)
+	}
+	close(start)
+	wg.Wait()
+	// The amplifier the operator actually clicked is first, and its failure is
+	// the one worth reporting: "the amp I pressed did not respond" beats the
+	// same message about its silent partner.
+	for _, err := range errs {
+		if err != nil {
+			return err
 		}
 	}
-	return firstErr
+	return nil
 }

 func (a *App) ampOperateOne(id string, on bool) error {
@@ -16774,15 +16816,11 @@ func (a *App) AmpPower(id string, on bool) (err error) {
 			applog.Printf("amp %s: power %v failed: %v", id, on, err)
 		}
 	}()
-	linked := a.GetLinkedAmps()
-	targets := a.ampTargets(id, linked)
-	var firstErr error
-	for _, tid := range targets {
-		if e := a.ampPowerOne(tid, on, len(targets) > 1); e != nil && firstErr == nil {
-			firstErr = e
-		}
-	}
-	return firstErr
+	targets := a.ampTargets(id, a.GetLinkedAmps())
+	multi := len(targets) > 1
+	return a.ampFanOut(targets, func(tid string) error {
+		return a.ampPowerOne(tid, on, multi)
+	})
 }

 func (a *App) ampPowerOne(id string, on, linked bool) error {
@@ -16812,12 +16850,31 @@ func (a *App) ampPowerOne(id string, on, linked bool) error {
 }

 // AmpPowerLevel selects the output power level — SPE only (L/M/H).
+//
+// Coupled like ON, OFF and OPERATE, which it was not: two combined amplifiers
+// left at different power levels feed the combiner unevenly, which is the thing
+// the coupling exists to prevent. It was simply the command nobody had linked.
+//
+// Running the pair concurrently matters more here than anywhere else. Setting a
+// level is not one command: an SPE has no "set level" at all, so the driver taps
+// the POWER key and waits for the amp to report the new level before tapping
+// again — up to three taps, up to two seconds each. One after the other, the two
+// amplifiers would sit at different levels for as long as six seconds.
 func (a *App) AmpPowerLevel(id, level string) error {
-	inst := a.ampInstByID(id)
-	if inst == nil || inst.spe == nil {
-		return fmt.Errorf("power level is an SPE feature")
-	}
-	return inst.spe.SetPowerLevel(level)
+	targets := a.ampTargets(id, a.GetLinkedAmps())
+	multi := len(targets) > 1
+	return a.ampFanOut(targets, func(tid string) error {
+		inst := a.ampInstByID(tid)
+		if inst == nil || inst.spe == nil {
+			// A PowerGenius sitting in the group has no L/M/H, and saying so
+			// would make a successful SPE pair look like a failure.
+			if multi {
+				return nil
+			}
+			return fmt.Errorf("power level is an SPE feature")
+		}
+		return inst.spe.SetPowerLevel(level)
+	})
 }

 // AmpFanMode sets the fan mode — PGXL only (STANDARD/CONTEST/BROADCAST).
@@ -9,7 +9,8 @@
      "TCI radios: when the rig forbids transmitting, PTT says so instead of doing nothing silently.",
      "WAJA carried Japan’s civil prefecture numbers instead of the JARL’s: 35 of the 47 references are renumbered.",
      "Award references can be renumbered in the editor — the number was the one field it would not let you correct.",
-      "The compass fills the moment Station Control opens, instead of waiting out the rest of a polling interval."
+      "The compass fills the moment Station Control opens, instead of waiting out the rest of a polling interval.",
+      "Combined amplifiers: the power level (L/M/H) is coupled too, and both amps are commanded at once so the combiner stops beeping."
    ],
    "fr": [
      "Une entité qui est un seul groupe d’îles remplit désormais la référence IOTA toute seule, sans abonnement callbook.",
@@ -18,7 +19,8 @@
      "Radios TCI : quand la radio interdit l’émission, le PTT le dit au lieu de ne rien faire en silence.",
      "WAJA portait les numéros civils des préfectures japonaises et non ceux de la JARL : 35 des 47 références sont renumérotées.",
      "Les références d’un diplôme se renumérotent dans l’éditeur : le numéro était le seul champ qu’il refusait de corriger.",
-      "La boussole se remplit dès l’ouverture de Station Control, au lieu d’attendre la fin d’un intervalle d’interrogation."
+      "La boussole se remplit dès l’ouverture de Station Control, au lieu d’attendre la fin d’un intervalle d’interrogation.",
+      "Amplis combinés : le niveau de puissance (L/M/H) est couplé lui aussi, et les deux amplis sont commandés en même temps — fini le bip du combineur."
    ]
  },
  {