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) + } +}