diff --git a/config.ini b/config.ini index e96c0fef..5d5d2a6d 100644 --- a/config.ini +++ b/config.ini @@ -293,6 +293,32 @@ autonomous.platform_role = ground_isr autonomous.safe_mode = HOLD autonomous.default_features = detect,mavlink,tak_output,logging +[vehicle.ugv_strike] +; Vehicle profiles do NOT inherit from each other — bootstrap.py reads only +; the one [vehicle.] section named by HYDRA_VEHICLE. reserved_channels +; is a vehicle-LOCAL key (no section prefix); omitting it here would skip the +; servo/light-bar channel-conflict check entirely. 1,3 are the truck's +; steering and throttle. +reserved_channels = 1,3 +autonomous.post_drop_mode = SMART_RTL +autonomous.default_features = detect,mavlink,tak_output,logging,autonomy,servo +autonomous.enabled = true +autonomous.geofence_lat = 35.15 +autonomous.geofence_lon = -79.52 +autonomous.geofence_radius_m = 25000 +autonomous.allowed_classes = truck, car +autonomous.allowed_vehicle_modes = GUIDED +autonomous.require_operator_lock = true +autonomous.arm_channel = 0 +autonomous.hardware_arm_channel = 0 +autonomous.post_strike_mode = HOLD +autonomous.safe_mode = HOLD +autonomous.platform_role = ground_isr +servo_tracking.enabled = true +approach.strike_approach_m = 0.5 +approach.abort_mode = HOLD +approach.waypoint_interval = 0.1 + [vehicle.drone_10in] reserved_channels = 1,2,3,4 autonomous.post_drop_mode = DOGLEG_RTL diff --git a/docs/adversarial/317.md b/docs/adversarial/317.md new file mode 100644 index 00000000..f9616511 --- /dev/null +++ b/docs/adversarial/317.md @@ -0,0 +1,129 @@ +# Adversarial review — PR #317 (ugv_strike vehicle profile) + +Touched adversarial path: `hydra_detect/pipeline/facade.py`. + +Reviewer: Claude (orchestrator). Implementation drafted by Codex (gpt-5.6-sol), +reviewed and corrected here. Codex could not execute Python in its environment, +so **every test result in this report was produced by the reviewer**, not the +implementer. All red-green claims below were observed directly. + +## Round 1 — behavior change in the autonomous strike path + +The PR rewires which callback `AutonomousController.evaluate()` receives. That +is the callback that fires a real engagement, so the review started there. + +**Finding 1.1 (CONFIRMED, fixed): strike re-entry produces false failure records.** + +`autonomous.py` updates `_last_strike_time` only when `strike_cb` returns +truthy (line ~482). The cooldown gate (line ~241) is the only re-entry control. +`_handle_approach_strike_command` returned `False` whenever +`approach.mode != IDLE`. + +Sequence on the new profile: +1. Gates pass, `start_strike()` succeeds, returns True, cooldown starts (30 s). +2. Cooldown correctly suppresses for 30 s. +3. Cooldown expires while the approach is *still* in `STRIKE` — normal for a + moving truck. Every qualifying frame now calls `strike_cb`, gets `False`, + never refreshes `_last_strike_time`, and emits + `AUTONOMOUS STRIKE FAILED` plus a reject decision record. + +Impact: the audit log fills with strike failures during a *working* strike. +Anyone reading the audit trail afterward would conclude the engagement failed +repeatedly when it succeeded once and stayed active. + +Fix: re-striking the track already under an active approach returns True +without restarting. A different track still returns False — that refusal is +load-bearing and was preserved. + +Verified red: `test_same_active_strike_after_cooldown_is_not_callback_failure` +fails against pre-fix code with the predicted +`AUTONOMOUS STRIKE FAILED: track_id=7 (strike_cb returned False)` in the log. + +## Round 2 — what the profile silently fails to inherit + +**Finding 2.1 (CONFIRMED, fixed): `reserved_channels` was dropped.** + +`bootstrap.py` resolves `f"vehicle.{vehicle}"` and reads **only** that section. +Vehicle profiles do not inherit. `reserved_channels` is a vehicle-local key +(`_VEHICLE_LOCAL_KEYS`), read in `facade.py` with `fallback=""`. + +Absence does not fail loudly — it skips the entire servo and light-bar +channel-conflict block. That block is what prints +`SAFETY: ... conflicts with reserved channels — servo tracking DISABLED` and +sets `_servo_tracker = None`. + +`[vehicle.ugv]` declares `1,3`. The new profile omitted them while also setting +`servo_tracking.enabled = true`. On the truck, 1 is steering and 3 is throttle. + +No live collision today (pan 10, strike 11), so this is a latent hole rather +than an active fault: the guard that would catch a future channel edit was not +running on this profile. Fixed by declaring `reserved_channels` explicitly, +pinned by a test that asserts parity with `[vehicle.ugv]`. + +Verified red: commenting the line out fails +`test_ugv_strike_reserves_the_same_channels_as_ugv`. + +**Finding 2.2 (considered, not a defect): `strike_approach_m = 0.5`.** +Schema minimum, closes to contact. Intended for this profile — the platform is +meant to physically reach the object. No payload and no arm channel is wired +(`arm_channel = 0`), so `start_strike()` skips its software-arm servo write. + +## Round 3 — the new selector as a failure surface + +`_autonomous_strike_handler()` is invoked inside the detection loop on every +frame. Anything that can raise there stops detection. + +**Finding 3.1 (CONFIRMED, fixed): loop-killing `AttributeError`.** + +The drafted selector read `self._vehicle` directly. `_refuse_approach_for_fw` +in the same file uses `getattr(self, "_vehicle", None)` with a comment stating +that pre-existing fixtures construct `Pipeline` by patching `__init__` and +never set `_vehicle`. The new code did not honor that established contract, +in a hotter code path than the one that documented it. + +Fixed with the same idiom; missing profile falls back to the one-shot handler, +which is pre-existing behavior for every other profile. + +Verified red: `test_strike_handler_survives_pipeline_without_vehicle_attr` +raises `AttributeError` against the naive version. + +**Finding 3.2 (CONFIRMED, fixed): case-sensitive profile match.** + +`_is_fw_profile` normalizes with `.strip().lower()`. The drafted selector used +`==` against a literal. `HYDRA_VEHICLE=UGV_STRIKE` would silently select the +one-shot handler — a quiet downgrade of the engagement path, not an error. +Now normalized through `_is_ugv_strike_profile`, matching the file's contract. + +Verified red: `test_strike_handler_profile_match_is_case_insensitive`. + +**Finding 3.3 (accepted, not fixed): TOCTOU between two property reads.** + +`_handle_approach_strike_command` reads `self._approach.mode` and +`self._approach.target_track_id` under two separate lock acquisitions. The mode +can change between them. Both outcomes are benign: either the idempotent True +(approach just completed, cooldown refreshes, next frame re-evaluates) or fall +through to the existing `!= IDLE` refusal. Fixing it would mean a combined +locked accessor on `ApproachController`, widening that class's public surface +for no behavioral gain. Documented rather than changed. + +## Pre-existing issue surfaced, deliberately not fixed here + +`autonomous.notify_strike_complete()` (autonomous.py:565) clears +`_strike_in_progress`. Repository-wide search finds **no callers**, and +`_strike_in_progress` is never read as a gate — it is set at line 478 and +otherwise write-only. The cooldown is the only real re-entry control, which is +precisely why finding 1.1 had teeth. Wiring a completion signal is a larger +change to shared autonomous logic affecting every profile; out of scope for a +profile addition. Should be filed. + +## Verification summary + +- `2654 passed`, flake8 clean on `hydra_detect/ tests/`. +- 19 failures / 10 errors in `test_ota_verify_pull.py` and + `test_tak_pytak_emitter.py` are environment-only (missing `pytak` on the + Windows dev box) and reproduce with identical counts on a stashed clean tree. +- Two tests in `TestUGVStrikePipeline` are `fcntl`-gated and skip on Windows; + they execute in Linux CI only and were not observed locally. Flagged rather + than claimed. +- `_eval_drop` and `_eval_rf_hunt` unmodified; drop and RF hunt remain blocked + on #147. diff --git a/docs/setup/ugv-strike-profile.md b/docs/setup/ugv-strike-profile.md new file mode 100644 index 00000000..436696f2 --- /dev/null +++ b/docs/setup/ugv-strike-profile.md @@ -0,0 +1,64 @@ +# UGV strike profile + +## Select the profile + +On each kit, set `/etc/hydra/vehicle.env` to: + +```sh +HYDRA_VEHICLE=ugv_strike +``` + +Then reload the service: + +```sh +sudo systemctl restart hydra-detect +``` + +The profile enables autonomous evaluation, servo tracking, truck/car target +selection, GUIDED operation, operator-selected target locking, a 25 km circular +geofence centered at 35.15, -79.52, and continuous strike approach updates at +0.1-second intervals to a 0.5 m close-approach estimate. Arm channels remain +disabled. Post-strike, abort, and autonomous fallback modes are HOLD. + +## Profiles do not inherit + +`hydra_detect/pipeline/bootstrap.py` reads only the single `[vehicle.]` +section named by `HYDRA_VEHICLE`. `[vehicle.ugv_strike]` gets nothing from +`[vehicle.ugv]`. Any key you need must be restated in full. + +This matters most for `reserved_channels`, which is a vehicle-local key with no +section prefix. `facade.py` reads it with `fallback=""`, so a missing value +skips the servo and light-bar channel-conflict check rather than failing +loudly. On the truck, channels 1 and 3 are steering and throttle. + +## Capability preconditions + +- **Follow** checks MAVLink connectivity, GPS fix type 3 or higher, and a locked + track in `hydra_detect/capability_status.py::_eval_follow`. +- **Servo Tracking** checks that a servo controller has claimed its channel. A + claimed controller with no locked track reports WARN; a claimed controller + with a locked track reports READY. The check is in + `hydra_detect/capability_status.py::_eval_servo_tracking`. +- **Autonomy Live** checks MAVLink connectivity, GPS fix type 3 or higher, a + valid configured geofence, `autonomous.enabled=true`, and operating mode + `ARMED`. The check is in + `hydra_detect/capability_status.py::_eval_autonomy_live`. + +## Strike paths + +`hydra_detect/pipeline/facade.py::_handle_strike_command` sends one GUIDED +waypoint at the estimated target GPS position. Other profiles retain this +one-shot autonomous path. + +`hydra_detect/pipeline/facade.py::_handle_approach_strike_command` calls +`approach.start_strike()` and enters `ApproachMode.STRIKE`. That path updates a +GUIDED waypoint continuously from the current locked track and commands twice +the configured follow maximum speed. Use the continuous path for a moving +target. + +For `ugv_strike`, the autonomous controller is wired to the continuous handler +by `hydra_detect/pipeline/facade.py::_autonomous_strike_handler`. + +Payload drop and RF hunt remain unconditionally BLOCKED in +`hydra_detect/capability_status.py::_eval_drop` and `_eval_rf_hunt` on issue +#147. diff --git a/hydra_detect/pipeline/facade.py b/hydra_detect/pipeline/facade.py index ba142051..ef69dc88 100644 --- a/hydra_detect/pipeline/facade.py +++ b/hydra_detect/pipeline/facade.py @@ -128,6 +128,11 @@ def _is_fw_profile(vehicle_type: str | None) -> bool: return (vehicle_type or "").strip().lower() == "fw" +def _is_ugv_strike_profile(vehicle_type: str | None) -> bool: + """Return True if the active vehicle profile is the UGV strike profile.""" + return (vehicle_type or "").strip().lower() == "ugv_strike" + + class Pipeline: """Top-level orchestrator that ties all modules together.""" @@ -1753,7 +1758,8 @@ def _run_loop(self) -> None: if self._autonomous is not None and self._mavlink is not None: self._autonomous.evaluate( track_result, self._mavlink, - self._handle_target_lock, self._handle_strike_command, + self._handle_target_lock, + self._autonomous_strike_handler(), ) # Update approach controller with locked track @@ -2139,6 +2145,20 @@ def _handle_strike_command(self, track_id: int) -> bool: self._servo_tracker.fire_strike() return success + def _autonomous_strike_handler(self): + """Select the profile-specific autonomous strike implementation. + + Uses ``getattr`` for the same reason as ``_refuse_approach_for_fw``: + pre-existing test fixtures construct Pipeline by patching __init__ + and never set ``_vehicle``. This runs inside the detection loop, so a + bare attribute access would turn that into a loop-killing + AttributeError. Missing profile falls back to the one-shot handler, + which is the pre-existing behavior for every other profile. + """ + if _is_ugv_strike_profile(getattr(self, "_vehicle", None)): + return self._handle_approach_strike_command + return self._handle_strike_command + def _get_preflight(self) -> dict: """Run pre-flight checks and return structured results.""" import os @@ -2437,9 +2457,15 @@ def _handle_approach_strike_command(self, track_id: int) -> bool: if self._approach is None: logger.warning("Approach strike failed: approach controller not available") return False - if self._approach.mode != ApproachMode.IDLE: + approach_mode = self._approach.mode + if ( + approach_mode == ApproachMode.STRIKE + and self._approach.target_track_id == track_id + ): + return True + if approach_mode != ApproachMode.IDLE: logger.warning("Approach strike failed: approach already active in %s mode", - self._approach.mode.value) + approach_mode.value) return False with self._state_lock: diff --git a/tests/test_autonomous.py b/tests/test_autonomous.py index 6d23c3ef..64e41ae1 100644 --- a/tests/test_autonomous.py +++ b/tests/test_autonomous.py @@ -13,6 +13,8 @@ parse_polygon, point_in_polygon, ) +from hydra_detect.approach import ApproachMode +from hydra_detect.pipeline.facade import Pipeline from hydra_detect.tracker import TrackedObject, TrackingResult @@ -282,6 +284,32 @@ def test_cooldown_enforced(self): assert strike_cb.call_count == 1 # Still just the first + def test_same_active_strike_after_cooldown_is_not_callback_failure(self): + ctrl = _make_controller(min_track_frames=1, strike_cooldown_sec=30.0) + tracks = _make_tracks((7, "mine", 0.92)) + pipeline = Pipeline.__new__(Pipeline) + pipeline._vehicle = "ugv_strike" + pipeline._mavlink = MagicMock() + pipeline._approach = MagicMock() + pipeline._approach.mode = ApproachMode.STRIKE + pipeline._approach.target_track_id = 7 + + fake_time = [1000.0] + with pytest.MonkeyPatch.context() as mp: + mp.setattr(time, "monotonic", lambda: fake_time[0]) + mav = _make_mavlink() + strike_cb = pipeline._handle_approach_strike_command + + ctrl.evaluate(tracks, mav, MagicMock(return_value=True), strike_cb) + fake_time[0] = 1031.0 + ctrl.evaluate(tracks, mav, MagicMock(return_value=True), strike_cb) + + decisions = ctrl.get_dashboard_snapshot()["log"] + assert not any( + item["reason"] == "strike callback failed" for item in decisions + ) + pipeline._approach.start_strike.assert_not_called() + def test_no_gps_fix(self): ctrl = _make_controller() mav = _make_mavlink() diff --git a/tests/test_ugv_strike_profile.py b/tests/test_ugv_strike_profile.py new file mode 100644 index 00000000..76915f5f --- /dev/null +++ b/tests/test_ugv_strike_profile.py @@ -0,0 +1,203 @@ +"""Regression coverage for the committed UGV strike vehicle profile.""" + +from __future__ import annotations + +import configparser +import os +import sys +from pathlib import Path +from unittest.mock import MagicMock + +import pytest + +from hydra_detect.approach import ApproachMode +from hydra_detect.capability_status import ( + CapabilityStatus, + SystemState, + _eval_autonomy_live, + _eval_follow, +) +from hydra_detect.config_schema import validate_config +from hydra_detect.pipeline.bootstrap import PipelineBootstrap + + +REPO_ROOT = Path(__file__).resolve().parents[1] +CONFIG_PATH = REPO_ROOT / "config.ini" + +_skip_no_fcntl = pytest.mark.skipif( + sys.platform == "win32", + reason="hydra_detect.web.config_api requires fcntl (Linux-only)", +) + + +def _read_config() -> configparser.ConfigParser: + cfg = configparser.ConfigParser(inline_comment_prefixes=(";", "#")) + cfg.read(CONFIG_PATH) + return cfg + + +def test_ugv_strike_profile_passes_schema_validation(): + result = validate_config(_read_config()) + profile_messages = [ + message for message in (*result.errors, *result.warnings) + if "vehicle.ugv_strike" in message + ] + assert profile_messages == [] + + +def test_ugv_strike_profile_resolves_dotted_overrides(monkeypatch): + monkeypatch.setenv("HYDRA_VEHICLE", "ugv_strike") + selected = os.environ["HYDRA_VEHICLE"] + ctx = PipelineBootstrap().load_config(str(CONFIG_PATH), vehicle=selected) + + assert ctx.cfg.getboolean("autonomous", "enabled") is True + assert ctx.cfg.getfloat("autonomous", "geofence_lat") == 35.15 + assert ctx.cfg.getfloat("autonomous", "geofence_lon") == -79.52 + assert ctx.cfg.getfloat("autonomous", "geofence_radius_m") == 25000 + assert ctx.cfg.get("autonomous", "allowed_classes") == "truck, car" + assert ctx.cfg.get("autonomous", "allowed_vehicle_modes") == "GUIDED" + assert ctx.cfg.getboolean("autonomous", "require_operator_lock") is True + assert ctx.cfg.getint("autonomous", "arm_channel") == 0 + assert ctx.cfg.getint("autonomous", "hardware_arm_channel") == 0 + assert ctx.cfg.get("autonomous", "post_strike_mode") == "HOLD" + assert ctx.cfg.get("autonomous", "safe_mode") == "HOLD" + assert ctx.cfg.get("autonomous", "platform_role") == "ground_isr" + assert ctx.cfg.getboolean("servo_tracking", "enabled") is True + assert ctx.cfg.getfloat("approach", "strike_approach_m") == 0.5 + assert ctx.cfg.get("approach", "abort_mode") == "HOLD" + assert ctx.cfg.getfloat("approach", "waypoint_interval") == 0.1 + + +def test_ugv_strike_reserves_the_same_channels_as_ugv(): + """reserved_channels must be declared explicitly on this profile. + + Vehicle profiles do not inherit: bootstrap.py reads only the single + [vehicle.] section. reserved_channels is a vehicle-LOCAL key, and + facade.py reads it with fallback="" — so if it is missing, the servo and + light-bar channel-conflict check is skipped entirely rather than failing + loudly. On the truck, 1 and 3 are steering and throttle. + """ + cfg = _read_config() + strike_reserved = cfg.get("vehicle.ugv_strike", "reserved_channels", fallback="") + assert strike_reserved.strip(), ( + "ugv_strike must declare reserved_channels; absence silently disables " + "the servo/light-bar channel-conflict guard" + ) + ugv_reserved = cfg.get("vehicle.ugv", "reserved_channels", fallback="") + assert {c.strip() for c in strike_reserved.split(",")} == { + c.strip() for c in ugv_reserved.split(",") + }, "ugv_strike must reserve the same airframe channels as ugv" + + +def test_autonomy_live_profile_state_gates_on_armed_and_geofence(): + state = SystemState( + mavlink_connected=True, + gps_fix=3, + autonomy_enabled=True, + autonomy_geofence_present=True, + operating_mode="ARMED", + ) + assert _eval_autonomy_live(state).status == CapabilityStatus.READY + + state.operating_mode = "OBSERVE" + assert _eval_autonomy_live(state).status == CapabilityStatus.BLOCKED + + state.operating_mode = "ARMED" + state.autonomy_geofence_present = False + assert _eval_autonomy_live(state).status == CapabilityStatus.BLOCKED + + +def test_follow_ready_with_mavlink_3d_gps_and_locked_track(): + state = SystemState( + mavlink_connected=True, + gps_fix=3, + servo_locked_track_id=42, + ) + assert _eval_follow(state).status == CapabilityStatus.READY + + +def test_ugv_strike_autonomy_uses_continuous_approach_handler(): + from hydra_detect.pipeline.facade import Pipeline + + pipeline = Pipeline.__new__(Pipeline) + pipeline._vehicle = "ugv_strike" + pipeline._handle_approach_strike_command = MagicMock() + pipeline._handle_strike_command = MagicMock() + + selected = pipeline._autonomous_strike_handler() + assert selected is pipeline._handle_approach_strike_command + + +def test_strike_handler_survives_pipeline_without_vehicle_attr(): + """_autonomous_strike_handler runs in the detection loop every frame. + + Pre-existing fixtures (tests/test_pipeline_callbacks.py) build Pipeline by + patching __init__ and never set _vehicle. A bare attribute access would + raise AttributeError inside the loop and kill detection. Mirrors the + defensive getattr in _refuse_approach_for_fw. + """ + from hydra_detect.pipeline.facade import Pipeline + + pipeline = Pipeline.__new__(Pipeline) + pipeline._handle_approach_strike_command = MagicMock() + pipeline._handle_strike_command = MagicMock() + + selected = pipeline._autonomous_strike_handler() + assert selected is pipeline._handle_strike_command + + +def test_strike_handler_profile_match_is_case_insensitive(): + """HYDRA_VEHICLE casing must not silently downgrade the strike path. + + _is_fw_profile normalizes case; this selector must match that contract or + 'UGV_STRIKE' quietly falls back to the one-shot handler. + """ + from hydra_detect.pipeline.facade import Pipeline + + for name in ("UGV_STRIKE", " ugv_strike ", "Ugv_Strike"): + pipeline = Pipeline.__new__(Pipeline) + pipeline._vehicle = name + pipeline._handle_approach_strike_command = MagicMock() + pipeline._handle_strike_command = MagicMock() + + selected = pipeline._autonomous_strike_handler() + assert selected is pipeline._handle_approach_strike_command, name + + +@_skip_no_fcntl +class TestUGVStrikePipeline: + @staticmethod + def _make_pipeline(vehicle): + """Build the minimal Pipeline state needed by strike idempotency.""" + from hydra_detect.pipeline import Pipeline + + pipeline = Pipeline.__new__(Pipeline) + pipeline._vehicle = vehicle + pipeline._mavlink = None + pipeline._approach = MagicMock() + return pipeline + + def test_same_track_strike_is_idempotent(self): + p = self._make_pipeline("ugv_strike") + p._approach.mode = ApproachMode.STRIKE + p._approach.target_track_id = 7 + + assert p._handle_approach_strike_command(7) is True + p._approach.start_strike.assert_not_called() + + def test_different_track_strike_remains_refused(self): + p = self._make_pipeline("ugv_strike") + p._approach.mode = ApproachMode.STRIKE + p._approach.target_track_id = 7 + + assert p._handle_approach_strike_command(8) is False + p._approach.start_strike.assert_not_called() + + +def test_headless_gcs_statustext_alert_defaults_remain_enabled(): + cfg = _read_config() + assert cfg.getboolean("mavlink", "alert_statustext") is True + alert_classes = { + value.strip() for value in cfg.get("mavlink", "alert_classes").split(",") + } + assert "truck" in alert_classes