fix(tci): a refused un-key no longer leaves the rig keyed for good

The trx handler stamped its PTT cache BEFORE commanding the radio and left
it in place when the command failed. An operator running JTDX over TCI with
an Icom on CI-V lost an un-key to a lost acknowledgement: the cache recorded
"off" regardless, and from then on every trx:0,false was dismissed as a
repeat of a state the radio had never reached. The cache is per-server, not
per-connection, so reconnecting JTDX changed nothing either -- the
transmitter stayed keyed into the amplifier, with no drive, until the radio
was switched off by hand.

The cache is now written only on success, and a failure clears "known"
outright so the next command reaches the radio whatever it is.

Second guard: releasePTT drops a PTT this server asserted when the client
disconnects, and when the server stops -- before the CAT backend goes down,
while the rig is still reachable. rigctld has had that since a K3 sat in
transmit for 29 s; the TCI server was written without it, so an operator
moving from Hamlib to TCI silently lost the protection. A later log shows
the rig keyed for 40 s across a JTDX reconnect for exactly that reason.

internal/tciserver/tciserver.go

@@ -178,7 +178,41 @@ func (s *Server) Start() error {
 }
 
 // Stop closes the listener and every client.
+// releasePTT drops a PTT this server asserted, and does it once.
+//
+// A client that dies mid-over — or a settings save that closes the server —
+// leaves the rig keyed with nobody left to un-key it, into an amplifier that has
+// no idea the transmission ended. rigctld has had this guard for a while (a K3
+// once sat in transmit for 29 s until the CAT link happened to be rebuilt); the
+// TCI server was written without it, so an operator who moved from Hamlib to TCI
+// silently lost the protection.
+//
+// pttKnown is cleared whatever happens: after an emergency unkey the radio's
+// state is a guess, and the next command must reach it rather than be dismissed
+// as a repeat.
+func (s *Server) releasePTT(why string) {
+	s.mu.Lock()
+	keyed := s.ptt
+	s.ptt, s.pttKnown = false, false
+	s.mu.Unlock()
+	if !keyed {
+		return
+	}
+	s.log("tci server: %s while the rig was keyed — dropping PTT", why)
+	if err := s.rig.SetPTT(false); err != nil {
+		s.log("tci server: emergency unkey FAILED: %v", err)
+		return
+	}
+	// Any client still attached is told, so a second logger's transmit indicator
+	// does not stay lit over a rig that is back in receive.
+	s.broadcast("trx:0,false;")
+}
+
 func (s *Server) Stop() {
+	// Before anything is torn down: the CAT backend is still up here, so an unkey
+	// still lands. Same ordering as rigctld.Stop for the same reason.
+	s.releasePTT("TCI server stopped")
+
 	s.mu.Lock()
 	if s.closed {
 		s.mu.Unlock()
@@ -266,6 +300,8 @@ func (s *Server) serve(c *client, remote string) {
 	s.mu.Unlock()
 	_ = c.conn.Close()
 	s.log("tci server: %s disconnected", remote)
+	// A client that walks away mid-over must not leave the rig transmitting.
+	s.releasePTT("client " + remote + " left")
 }
 
 // initBlock is the initialisation set from §4.1 of the protocol document, in
@@ -484,16 +520,33 @@ func (s *Server) handle(c *client, cmd string) string {
 		// 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 {
+			// The cache is stamped ONLY on success, and a failure clears "known"
+			// outright so the NEXT command — whatever it is — reaches the radio.
+			//
+			// It used to be written before the radio was commanded and left in
+			// place when the command failed. That is how a rig got stuck keyed for
+			// good: the un-key failed on a lost CI-V acknowledgement, the cache
+			// recorded "off" regardless, and from then on every trx:0,false was
+			// dismissed as a repeat of a state the radio had never reached. Not
+			// even reconnecting the client cleared it — this cache is per-server,
+			// not per-connection — so the transmitter stayed keyed into the
+			// amplifier until the operator switched the radio off. A cache must
+			// never claim something the radio refused.
+			s.mu.Lock()
+			s.pttKnown = false
+			s.mu.Unlock()
 			s.log("tci server: PTT %v refused: %v", on, err)
 			return ""
 		}
+		s.mu.Lock()
+		s.ptt, s.pttKnown = on, true
+		s.mu.Unlock()
 		s.log("tci server: PTT %s", map[bool]string{true: "ON", false: "off"}[on])
 		s.broadcast(fmt.Sprintf("trx:0,%t;", on))
 		return ""

internal/tciserver/tciserver_test.go

@@ -16,6 +16,7 @@ type fakeRig struct {
 	txHz         int64
 	ptt          bool
 	splitErr     error
+	pttErr       error
 	calls        []string
 }
 
@@ -34,6 +35,10 @@ func (r *fakeRig) SetMode(m string) error {
 	return nil
 }
 func (r *fakeRig) SetPTT(on bool) error {
+	// Refused BEFORE the state moves, like a radio that never got the frame.
+	if r.pttErr != nil {
+		return r.pttErr
+	}
 	r.calls = append(r.calls, fmt.Sprintf("ptt=%v", on))
 	r.ptt = on
 	return nil
@@ -328,3 +333,54 @@ func TestRepeatedPTTIsNotResentToTheRadio(t *testing.T) {
 		t.Errorf("the radio was told %v, want the change through and the repeat dropped", r.calls)
 	}
 }
+
+// A refused un-key must never be remembered as done.
+//
+// The failure an operator hit running JTDX over TCI with an Icom on CI-V: the
+// rig went to transmit, the un-key was refused on a lost acknowledgement, and
+// from then on NOTHING could take it out of transmit. The cache had stamped
+// "off" before the radio was even commanded and kept it after the refusal, so
+// every later trx:0,false was dismissed as a repeat of a state the radio had
+// never reached. It is per-server, not per-connection, so reconnecting the
+// client changed nothing either — the transmitter stayed keyed into the
+// amplifier, with no drive, until the radio was switched off by hand.
+func TestARefusedUnkeyIsNotRememberedAsDone(t *testing.T) {
+	r := &fakeRig{freq: 14074000, rxFreq: 14074000, mode: "USB"}
+	s := srv(r)
+	ask(t, s, "trx:0,true")
+	if !r.ptt {
+		t.Fatal("the rig was never keyed — the test would prove nothing")
+	}
+
+	r.pttErr = fmt.Errorf("icom: timeout waiting for response")
+	ask(t, s, "trx:0,false")
+	if !r.ptt {
+		t.Fatal("the fake rig un-keyed on a refusal — the test would prove nothing")
+	}
+
+	// The client asks again, and this time the radio answers. It MUST be told.
+	r.pttErr = nil
+	ask(t, s, "trx:0,false")
+	if r.ptt {
+		t.Error("still keyed: the refused un-key was cached as done and the retry was dropped as a repeat")
+	}
+}
+
+// A client that walks away mid-over must not leave the rig transmitting, and
+// the release must be once-only — a second call has nothing to un-key and must
+// not re-command a radio that is already receiving.
+func TestReleasePTTUnkeysOnceWhenTheClientLeaves(t *testing.T) {
+	r := &fakeRig{freq: 14074000, rxFreq: 14074000, mode: "USB"}
+	s := srv(r)
+	ask(t, s, "trx:0,true")
+
+	s.releasePTT("client left")
+	if r.ptt {
+		t.Error("the rig is still keyed after the client left")
+	}
+	n := len(r.calls)
+	s.releasePTT("client left")
+	if len(r.calls) != n {
+		t.Errorf("released twice — the radio was told %v", r.calls)
+	}
+}