diff --git a/changelog.json b/changelog.json index f4e6d28..1dbe635 100644 --- a/changelog.json +++ b/changelog.json @@ -12,7 +12,8 @@ "Each satellite slice now gets both of its antennas, RX and TX, from its own band — the downlink slice was left with no transmit antenna at all.", "The satellite ground track no longer draws a straight line across the map when it crosses the antimeridian.", "A correction you make on the transmit VFO is now kept for the whole pass, and remembered for that transponder. Doppler tracking used to undo it a second later.", - "The Green Heron RT-21 is named in the rotator list. It speaks the DCU-1 command set OpsLog already drives, over its COM port or straight over TCP with the Ethernet option — set the controller to DCU-1 / Rotor-EZ." + "The Green Heron RT-21 is named in the rotator list. It speaks the DCU-1 command set OpsLog already drives, over its COM port or straight over TCP with the Ethernet option — set the controller to DCU-1 / Rotor-EZ.", + "Retract and Calibrate on a SteppIR now show the elements moving, and inhibit the transmitter while they do. Neither said anything before — the same was missing on an Ultrabeam retract, where the element lengths counting down hid it." ], "fr": [ "Le tracé du ciel et la position passent dans leur propre colonne, à gauche de la carte. Largeur réglable, et la colonne se replie comme celle de droite.", @@ -25,7 +26,8 @@ "Chaque slice satellite reçoit désormais ses deux antennes, RX et TX, depuis sa propre bande — la slice de descente restait sans antenne d’émission.", "Le tracé au sol du satellite ne trace plus une ligne droite en travers de la carte lorsqu’il franchit l’antiméridien.", "Une correction faite sur le VFO d’émission est désormais conservée pour toute la passe, et mémorisée pour ce transpondeur. Le suivi Doppler l’effaçait une seconde plus tard.", - "Le Green Heron RT-21 est nommé dans la liste des rotators. Il parle le jeu de commandes DCU-1 que OpsLog pilote déjà, via son port COM ou directement en TCP avec l’option Ethernet — régler le contrôleur sur DCU-1 / Rotor-EZ." + "Le Green Heron RT-21 est nommé dans la liste des rotators. Il parle le jeu de commandes DCU-1 que OpsLog pilote déjà, via son port COM ou directement en TCP avec l’option Ethernet — régler le contrôleur sur DCU-1 / Rotor-EZ.", + "Rétracter et Calibrer sur une SteppIR montrent désormais les éléments en mouvement, et inhibent l’émission pendant ce temps. Ni l’un ni l’autre ne le signalait — même manque sur la rétraction d’une Ultrabeam, où le défilement des longueurs d’éléments le masquait." ] }, { diff --git a/internal/steppir/moving_test.go b/internal/steppir/moving_test.go index e249847..eb65e59 100644 --- a/internal/steppir/moving_test.go +++ b/internal/steppir/moving_test.go @@ -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") + } +} diff --git a/internal/steppir/steppir.go b/internal/steppir/steppir.go index 97edf04..ee51846 100644 --- a/internal/steppir/steppir.go +++ b/internal/steppir/steppir.go @@ -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 } diff --git a/internal/ultrabeam/ultrabeam.go b/internal/ultrabeam/ultrabeam.go index 44c2ac5..9b2847e 100644 --- a/internal/ultrabeam/ultrabeam.go +++ b/internal/ultrabeam/ultrabeam.go @@ -894,9 +894,20 @@ func (c *Client) SetDirection(direction int) error { } // Retract retracts all elements (command 2) +// +// Reports motion at once, like a frequency change does: this is the longest +// move the antenna makes, and the flag it sets is what inhibits the +// transmitter while the elements travel. The element lengths counting down +// made the omission less visible here than on a SteppIR, which reports no +// lengths at all — but the inhibit was equally missing. func (c *Client) Retract() error { - _, err := c.sendCommand(CMD_RETRACT, nil) - return err + if _, err := c.sendCommand(CMD_RETRACT, nil); err != nil { + return err + } + c.statusMu.Lock() + c.moveCmdAt = time.Now() + c.statusMu.Unlock() + return nil } // ModifyElement modifies element length (command 12)