Skip to content

Defense-in-depth pass on companion safety - #1

Open
gasantiago16 wants to merge 3 commits into
mainfrom
safety-defense-in-depth
Open

Defense-in-depth pass on companion safety#1
gasantiago16 wants to merge 3 commits into
mainfrom
safety-defense-in-depth

Conversation

@gasantiago16

@gasantiago16 gasantiago16 commented Apr 25, 2026

Copy link
Copy Markdown
Owner

Summary

Closes the ten issues from the v0.3 logic ultrareview. The keystone safety invariant — MSP_SET_RAW_RC sent only when state.is_active(); silent companion → BF 500ms timeout → RX takeover — was already correct. This PR tightens the second-tier checks the docs implied but the code didn't fully enforce.

Critical bug fixes

  • analog=None silently passed safety (safety.py:66) — a loose VBAT pad would have flown a tired pack into a tree. Now mirrors the GPS pattern with no_analog_telemetry + stale_analog.
  • HOLD never re-checked distance (state.py) — wind drift kept centered sticks while the drone drifted out of the hold disc. Now re-engages TRANSIT past arrival_radius_m × 1.5 with hysteresis to prevent ping-pong.
  • max_distance_from_target_during_transit_m was a dead config field — declared, never consumed. Now wired as transit_runaway_*m reason while in TRANSIT.
  • CLIMB ran full nav at low altitude — nav.compute_rc(climb_phase=True) now zeros roll/pitch/yaw until current alt ≥ 80% of target. Removes the downhill-takeoff treeline hazard.

New defense-in-depth checks

  • HeadingDivergenceTracker — sliding-window watchdog catches mag-NONE yaw drift before geofence does. Trips when forward-flight |heading_err| mean exceeds 60° for 3s.
  • stale_rc / no_rc_telemetry — companion now publishes MSP_RC outage instead of silently defaulting aux_active=False.
  • FlightModeReader — subscribes MSP_STATUS_EX + MSP_BOXNAMES, decodes ANGLE/HORIZON box bits, refuses overrides if BF reports ACRO. Fail-open until BOXNAMES received; safelock.lua remains primary defense.

Tests + tooling

  • scripts/test_lua_logic.py — drives racestrt.lua state machine and safelock.lua threshold logic via lupa with controlled time. Catches transition bugs the parse-only check missed. 11 logic tests, wired into CI alongside the syntax check.
  • MAVLink byte-for-byte coverage: STATUSTEXT vs pymavlink 2.4.49, COMPANION_STATE frozen reference vector, plus HEARTBEAT + STATUSTEXT pymavlink round-trip.
  • SITL hard-fail: sitl/start.sh now reads back the dump and exits 1 if msp_override_channels_mask = 15 or mag_hardware = NONE are missing. Was best-effort nc | echo WARNING before — would have let the smoke test "pass" with overrides on AUX channels unbounded.
  • MEMORY.md updated with the v0.4 snapshot and 4 new sharp-edge entries.

Test counts

Before After
Python (companion) 71 always + 3 SITL 102 always + 3 SITL
Lua parse only parse + 11 logic

Test plan

  • CI green on push (Python + Lua syntax + Lua logic)
  • Manual SITL run validates the new readback assert (build SITL container, start, confirm logs [sitl] OK: msp_override_channels_mask = 15)
  • First field flight uses the new transit_runaway cap (default 200m) — verify no false trips on legit racing distances; tune if needed
  • Heading-divergence threshold (60° / 3s) tuned against real mag-NONE flight log; current values are conservative against typical 1°/min drift

🤖 Generated with Claude Code

