mirror of
https://github.com/azaion/autopilot.git
synced 2026-06-21 12:31:09 +00:00
[AZ-652] mission_executor safety + resume + middle-waypoint (batch 9)
Geofence (INCLUSION+EXCLUSION, ≤500 ms detect→RTL), battery thresholds (RTL@25%/land@15% + signed override), middle-waypoint re-upload (CLEAR_ALL→upload→SET_CURRENT(0)), and post-flight mapobjects push trigger. Adds production MAVLink command issuers for both geofence and battery failsafe families. Implements 6 ACs with 12 integration tests + module unit tests; full workspace test suite green. See batch_09_cycle1_report.md for AC coverage and known limitations. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
Oleksandr Bezdieniezhnykh
@@ -0,0 +1,139 @@
|
||||
# Batch 9 (cycle 1) implementation report
|
||||
|
||||
**Tasks**: AZ-652
|
||||
**Component scope**: `mission_executor`
|
||||
**Verdict**: PASS_WITH_WARNINGS — proceed; flagged items below.
|
||||
|
||||
## Tasks
|
||||
|
||||
### AZ-652 mission_executor_safety_and_resume — Geofence + battery + middle-waypoint + post-flight
|
||||
|
||||
**Outcome**: Implemented. All six acceptance criteria green; production MAVLink command issuers wired for both geofence and battery families.
|
||||
|
||||
**Production code added**:
|
||||
|
||||
- `crates/mission_executor/src/internal/geofence.rs`
|
||||
- `GeofenceVerdict { Ok, InclusionExit, ExclusionEntry }` — symmetric semantics (both variants treated as faults; the C++ behaviour of silently ignoring EXCLUSION is rejected).
|
||||
- `GeofenceMonitor` — pure point-in-polygon evaluator (ray-casting, no external crate dependency; `geo` would have pulled `num-traits` etc. for one function we can implement in 25 LOC).
|
||||
- `GeofenceEvent { Violation, RtlIssued, RtlSendFailed }` — broadcast surface.
|
||||
- `GeofenceCommandIssuer` trait — separate from the lost-link issuer per the AZ-651 "each failsafe family owns its command surface" pattern.
|
||||
- `MavlinkGeofenceCommandIssuer` — production impl that calls `mavlink_layer::MavlinkHandle::send_command(MAV_CMD_NAV_RETURN_TO_LAUNCH)`.
|
||||
- `GeofenceDriver` — wiring layer; 100 ms tick, edge-triggered RTL (only on Ok→violation), shutdown-aware.
|
||||
|
||||
- `crates/mission_executor/src/internal/battery_thresholds.rs`
|
||||
- `BatteryConfig { rtl_threshold_pct, hard_floor_pct }` — defaults 25 % / 15 % per task spec.
|
||||
- `BatteryOverride` — signed (signature pre-validated by `operator_bridge` per AZ-689); fields carry operator id + rationale for audit logging.
|
||||
- `BatteryAction { None, IssueRtl, IssueLandNow }` — discriminator returned by the pure monitor.
|
||||
- `BatteryMonitor` — pure logic: latches once it has fired so the same RTL is not re-issued on the next tick; honours active override (suppresses RTL only — hard-floor land is **not** override-able).
|
||||
- `BatteryCommandIssuer` trait + `MavlinkBatteryCommandIssuer` production impl (`MAV_CMD_NAV_RETURN_TO_LAUNCH` for RTL, `MAV_CMD_NAV_LAND` for hard-floor land-now).
|
||||
- `BatteryDriver` — wiring layer; subscribes to `SYS_STATUS`-projected battery percentages, emits audit-log entries for overrides via tracing.
|
||||
|
||||
- `crates/mission_executor/src/internal/middle_waypoint.rs`
|
||||
- `MiddleWaypointHint { at, insert_after_seq, label }` — externally supplied by `scan_controller` (the spec excludes the **placement** algorithm from this task).
|
||||
- `MissionRePlanner::on_middle_waypoint(hint, current_mission)` — runs `MISSION_CLEAR_ALL` → upload patched waypoints → `MISSION_SET_CURRENT(0)` via the `MissionDriver` trait. Returns the patched mission so the executor can mirror it into the FSM's `mission` field.
|
||||
- `MissionRePlanner::on_target_follow_release(reason, original_mission, current_position)` — re-uploads the original mission anchored at the current position.
|
||||
|
||||
- `crates/mission_executor/src/internal/post_flight.rs`
|
||||
- `MapObjectsPusher` trait (production impl is `mission_client::MissionClientHandle::push_mapobjects_diff` per AZ-647); `MapObjectsDiffSource` trait (production impl is `mapobjects_store::MapObjectsStoreHandle::dump_pending` per AZ-654).
|
||||
- `PostFlightPusher::push_once(mission_id)` — called from the `POST_FLIGHT_SYNC` entry guard. Errors are logged but never block the executor's progression to `DONE` (spec is explicit: degraded push surfaces a manual-replay warning; FSM still reaches `DONE`).
|
||||
|
||||
- `crates/mission_executor/src/lib.rs`
|
||||
- `MissionExecutorHandle` gained `driver: Arc<dyn MissionDriver>` and `hard_floor_active: Arc<AtomicBool>` fields.
|
||||
- `insert_middle_waypoint(Coordinate)` now delegates to `MissionRePlanner` and updates the FSM's mission on success.
|
||||
- `failsafe_trigger(FailsafeKind)` extended to handle `BatteryRtl`, `BatteryHardFloor`, `GeofenceInclusion`, `GeofenceExclusion` — all transition `FlyMission → Land` via the existing `transition_flymission_to_land` helper; `BatteryHardFloor` additionally latches `hard_floor_active`.
|
||||
- `health()` flips to red while `hard_floor_active` is set regardless of FSM state.
|
||||
- `clear_hard_floor()` — operator-driven recovery (ground-test workflow, swapped battery).
|
||||
- `#[doc(hidden)] force_state_for_tests(state)` — integration-test back-door so failsafe behaviour can be asserted in the `FlyMission` state without wiring the full transition harness. Hidden from rustdoc and not part of the public API.
|
||||
|
||||
**Tests**:
|
||||
|
||||
- `crates/mission_executor/tests/safety_and_resume.rs` (12 integration tests; all green):
|
||||
- `ac1_inclusion_geofence_exit_triggers_rtl` (AC-1).
|
||||
- `ac2_exclusion_geofence_entry_triggers_rtl` (AC-2).
|
||||
- `ac3a_battery_rtl_at_threshold` (AC-3, RTL branch).
|
||||
- `ac3b_battery_land_now_at_hard_floor_and_flips_health_red` (AC-3, hard-floor branch + health).
|
||||
- `ac4_signed_override_suppresses_battery_rtl` (AC-4).
|
||||
- `ac5_middle_waypoint_reupload_sequence` (AC-5; asserts `MISSION_CLEAR_ALL` → upload → `MISSION_SET_CURRENT(0)` order via spy driver).
|
||||
- `ac6_post_flight_push_triggered_once_executor_reaches_done` (AC-6).
|
||||
- `ac6_degraded_push_does_not_block_caller` (AC-6 negative path).
|
||||
- `battery_rtl_failsafe_transitions_flymission_to_land` — `failsafe_trigger` plumbing.
|
||||
- `battery_hard_floor_failsafe_latches_health_red` — latch persistence + recovery.
|
||||
- `target_follow_release_recomputes_and_reuploads` — `MissionRePlanner::on_target_follow_release`.
|
||||
- `battery_override_can_be_applied_via_handle_apply_override_channel` — override propagation surface.
|
||||
- Module unit tests (`internal::geofence::tests` 6 tests; `internal::battery_thresholds::tests` 8 tests; `internal::middle_waypoint::tests` 4 tests; `internal::post_flight::tests` 2 tests) cover the pure-logic surface.
|
||||
|
||||
## AC coverage
|
||||
|
||||
| AC | Behaviour | Test | Status |
|
||||
|----|-----------|------|--------|
|
||||
| AC-1 | INCLUSION exit → RTL ≤500 ms; FSM → `Land`; alert observable | `ac1_inclusion_geofence_exit_triggers_rtl` | PASS |
|
||||
| AC-2 | EXCLUSION entry → RTL ≤500 ms (parity with INCLUSION); alert observable | `ac2_exclusion_geofence_entry_triggers_rtl` | PASS |
|
||||
| AC-3a | `SYS_STATUS` ≤25 % → RTL; FSM → `Land` | `ac3a_battery_rtl_at_threshold` | PASS |
|
||||
| AC-3b | `SYS_STATUS` <15 % → `MAV_CMD_NAV_LAND`; health → red | `ac3b_battery_land_now_at_hard_floor_and_flips_health_red` | PASS |
|
||||
| AC-4 | Signed `BatteryOverride { until_ts }` suppresses RTL; audit-log entry | `ac4_signed_override_suppresses_battery_rtl` | PASS |
|
||||
| AC-5 | `MISSION_CLEAR_ALL` → upload → `MISSION_SET_CURRENT(0)` in order, ≤2 s e2e | `ac5_middle_waypoint_reupload_sequence` | PASS |
|
||||
| AC-6 | On `POST_FLIGHT_SYNC` entry → `push_mapobjects_diff` exactly once; FSM still reaches `DONE` on push failure | `ac6_post_flight_push_triggered_once_executor_reaches_done`, `ac6_degraded_push_does_not_block_caller` | PASS |
|
||||
|
||||
## Code review
|
||||
|
||||
**Spec compliance**: PASS. All six ACs implemented with test seams that demonstrate the spec'd state transitions. The two AC-3 branches and the two AC-6 branches (happy + degraded) are split into separate tests for blast-radius isolation.
|
||||
|
||||
**Architecture compliance**: PASS.
|
||||
|
||||
- Layer 3 coordinator (`mission_executor`) imports only `shared`, `mavlink_layer`, `mission_client` (via traits in this batch), and `mapobjects_store` (via traits in this batch). No new Layer 3 ↔ Layer 3 imports.
|
||||
- `MavlinkGeofenceCommandIssuer` and `MavlinkBatteryCommandIssuer` are the production wiring for the two new failsafe families; both call `mavlink_layer::MavlinkHandle::send_command(CommandLong)` via the existing `mavlink_layer` Public API (same surface AZ-651's `MavlinkCommandIssuer` uses for lost-link).
|
||||
- The `MAV_CMD_NAV_LAND` constant is co-located with the battery driver since that is the only family that issues it; `MAV_CMD_NAV_RETURN_TO_LAUNCH` continues to live in `internal::lost_link` and is re-exported (both families share the constant rather than defining a duplicate).
|
||||
|
||||
**SRP**: PASS.
|
||||
- `geofence.rs` — pure monitor + driver + production command issuer; one file because the three concepts are tightly coupled and the file is ~470 LOC.
|
||||
- `battery_thresholds.rs` — same structure for battery.
|
||||
- `middle_waypoint.rs` — pure replanner + types; no driver task (it is invoked synchronously by `MissionExecutorHandle::insert_middle_waypoint`).
|
||||
- `post_flight.rs` — pure orchestrator + two traits; no MAVLink dependency (the push goes through `mission_client`).
|
||||
|
||||
**Runtime completeness**: PASS. The `Runtime Completeness` section of the spec required real point-in-polygon, real `SYS_STATUS` decode, and real `MAV_CMD_*` issuance. All three are present:
|
||||
- Point-in-polygon: ray-casting in `geofence::point_in_polygon` (deterministic, branch-coverage tested).
|
||||
- `SYS_STATUS` decode: the battery driver consumes `shared::models::telemetry::UavSysStatus` which is already produced by `mavlink_layer`'s `MavlinkProjection` (AZ-649).
|
||||
- `MAV_CMD_*` issuance: `MavlinkGeofenceCommandIssuer` and `MavlinkBatteryCommandIssuer` both call the production `MavlinkHandle::send_command` surface.
|
||||
|
||||
**Test discipline**: PASS. Each AC maps to one named test (two branches each for AC-3 and AC-6). AAA pattern with language-appropriate comment syntax (`// Arrange` / `// Act` / `// Assert`). Spy implementations (`SpyGeofenceIssuer`, `SpyBatteryIssuer`, `SpyMissionDriver`, `SpyPusher`) record calls in `Arc<Mutex<Vec<_>>>` and are asserted on directly — no "no error thrown" tests.
|
||||
|
||||
**Security quick-scan**: PASS. No string-interpolated commands; no untrusted input parsing in this batch. `BatteryOverride` signature validation is **excluded from this task's scope** (handled by `operator_bridge` per AZ-689). The driver assumes the override surface has already verified signatures upstream — this is documented in the type's docstring.
|
||||
|
||||
**Performance scan**: PASS. Geofence monitor ticks at 10 Hz × O(total vertices); with the operational ≤8 fences × ≤32 vertices typical for a single mission this is a few hundred FLOPs per tick — well under the AZ-652 ≤500 ms response budget. The 100 ms tick gives a worst-case 100 ms detection latency, plus the MAVLink command round-trip; well inside ≤500 ms.
|
||||
|
||||
**Cross-task consistency**: N/A — this batch contains a single task.
|
||||
|
||||
## Module-layout drift (minor)
|
||||
|
||||
`_docs/02_document/module-layout.md` lists `crates/mission_executor/src/internal/geofence/*` (a folder). This batch implements it as a single file (`crates/mission_executor/src/internal/geofence.rs`). The file is ~470 LOC and cohesive (pure monitor + driver + production command issuer); splitting into a folder for this batch would be premature. If a future batch adds new geofence variants (cylinder, altitude floor) or polygon preprocessing (R-tree), the file becomes a folder at that point. Flagged here so the next module-layout sync picks it up.
|
||||
|
||||
## Known limitations (warnings)
|
||||
|
||||
1. **`MavlinkBatteryCommandIssuer::issue_land_now` passes all `param_*` zeroed.** Per `architecture.md §7.7` this asks the airframe to pick the safest reachable landing point. If a future BIT item or operator setting wants to bias toward a specific recovery point, the issuer gains a `Coordinate` parameter at that point. Currently no caller supplies one.
|
||||
|
||||
2. **`force_state_for_tests` is hidden from rustdoc but is a public symbol.** It is marked `#[doc(hidden)]` and only used by `tests/safety_and_resume.rs`. An alternative would be a `cfg(test)`-only module, but that does not work for integration tests (which compile against the public API). This is the same back-door pattern used by several existing FSM crates in the workspace.
|
||||
|
||||
3. **Audit-log persistence is a `tracing::info!` call, not a database write.** The spec excludes `shared::audit` persistence from this task; the driver emits a structured `tracing::info!(target = "audit", ...)` entry which the runtime's `tracing` subscriber routes to the audit sink wired by `shared::audit` (when it lands). This matches the AZ-651 lost-link audit-log pattern.
|
||||
|
||||
## Auto-fix attempts during the batch
|
||||
|
||||
- `cargo fmt -p mission_executor` straightened `use mavlink_layer::{CommandLong, MavlinkHandle, SendCommandError};` after adding the production issuers.
|
||||
- Removed an unused `mpsc` import from `tests/safety_and_resume.rs` (initial draft used a channel; final version uses a `watch` for telemetry replay).
|
||||
- `clippy -p mission_executor --tests -- -D warnings` is green.
|
||||
|
||||
## Test reproduction
|
||||
|
||||
```
|
||||
cargo build -p mission_executor --tests
|
||||
cargo test -p mission_executor # all green
|
||||
cargo test --test safety_and_resume -p mission_executor # 12 tests; 0 failed
|
||||
cargo clippy -p mission_executor --tests -- -D warnings
|
||||
cargo test --workspace # all green
|
||||
```
|
||||
|
||||
## Candidates for batch 10
|
||||
|
||||
- **AZ-653** `gimbal_a40_transport` — opens up the `gimbal_link` BIT evaluator slot (AZ-650 batch 8 noted it as the natural next slot).
|
||||
- **AZ-689** `operator_bridge_signed_commands` — closes the upstream signature-validation gap referenced by AC-4's audit-log note here.
|
||||
|
||||
Batch 10 sizing: one of the above; not both. AZ-653 unblocks more downstream BIT slots; AZ-689 closes a documented gap in this batch's audit-log surface.
|
||||
Reference in New Issue
Block a user