fix(tci server): split armed on the frequency asked for, in either order

Audit prompted by "are we sure the commands are implemented — split, Fake It,
Split rig?". The rigctl server is complete and hardened; the TCI one, three days
old, had reintroduced a bug rigctld had already paid for.

A client working split says two things — where to transmit, and that split is
on — and nothing obliges it to say them in that order. A write to channel B
while the rig was still simplex was DISCARDED, on the sound principle that
preparing a transmit frequency is not a request to QSY. But then the split was
armed on whatever the transmit VFO held, which is the receive frequency: the
operator transmits straight onto the DX while their software shows exactly what
they asked for. The frequency is now remembered and used when the split arrives,
which is what rigctld does with set_split_vfo / set_split_freq.

Two more from the same source:

Asking for a split state the rig is already in touches nothing. A client in Fake
It uses no split but still says so to be sure, and answering an error to a
request that was already true is what made JTDX abandon a transmission a second
into the frame through the rigctl server.

A repeated PTT command is not re-sent. One client restated it sixteen times a
second, and the Flex's own "xmit 1" was overwritten between two of them inside a
millisecond. The same radio sits behind this server — the operator reporting
this is on the Flex API backend.

Fake It itself needs nothing but channel A, and now has a test saying so.

changelog.json

@@ -7,14 +7,16 @@
       "CAT sharing can now speak TCI instead of Hamlib, so a TCI-only program reaches whatever radio OpsLog is on.",
       "Lookup cache: a TTL of 0 switches it off, so a callbook record you are correcting is re-read every time.",
       "TCI: when the radio forbids transmitting, PTT now says so instead of doing nothing silently.",