Gabriel Santiago and others added 3 commits April 25, 2026 09:53
Gabriel Santiago @claude
Defense-in-depth pass on companion safety
9e45f9f 
Closes ten issues found by the v0.3 logic ultrareview. The keystone safety
invariant ("MSP_SET_RAW_RC sent only when state is_active(); silent companion
→ BF 500ms timeout → RX takeover") was already correct. This PR tightens the
second-tier checks the docs claimed but the code didn't fully enforce.

Critical bug fixes
- safety.evaluate now blocks on `analog is None`, mirroring the GPS pattern;
  a loose VBAT pad would have silently passed the gate.
- state machine: HOLD now re-engages TRANSIT when drift exceeds
  arrival_radius_m * 1.5. Wind in HOLD used to hold centered sticks while
  drifting indefinitely.
- safety.max_distance_from_target_during_transit_m was a dead config field;
  now wired as a `transit_runaway_*m` reason while in TRANSIT. Documented
  protection that didn't exist.
- nav.compute_rc(climb_phase=True) zeros roll/pitch/yaw during CLIMB until
  current alt >= 80% of target. Removes the downhill-takeoff treeline hazard.

New defense-in-depth checks
- HeadingDivergenceTracker — sliding-window watchdog catches mag-NONE yaw
  drift before geofence does (geofence triggers only after lateral drift
  exceeds the radius, by which time the drone is far off-bearing).
- stale_rc / no_rc_telemetry safety reasons — companion now reports MSP_RC
  outage instead of silently defaulting aux_active to False with no signal.
- FlightModeReader (MSP_STATUS_EX + MSP_BOXNAMES) — companion subscribes,
  decodes ANGLE/HORIZON box bits, refuses overrides if BF reports ACRO.
  Fail-open until BOXNAMES received; safelock.lua remains primary defense.

Tests + tooling
- scripts/test_lua_logic.py — drives racestrt.lua's state machine and
  safelock.lua's threshold logic via lupa with controlled time. Catches
  transition bugs the parse-only check missed. 11 logic tests, wired into CI.
- MAVLink: STATUSTEXT byte-for-byte vs pymavlink 2.4.49, COMPANION_STATE
  frozen reference vector, HEARTBEAT + STATUSTEXT pymavlink round-trip.
- sitl/start.sh — hard-fails if `msp_override_channels_mask = 15` or
  `mag_hardware = NONE` are missing from the post-defaults dump. Was a
  best-effort `nc | echo WARNING` before.

Test counts: 71 → 105 Python (102 always + 3 SITL-gated) + 11 Lua logic.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Gabriel Santiago @claude
Add 6 capability/quality items + 6-agent peer review fixes
8fe2261 
Six additions, each independently reviewed by a separate agent for bugs;
agents found 11 real issues (one per finding listed below) which are also
fixed in this commit.

## Items shipped

1. **Pre-flight config validator** (`scripts/preflight_validator.py`).
   Diff a per-drone manifest against an FC `dump`, exit 1 on mismatch.
   18 tests. *Reviewer found:* `#`-stripping corrupted values containing
   `#` (`set craft_name = jacob#1`); blanket `value.upper()` made
   case-sensitive name fields silently match wrong-case values. Both
   fixed: comments only at line start; case-folding only for uniformly-
   cased single-token enum values.

2. **Telemetry recorder + replayer** (`racer_companion/recorder.py`,
   `tools/replay.py`). Tee MSP bytes to JSONL during a flight; replay
   through MspClient parser offline. 10 tests. *Reviewer found:* file
   order trusted (out-of-order events bypass pacing); non-monotonic
   `time_source` silently stalls; disk-full kills the FC link; malformed
   JSONL lines abort the load. All fixed: events sorted by timestamp on
   load, elapsed clamped to >=0 (with NaN→inf for drain mode), `_emit`
   wrapped in try/except OSError, malformed lines skipped with continue.

3. **FC adapter pattern** (`racer_companion/fc/`). FlightController
   Protocol + TelemetrySnapshot + BetaflightAdapter wrapping
   MspClient + FlightModeReader. main.py refactored to consume.
   12 tests. *Reviewer found:* only MSP_STATUS_EX decoder was wrapped
   — a malformed GPS/ATTITUDE/etc frame would have killed the loop;
   BOXNAMES log line was lost in the refactor; snapshot returned by
   reference (callers retaining across ticks would see back-mutation).
   All fixed: every decoder branch wrapped, BOXNAMES log restored,
   `tick()` now returns `dataclasses.replace(self._snapshot)`.

4. **Property-based safety tests** (`tests/test_safety_properties.py`).
   hypothesis sampling for clamp invariants, evaluate() totality, and
   tracker invariants. *Reviewer found:* `test_ok_iff_no_reasons` was
   vacuous — `received_at` and `now` independent uniforms made stale
   telemetry universal, so the `ok=True` branch was unreachable;
   `test_no_exceptions_on_any_input` left geofence/transit/divergence
   defaulted; GPS strategy excluded poles/antimeridian; `test_never_trips
   _below_threshold` only sampled |err|<30 against a 60° threshold.
   Fixed: `FRESH_RECEIVED_AT` strategy anchored within staleness window,
   all evaluate() args randomized, lat/lon include ±90 / ±180,
   sub-threshold strategy widened to ±59. Hypothesis pinned >=6.100.

5. **Wind correction in HOLD** (`nav.py: hold_command_us`,
   `position_error_body_m`). Body-frame P controller for HOLD with
   small gains (8 us/m, ±60us cap) — drone now lazily opposes wind
   drift instead of holding centered sticks. 11 tests. *Reviewer*
   independently rederived the body-frame math and confirmed signs.
   *Reviewer found:* no NaN/Inf guard would surface as a `ValueError`
   from `int(round(NaN))`; sign-correctness untested for heading
   180°/270°. Fixed: `math.isfinite` guard returns centered sticks
   on bad input; explicit tests added for the high-risk headings.

6. **Heading-threshold tuning harness** (`scripts/tune_heading
   _threshold.py`). Replay a JSONL flight log through the tracker
   across a grid of thresholds × windows; report two metrics.
   12 tests. *Reviewer found:* degree-sign broke Windows console
   (cp1252); `load_log()` aborted on a single torn last line;
   `--log` and `--synthetic` not mutually exclusive; trip-count
   alone was misleading (a flapper at 40°/2s with 12 brief trips
   looks the same as a sustained trip). Fixed: ASCII-only output,
   per-line tolerance with stderr summary, mutually-exclusive
   argparse group, second metric `tripped_seconds` rendered as
   its own table.

## Test counts

- companion: 105 → 154 always-run (+ 3 SITL-gated, unchanged)
- scripts: 11 Lua logic + 18 preflight + 12 tune-heading

## CI

`scripts-tests` job (renamed from `lua-syntax`) now runs Lua syntax,
Lua logic, preflight tests, and tune-heading tests. `python-tests`
installs `[dev]` extras (hypothesis, pymavlink) for property-based
and round-trip MAVLink tests.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Gabriel Santiago @claude
Fix holistic-review findings + integration tests + docs
0419649 
A holistic cross-cutting review (the 7th agent, after the six per-item
reviewers) found one critical bug and several documentation gaps the
per-item reviewers couldn't see. Fixes here.

## Critical fix

**HeadingDivergenceTracker was silent dead code in default config.**
main.py built `pitching_forward = (state==TRANSIT and |err| < 25°)`,
combined with the tracker's 60° trip threshold meant the tracker was
only fed samples whose mean was bounded by 25 — it could NEVER trip.
The mag-NONE drift watchdog advertised in the v0.4 commit was silent.

Fix: `transit_active = (state == TRANSIT)` only. The error-magnitude
gate is removed; the tracker now sees every TRANSIT-state sample,
which is what the threshold check is for. Same fix applied in
`scripts/tune_heading_threshold.py` so the tuning grid reproduces
what the live tracker actually sees. Sharp-edge entry added in
MEMORY.md so future maintainers don't re-introduce the gate.

A new end-to-end test (`test_recorder_fc_integration.py`) drives the
same boolean main.py builds, confirming the tracker DOES trip on
sustained 90° error in TRANSIT.

## Other fixes

- **`tools/replay.py` invocation**: docstring said
  `python -m racer_companion.tools.replay`, which doesn't exist.
  Corrected to `python -m tools.replay` (matching the `tools/`
  package layout used by `msp_loopback_test.py` and `replay_synth.py`).
- **`RecordingAdapter` is now wired into main.py via `--record <path>`.**
  Previously the recorder was implemented and tested but unreachable
  from the running binary. `BetaflightAdapter.open()` accepts an
  optional `record_path` and composes the chain
  `serial → RecordingAdapter → MspClient → adapter`.
- **`is_acro_active()` docstring** said "armed and neither ANGLE nor
  HORIZON active." Code never consulted ARM. Docstring corrected to
  match (intentional fail-active behavior, paired with safelock.lua
  upstream).
- **`bf_config/per_drone/example_manifest.txt`** added — the floor
  every drone must satisfy. Plus a "Pre-flight validation" section
  in the per-drone README so a new pilot has an on-ramp.
- **`companion/README.md`** now lists `fc/`, `recorder.py`,
  `tools/replay.py`, and the `--record` flag on main.py.
- **MEMORY.md** v0.4 → v0.5: snapshot table updated with the 6 new
  items + correct test count (105 → 159 unit + 9 hypothesis); 6 new
  sharp-edge entries (heading-tracker gate, is_acro arm-check absence,
  tools layout, snapshot copy contract, recorder error-swallow, validator
  normalize semantics); pymavlink "NOT done" row qualified as dev-dep only.

## New tests

- `test_main_smoke.py` — imports `racer_companion.main`,
  `racer_companion.fc.betaflight`, `racer_companion.recorder` to
  catch typos / circular-import regressions that would only surface
  on the Pi at startup.
- `test_recorder_fc_integration.py` — round-trips a recorded byte
  stream through `RecordingAdapter → MspClient → BetaflightAdapter`,
  then back through `ReplayAdapter`. Plus the heading-tracker
  end-to-end test described above.

## Test counts

- companion: 154 → 159 always-run (+ 3 SITL-gated)
- scripts: unchanged (11 Lua + 18 preflight + 12 tune)

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
gasantiago16 pushed a commit that referenced this pull request Apr 28, 2026
Gabriel Santiago
docs: v0.7 snapshot — Isaac Sim flight working end-to-end + open items
159a62f 
End-of-night doc pass after landing the two-stage ARM_SWITCH recovery
in `pegasus-bridge`. Captures what changed and what's still open.

- MEMORY.md: new v0.7 snapshot at top documenting the latch fix path
  (shim send-lock + two-stage recovery + failsafe_delay=200), the
  mission4 artifact (drone survives 6 latches over 220 s, completes
  X_LEG_1→X_LEG_2→X_LEG_3→TO_CENTER), and four new sharp edges.
- HITL_SIM_HANDOFF.md: rewrote § 2 (ARM_SWITCH latch) marking it
  FIXED, with the full root-cause and recovery-flow explanation.
- TODO.md (new): ranked open items. #1 is the recurring ~18 s
  RX-stall under sustained load — recovery handles it but it costs
  leg accuracy. Recommended diagnostic next step is per-tick wall
  delta logging in shim's _consume_requests.
- mission{2,3,4}.png/.csv: in-repo artifacts of the flight progression
  (mission2/3 = pre-fix latch fatal, mission4 = post-fix recoverable).
gasantiago16 pushed a commit that referenced this pull request Apr 28, 2026
Gabriel Santiago
sitl: v0.8 — mission_demo dual-write kills chronic RX_FAILSAFE
82242ca 
TODO #1(a) from yesterday's wrap-up. mission_demo now sends the same
RC values to both MSP TCP 5761 AND UDP 9004 every tick. BF's RX
state machine treats the UDP path as "real RX", clearing the chronic
arming-disable bit-2 set that drove the original 25-30 s ARM_SWITCH
latch cycle.

Empirical: mission7.png + mission7_md.log show 0 RX_FAILSAFE hits in
240 s of [rc] logs vs mission6's 42 hits. Single-source-of-truth
design — both paths read the same `rc[]` from compute_rc, no
precedence fight.

The drone's leg-arrival behavior is still the same as mission6
because a SECOND failsafe trigger surfaced once we silenced bit-2:
bit-1 FAILSAFE pulses on a ~10 s cycle without bit-2 ever appearing.
Recovery flow handles each. Source unknown — queued as TODO.md item
2 with a concrete diagnostic recipe.

Files:
- integrations/tools/mission_demo.py: +socket, +struct, _UDP_RC_PACKET
  pin (<d16H, 40 B asserted), --udp-rc-host/--udp-rc-port/--no-udp-rc
  CLI args, connect()-pattern socket setup, dual-send block right
  after fc.send_overrides(rc), error counter with gated logging,
  socket close in finally. Default ON. Pre-existing duplicate `rel
  =` line cleaned up while we were there.
- integrations/tests/test_mission_demo.py: docstring "(bit 7)"→
  "(bit 3)" fix. Constant was already right; just a stale comment.
- MEMORY.md: v0.8 snapshot at top with 0-vs-42 measurement, new
  sharp edge ("two bit-1 FAILSAFE triggers exist"), wire-format-
  pinned-in-three-places warning.
- TODO.md: closed item 1 (chronic RX_FAILSAFE), opened item 2
  (residual bit-1 FAILSAFE). Renumbered remaining items.

Counter-agent reviewed twice — once mid-session on the patch alone,
once on the consolidated diff. Findings applied or queued.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@gasantiago16