fix(steppir): the same fast poll, and motion reported at the command
The shortened transmit gag was written for an Ultrabeam that is now polled four times a second while it moves. The SteppIR was still on two seconds and reported nothing at all until its own poll came round, so the gap between the 900 ms grace and the first poll that would have seen the movement was a hole in "block TX while the elements travel" — the transmitter released in the middle of a move. It now reports a commanded move at once (bounded, so an antenna that never answers cannot latch the inhibit on), on a COPY of the cached status so the flag cannot leak into what the poll goroutine owns, and follows the motors with its poll rate exactly as the Ultrabeam does.
This commit is contained in:
@@ -0,0 +1,41 @@
|
||||
package steppir
|
||||
|
||||
import (
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
// The controller says nothing until its own poll comes round, so a move has to
|
||||
// be reported from the moment it is COMMANDED. Without it the app's "block TX
|
||||
// while the elements travel" had a hole: the grace covering the command ran out
|
||||
// before the first poll that would have seen the movement, and the transmitter
|
||||
// was released in the middle of a move.
|
||||
func TestAJustCommandedMoveReportsMotion(t *testing.T) {
|
||||
c := &Client{}
|
||||
c.lastStatus = &Status{Connected: true} // idle, as the controller last said
|
||||
|
||||
if st, _ := c.GetStatus(); st.MotorsMoving != 0 {
|
||||
t.Fatal("an idle antenna reports motion")
|
||||
}
|
||||
|
||||
c.statusMu.Lock()
|
||||
c.moveCmdAt = time.Now()
|
||||
c.statusMu.Unlock()
|
||||
|
||||
st, _ := c.GetStatus()
|
||||
if st.MotorsMoving == 0 {
|
||||
t.Error("a move commanded a moment ago is not reported as motion")
|
||||
}
|
||||
if c.lastStatus.MotorsMoving != 0 {
|
||||
t.Error("the optimistic flag leaked into the cached status the poll owns")
|
||||
}
|
||||
|
||||
// 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.statusMu.Unlock()
|
||||
if st, _ := c.GetStatus(); st.MotorsMoving != 0 {
|
||||
t.Error("the optimistic window never expires")
|
||||
}
|
||||
}
|
||||
@@ -65,6 +65,25 @@ const (
|
||||
// self-corrects instead of lying forever.
|
||||
const pendingDirTTL = 45 * time.Second
|
||||
|
||||
// The same two cadences the Ultrabeam uses, and for the same reason: transmit
|
||||
// is inhibited while the elements travel, so every poll interval between the
|
||||
// motors stopping and this client noticing is a second the operator cannot call
|
||||
// with the antenna already in place.
|
||||
const (
|
||||
pollIdle = 2 * time.Second
|
||||
pollMoving = 250 * time.Millisecond
|
||||
)
|
||||
|
||||
// moveOptimisticWindow is how long after a commanded move GetStatus reports
|
||||
// motion before the controller has had a chance to say so.
|
||||
//
|
||||
// The SteppIR reports nothing until its own poll comes round, so without this
|
||||
// the app's "block TX while moving" had a hole in it: the grace that covers the
|
||||
// command ran out before the first poll that would have seen the movement, and
|
||||
// the transmitter was released in the middle of a move. Bounded, so an antenna
|
||||
// that never reports motion cannot latch the inhibit on for ever.
|
||||
const moveOptimisticWindow = 3 * time.Second
|
||||
|
||||
// Transport says how to reach the controller.
|
||||
type Transport struct {
|
||||
Mode string // "tcp" | "serial"
|
||||
@@ -103,6 +122,8 @@ type Client struct {
|
||||
statusMu sync.RWMutex
|
||||
lastStatus *Status
|
||||
lastSetKHz int
|
||||
// moveCmdAt is when a move was last COMMANDED — see moveOptimisticWindow.
|
||||
moveCmdAt 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.
|
||||
@@ -191,7 +212,28 @@ func (c *Client) GetStatus() (*Status, error) {
|
||||
if c.lastStatus == nil {
|
||||
return &Status{Connected: false}, nil
|
||||
}
|
||||
return c.lastStatus, nil
|
||||
// A COPY, so the optimistic flag below can never leak into the status the
|
||||
// poll goroutine owns — and so a caller holding the value cannot see it
|
||||
// change under them mid-read.
|
||||
st := *c.lastStatus
|
||||
if st.Connected && st.MotorsMoving == 0 && c.movingOptimisticallyLocked() {
|
||||
st.MotorsMoving = 1 // sentinel: commanded, not yet reported (read as != 0)
|
||||
}
|
||||
return &st, nil
|
||||
}
|
||||
|
||||
// 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
|
||||
}
|
||||
|
||||
// MovingOptimistically is the same question from outside the lock — the poll
|
||||
// loop asks it to decide its cadence.
|
||||
func (c *Client) MovingOptimistically() bool {
|
||||
c.statusMu.RLock()
|
||||
defer c.statusMu.RUnlock()
|
||||
return c.movingOptimisticallyLocked()
|
||||
}
|
||||
|
||||
// open dials the transport. Callers hold connMu.
|
||||
@@ -266,8 +308,9 @@ func (c *Client) pollLoop() {
|
||||
close(c.done)
|
||||
}
|
||||
}()
|
||||
ticker := time.NewTicker(2 * time.Second)
|
||||
ticker := time.NewTicker(pollIdle)
|
||||
defer ticker.Stop()
|
||||
fast := false
|
||||
for {
|
||||
select {
|
||||
case <-c.stopChan:
|
||||
@@ -315,6 +358,17 @@ func (c *Client) pollLoop() {
|
||||
c.lastStatus = st
|
||||
c.statusMu.Unlock()
|
||||
c.checkDrift(st)
|
||||
|
||||
// Follow the motors with the poll rate; changed only when it changes,
|
||||
// since the antenna is polled for hours on end.
|
||||
if moving := st.MotorsMoving != 0 || c.MovingOptimistically(); moving != fast {
|
||||
fast = moving
|
||||
if fast {
|
||||
ticker.Reset(pollMoving)
|
||||
} else {
|
||||
ticker.Reset(pollIdle)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -639,6 +693,7 @@ 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
|
||||
c.statusMu.Unlock()
|
||||
return nil
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user