fix(antenna): Retract and Calibrate report the elements moving
An operator reported that retracting a SteppIR shows nothing, while an Ultrabeam visibly does. The cause is not the display: both antennas feed one `moving` flag and the widget renders it the same way. moveCmdAt — the bridge that reports motion from the moment a move is COMMANDED, before the controller's own poll can confirm it — was set only by SetFrequency. Retract and Calibrate write their frame directly and armed nothing, so three things were missing at once: nothing said the elements were moving, the poll stayed on its two-second idle cadence instead of speeding up to 250 ms to watch, and the transmit inhibit was not engaged while the elements travelled. That last one is the reason this is not cosmetic. The Ultrabeam's Retract armed nothing either. It was less visible there because that controller reports element lengths, which count down on screen; a SteppIR reports none at all, so the retract looked inert. The inhibit was equally absent. The window becomes a deadline rather than a timestamp, so the caller can say how long a bridge it needs: three seconds for a tune, ten for a retract or a calibrate. Those are tens of seconds of travel, and a retract drops the controller out of AUTOTRACK where its motor-bit reporting is less predictable. Still bounded — an antenna that never reports motion must not latch the inhibit on for ever — and markMoving only ever extends, so a tune issued just after a retract cannot cut the retract's bridge down to its own. Four tests, including that a command which failed to reach the controller does not claim the antenna is moving.
This commit is contained in:
@@ -19,7 +19,7 @@ func TestAJustCommandedMoveReportsMotion(t *testing.T) {
|
||||
}
|
||||
|
||||
c.statusMu.Lock()
|
||||
c.moveCmdAt = time.Now()
|
||||
c.moveUntil = time.Now().Add(moveOptimisticWindow)
|
||||
c.statusMu.Unlock()
|
||||
|
||||
st, _ := c.GetStatus()
|
||||
@@ -33,9 +33,68 @@ func TestAJustCommandedMoveReportsMotion(t *testing.T) {
|
||||
// Bounded: an antenna that never reports motion must not latch the transmit
|
||||
// inhibit on for ever.
|
||||
c.statusMu.Lock()
|
||||
c.moveCmdAt = time.Now().Add(-moveOptimisticWindow - time.Second)
|
||||
c.moveUntil = time.Now().Add(-time.Second)
|
||||
c.statusMu.Unlock()
|
||||
if st, _ := c.GetStatus(); st.MotorsMoving != 0 {
|
||||
t.Error("the optimistic window never expires")
|
||||
}
|
||||
}
|
||||
|
||||
// Retract and Calibrate move every element, and they were the two commands that
|
||||
// reported nothing at all: no "moving" on screen, the poll left on its
|
||||
// two-second idle cadence instead of speeding up to watch, and no transmit
|
||||
// inhibit while the elements travelled. Reported by an operator whose retract
|
||||
// looked inert next to an Ultrabeam's.
|
||||
func TestRetractAndCalibrateReportMotion(t *testing.T) {
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
call func(*Client) error
|
||||
}{
|
||||
{"retract", (*Client).Retract},
|
||||
{"calibrate", (*Client).Calibrate},
|
||||
} {
|
||||
c := &Client{}
|
||||
c.lastStatus = &Status{Connected: true, Frequency: 14074}
|
||||
// No connection, so the write fails and the command returns an error —
|
||||
// which is the point: a command that did NOT reach the controller must
|
||||
// not claim the antenna is moving.
|
||||
if err := tc.call(c); err == nil {
|
||||
t.Fatalf("%s: expected an error with no connection", tc.name)
|
||||
}
|
||||
if st, _ := c.GetStatus(); st.MotorsMoving != 0 {
|
||||
t.Errorf("%s: a command that failed to send reports motion", tc.name)
|
||||
}
|
||||
|
||||
// And with the write accepted, motion is reported at once.
|
||||
c.markMoving(retractOptimisticWindow)
|
||||
if st, _ := c.GetStatus(); st.MotorsMoving == 0 {
|
||||
t.Errorf("%s: a commanded move is not reported as motion", tc.name)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// The retract bridge is longer than the tune bridge: winding every element into
|
||||
// its hub takes tens of seconds, and a retract drops the controller out of
|
||||
// AUTOTRACK, where its motor-bit reporting is less predictable.
|
||||
func TestRetractBridgeOutlastsTheTuneBridge(t *testing.T) {
|
||||
if retractOptimisticWindow <= moveOptimisticWindow {
|
||||
t.Errorf("retract window %v is not longer than the tune window %v",
|
||||
retractOptimisticWindow, moveOptimisticWindow)
|
||||
}
|
||||
// Bounded all the same — see the note on the constant.
|
||||
if retractOptimisticWindow > time.Minute {
|
||||
t.Errorf("retract window %v could latch the transmit inhibit on", retractOptimisticWindow)
|
||||
}
|
||||
}
|
||||
|
||||
// markMoving extends, never shortens: a tune issued a moment after a retract
|
||||
// must not cut the retract's bridge down to the tune's.
|
||||
func TestMarkMovingOnlyExtends(t *testing.T) {
|
||||
c := &Client{}
|
||||
c.markMoving(retractOptimisticWindow)
|
||||
far := c.moveUntil
|
||||
c.markMoving(moveOptimisticWindow)
|
||||
if c.moveUntil.Before(far) {
|
||||
t.Error("a shorter bridge shortened a longer one")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -84,6 +84,16 @@ const (
|
||||
// that never reports motion cannot latch the inhibit on for ever.
|
||||
const moveOptimisticWindow = 3 * time.Second
|
||||
|
||||
// retractOptimisticWindow is the same bridge for a RETRACT or a CALIBRATE.
|
||||
//
|
||||
// Longer, because those are the two longest movements the antenna makes —
|
||||
// tens of seconds to wind every element into its hub — and because a retract
|
||||
// drops the controller out of AUTOTRACK, which is a state its motor-bit
|
||||
// reporting is less predictable in. Still bounded, for the reason above: an
|
||||
// antenna that never reports motion must not latch the transmit inhibit on
|
||||
// for ever.
|
||||
const retractOptimisticWindow = 10 * time.Second
|
||||
|
||||
// Transport says how to reach the controller.
|
||||
type Transport struct {
|
||||
Mode string // "tcp" | "serial"
|
||||
@@ -122,8 +132,11 @@ type Client struct {
|
||||
statusMu sync.RWMutex
|
||||
lastStatus *Status
|
||||
lastSetKHz int
|
||||
// moveCmdAt is when a move was last COMMANDED — see moveOptimisticWindow.
|
||||
moveCmdAt time.Time
|
||||
// moveUntil is how long a commanded move is reported as moving without the
|
||||
// controller having said so — see moveOptimisticWindow. A DEADLINE rather
|
||||
// than the command's timestamp, because a retract needs a longer bridge than
|
||||
// a tune and the caller is what knows which it asked for.
|
||||
moveUntil time.Time
|
||||
// lastDriftKHz is the frequency last reported for a controller that had gone
|
||||
// somewhere other than where it was told, so the disagreement is stated once
|
||||
// and not on every poll. Zero when it is where it should be.
|
||||
@@ -225,7 +238,25 @@ func (c *Client) GetStatus() (*Status, error) {
|
||||
// movingOptimisticallyLocked reports a move commanded too recently for the
|
||||
// controller to have answered. Callers hold statusMu.
|
||||
func (c *Client) movingOptimisticallyLocked() bool {
|
||||
return !c.moveCmdAt.IsZero() && time.Since(c.moveCmdAt) < moveOptimisticWindow
|
||||
return !c.moveUntil.IsZero() && time.Now().Before(c.moveUntil)
|
||||
}
|
||||
|
||||
// markMoving reports motion for d, bridging the gap until the controller says
|
||||
// so itself.
|
||||
//
|
||||
// Every command that MOVES something has to call this. Retract and Calibrate
|
||||
// did not, and they are the two that need it most: nothing on screen said the
|
||||
// elements were moving, the poll stayed on its two-second idle cadence
|
||||
// instead of speeding up to watch, and the transmit inhibit was not engaged
|
||||
// while the elements travelled. An operator reported the retract as showing
|
||||
// nothing at all, next to an Ultrabeam that shows its element lengths
|
||||
// counting down.
|
||||
func (c *Client) markMoving(d time.Duration) {
|
||||
c.statusMu.Lock()
|
||||
if until := time.Now().Add(d); until.After(c.moveUntil) {
|
||||
c.moveUntil = until
|
||||
}
|
||||
c.statusMu.Unlock()
|
||||
}
|
||||
|
||||
// MovingOptimistically is the same question from outside the lock — the poll
|
||||
@@ -693,7 +724,9 @@ func (c *Client) SetFrequency(freqKhz int, direction int) error {
|
||||
c.statusMu.Lock()
|
||||
c.lastSetKHz = freqKhz
|
||||
c.pendingDir, c.pendingDirAt, c.pendingDirSet = direction, time.Now(), true
|
||||
c.moveCmdAt = time.Now() // report motion at once — see moveOptimisticWindow
|
||||
if until := time.Now().Add(moveOptimisticWindow); until.After(c.moveUntil) {
|
||||
c.moveUntil = until // report motion at once — see moveOptimisticWindow
|
||||
}
|
||||
c.statusMu.Unlock()
|
||||
return nil
|
||||
}
|
||||
@@ -726,7 +759,11 @@ func (c *Client) Retract() error {
|
||||
khz = 14000 // any in-range value; the controller just homes
|
||||
}
|
||||
}
|
||||
return c.writeCmd(buildSet(khz*1000, DirNormal, 'S'))
|
||||
if err := c.writeCmd(buildSet(khz*1000, DirNormal, 'S')); err != nil {
|
||||
return err
|
||||
}
|
||||
c.markMoving(retractOptimisticWindow)
|
||||
return nil
|
||||
}
|
||||
|
||||
// portBusyHint turns "Serial port busy" into something actionable — see the
|
||||
@@ -764,5 +801,9 @@ func (c *Client) Calibrate() error {
|
||||
khz = 14000
|
||||
}
|
||||
}
|
||||
return c.writeCmd(buildSet(khz*1000, DirNormal, 'V'))
|
||||
if err := c.writeCmd(buildSet(khz*1000, DirNormal, 'V')); err != nil {
|
||||
return err
|
||||
}
|
||||
c.markMoving(retractOptimisticWindow)
|
||||
return nil
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user