-      "TCI sharing: the server now announces transmit permission, without which a client such as MSHV never keys at all."
+      "TCI sharing: the server now announces transmit permission, without which a client such as MSHV never keys at all.",
+      "TCI sharing: split is armed on the frequency the client asked for, whichever order it sent the two commands in."
     ],
     "fr": [
       "Une entité qui est un seul groupe d’îles remplit désormais la référence IOTA toute seule, sans abonnement callbook.",
       "Le partage CAT peut désormais parler TCI au lieu de Hamlib : un logiciel TCI atteint la radio, quelle qu’elle soit.",
       "Cache des recherches : un TTL à 0 le désactive, pour relire à chaque fois une fiche callbook en cours de correction.",
       "TCI : quand la radio interdit l’émission, le PTT le dit désormais au lieu de ne rien faire en silence.",
-      "Partage TCI : le serveur annonce désormais l’autorisation d’émettre, sans laquelle un client comme MSHV ne passe jamais en émission."
+      "Partage TCI : le serveur annonce désormais l’autorisation d’émettre, sans laquelle un client comme MSHV ne passe jamais en émission.",
+      "Partage TCI : le split s’arme sur la fréquence demandée par le logiciel, quel que soit l’ordre de ses deux commandes."
     ]
   },
   {

internal/tciserver/tciserver.go

@@ -75,6 +75,27 @@ type Server struct {
 	conns  map[*client]struct{}
 	closed bool
 
+	// pendingTxHz is a transmit frequency a client set on channel B while the rig
+	// was still simplex.
+	//
+	// It must be REMEMBERED, not discarded. A client working split sends two
+	// commands and is free to send them in either order; when the frequency comes
+	// first, throwing it away means the split is then armed on whatever the
+	// transmit VFO happened to hold — the receive frequency — and the operator
+	// transmits straight onto the DX while their software shows exactly what they
+	// asked for. rigctld learned this the same way, and pairs set_split_vfo with
+	// set_split_freq for the same reason.
+	pendingTxHz int64
+
+	// ptt mirrors the last PTT state a client commanded, so a repeat can be
+	// recognised. A client is free to restate PTT as often as it likes, and one
+	// does: through the rigctl server Nexus sent set_ptt 0 about sixteen times a
+	// second, and the Flex's own "xmit 1" landed between two of them and was
+	// overwritten inside a millisecond — a transmit request that simply did
+	// nothing. The same radio sits behind this server.
+	ptt      bool
+	pttKnown bool
+
 	// last is what the clients have been told, so only changes are sent. TCI
 	// clients redraw on every command they receive; re-sending an unchanged
 	// frequency four times a second makes a VFO readout flicker and, in some
@@ -416,11 +437,15 @@ func (s *Server) handle(c *client, cmd string) string {
 			return ""
 		}
 		if arg(1) == "1" {
-			// Channel B is the transmit frequency, and it only means anything
+			// Channel B is the transmit frequency. Setting it while simplex must
-			// with split armed. Setting it while simplex would silently move the
+			// not move the rig's only VFO — the client asked to prepare a split
-			// rig's only VFO — the client asked to prepare a split TX frequency,
+			// transmit frequency, not to QSY — but it must not be thrown away
-			// not to QSY.
+			// either: it is where the split will be armed a moment from now.
+			s.mu.Lock()
+			s.pendingTxHz = hz
+			s.mu.Unlock()
 			if !split {
+				s.broadcast(fmt.Sprintf("vfo:0,1,%d;", hz))
 				return ""
 			}
 			if err := s.rig.SetSplit(true, hz); err != nil {
@@ -454,10 +479,22 @@ func (s *Server) handle(c *client, cmd string) string {
 			return reply("trx:0,false;")
 		}
 		on := strings.EqualFold(arg(1), "true")
+		// Only touch the radio on a CHANGE — restating a state is not a request
+		// to change it. The first command always goes through, since there is no
+		// knowing how the radio was left.
+		s.mu.Lock()
+		known, prev := s.pttKnown, s.ptt
+		s.ptt, s.pttKnown = on, true
+		s.mu.Unlock()
+		if known && prev == on {
+			s.broadcast(fmt.Sprintf("trx:0,%t;", on))
+			return ""
+		}
 		if err := s.rig.SetPTT(on); err != nil {
 			s.log("tci server: PTT %v refused: %v", on, err)
 			return ""
 		}
+		s.log("tci server: PTT %s", map[bool]string{true: "ON", false: "off"}[on])
 		s.broadcast(fmt.Sprintf("trx:0,%t;", on))
 		return ""
 
@@ -466,13 +503,34 @@ func (s *Server) handle(c *client, cmd string) string {
 			return reply(fmt.Sprintf("split_enable:0,%t;", split))
 		}
 		on := strings.EqualFold(arg(1), "true")
-		if err := s.rig.SetSplit(on, tx); err != nil {
+		// Already in the state asked for? Then it is done, and nothing goes to
+		// the radio. This is the lesson the rigctl server paid for: JTDX in "Fake
+		// It" uses no split but still says so to be sure, and a backend that
+		// cannot set split answered an error to a request that was already true.
+		// JTDX read that as rig control failing and abandoned the transmission a
+		// second into the frame. A refusal is only honest when something actually
+		// needed doing.
+		if on == split {
+			s.broadcast(fmt.Sprintf("split_enable:0,%t;", on))
+			return ""
+		}
+		// Arm on the frequency the client gave for channel B, which it is free to
+		// have sent before this command rather than after.
+		s.mu.Lock()
+		pending := s.pendingTxHz
+		s.mu.Unlock()
+		txHz := tx
+		if on && pending > 0 {
+			txHz = pending
+		}
+		if err := s.rig.SetSplit(on, txHz); err != nil {
 			// The refusal is the useful part: a backend that cannot split says
 			// so, and the client can tell the operator instead of transmitting
 			// on the wrong frequency believing all is well.
 			s.log("tci server: split %v refused: %v", on, err)
 			return ""
 		}
+		s.log("tci server: split %s, TX %d Hz", map[bool]string{true: "ON", false: "off"}[on], txHz)
 		s.broadcast(fmt.Sprintf("split_enable:0,%t;", on))
 		return ""
 

internal/tciserver/tciserver_test.go

@@ -248,3 +248,83 @@ func TestUnknownCommandsAreQuiet(t *testing.T) {
 		}
 	}
 }
+
+// Split, with the client sending the two commands in the order it prefers.
+//
+// A client working split has to say two things: where to transmit, and that
+// split is on. Nothing obliges it to say them in that order, and the frequency
+// arriving first is the dangerous case: discarding it and then arming split
+// leaves the transmit VFO on whatever it held — the RECEIVE frequency — so the
+// operator transmits straight onto the DX while their software shows exactly
+// what they asked for.
+func TestSplitIsArmedOnTheFrequencyTheClientGaveWhicheverOrderItCame(t *testing.T) {
+	// Frequency first, then split — the order that used to lose the frequency.
+	r := &fakeRig{freq: 14025000, rxFreq: 14025000, mode: "CW"}
+	s := srv(r)
+	ask(t, s, "vfo:0,1,14027000")
+	ask(t, s, "split_enable:0,true")
+	if len(r.calls) != 1 || r.calls[0] != "split=true,14027000" {
+		t.Errorf("frequency first: the radio was told %v, want split armed on 14027000", r.calls)
+	}
+
+	// Split first, then the frequency — the order that always worked.
+	r2 := &fakeRig{freq: 14025000, rxFreq: 14025000, mode: "CW"}
+	s2 := srv(r2)
+	ask(t, s2, "split_enable:0,true")
+	ask(t, s2, "vfo:0,1,14027000")
+	if len(r2.calls) == 0 || r2.calls[len(r2.calls)-1] != "split=true,14027000" {
+		t.Errorf("split first: the radio was told %v, want it to end on 14027000", r2.calls)
+	}
+}
+
+// "Fake It" uses no split at all: the client shifts the DIAL at the start of
+// transmit and shifts it back at the end. All it needs is channel A, and it
+// must reach the radio both ways.
+func TestFakeItIsJustTheDialMoving(t *testing.T) {
+	r := &fakeRig{freq: 14074000, rxFreq: 14074000, mode: "USB"}
+	s := srv(r)
+	ask(t, s, "vfo:0,0,14075300") // up for the over
+	ask(t, s, "vfo:0,0,14074000") // and back
+	want := []string{"freq=14075300", "freq=14074000"}
+	if strings.Join(r.calls, " ") != strings.Join(want, " ") {
+		t.Errorf("the radio was told %v, want %v", r.calls, want)
+	}
+}
+
+// A client in Fake It still says "split off" to be sure. The rig is already
+// simplex, so there is nothing to do — and saying so beats asking a backend
+// that may not be able to set split at all.
+//
+// This is what broke JTDX through the rigctl server: an error answered to a
+// request that was already true, read as rig control failing, and the
+// transmission abandoned a second into the frame.
+func TestSayingSplitOffWhenAlreadySimplexTouchesNothing(t *testing.T) {
+	r := &fakeRig{freq: 14074000, rxFreq: 14074000, mode: "USB",
+		splitErr: fmt.Errorf("this backend cannot split")}
+	s := srv(r)
+	ask(t, s, "split_enable:0,false")
+	if len(r.calls) != 0 {
+		t.Errorf("the radio was told %v for a state it was already in", r.calls)
+	}
+}
+
+// A client restating PTT must not re-command the radio. Through the rigctl
+// server, one sent set_ptt 0 sixteen times a second and the Flex's own transmit
+// request was overwritten between two of them inside a millisecond.
+func TestRepeatedPTTIsNotResentToTheRadio(t *testing.T) {
+	r := &fakeRig{freq: 14074000, rxFreq: 14074000, mode: "USB"}
+	s := srv(r)
+	for i := 0; i < 5; i++ {
+		ask(t, s, "trx:0,false")
+	}
+	if len(r.calls) != 1 || r.calls[0] != "ptt=false" {
+		// The FIRST one always goes through: there is no knowing how the radio
+		// was left.
+		t.Errorf("the radio was told %v, want one unkey and no repeats", r.calls)
+	}
+	ask(t, s, "trx:0,true")
+	ask(t, s, "trx:0,true")
+	if len(r.calls) != 2 || r.calls[1] != "ptt=true" {
+		t.Errorf("the radio was told %v, want the change through and the repeat dropped", r.calls)
+	}
+}