From bbe1b3ce80c5303386a6e87fd5cae09741d2afc6 Mon Sep 17 00:00:00 2001 From: Gregory Salaun Date: Mon, 17 Aug 2026 16:23:53 +0200 Subject: [PATCH] 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 | 55 ++++++++++++++++++++++++++- internal/tciserver/tciserver_test.go | 56 ++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+), 1 deletion(-) diff --git a/internal/tciserver/tciserver.go b/internal/tciserver/tciserver.go index ab858ed..172bce5 100644 --- a/internal/tciserver/tciserver.go +++ b/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 "" diff --git a/internal/tciserver/tciserver_test.go b/internal/tciserver/tciserver_test.go index 323468f..686a6eb 100644 --- a/internal/tciserver/tciserver_test.go +++ b/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) + } +}