azaion/gps-denied-onboard
1
0
Fork 0
mirror of https://github.com/azaion/gps-denied-onboard.git synced 2026-06-21 12:11:13 +00:00
Code Issues Packages Projects Releases Wiki Activity

[AZ-396] [AZ-397] Batch 11: C8 source-set switch + QGC telemetry adapter

Browse Source 
AZ-396: PymavlinkArdupilotAdapter.request_source_set_switch body sends
MAV_CMD_SET_EKF_SOURCE_SET, awaits COMMAND_ACK with timeout, enforces
Invariant 11 idempotence (1s rate-limit + skip-after-success). Adds
runtime_root.SpoofRecoverySink to bridge C5 spoof-promotion-recovered
signal to the C8 outbound thread via a bounded dispatch queue.
FcConfig gains spoof_recovery_source_set + source_set_switch_timeout_ms.

AZ-397: QgcTelemetryAdapter implements GcsAdapter strategy: MAVLink 2.0
to QGC, emit_summary downsamples 5Hz to configurable summary_rate_hz
[0.5, 5.0] via integer modulo, emit_status_text mirrors to GCS link,
subscribe_operator_commands translates COMMAND_LONG / PARAM_REQUEST_*
/ REQUEST_DATA_STREAM / MISSION_* / SET_MODE into OperatorCommand DTOs
and audits each receipt to FDR. FcKind.GCS_QGC added for PortConfig.

