diff --git a/internal/cat/kenwood.go b/internal/cat/kenwood.go index 9b96a66..967336e 100644 --- a/internal/cat/kenwood.go +++ b/internal/cat/kenwood.go @@ -130,8 +130,30 @@ type Kenwood struct { tx bool txAt time.Time lastState RigState + // ifRejects counts consecutive "?;" answers to IF;. See State(). + ifRejects int } +// errRigRejected marks a "?;" — the rig understood the frame and declined it. +// Distinct from a serial fault on purpose: one is "ask again in a moment", the +// other is "the link is gone", and collapsing them is what dropped CAT sharing. +var errRigRejected = errors.New("rejected by rig") + +// ifRejectGrace is how many consecutive "?;" answers to IF; are ridden out +// before the link is called dead. +// +// A Kenwood answers "?;" while it is busy — the tail of a transmission, a menu +// open on the front panel. The TS-590SG does it for a moment after RX;, which is +// exactly when the poll resumes: WSJT-X keyed through shared CAT, dropped PTT, +// and the very next IF; came back "?;". One rejected poll then tore down the +// whole link, WSJT-X lost the rig, and Hamlib went on to send an uninitialised +// frequency (2^63) that OpsLog rightly refused — an alarming error message whose +// real cause was three lines earlier in the log. +// +// Three at 250 ms is under a second of tolerance: long enough for the rig to +// finish whatever it was doing, far too short to hide an unplugged cable. +const ifRejectGrace = 3 + // SetLowerLines chooses whether DTR and RTS are deasserted on connect. Set // before Connect. func (k *Kenwood) SetLowerLines(v bool) { @@ -281,8 +303,21 @@ func (k *Kenwood) ReadState() (RigState, error) { } raw, err := k.ask("IF;") if err != nil { + // A "?;" is the rig saying "busy", not "gone". Ride out a few and keep + // serving the last known state, so the CAT link the digital software keys + // through survives the moment after a transmission. A serial fault is NOT + // covered: that returns a different error and drops through at once. + if errors.Is(err, errRigRejected) && k.lastState.Connected && k.ifRejects < ifRejectGrace { + k.ifRejects++ + debugLog.Printf("kenwood: IF; rejected (%d/%d) — rig busy, keeping the link", k.ifRejects, ifRejectGrace) + s := k.lastState + s.Connected = true + return s, nil + } + k.ifRejects = 0 return RigState{}, err } + k.ifRejects = 0 f, ok := parseKenwoodIF(raw) if !ok { return RigState{}, fmt.Errorf("kenwood: unparsable IF frame %q", raw) @@ -524,6 +559,10 @@ func (k *Kenwood) ask(cmd string) (string, error) { k.rx = k.rx[i+1:] traceText("kenwood", "RX", frame) if frame == "?;" { + // Sentinel-wrapped so the poll loop can tell "the rig said no" apart + // from a serial fault. The two look identical as plain errors and must + // not be handled the same way: one means try again in a moment, the + // other means the link is gone. // IF; and ID; are universal on Kenwood/Elecraft — a "?;" to them is a // transient "busy" (typically mid-transmit, or a menu open on the rig), // NOT "unsupported". Latching them off would blind the poll loop for @@ -533,7 +572,7 @@ func (k *Kenwood) ask(cmd string) (string, error) { k.unsupported[want] = true debugLog.Printf("kenwood: this rig does not support %q — not asking again", cmd) } - return "", fmt.Errorf("kenwood: %s rejected", want) + return "", fmt.Errorf("kenwood: %s rejected: %w", want, errRigRejected) } if strings.HasPrefix(frame, want) { return frame, nil diff --git a/internal/cat/kenwood_loopback_test.go b/internal/cat/kenwood_loopback_test.go index a0a87f0..9f1994e 100644 --- a/internal/cat/kenwood_loopback_test.go +++ b/internal/cat/kenwood_loopback_test.go @@ -174,6 +174,19 @@ func dialTo(rig *ts2000) func() (serial.Port, error) { } } +// dialToBusy is dialTo with a hook that makes the rig answer "?;" instead — +// what a real Kenwood does while it is busy, rather than staying silent. +func dialToBusy(rig *ts2000, busy func(cmd string) bool) func() (serial.Port, error) { + return func() (serial.Port, error) { + return &fakeSerial{toRig: &strings.Builder{}, answer: func(cmd string) string { + if busy(cmd) { + return "?;" + } + return rig.answer(cmd) + }}, nil + } +} + func TestKenwoodAgainstEmulatedRig(t *testing.T) { rig := &ts2000{vfoA: 14250000, vfoB: 14260000, mode: '2'} // 20 m USB k := NewKenwood("COM-TEST", 9600, "FT8") @@ -383,3 +396,63 @@ func TestKenwoodNoisyRigIsReportedAsNoiseNotSilence(t *testing.T) { t.Errorf("error was %q — it should say data arrived, and quote it", err) } } + +// A "?;" to IF; must not tear the link down. +// +// The TS-590SG answers "?;" for a moment after coming out of transmit, which is +// exactly when the poll resumes. In the field that single rejected poll dropped +// the whole CAT link: WSJT-X, keying through shared CAT, lost the rig, and +// Hamlib then sent an uninitialised frequency (2^63) that OpsLog refused — an +// alarming error whose real cause was three lines earlier. +func TestKenwoodBusyRigKeepsTheLink(t *testing.T) { + rig := &ts2000{vfoA: 14250000, vfoB: 14260000, mode: '2'} + busy := 0 + k := NewKenwood("COM-TEST", 9600, "FT8") + k.dialPort = dialToBusy(rig, func(cmd string) bool { + if strings.HasPrefix(cmd, "IF") && busy > 0 { + busy-- + return true + } + return false + }) + if err := k.Connect(); err != nil { + t.Fatalf("connect: %v", err) + } + defer k.Disconnect() + + // One good read establishes the state the busy window will keep serving. + first, err := k.ReadState() + if err != nil || !first.Connected { + t.Fatalf("first read: %+v err=%v", first, err) + } + + // Inside the grace: still connected, still reporting the last good frequency. + busy = ifRejectGrace + for i := 1; i <= ifRejectGrace; i++ { + s, err := k.ReadState() + if err != nil { + t.Fatalf("reject %d dropped the link: %v", i, err) + } + if !s.Connected || s.FreqHz != first.FreqHz { + t.Errorf("reject %d gave %+v — want the last good state, still connected", i, s) + } + } + + // The rig answers again → the tolerance resets, so a later busy spell gets + // the full allowance instead of inheriting the previous one. + if s, err := k.ReadState(); err != nil || !s.Connected { + t.Fatalf("recovery read failed: %+v err=%v", s, err) + } + + // Past the grace, a persistent refusal IS a fault and must surface. + busy = ifRejectGrace + 1 + var lastErr error + for i := 0; i <= ifRejectGrace; i++ { + if _, lastErr = k.ReadState(); lastErr != nil { + break + } + } + if lastErr == nil { + t.Error("a rig refusing IF; forever was still reported as healthy") + } +}