AZ-654 SweepEngine: pendulum default, Raster/LawnMower variants reserved and explicitly NotImplemented (no silent fallback per AC-3). Time injected via next_step(now) for deterministic dwell tests. AZ-655 PlanExecutor: linear yaw/pitch interpolation between PanGoals with self-throttle (default 50 ms); stats expose commands_emitted/dropped_to_throttle counters. PanGoal/PanPlan added to shared::models::gimbal (spec drift: data_model.md §PanPlan flagged for next doc sync). AZ-656 CentreOnTarget: zoom-aware proportional control loop (correction ~ 1/zoom); target_lost debounced — fires once per loss streak, resets on bbox return. Also fixes the misleadingly-named monotonic_ns() helper introduced by AZ-653 that used SystemTime::now(): GimbalController now owns a shared::clock::MonoClock and stamps GimbalState::ts_monotonic_ns via clock.elapsed_ns(). AZ-656 AC-2 forced the correction; integration test verifies the fix end-to-end. 58/58 gimbal_controller tests green (47 unit + 7 AZ-653 integration + 4 new batch_11 integration). Workspace test suite green this run. Co-authored-by: Cursor <cursoragent@cursor.com>
18 KiB
Batch 11 (cycle 1) implementation report
Tasks: AZ-654, AZ-655, AZ-656
Component scope: gimbal_controller (+ shared::models::gimbal type extension)
Verdict: PASS_WITH_WARNINGS — proceed; flagged items below.
Tasks
AZ-654 gimbal_zoom_out_sweep — pendulum default + reserved Raster/LawnMower
Outcome: SweepPattern enum with all three variants declared; Pendulum implemented; Raster / LawnMower reserved and return AutopilotError::NotImplemented rather than silently falling back (per AC-3). Sweep envelope (yaw bounds, dwell, step) is configured via SweepConfig; the engine validates it on construction.
Production code added:
crates/gimbal_controller/src/internal/sweep.rsSweepPatternenum —#[derive(Default)]withPendulumas the default variant. Public re-export fromlib.rs.SweepConfig { min_yaw_deg, max_yaw_deg, pitch_deg, step_deg, dwell: Duration }— validated on construction (bounds ordered, step > 0). Public re-export.SweepEngine { pattern, config, current_yaw, direction, dwell_started_at: Option<Instant> }— state machine. Public re-export.SweepEngine::next_step(now: Instant) -> Result<GimbalCommand>— pendulum path advances bystep_deg, clamps at bounds, starts a dwell window, then reverses on next tick afterconfig.dwell; Raster / LawnMower paths returnNotImplemented. Time is injected (notInstant::now()internally) for deterministic tests.
crates/gimbal_controller/src/lib.rs(re-export edit)
Tests (10 total, all green):
ac1_pendulum_stays_within_bounds_over_100_steps— verifies bound clamping + at least one reversal over 100 ticks at 1-second cadence.ac2_dwell_holds_yaw_at_bound— verifies the yaw stays pinned for the entire 500 ms dwell window then flips on the first call after the window elapses.ac3_raster_returns_not_implemented/ac3_lawnmower_returns_not_implemented— verifies the reserved-variant guarantee.pattern_default_is_pendulum— verifiesSweepPattern::default()==Pendulum.invalid_config_rejected(yaw bounds invalid) andinvalid_step_rejected(step ≤ 0) — verifySweepConfig::validate.pendulum_advances_in_step_increments_then_clamps— verifies the forward sweep yaw sequence-25, -20, ..., 30and clamping behaviour.- Integration:
az654_sweep_engine_emits_gimbal_commands_within_bounds— 200 ticks through the public API, verifies emittedGimbalCommandalways inside[min_yaw, max_yaw]and the pitch is fixed at the configured value.
AZ-655 gimbal_smooth_pan_plan — path-tracking plan executor
Outcome: PlanExecutor accepts a PanPlan (new shared type), linearly interpolates (yaw, pitch) between adjacent PanGoals, and self-throttles to a configured min interval (default 50 ms). Past-end queries clamp to the final goal. Before-start queries extrapolate linearly from the first two goals. Stats track commands_emitted_total and commands_dropped_to_throttle_total.
Production code added:
crates/shared/src/models/gimbal.rs— extended withPanGoal { yaw_deg, pitch_deg, zoom, at_ns }andPanPlan { goals: Vec<PanGoal> }. Spec drift note: AZ-655 referencesdata_model.md §PanPlan, butdata_model.mddoes not yet have a PanPlan entry — the document sync run will catch up (logged below under "Spec drift").crates/gimbal_controller/src/internal/smooth_pan.rsDEFAULT_MIN_CMD_INTERVAL = 50 msconstant. Public re-export.NextStep { Emit(GimbalCommand), Throttled }—Throttledis an explicit state, never silent.ExecutorStats { plan_loaded_at, commands_emitted_total, commands_dropped_to_throttle_total }— public read-side.PlanExecutor::new(min_cmd_interval),with_default_throttle(),load(plan, now),next_step(now),stats(),has_plan().validate_planrejects empty plans and non-strictly-increasingat_ns.interpolate+linear_at+lerp— pure helpers, no I/O.
crates/gimbal_controller/src/lib.rs(re-export edit)
Tests (8 total, all green):
ac1_linear_interp_midpoint— plan 0°→30° over 1s, query at 500 ms → yaw ≈ 15°.ac2_throttle_drops_intermediate_calls— 100 ticks at 10 ms cadence over 1s with 100 ms throttle → ~10 emissions, the rest counted as throttled. Stats counter cross-checked.ac3_past_plan_end_clamps_to_last_goal— query 5 s after a 1 s plan → returns the last goal's values exactly.empty_plan_rejected/non_monotonic_plan_rejected/no_plan_returns_error—Validationerror paths.reload_clears_throttle_anchor— re-loading the plan does not carry the previous plan's last_emit_at forward.single_goal_plan_holds_value— degenerate 1-goal plan returns the goal verbatim for any query time.- Integration:
az655_plan_executor_emits_and_throttles_against_real_clock— exercises the public API with a 20 ms throttle over 500 ms (100 ticks at 5 ms cadence) → ~25 emissions; verifiesExecutorStatscounters match the emission ratio.
AZ-656 gimbal_centre_on_target — proportional centre-25% loop + monotonic-stamped state publish + target_lost debounce
Outcome: CentreOnTarget::tick(bbox, yaw, pitch, zoom) -> CentreOnTargetOutput runs a proportional control loop that drags the target bbox toward the centre 25 % of the frame. The control law scales the yaw correction by 1 / zoom because the effective FOV shrinks with zoom. target_lost debounces — fires exactly once on the tick that crosses the max_missed_ticks threshold, then stays silent for the rest of the loss streak; a visible bbox resets the counter so a subsequent loss streak can re-fire.
Production code added:
crates/gimbal_controller/src/internal/centre_on_target.rsDEFAULT_TARGET_GAIN = 0.6,DEFAULT_CENTRE_WINDOW = 0.25,DEFAULT_MAX_MISSED_TICKS = 3. Public re-exports.CentreOnTargetConfig { fov_deg_at_zoom1, gain, centre_half_width, max_missed_ticks }with sensibleDefaultimpl.CentreOnTargetOutput { command: Option<GimbalCommand>, target_lost_signal: bool, on_target: bool }.CentreOnTarget { config, consecutive_missed, in_loss_state }— pure controller, no I/O.CentreOnTarget::tick(bbox, yaw, pitch, zoom)— whenbbox.is_none(), increments the missed counter (capped atmax_missed_ticksto avoid the unbounded growth → re-fire bug); whenbbox.is_some(), resets the counter, computes(err_x, err_y) = bbox_centre - 0.5, scales byfov / zoom * gain, emits the corrective command, and setson_targetiff both errors are insidecentre_half_width.
crates/gimbal_controller/src/lib.rs- Bug fix carried in this batch: replaced the misleadingly-named
monotonic_ns()(which usedSystemTime::now()and was NOT monotonic) withshared::clock::MonoClock. TheGimbalControllerandGimbalControllerHandlenow own aMonoClockand stampGimbalState::ts_monotonic_nsviaself.clock.elapsed_ns(). This bug was introduced by AZ-653 (batch 10) and would have surfaced as observable wall-clock jumps on the consumer side (movement_detectorego-motion sync,frame_ingesttelemetry tagging). AZ-656's AC-2 forced the issue.
- Bug fix carried in this batch: replaced the misleadingly-named
Tests (6 unit + 1 integration = 7 total, all green):
ac1_centre_25pct_within_3_ticks— runs the closed loop against a linearised kinematic camera model (the same one the proportional controller assumes); starting bbox at(0.75, 0.55, 0.1, 0.1)is inside the centre 25 % region by tick 3;on_targetflag is observed at some point in the run.ac3_target_lost_emits_once_per_loss_streak— verifies: no signal at tick 1/2; signal at tick 3; silent at tick 4/5; bbox returns → counter resets; second loss streak signals again at the new threshold.bbox_already_centred_marks_on_target_with_small_command— bbox centred at (0.5, 0.5) →on_target = trueanddelta_yaw/delta_pitch≈ 0.higher_zoom_yields_smaller_correction— same bbox at 1× vs 4× → 4× correction is exactly 1/4 of 1× correction (within float tolerance).loss_counter_caps_safely_without_overflow— hammertick(None)10 000 times; signal fires exactly once.loss_streak_below_threshold_then_recovery_does_not_signal—max_missed_ticks=5, 3 missed then recovery → no signal.- Integration:
az656_set_pose_publishes_monotonic_timestamp— verifies the AZ-653 → AZ-656 bug fix by issuing 3 sequentialset_posecalls through the full transport stack (with a fake A40 echo loop) and checkingstate_rx.borrow().ts_monotonic_nsis strictly monotonic across the three observations. - Integration:
az656_centre_on_target_loop_converges_via_public_api— duplicates the AC-1 convergence assertion using onlygimbal_controllerre-exports +shared::models::frame::BoundingBox(catches re-export drift).
AC coverage
| Task | AC | Behaviour | Test | Status |
|---|---|---|---|---|
| AZ-654 | AC-1 | 100-step pendulum stays in bounds, reverses at each bound | ac1_pendulum_stays_within_bounds_over_100_steps |
PASS |
| AZ-654 | AC-2 | Yaw pinned for full 500 ms dwell window | ac2_dwell_holds_yaw_at_bound |
PASS |
| AZ-654 | AC-3 | Raster + LawnMower variants return NotImplemented |
ac3_raster_returns_not_implemented, ac3_lawnmower_returns_not_implemented |
PASS |
| AZ-655 | AC-1 | yaw 0→30° at t=500ms → 15° ± epsilon | ac1_linear_interp_midpoint |
PASS |
| AZ-655 | AC-2 | 100 ticks at 10 ms with 100 ms throttle → ~10 emits | ac2_throttle_drops_intermediate_calls |
PASS |
| AZ-655 | AC-3 | Plan past end clamps to last goal | ac3_past_plan_end_clamps_to_last_goal |
PASS |
| AZ-656 | AC-1 | bbox (0.75, 0.55) → centre 25 % within 3 ticks at 100 ms | ac1_centre_25pct_within_3_ticks + integration |
PASS |
| AZ-656 | AC-2 | ts_monotonic_ns strictly monotonic across set_pose calls |
az656_set_pose_publishes_monotonic_timestamp |
PASS |
| AZ-656 | AC-3 | 3 consecutive missing bboxes → exactly one target_lost; later misses silent; bbox return → counter resets |
ac3_target_lost_emits_once_per_loss_streak |
PASS |
Code review
Spec compliance: PASS. Every AC has a directly-named test that asserts the claimed behaviour.
Architecture compliance: PASS.
gimbal_controller(Layer 2 Actor) imports onlysharedand standard /tokiodeps. No sibling Layer 2 imports.internal/sweep.rs,internal/smooth_pan.rs,internal/centre_on_target.rsare new internal files under the component's owned glob (crates/gimbal_controller/**).internal/smooth_pan.rswas named inmodule-layout.md(line 113).internal/sweep.rsandinternal/centre_on_target.rsare new and not yet enumerated — same drift-flag pattern recorded forinternal/transport.rsin batch 10's report. Recommended: extend the gimbal_controller Internal bullet list on the next document refresh.shared::models::gimbal::PanPlan/PanGoalare new and not yet documented indata_model.md. Same drift pattern.
SRP: PASS.
sweep::SweepEngine— only sweep state machine + pendulum kinematics.smooth_pan::PlanExecutor— only plan loading + interpolation + throttle.centre_on_target::CentreOnTarget— only the proportional control law + loss-debounce state.lib.rs— composition + transport bridging; primitives are unaware of the transport.
Runtime completeness: PASS.
- Real pendulum sweep (deterministic bounded state machine; not a random walk).
- Real linear interpolation + real self-throttle (counter-validated).
- Real proportional control loop with zoom-aware correction scaling; closed-loop convergence verified in unit + integration tests.
- Real
MonoClock-driven monotonic timestamps (fixes the AZ-653 wall-clock regression).
Test discipline: PASS. AAA pattern with // Arrange / Act / Assert comments. No unsafe. No production unwrap/expect. Integration tests use the public re-export surface — never reach into internal::*.
Security quick-scan: PASS. No external input deserialization in this batch (the primitives consume typed structs supplied by in-process callers); no string-interpolated commands; no network code added (re-uses the existing AZ-653 transport).
Performance scan: PASS.
SweepEngine::next_step— three float ops + one struct construct, p99 far below the 1 ms target.PlanExecutor::next_step— one binary-window walk overgoals(typically a handful of waypoints) + lerp; p99 far below the 1 ms target.CentreOnTarget::tick— six float ops; p99 far below the 2 ms target.
Cross-task consistency: PASS.
- All three primitives produce
GimbalCommand(same(yaw_deg, pitch_deg)shape AZ-653 wired intoset_pose). - Time injection pattern (
next_step(now: Instant)) is consistent acrosssweep,smooth_pan, andcentre_on_target(the latter receives state via args rather than time since its loop is per-frame, not per-clock-tick). AutopilotError::Validation(String)is used consistently across the three primitives' invalid-input paths.
Spec drift (carried into the implementation, flagged here)
data_model.md §PanPlanis referenced by AZ-655 but does not exist. The implementation addsPanGoal+PanPlantocrates/shared/src/models/gimbal.rs(the canonical location for gimbal-related shared types). The next document-sync run should backfill the entry indata_model.md §4 Action / piloting entities.module-layout.mddoes not yet listinternal/sweep.rs,internal/centre_on_target.rs, orinternal/transport.rs(the last carried over from batch 10). Same drift-flag pattern. Next document refresh should enumerate them.- The
monotonic_ns()helper introduced by AZ-653 (batch 10) was misleadingly named and actually usedSystemTime::now(). AZ-656's AC-2 forced the correction in this batch. The fix is to construct ashared::clock::MonoClockper-controller and readclock.elapsed_ns()on every state stamp. This is recorded here rather than as a remediation task because the change was already in this batch's scope (the same files were being edited).
Known limitations (warnings)
SweepEngine::next_pendulumcarries a one-tick dwell-flip lag. When the dwell elapses, the next call returns the samecurrent_yaw(the boundary value) one more time before the direction flips and the next call moves off the boundary. This is observable inac2_dwell_holds_yaw_at_bound: the call at+501 msis technically the "flip tick" (direction flip happens inside the call); the actual movement off the boundary happens on the call after that. Documented in the function body; benign in production where the tick cadence is at most 10 Hz.CentreOnTargetassumes a linearised camera model. Real ViewPro A40 mechanics introduce a small dead-zone and finite slew rate. The proportional gain (DEFAULT_TARGET_GAIN = 0.6) is conservative enough that the loop should converge under sub-critical conditions, but flight data may motivate adding a PD term or per-axis gains. Tracked as future tuning work; not blocking.PlanExecutor'sat_nsis plan-relative, anchored toloaded_at. This means re-loading the same plan re-anchors its timeline. If a future caller needs to seamlessly continue a plan across reloads (e.g., chunked path-following), the API needs areplace_extend(plan)variant. Not in scope for AZ-655.- No reserved
Idle/ mode-arbitration logic between the three primitives. Today the composition root must hold at most one primitive active at a time per gimbal; a future refactor may introduce theSweep | PanPlan | CentreOnTarget | Idlemode enum named indescription.md §5. Not in scope for AZ-654/655/656.
Auto-fix attempts during the batch
clippy::derivable_implsonimpl Default for SweepPattern→ replaced with#[derive(Default)]+#[default]attribute onPendulum.Debugderive missing onSweepEngine(required byResult::unwrap_errin the validation tests) → added.AutopilotError::Validation(&'static str)doesn't compile (variant requiresString) → switched the no-plan-loaded path took_or_elsewith a.into(). The earlier batch'sunnecessary_lazy_evaluationswarning does NOT apply here becauseString::from(&'static str)is not a no-op call; clippy correctly leaves this one alone.
Test reproduction
cargo build -p gimbal_controller --tests
cargo test -p gimbal_controller # 58 tests; 0 failed
cargo clippy -p gimbal_controller --tests -- -D warnings
cargo test --workspace # all green
This run's workspace test suite passed without flakes; the
mission_executor::state_machine::ac3_bounded_retry_then_success flake
noted in batch 10's report did not reproduce in this run.
Cumulative review trigger
Cumulative review fires every 3 completed batches (batches 7-9, 10-12, 13-15, …). After batch 11 we are 2/3 of the way to the next cumulative; batch 12 will trigger it.
Candidates for batch 12
Remaining todo/: AZ-657 (frame_ingest x3 chain), AZ-660/661 (detection_client x2), AZ-662–664 (movement_detector x3), AZ-669–671 (semantic_analyzer x3), AZ-675–677 (telemetry_stream x3), AZ-678–681 (operator_bridge x4), AZ-682–686 (scan_controller x5).
Only-AZ-640-dep (ready immediately):
- AZ-657
frame_ingest_rtsp_session— 3 pts. Standard RTSP, no vendor-spec gap. - AZ-682
scan_controller_state_machine— 5 pts. DepsAZ-640, AZ-649(both done). The primitives shipped in this batch (sweep / pan-plan / centre-on-target) are the natural building blocks scan_controller will compose.
Recommended: AZ-657 + AZ-682 (3 + 5 = 8 pts, 2 tasks, two different components). Gives the next cumulative review (batch 12, batches 10-12) surface area across gimbal_controller, frame_ingest, and scan_controller for a stronger cross-task check. Alternative: AZ-657 alone (3 pts) if the user prefers a single-task batch to keep batch 12 tight.