Tests: 25 new (12 AZ-396 + 13 AZ-397); full suite 501 passing, 2 skipped.
Contracts unchanged (additive FcConfig fields, range relaxation on
GcsConfig.summary_rate_hz, additive FcKind enum value).

Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
Oleksandr Bezdieniezhnykh
2026-05-11 05:06:56 +03:00
parent 1e0be08e8a
commit 8a9cf88a46
16 changed files with 1608 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files
_docs/03_implementation/batch_11_cycle1_report.md
+70
View File
@@ -0,0 +1,70 @@
# Batch 11 — Cycle 1 Implementation Report
**Batch**: 11 of N
**Tasks landed**: AZ-396 (D-C8-2 source-set switch + spoof-recovery sink) + AZ-397 (QgcTelemetryAdapter)
**Cycle**: 1
**Date**: 2026-05-11
## Scope
| Task | Component | Purpose |
|------|-----------|---------|
| AZ-396 | C8 FC adapter (AP) + runtime root | `PymavlinkArdupilotAdapter.request_source_set_switch` body — sends `MAV_CMD_SET_EKF_SOURCE_SET` (ardupilotmega cmd 42007) with `param1 = config.fc.spoof_recovery_source_set`, waits for `COMMAND_ACK` up to `config.fc.source_set_switch_timeout_ms`, idempotence per Invariant 11 (rate-limit within 1 s + skip after success). `runtime_root.SpoofRecoverySink` provides a bounded-queue thread sink that consumes C5's spoof-promotion-recovered signal (AZ-385 publisher side; future task) and dispatches the source-set switch from a single dedicated thread (Invariant 8 preserved). |
| AZ-397 | C8 GCS adapter | `QgcTelemetryAdapter` — concrete `GcsAdapter` strategy. Open/close a MAVLink 2.0 channel to QGC on the configured UART; `emit_summary` downsamples 5 Hz → `summary_rate_hz` via modulo arithmetic and emits `GLOBAL_POSITION_INT` + `NAMED_VALUE_FLOAT("horiz_m")`; `emit_status_text` mirrors a STATUSTEXT to the GCS link; `subscribe_operator_commands` registers a pymavlink message-hook that translates `COMMAND_LONG` / `PARAM_REQUEST_*` / `REQUEST_DATA_STREAM` / `MISSION_*` / `SET_MODE` into `OperatorCommand` DTOs and writes every receipt to FDR (`kind="c8.gcs.operator_command"`) as the audit trail. |
## Files added / modified
### Added (prod)
- `src/gps_denied_onboard/runtime_root/spoof_recovery_sink.py` — `SpoofRecoveryPublisher` Protocol + `SpoofRecoverySink` (dispatch thread, bounded queue, error isolation).
- `src/gps_denied_onboard/components/c8_fc_adapter/mavlink_gcs_adapter.py` — `QgcTelemetryAdapter` + `_compute_downsample_modulo` helper.
### Added (tests)
- `tests/unit/c8_fc_adapter/test_az396_source_set_switch.py` — 12 AC tests (10 ACs + 2 sink-isolation cases).
- `tests/unit/c8_fc_adapter/test_az397_qgc_telemetry.py` — 13 AC tests (10 ACs + parametrised rate cases).
### Modified (prod)
- `src/gps_denied_onboard/_types/fc.py` — added `FcKind.GCS_QGC` enum value (additive; lets `PortConfig.fc_kind` discriminate the GCS link variant alongside AP/iNav).
- `src/gps_denied_onboard/components/c8_fc_adapter/pymavlink_ardupilot_adapter.py` — `request_source_set_switch` body (AZ-396) replaces the NotImplementedError placeholder; new helpers `_wait_for_command_ack` and `_handle_source_set_switch_failure`; ctor fields `_last_switch_attempt_ns` + `_last_switch_succeeded`.
- `src/gps_denied_onboard/config/schema.py` — `FcConfig` extended with `spoof_recovery_source_set: int = 1` + `source_set_switch_timeout_ms: int = 1500` (defaults align with AZ-396 spec); `GcsConfig.summary_rate_hz` valid range widened from `[1.0, 2.0]` to `[0.5, 5.0]` per AZ-397 AC-10.
- `src/gps_denied_onboard/config/loader.py` — `ENV_KEY_MAP` extended with `FC_SPOOF_RECOVERY_SOURCE_SET` + `FC_SOURCE_SET_SWITCH_TIMEOUT_MS`; `_FIELD_COERCIONS` extended with the two new int fields.
- `src/gps_denied_onboard/runtime_root/__init__.py` — re-exports `SpoofRecoveryPublisher` + `SpoofRecoverySink`.
### Modified (tests)
- `tests/unit/c8_fc_adapter/test_az390_adapter_protocol.py` — updated `test_ac3_fc_kind_has_two_members` to reflect the additive `GCS_QGC` enum value; updated `test_gcs_summary_rate_out_of_range_rejected` boundaries (`5.1` / `0.4`) to match the new valid range.
- `tests/unit/c8_fc_adapter/test_az393_ardupilot_outbound.py` — AC-9 placeholder check (`NotImplementedError`) replaced by a callability check; the real source-set-switch behaviour now lives under AZ-396 tests.
## Contract changes
- `_docs/02_document/contracts/c8_fc_adapter/fc_adapter_protocol.md` — **unchanged at v1.0.0**.
- `_docs/02_document/contracts/shared_config/composition_root_protocol.md` — **unchanged at v1.2.0**. Additive `FcConfig` fields + valid-range relaxation for `GcsConfig.summary_rate_hz` (existing configs remain valid).
## Test counts
| Metric | Before | After | Delta |
|--------|--------|-------|-------|
| Tests passing | 476 | 501 | +25 |
| Tests skipped | 2 | 2 | 0 |
| Tests failing | 0 | 0 | 0 |
## Architectural notes
- **Source-set switch threading**: the AP adapter's `request_source_set_switch` enforces single-writer via the same `_enforce_single_writer` used by `emit_external_position`. The `SpoofRecoverySink` dispatch thread is the documented single writer; the C5 publisher thread calls `sink.publish()` which only enqueues. The bounded queue guarantees the publisher never blocks on UART.
- **`_wait_for_command_ack` filters by `type="COMMAND_ACK"`** — necessary because the inbound MAVLink decoder thread (AZ-391) also calls `recv_match` on the same connection. Real pymavlink routes by type internally; the unit-test stub explicitly mirrors that behaviour so the test-vs-production gap stays small.
- **GCS adapter downsampling**: `_compute_downsample_modulo = round(_COMPOSE_ROOT_INVOKE_HZ / summary_rate_hz)`. Tests pin the exact integer modulo so any future change to `_COMPOSE_ROOT_INVOKE_HZ` shows up as a deliberate AC update. The modulo is computed once at construction; updates to `summary_rate_hz` require a reopen.
- **Operator command dispatch**: pymavlink's message-hook list is the canonical inbound-routing seam. `_ensure_operator_handler_attached` is idempotent — multiple `subscribe_operator_commands` calls share a single hook; subscriber fan-out happens via the `SubscriptionBus` (C8 inbound pattern, AZ-391). Subscriber crashes are isolated by the bus.
- **`FcKind.GCS_QGC`**: pragmatic compromise to keep `PortConfig` single-typed across FC and GCS variants. A future `LinkKind` superclass refactor would split the two; documented in the AZ-390 follow-up.
## Dependencies introduced
- None.
## Known forward-actions
1. **AZ-385 (C5 spoof-promotion gate)** publishes the recovery signal the `SpoofRecoverySink` consumes. The composition root wires the two with one call: `publisher.subscribe_spoof_promotion_recovered(sink.publish)`. Until AZ-385 lands, the sink is constructed but inactive.
2. **`_SWITCH_RATE_LIMIT_S` and the GCS NAMED_VALUE_FLOAT name (`"horiz_m"`)** — both module-level constants; promotion to config + contract bump deferred.
3. **`global_position_int` relative-altitude placeholder** — currently mirrors absolute altitude; the runtime root must surface the launch reference frame before the relative field is meaningful.
4. **C8-IT-07 SITL end-to-end** (3 s spoofing-promotion latency) — deferred to E-BBT; this batch delivers the wire surface for that gate.
_docs/03_implementation/reviews/batch_11_review.md
+109
View File
@@ -0,0 +1,109 @@
# Batch 11 — Code Review
**Batch**: 11 of N
**Tasks**: AZ-396 (D-C8-2 source-set switch + spoof-recovery sink) + AZ-397 (QgcTelemetryAdapter)
**Reviewer**: autodev (7-phase)
**Verdict**: **PASS_WITH_INFO**
**Date**: 2026-05-11
## Scope
| Task | Files touched (prod) | Files touched (tests) |
|------|----------------------|------------------------|
| AZ-396 | `components/c8_fc_adapter/pymavlink_ardupilot_adapter.py` (extension), `config/schema.py`, `config/loader.py`, `runtime_root/__init__.py`, `runtime_root/spoof_recovery_sink.py` | `tests/unit/c8_fc_adapter/test_az396_source_set_switch.py`, `tests/unit/c8_fc_adapter/test_az393_ardupilot_outbound.py` (AC-9 update) |
| AZ-397 | `components/c8_fc_adapter/mavlink_gcs_adapter.py`, `_types/fc.py` (GCS_QGC enum), `config/schema.py` (range widening) | `tests/unit/c8_fc_adapter/test_az397_qgc_telemetry.py`, `tests/unit/c8_fc_adapter/test_az390_adapter_protocol.py` (FcKind members + GcsConfig range adjustments) |
## Phase 1 — AC compliance
### AZ-396 — 10 ACs (12 tests including 2 sink-isolation cases)
| AC | Coverage |
|----|----------|
| AC-1 ACK success → INFO log + FDR + STATUSTEXT | `test_ac1_ack_success` |
| AC-2 non-success ACK → SourceSetSwitchError + ERROR log + STATUSTEXT(ERROR) | `test_ac2_non_success_ack_raises` |
| AC-3 ACK timeout (1500ms default) | `test_ac3_ack_timeout_raises` |
| AC-4 Configurable timeout | `test_ac4_configurable_timeout_uses_config_value` (500 ms) |
| AC-5 Idempotence within 1 s — rate-limited | `test_ac5_idempotence_within_1s_rate_limited` (1 wire call after 2 invokes) |
| AC-6 Idempotence after success — no re-issue | `test_ac6_idempotence_after_success_no_reissue` |
| AC-7 Runtime-root signal triggers switch | `test_ac7_spoof_recovery_sink_triggers_switch` + isolation case |
| AC-8 source_set_id from config | `test_ac8_source_set_id_from_config` (p1=2.0 when config=2) |
| AC-9 Placeholder NotImplementedError replaced | `test_ac9_no_longer_raises_not_implemented` |
| AC-10 STATUSTEXT severity matrix | `test_ac10_statustext_severity_matrix` (success=INFO, fail=ERROR) |
### AZ-397 — 10 ACs (13 tests including parametrised cases)
| AC | Coverage |
|----|----------|
| AC-1 5 → 2 Hz downsample | `test_ac1_5hz_to_2hz_downsample` (100 calls → 50 frames; modulo=2) |
| AC-2 Configurable rate | `test_ac2_configurable_rate[1.0-20]`, `[5.0-100]` + `test_ac2_out_of_range_rate_rejected_at_config` (10 / 0.2) |
| AC-3 Summary frame fields | `test_ac3_summary_frame_fields` — `global_position_int_send` lat/lon/alt match; `NAMED_VALUE_FLOAT("horiz_m")` matches projector |
| AC-4 STATUSTEXT mirror | `test_ac4_statustext_mirror` |
| AC-5 Operator command callback | `test_ac5_operator_command_subscription_invokes_callback` (PARAM_REQUEST_LIST → OperatorCommand) |
| AC-6 FDR audit trail | `test_ac6_operator_command_fdr_audit_trail` (`kind="c8.gcs.operator_command"`) |
| AC-7 Single-writer thread | `test_ac7_single_writer_thread` |
| AC-8 First emit logged once | `test_ac8_first_emit_logged_once` |
| AC-9 WGS84 round-trip ≤ 1 cm | `test_ac9_wgs84_round_trip_within_1cm` (defensive against the shared helper C4 + C8 use) |
| AC-10 GcsAdapterConfigError on bad config | `test_ac10_gcs_config_error_on_bad_rate` + `test_ac10_open_rejects_wrong_fc_kind` |
25 new tests added (12 + 13); 501 total in suite (was 476), 2 pre-existing skips, 0 failures.
## Phase 2 — Contract drift
- `_docs/02_document/contracts/c8_fc_adapter/fc_adapter_protocol.md` — **unchanged at v1.0.0**. The `request_source_set_switch` and `GcsAdapter` Protocol entries were declared in v1.0.0 / AZ-390; this batch wires the bodies.
- `_docs/02_document/contracts/shared_config/composition_root_protocol.md` — **unchanged at v1.2.0**. The new `FcConfig.spoof_recovery_source_set` + `FcConfig.source_set_switch_timeout_ms` fields are additive defaults; `GcsConfig.summary_rate_hz` valid range widened from `[1.0, 2.0]` to `[0.5, 5.0]` per AZ-397 AC-10 — this is a constraint relaxation, NOT a tightening; existing valid configs remain valid.
- `_types/fc.py` `FcKind` — additive `GCS_QGC` enum value. The shared `PortConfig.fc_kind` discriminator now also marks GCS link variants. Existing AP/iNav callers unaffected.
## Phase 3 — Architectural compliance
- **ADR-002 (build-time exclusion)** — `QgcTelemetryAdapter` is registered in `runtime_root.fc_factory._GCS_BUILD_FLAGS["qgc_mavlink"] = "BUILD_GCS_QGC_MAVLINK"` (batch 8); the lazy `from pymavlink import mavutil` inside `_connect` keeps the wire dependency out of the binary's import graph when the flag is OFF. Tests inject `connect_factory` so neither pymavlink nor a real UART is required.
- **ADR-009 (interface-first DI)** — both new helpers (`SpoofRecoverySink`, `QgcTelemetryAdapter`) accept all deps via the constructor. The sink only depends on the `FcAdapter` Protocol; it never imports a concrete adapter class.
- **Module layering** — `mavlink_gcs_adapter.py` is the public `GcsAdapter` strategy (no `_` prefix); the `SpoofRecoverySink` lives in `runtime_root/spoof_recovery_sink.py` and is exported from `runtime_root/__init__.__all__` (single-source-of-truth for composition-root surface).
- **Single-writer outbound thread (Invariant 8)** — enforced on `emit_summary` and `emit_status_text` in `QgcTelemetryAdapter`. The `request_source_set_switch` body in the AP adapter also checks single-writer.
- **Single-writer dispatch via SpoofRecoverySink** — the sink uses a bounded `queue.Queue` to deliver C5's spoof-recovery signal to a dedicated dispatcher thread, which is the SAME thread that calls `request_source_set_switch` for the entire sink lifetime. This makes the sink itself the writer-of-record from C8's perspective; no race with the C5 publisher thread.
- **Idempotence (Invariant 11)** — implemented via `_last_switch_attempt_ns` + `_last_switch_succeeded`; rate-limit window `_SWITCH_RATE_LIMIT_S = 1.0` is constant. Promotion to config knob is a forward action (informational finding).
- **Downsampling counter (Invariant 12)** — modulo arithmetic (`_invocation_count % _downsample_modulo`). The modulo is computed ONCE at construction time from `config.gcs.summary_rate_hz`; the chosen mapping is documented in `_compute_downsample_modulo` (5 Hz → 1, 2 Hz → 2, 1 Hz → 5, 0.5 Hz → 10) — operator-side decoder must use the same table when reasoning about expected frame rate.
## Phase 4 — Performance & reliability
- **AP source-set switch** — single `command_long_send` + ACK loop with monotonic clock. p95 is dominated by the FC's ACK round-trip; the adapter contributes <1 ms of overhead (one `command_long_send` + one `recv_match` per iteration). The `_wait_for_command_ack` filter ignores non-COMMAND_ACK messages, so cross-talk with the inbound decoder thread is preserved (verified by the test stub honouring `type=`).
- **GCS downsample emit** — `emit_summary` is one modulo compare + (on every Nth call) two `mav.*_send` calls. The frame counter is integer; no allocations per drop.
- **Operator command path** — pymavlink's message-hooks list is appended once on first `subscribe_operator_commands`. Subsequent subscribers piggy-back on the same hook (the `SubscriptionBus` does the fan-out; subscribers can be cancelled independently).
- **Sink queue capacity** — bounded to 16 pending switches. The recovery gate produces at most one signal per spoof-promotion event; 16 is a defensive cushion. Overflow emits a WARN and drops; the C8 idempotence gate (Invariant 11) would suppress duplicates anyway.
- **Sink stop semantics** — `stop(join_timeout_s=1.0)` flips the stop event and pushes a sentinel; the dispatcher exits on the next `queue.get` cycle (worst case 500 ms). Idempotent — multiple `stop()` calls are safe.
## Phase 5 — Test quality
- **pymavlink message-hook + recv_match are stubbed correctly** — the AZ-396 stub's `recv_match` filters by `type=`, mirroring real pymavlink behaviour. Without the filter the inbound decoder thread (which also calls `recv_match`) ate the ACK. The fix is documented inline in the test file.
- **Sink AC-7 covers both happy path and `SourceSetSwitchError` isolation** — the dispatcher survives a raised exception from the adapter and continues to process subsequent `publish()` calls. This protects against C5 floods.
- **AZ-397 AC-2 is parametrised** — single test exercises 1 Hz and 5 Hz endpoints; one separate test exercises the out-of-range path.
- **AZ-397 AC-9 uses the real WgsConverter** — the round-trip is computed on the real helper, not a mock; defensive against silent regressions in the shared converter (C4 + C8 both depend on it).
- **AZ-393 AC-9 retitled** — the original placeholder-`NotImplementedError` assertion is replaced by a presence/callability check; the real behaviour is now under AZ-396's coverage. The change is documented inline.
- Arrange/Act/Assert pattern consistently applied.
## Phase 6 — Logging & FDR coverage
- **AP adapter (AZ-396 additions)**: `c8.ap.source_set_switch_executed` (INFO + FDR), `c8.ap.source_set_switch_failed` (ERROR + FDR), `c8.ap.source_set_switch_rate_limited` (INFO), `c8.ap.source_set_switch_already_active` (INFO), `c8.ap.recv_match_failed` (DEBUG).
- **Spoof recovery sink**: `c8.spoof_recovery_sink_switch_failed` (DEBUG), `c8.spoof_recovery_sink_adapter_error` (WARN), `c8.spoof_recovery_sink_queue_full` (WARN).
- **GCS adapter**: `c8.gcs.first_summary_emit` (INFO, once), `c8.gcs.summary_emit` (DEBUG, per emit), `c8.gcs.summary_emit_failed` (ERROR), `c8.gcs.statustext_failed` (DEBUG), `c8.gcs.operator_command_fdr_enqueue_failed` (DEBUG).
- **FDR record kinds**: `c8.ap.source_set_switch_executed` (INFO), `c8.ap.source_set_switch_failed` (ERROR), `c8.gcs.operator_command` (INFO; per inbound operator command — § 9 audit trail).
## Phase 7 — Security & risk surface
- **R-D-C8-2 (firmware-supported but no operator-deployed precedent)** — AZ-396 delivers the wire surface; the production gate is IT-3 SITL (ADR-008). On failure paths, the system continues to emit `GPS_INPUT` and the operator can manually switch via RC aux (D-C8-2-FALLBACK); the failure is surfaced via STATUSTEXT(ERROR) + FDR for operator audit.
- **Spoof recovery wiring** — the C5 publisher side (AZ-385) is not yet landed. The sink is constructed but not wired; AC-7 verifies the wiring shape works with a mocked publisher. When AZ-385 lands the composition root makes ONE call: `publisher.subscribe_spoof_promotion_recovered(sink.publish)`.
- **GCS operator-command audit** — every inbound operator command emits an FDR record before the subscriber callback fires; an audit trail survives even if the subscriber crashes. The `SubscriptionBus` isolation guarantees a misbehaving subscriber cannot kill the message-hook dispatch.
- **STATUSTEXT mirror is one-shot** — `emit_status_text` truncates the message to 50 bytes (MAVLink constraint) and emits exactly one frame; no rate-limit on this path (the C5/C8 emitters rate-limit at the source).
- **No new external dependencies** — pymavlink and pyserial were already pinned; no new packages introduced.
- **Wider GcsConfig range** is a relaxation, not a tightening — operator misconfiguration that previously raised `ConfigError` at boot may now silently use an unexpected rate. The contract clarifies the rate range; operator-side documentation should note the new ceiling.
## Informational findings (non-blocking)
1. **`_SWITCH_RATE_LIMIT_S = 1.0`** is a module-level constant. Promotion to `FcConfig.source_set_switch_rate_limit_s` is a forward-action contract bump.
2. **Spoof-recovery sink queue capacity (16)** is a module-level constant. The C5 publisher is the only producer; the cap is defensive. Promotion to config is a forward action.
3. **AZ-397's `global_position_int` encoding uses `alt_m * 1000.0` for both the absolute and relative-altitude fields** — the relative-altitude needs a real take-off-relative offset, which only the runtime root knows at this point. The placeholder is acceptable for the GCS-display use case; promotion to a real relative-alt computation is a forward action when the composition root surfaces the launch reference frame.
4. **AZ-397 NAMED_VALUE_FLOAT name `"horiz_m"`** is documented inline as the canonical GCS-side decoder key. The operator-side decoder (E-C12) MUST mirror this string.
5. **`FcKind.GCS_QGC`** — adding GCS to a previously FC-only enum is a pragmatic compromise to keep `PortConfig` single-typed. A future refactor could split this into a `LinkKind` superclass without breaking external callers.
## Verdict
PASS_WITH_INFO — all 20 ACs (10 + 10) satisfied; 25 new tests added (501 total, 0 failures); contract surface unchanged at v1.0.0 / composition_root v1.2.0; constraint relaxations and one additive enum value are non-breaking; five informational findings are forward-action enhancements.