fix(steppir): a silent controller wedged the driver for good

io.ReadFull cannot be used on a serial port, and it was.

On Windows a serial read that times out returns (0, nil) — on this transport a
timeout is not an error. io.ReadFull loops while err == nil, so a controller
that goes quiet for one poll, or answers with a truncated frame, spins it
forever. It holds ioMu throughout, and that is the whole failure the operator
sees:

  - the poll goroutine never returns, so nothing is ever logged about a fault
    and the cached status keeps the antenna looking connected;
  - every command blocks on the same mutex, and the trace line sat AFTER the
    lock, so even the attempt left no trace.

One dropped reply on a 4800-baud link therefore stopped the antenna responding
until OpsLog was restarted, with a log that showed the antenna starting, a few
status frames, and then nothing — which is exactly how it was reported.

Reads are now bounded: three seconds for an 11-byte frame that takes 23 ms on
the wire. Giving up returns an error, and the poll loop already knows what to do
with one — say so and reconnect. The command trace moved ahead of the lock, so
what the operator asked for is in the log even when the answer is not.

The SPE driver reads through a bufio.Reader, whose ErrNoProgress guard already
covers this; the ADIF parser reads a file. This was the only exposed one.
This commit is contained in:
2026-08-16 00:33:32 +02:00
parent 331db58705
commit b49599150c
3 changed files with 161 additions and 2 deletions
+46 -2
View File
@@ -419,6 +419,45 @@ func drain(conn io.ReadWriteCloser) int {
return total
}
// frameTimeout bounds the wait for one 11-byte status reply. At 4800 baud the
// frame itself takes ~23 ms; three seconds is a controller that is not going to
// answer this query.
const frameTimeout = 3 * time.Second
// readFrame reads exactly len(buf) bytes, or gives up.
//
// io.ReadFull CANNOT be used on a serial port, and using it here is what made an
// antenna "stop responding after a while" with nothing whatsoever in the log.
//
// On Windows a serial read that times out returns (0, nil) — a timeout is not an
// error on this transport. io.ReadFull loops while err == nil, so a controller
// that goes quiet, or sends a truncated frame, spins it forever. It holds ioMu
// the whole time, and that is the part the operator sees: the poll goroutine
// never returns to report a fault, so the last status stays on screen and the
// link still looks connected — while every command blocks on the same mutex.
// The trace line used to sit AFTER that lock, so even the attempt went unlogged.
// One dropped reply on a 4800-baud link wedged the driver until OpsLog restarted.
//
// Giving up returns an error, which the poll loop already knows how to handle:
// it says so in the log and reconnects.
func readFrame(conn io.ReadWriteCloser, buf []byte, d time.Duration) error {
deadline := time.Now().Add(d)
for n := 0; n < len(buf); {
m, err := conn.Read(buf[n:])
if err != nil {
return err
}
n += m
if n >= len(buf) {
return nil
}
if time.Now().After(deadline) {
return fmt.Errorf("timed out after %s with %d of %d bytes", d, n, len(buf))
}
}
return nil
}
func (c *Client) queryStatus() (*Status, error) {
c.connMu.Lock()
conn := c.conn
@@ -438,7 +477,7 @@ func (c *Client) queryStatus() (*Status, error) {
return nil, fmt.Errorf("write status cmd: %w", err)
}
buf := make([]byte, 11)
if _, err := io.ReadFull(conn, buf); err != nil {
if err := readFrame(conn, buf, frameTimeout); err != nil {
return nil, fmt.Errorf("read status: %w", err)
}
// Reject anything that isn't a framed reply rather than decoding garbage into
@@ -523,9 +562,14 @@ func (c *Client) writeCmd(pkt []byte) error {
if conn == nil {
return fmt.Errorf("steppir: not connected")
}
// Traced BEFORE taking the lock, not after. A command waits here for the poll
// in flight, and when that wait was unbounded the log showed no sign the
// operator had asked for anything at all — the one fact that would have named
// the fault. The line now means "asked for"; a failure to write is reported
// by the caller.
log.Printf("steppir: → % X", pkt)
c.ioMu.Lock()
defer c.ioMu.Unlock()
log.Printf("steppir: → % X", pkt)
setDeadline(conn, 3*time.Second)
if _, err := conn.Write(pkt); err != nil {
c.closeConn()