From 1aab8ebc2e14a1cb7760b008c9d674a9fc4fc62f Mon Sep 17 00:00:00 2001 From: OminousPanda110 Date: Fri, 14 Aug 2026 15:29:31 -0700 Subject: [PATCH 1/5] feat(ops243): add internal hardware trigger --- README.md | 6 +- docs/CHANGELOG.md | 6 + docs/hardware-trigger.md | 69 ++++++++ docs/rolling_buffer_spin_detection.md | 9 +- scripts/start-kiosk.sh | 25 ++- src/openflight/ops243.py | 207 +++++++++++++++++++++- src/openflight/rolling_buffer/__init__.py | 2 + src/openflight/rolling_buffer/monitor.py | 24 ++- src/openflight/rolling_buffer/trigger.py | 125 ++++++++++++- src/openflight/server.py | 53 +++++- tests/test_ops243.py | 147 +++++++++++++++ tests/test_rolling_buffer.py | 117 ++++++++++++ tests/test_server.py | 123 +++++++++++++ tests/test_start_kiosk.py | 43 +++++ 14 files changed, 937 insertions(+), 19 deletions(-) create mode 100644 docs/hardware-trigger.md diff --git a/README.md b/README.md index 71bec9afb..88cbfb040 100644 --- a/README.md +++ b/README.md @@ -60,7 +60,7 @@ See the **[Parts List](docs/PARTS.md)** for everything you need with purchase li ### 2. Wire it up -Follow the **[Sound Trigger Wiring Guide](docs/sound-trigger-wiring.md)** to connect the SEN-14262 to the OPS243-A. The (deprecated) K-LD7 modules connect via USB — no wiring needed. +Follow the **[Sound Trigger Wiring Guide](docs/sound-trigger-wiring.md)** to connect the SEN-14262 to the OPS243-A. The (deprecated) K-LD7 modules connect via USB — no wiring needed. The OPS243 also has an opt-in internal speed-trigger path; see the **[Internal Hardware Trigger Guide](docs/hardware-trigger.md)**. **Adding the IWR6843 angle radar?** The Pi cannot power both radars over USB, so the OPS243 moves to the Pi's GPIO UART header while the TI board takes the USB @@ -96,6 +96,9 @@ details and troubleshooting. # Default: rolling buffer mode with sound trigger scripts/start-kiosk.sh +# Opt-in OPS243 internal hardware trigger (30 ksps, S#6) +scripts/start-kiosk.sh --trigger hardware + # With the IWR6843 angle radar (OPS243 on the Pi GPIO UART). # Geometry values are examples — measure your own; see the operator guide. scripts/start-kiosk.sh --iwr6843 \ @@ -302,6 +305,7 @@ uv run pytest tests/ -v - **[Parts List](docs/PARTS.md)** — What to buy - **[Sound Trigger Wiring](docs/sound-trigger-wiring.md)** — How to wire the sound trigger +- **[Internal Hardware Trigger](docs/hardware-trigger.md)** — OPS243 internal trigger methodology and Pi retest checklist - **[Raspberry Pi Setup](docs/raspberry-pi-setup.md)** — Full setup guide - **[IWR6843 Operator Guide](docs/iwr6843/README.md)** — Wire, flash, mount, aim, and calibrate the angle radar - **[LIS3DH Inclinometer Setup](docs/inclinometer/README.md)**: Add enclosure-level compensation to IWR6843 tilt diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index ba2f60f62..04301f764 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -8,6 +8,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Added +- **Opt-in OPS243 internal hardware trigger.** `--trigger hardware` configures + the radar's internal speed trigger with 25 mph, magnitude 40, and the tested + `S#6` split at 30 ksps. Captures are parsed and the radar is re-armed with + `GC` after each dump. The existing sound-trigger default and + `--sound-pre-trigger` behavior are unchanged. See + [OPS243 Internal Hardware Trigger](hardware-trigger.md). - **System Prerequisites:** Documented missing binary dependencies (`swig`, `liblgpio-dev`, `python3-dev`) required prior to executing `./scripts/setup/setup.sh`. - **Environment Reload Guidance:** Added instructions for reloading terminal environment variables (`source ~/.bashrc`) when installed dependencies or scripts (`setup.sh`, `start-kiosk.sh`) are not recognized in the current terminal session. - **OPS243 over the Raspberry Pi GPIO UART.** The radar can now run on the J3 diff --git a/docs/hardware-trigger.md b/docs/hardware-trigger.md new file mode 100644 index 000000000..63fcb1bab --- /dev/null +++ b/docs/hardware-trigger.md @@ -0,0 +1,69 @@ +# OPS243 Internal Hardware Trigger + +OpenFlight’s `hardware` trigger mode lets the OPS243-A decide when a rolling-buffer capture starts. It is opt-in; the kiosk default remains the existing SEN-14262 sound trigger. + +## Methodology + +The host configures the radar once and then waits for the radar’s completed rolling-buffer dump. The host does not poll speed reports or send `S!` for each shot: + +1. Put the radar in idle mode with `PI`. +2. Arm the outbound internal speed threshold with `ST-n` (25 mph by default; + outbound radar velocity is negative). +3. Enter rolling-buffer mode with `GC`. +4. Restore the detector settings that `GC` resets: 30 ksps, MPH units, 128 samples, `X=2`, outbound filtering, JSON plus magnitude output, and the configured `S#n` split. +5. Restore `ST-n` and the magnitude gate `SMn`, then allow the 4,096-sample history to fill. +6. Wait for the board-triggered dump, parse the I/Q payload, and reject it if it has no outbound ball-speed reading at or above 35 mph. +7. Re-arm with `GC`, restore the cached settings, and wait for the buffer to fill again. A serial write timeout keeps the capture and reports a retryable re-arm failure instead of discarding the shot. + +The implementation intentionally excludes the optional `SZ` board-processing stream, SigMF/replay tooling, clock synchronization, and UI changes. Those are separate concerns and are not required for this focused trigger path. + +## Defaults and command + +| Setting | Hardware-mode default | +|---|---:| +| Trigger threshold | 25 mph | +| Trigger magnitude | 40 (`SM40`) | +| Pre-trigger split | 6 segments (`S#6`) | +| Sample rate | 30 ksps (required) | +| Minimum accepted outbound ball speed | 35 mph | + +Run the mode directly with: + +```bash +openflight-server \ + --trigger hardware \ + --trigger-threshold 25 \ + --trigger-magnitude 40 \ + --pre-trigger-segments 6 \ + --sample-rate 30 +``` + +The kiosk script forwards the same settings: + +```bash +scripts/start-kiosk.sh --trigger hardware +``` + +Use `--trigger-threshold`, `--trigger-magnitude`, and `--pre-trigger-segments` to override the hardware path. `S#6` applies only to this new mode. The established sound path continues to use `--sound-pre-trigger` and keeps its existing default and behavior: + +```bash +scripts/start-kiosk.sh --trigger sound --sound-pre-trigger 16 +``` + +The internal trigger does not depend on the SEN-14262 sound edge. Existing sound-trigger wiring can remain installed, but selecting `hardware` is the software choice that activates the OPS243 internal trigger. + +## Raspberry Pi retest checklist + +Before treating a PR as ready for review, run the hardware path on the target Pi and record the observations in the PR body: + +```bash +scripts/start-kiosk.sh --trigger hardware --radar-port /dev/ttyAMA0 +``` + +Record representative shots, including slow and fast swings; deliberate noise or nearby-impact false triggers; observed trigger-to-capture latency; whether each accepted capture produced a shot; and the observed `S#6` pre/post split. Also run the unchanged sound path with the same representative shots: + +```bash +scripts/start-kiosk.sh --trigger sound +``` + +The hardware test is incomplete until both modes are checked on the Pi. The Mac development environment can validate command ordering, parsing, re-arm recovery, and CLI forwarding, but it cannot verify the OPS243 electrical and firmware behavior. diff --git a/docs/rolling_buffer_spin_detection.md b/docs/rolling_buffer_spin_detection.md index 00e1840ab..6d0fac828 100644 --- a/docs/rolling_buffer_spin_detection.md +++ b/docs/rolling_buffer_spin_detection.md @@ -11,7 +11,7 @@ estimation from that capture remains experimental. 4. An overlapping timeline separates the club, impact, and ball regions. 5. An ungated multitaper estimator records an experimental spin candidate. -The normal kiosk command uses this pipeline: +The normal kiosk command uses this sound-trigger pipeline: ```bash scripts/start-kiosk.sh @@ -32,6 +32,12 @@ The runtime then starts in the persisted `GC` rolling-buffer mode without re-entering it. See [Sound Trigger Wiring](sound-trigger-wiring.md) for the recommended direct hardware trigger. +The OPS243 internal speed trigger is available as a separate, opt-in capture +strategy. It uses the tested 30 ksps configuration and defaults to `S#6`; see +the [Internal Hardware Trigger guide](hardware-trigger.md) for its command +ordering, re-arm behavior, and Raspberry Pi validation checklist. The sound +path and its defaults remain unchanged. + ## Current defaults | Setting | Value | @@ -75,4 +81,5 @@ in the session JSONL logs described in the - [Raspberry Pi Setup](raspberry-pi-setup.md) - [Sound Trigger Wiring](sound-trigger-wiring.md) +- [OPS243 Internal Hardware Trigger](hardware-trigger.md) - [Spin Replay and Diagnostics](spin-dechirp-replay.md) diff --git a/scripts/start-kiosk.sh b/scripts/start-kiosk.sh index 9766e0220..8aa469006 100755 --- a/scripts/start-kiosk.sh +++ b/scripts/start-kiosk.sh @@ -23,6 +23,10 @@ DRY_RUN=false # Rolling buffer mode is the only mode (streaming mode removed) TRIGGER="sound" # Default: hardware sound trigger (SEN-14262 → HOST_INT) SOUND_PRE_TRIGGER="" +TRIGGER_THRESHOLD=25 +TRIGGER_THRESHOLD_SET=false +TRIGGER_MAGNITUDE=40 +PRE_TRIGGER_SEGMENTS=6 BUFFER_SPLIT="" IWR6843=false IWR6843_PORT="" @@ -133,6 +137,19 @@ while [[ $# -gt 0 ]]; do TRIGGER="$2" shift 2 ;; + --trigger-threshold|--speed-trigger-threshold) + TRIGGER_THRESHOLD="$2" + TRIGGER_THRESHOLD_SET=true + shift 2 + ;; + --trigger-magnitude) + TRIGGER_MAGNITUDE="$2" + shift 2 + ;; + --pre-trigger-segments) + PRE_TRIGGER_SEGMENTS="$2" + shift 2 + ;; --sound-pre-trigger) SOUND_PRE_TRIGGER="$2" shift 2 @@ -587,7 +604,13 @@ if [ -n "$TRIGGER" ] && [ "$SWING_SPEED" != true ]; then SERVER_CMD="$SERVER_CMD --trigger $TRIGGER" fi -if [ -n "$SOUND_PRE_TRIGGER" ] && [ "$SWING_SPEED" != true ]; then +if [ "$TRIGGER" = "hardware" ] && [ "$SWING_SPEED" != true ]; then + SERVER_CMD="$SERVER_CMD --trigger-threshold $TRIGGER_THRESHOLD" + SERVER_CMD="$SERVER_CMD --trigger-magnitude $TRIGGER_MAGNITUDE" + SERVER_CMD="$SERVER_CMD --pre-trigger-segments $PRE_TRIGGER_SEGMENTS" +elif [ "$TRIGGER" = "speed" ] && [ "$TRIGGER_THRESHOLD_SET" = true ] && [ "$SWING_SPEED" != true ]; then + SERVER_CMD="$SERVER_CMD --trigger-threshold $TRIGGER_THRESHOLD" +elif [ -n "$SOUND_PRE_TRIGGER" ] && [ "$SWING_SPEED" != true ]; then SERVER_CMD="$SERVER_CMD --sound-pre-trigger $SOUND_PRE_TRIGGER" fi diff --git a/src/openflight/ops243.py b/src/openflight/ops243.py index 843704b17..697804c24 100644 --- a/src/openflight/ops243.py +++ b/src/openflight/ops243.py @@ -35,6 +35,7 @@ import json import logging +import math import re import threading import time @@ -222,6 +223,8 @@ def __init__( self._json_mode = False self._magnitude_enabled = False self._speed_read_buffer = "" + self._internal_speed_trigger_config = None + self._hardware_trigger_recovery_required = False self.last_hardware_trigger_first_byte_timestamp: Optional[float] = None # Most recent OPS-clock -> host-epoch sync (see read_clock_sync). self.last_clock_sync: Optional[dict] = None @@ -1510,14 +1513,40 @@ def wait_for_hardware_trigger( last_data_time = None bytes_received = 0 self.last_hardware_trigger_first_byte_timestamp = None + recovery_required = bool(getattr(self, "_hardware_trigger_recovery_required", False)) + capture_started = not recovery_required + recovery_buffer = "" + if recovery_required: + logger.info( + "[OPS] Re-arm recovery: discarding trailing output until a fresh capture starts" + ) while time.time() < deadline: waiting = self.serial.in_waiting if waiting: - first_byte_timestamp = time.time() if last_data_time is None else None chunk = self.serial.read(waiting) - response_lines.append(chunk.decode("ascii", errors="ignore")) - bytes_received += len(chunk) + decoded_chunk = chunk.decode("ascii", errors="ignore") + + if recovery_required and not capture_started: + candidate = recovery_buffer + decoded_chunk + capture_start = self._capture_start_index(candidate) + if capture_start is None: + recovery_buffer = candidate[-1024:] + time.sleep(0.01) + continue + decoded_chunk = candidate[capture_start:] + recovery_buffer = "" + capture_started = True + self._hardware_trigger_recovery_required = False + logger.info("[OPS] Re-arm recovery: fresh capture boundary found") + first_byte_timestamp = time.time() + else: + first_byte_timestamp = time.time() if last_data_time is None else None + + if not decoded_chunk: + continue + response_lines.append(decoded_chunk) + bytes_received += len(decoded_chunk) if first_byte_timestamp is not None: last_data_time = first_byte_timestamp self.last_hardware_trigger_first_byte_timestamp = last_data_time @@ -1559,7 +1588,7 @@ def wait_for_hardware_trigger( break time.sleep(0.02) - full_response = "".join(response_lines) if response_lines else "" + full_response = "".join(response_lines) if capture_started else "" if not full_response: logger.info("[OPS] Hardware trigger: no data received within %.0fs", timeout) @@ -1661,6 +1690,176 @@ def rearm_rolling_buffer(self, pre_trigger_segments: int = 16): self.serial.reset_input_buffer() logger.info("[OPS] Rolling buffer re-armed (S#%d)", pre_trigger_segments) + def _drain_rearm_serial(self, quiet_period: float = 0.2): + """Drain any tail bytes before changing the board mode. + + The internal trigger returns a complete I/Q dump without an ``S!`` + command. A few firmware versions can still have a short tail in the + UART buffer when the Q array is complete, so GC must wait for a quiet + gap before it is sent. The wait is bounded by the same dump budget + used by the ordinary re-arm path. + """ + if not self.serial or not self.serial.is_open: + raise ConnectionError("Not connected to radar") + + budget = self.transfer_budget_s(floor=self.REARM_DRAIN_TIMEOUT_S) + started = time.monotonic() + last_data = started + drained = 0 + + while time.monotonic() - started < budget: + waiting = self.serial.in_waiting + if waiting: + drained += len(self.serial.read(waiting)) + last_data = time.monotonic() + elif time.monotonic() - last_data >= quiet_period: + break + time.sleep(0.02) + + if drained: + logger.info("[OPS] Internal-trigger re-arm drained %d trailing bytes", drained) + + @staticmethod + def _format_internal_trigger_threshold(threshold_mph: float) -> str: + """Format the outbound (negative radar velocity) ``ST`` threshold.""" + return f"-{abs(float(threshold_mph)):g}" + + @staticmethod + def _capture_start_index(response: str) -> Optional[int]: + """Find the start of a fresh rolling-buffer dump in UART text.""" + sample_time_index = response.find('"sample_time"') + if sample_time_index < 0: + return None + line_start = response.rfind("\n", 0, sample_time_index) + 1 + object_start = response.find("{", line_start, sample_time_index + 1) + return object_start if object_start >= 0 else None + + def configure_for_internal_speed_trigger( + self, + trigger_threshold_mph: float = 25.0, + pre_trigger_segments: int = 6, + trigger_magnitude: int = 40, + sample_rate_ksps: int = 30, + ): + """Configure the OPS243's internal speed-triggered rolling buffer. + + The board owns the trigger edge in this mode. ``ST`` arms the + internal speed threshold, ``GC`` enters rolling-buffer mode, and the + detector settings are restored after GC because the firmware resets + them when changing modes. This path intentionally does not enable + the optional SZ board-processing stream. + """ + if not self.serial or not self.serial.is_open: + raise ConnectionError("Not connected to radar") + + threshold = float(trigger_threshold_mph) + if not math.isfinite(threshold) or threshold < 0: + raise ValueError("Trigger threshold must be non-negative") + if not isinstance(pre_trigger_segments, int): + raise ValueError("Pre-trigger segments must be an integer") + if not isinstance(trigger_magnitude, int) or not 1 <= trigger_magnitude <= 2000: + raise ValueError("Trigger magnitude must be between 1 and 2000") + if sample_rate_ksps != 30: + raise ValueError("Internal speed trigger requires a 30 ksps sample rate") + + pre_trigger_segments = max(0, min(32, pre_trigger_segments)) + signed_threshold = self._format_internal_trigger_threshold(threshold) + self._speed_read_buffer = "" + self.serial.reset_input_buffer() + self._hardware_trigger_recovery_required = False + + # OPS243 internal-trigger order: idle, arm the threshold, enter GC. + self._send_command("PI") + self.serial.write(f"ST{signed_threshold}\r".encode("ascii")) + self.serial.flush() + time.sleep(0.1) + self._send_command("GC") + + self._internal_speed_trigger_config = ( + threshold, + pre_trigger_segments, + trigger_magnitude, + sample_rate_ksps, + ) + self._restore_internal_speed_trigger_settings() + self.serial.reset_input_buffer() + + # A 4,096-sample buffer takes about 136.5 ms at 30 ksps. + time.sleep(0.3) + logger.info( + "[OPS] Internal speed trigger armed (ST%s, S#%d, S=%d, SM%d)", + signed_threshold, + pre_trigger_segments, + sample_rate_ksps, + trigger_magnitude, + ) + + def _restore_internal_speed_trigger_settings(self): + """Restore detector settings cleared by the last GC command.""" + if self._internal_speed_trigger_config is None: + raise RuntimeError("Internal speed trigger has not been configured") + + threshold, pre_trigger_segments, trigger_magnitude, sample_rate_ksps = ( + self._internal_speed_trigger_config + ) + threshold_text = f"{threshold:g}" + signed_threshold = self._format_internal_trigger_threshold(threshold) + + self.set_sample_rate(sample_rate_ksps * 1000) + self.set_units(SpeedUnit.MPH) + self.set_transmit_power(0) + self.set_buffer_size(128) + self.set_fft_size(2) + self._send_command("R-") + self._send_command(f"R>{threshold_text}") + self.enable_json_output(True) + self.enable_magnitude_report(True) + self._send_command("W0") + self._send_command(f"S#{pre_trigger_segments}") + + # ST/SM require a terminating carriage return and are deliberately + # sent after MPH is restored so the threshold is interpreted in mph. + self.serial.write(f"ST{signed_threshold}\r".encode("ascii")) + self.serial.write(f"SM{trigger_magnitude}\r".encode("ascii")) + self.serial.flush() + time.sleep(0.1) + + def rearm_internal_speed_trigger(self, sample_rate_ksps: int = 30) -> bool: + """Re-arm an internal hardware trigger after a completed dump. + + GC restarts the rolling buffer but also restores firmware defaults, so + the cached detector settings are applied again. A serial write timeout + is recoverable: the capture is valid and the next cycle can retry the + GC sequence. + """ + if not self.serial or not self.serial.is_open: + raise ConnectionError("Not connected to radar") + if sample_rate_ksps != 30: + raise ValueError("Internal speed trigger requires a 30 ksps sample rate") + if self._internal_speed_trigger_config is None: + raise RuntimeError("Internal speed trigger has not been configured") + + try: + self._drain_rearm_serial() + self.serial.reset_input_buffer() + self.serial.write(b"GC") + self.serial.flush() + time.sleep(0.15) + self._restore_internal_speed_trigger_settings() + self.serial.reset_input_buffer() + except serial.SerialTimeoutException as error: + self._hardware_trigger_recovery_required = True + logger.warning( + "[OPS] Internal trigger re-arm timed out; capture retained and " + "re-arm will be retried: %s", + error, + ) + return False + + time.sleep((4096 / (sample_rate_ksps * 1000)) + 0.05) + logger.info("[OPS] Internal speed trigger re-armed (GC)") + return True + def configure_for_rolling_buffer( self, pre_trigger_segments: int = 16, sample_rate_ksps: int = 30 ): diff --git a/src/openflight/rolling_buffer/__init__.py b/src/openflight/rolling_buffer/__init__.py index 0eb374463..54364bfad 100644 --- a/src/openflight/rolling_buffer/__init__.py +++ b/src/openflight/rolling_buffer/__init__.py @@ -27,6 +27,7 @@ ) from .processor import RollingBufferProcessor from .trigger import ( + HardwareTriggeredCapture, ManualTrigger, PollingTrigger, SpeedTriggeredCapture, @@ -57,6 +58,7 @@ "RollingBufferProcessor", # Triggers "TriggerStrategy", + "HardwareTriggeredCapture", "PollingTrigger", "ThresholdTrigger", "ManualTrigger", diff --git a/src/openflight/rolling_buffer/monitor.py b/src/openflight/rolling_buffer/monitor.py index e470e21aa..947156a41 100644 --- a/src/openflight/rolling_buffer/monitor.py +++ b/src/openflight/rolling_buffer/monitor.py @@ -213,11 +213,9 @@ class RollingBufferMonitor: Provides higher temporal resolution (~937 Hz vs ~56 Hz) and optional spin detection. - Recommended configuration uses "speed" trigger (default): - - Fast speed detection mode (~150-200Hz) watches for club swing - - Automatically switches to rolling buffer when speed detected - - Captures ball impact with high temporal resolution - - Per OmniPreSense manufacturer recommendation + Recommended configuration uses the existing "speed" trigger. The opt-in + "hardware" trigger delegates the threshold edge to the OPS243 internal + trigger while preserving the same rolling-buffer processor. Interface matches LaunchMonitor for compatibility with existing code. @@ -252,6 +250,7 @@ def __init__( is nominal. trigger_type: Trigger strategy: - "speed" (default, recommended): Fast speed trigger per manufacturer + - "hardware": OPS243 internal speed trigger - "polling": Continuous capture polling (slower, simpler) - "threshold": Speed threshold trigger - "manual": External trigger for testing @@ -287,9 +286,22 @@ def connect(self) -> bool: """ self.radar.connect() + if self.trigger_type == "hardware": + self.radar.configure_for_internal_speed_trigger( + trigger_threshold_mph=self.trigger.trigger_threshold_mph, + pre_trigger_segments=self.trigger.pre_trigger_segments, + trigger_magnitude=self.trigger.trigger_magnitude, + sample_rate_ksps=self.sample_rate_ksps, + ) + logger.info( + "[MONITOR] Internal hardware trigger configured (threshold %.1f, S#%d, SM%d)", + self.trigger.trigger_threshold_mph, + self.trigger.pre_trigger_segments, + self.trigger.trigger_magnitude, + ) # Speed trigger handles its own configuration (starts in speed mode) # Other triggers need rolling buffer mode configured upfront - if self.trigger_type != "speed": + elif self.trigger_type != "speed": # Get pre_trigger_segments from the trigger if available pre_trigger_segments = getattr(self.trigger, "pre_trigger_segments", 12) if self.trigger_type == "sound": diff --git a/src/openflight/rolling_buffer/trigger.py b/src/openflight/rolling_buffer/trigger.py index ca51b390c..9246624dd 100644 --- a/src/openflight/rolling_buffer/trigger.py +++ b/src/openflight/rolling_buffer/trigger.py @@ -5,6 +5,7 @@ """ import logging +import math import threading import time from abc import ABC, abstractmethod @@ -1142,13 +1143,132 @@ def reset(self): pass # No state to reset +class HardwareTriggeredCapture(TriggerStrategy): + """Capture using the OPS243's internal speed trigger. + + The radar detects the threshold crossing and dumps its rolling buffer over + HOST_INT. The host waits for that dump, parses it, validates that it + contains a ball-speed reading, and then re-arms the board with GC. + """ + + def __init__( + self, + trigger_threshold_mph: float = 25.0, + min_ball_speed_mph: float = 35.0, + pre_trigger_segments: int = 6, + trigger_magnitude: int = 40, + sample_rate_ksps: int = 30, + ): + super().__init__(pre_trigger_segments=pre_trigger_segments) + + threshold = float(trigger_threshold_mph) + if not math.isfinite(threshold) or threshold < 0: + raise ValueError("Trigger threshold must be non-negative") + if not isinstance(pre_trigger_segments, int) or not 0 <= pre_trigger_segments <= 32: + raise ValueError("Pre-trigger segments must be an integer from 0 to 32") + if not isinstance(trigger_magnitude, int) or not 1 <= trigger_magnitude <= 2000: + raise ValueError("Trigger magnitude must be between 1 and 2000") + if sample_rate_ksps != 30: + raise ValueError("Internal speed trigger requires a 30 ksps sample rate") + + self.trigger_threshold_mph = threshold + self.min_ball_speed_mph = float(min_ball_speed_mph) + self.trigger_magnitude = trigger_magnitude + self.sample_rate_ksps = sample_rate_ksps + + def wait_for_trigger( + self, + radar: "OPS243Radar", + processor: RollingBufferProcessor, + timeout: float = 30.0, + ) -> Optional[IQCapture]: + """Wait for one internal-trigger dump and return a valid capture.""" + logger.info( + "[TRIGGER] Waiting for OPS hardware trigger >= %.1f mph (timeout=%.0fs)...", + self.trigger_threshold_mph, + timeout, + ) + + response = radar.wait_for_hardware_trigger(timeout=timeout) + if not response: + logger.info("[TRIGGER] OPS hardware trigger timeout — no dump received") + return None + + response_bytes = len(response) + first_byte_timestamp = getattr( + radar, + "last_hardware_trigger_first_byte_timestamp", + None, + ) + capture = None + parse_error = None + started_at = time.time() + try: + capture = processor.parse_capture( + response, + first_byte_timestamp=first_byte_timestamp, + ) + except Exception as error: # pylint: disable=broad-exception-caught + parse_error = error + logger.warning("[TRIGGER] Hardware capture parse failed: %s", error, exc_info=True) + finally: + try: + rearmed = radar.rearm_internal_speed_trigger(self.sample_rate_ksps) + except Exception as error: # pylint: disable=broad-exception-caught + rearmed = False + logger.warning("[TRIGGER] Internal trigger re-arm failed: %s", error, exc_info=True) + + trigger_latency_ms = (time.time() - started_at) * 1000.0 + if not rearmed: + logger.warning("[TRIGGER] Hardware capture retained while radar re-arm is pending") + + if capture is None or parse_error is not None: + self._append_diagnostic( + accepted=False, + reason="parse_failed", + response_bytes=response_bytes, + trigger_latency_ms=trigger_latency_ms, + ) + return None + + summary = self._summarize_capture_activity(processor, capture) + valid_outbound = [ + speed + for speed in summary["all_outbound_speeds"] + if speed >= self.min_ball_speed_mph + ] + if not valid_outbound: + self._append_activity_diagnostic( + summary, + accepted=False, + reason="no_ball_speed", + response_bytes=response_bytes, + trigger_latency_ms=trigger_latency_ms, + ) + logger.info( + "[TRIGGER] OPS hardware capture rejected — no outbound speed >= %.1f mph", + self.min_ball_speed_mph, + ) + return None + + logger.info( + "[TRIGGER] OPS hardware capture accepted — peak %.1f mph", + max(valid_outbound), + ) + return capture + + def reset(self): + """Reset trigger state; the radar owns the armed state.""" + pass + + def create_trigger(trigger_type: str = "speed", **kwargs) -> TriggerStrategy: """ Factory function to create trigger strategy. Args: - trigger_type: "speed" (recommended), "polling", "threshold", "manual", - "sound", or "sound-gpio" + trigger_type: "speed" (recommended), "hardware", "polling", "threshold", + "manual", "sound", or "sound-gpio" **kwargs: Arguments passed to trigger constructor Returns: @@ -1167,6 +1287,7 @@ def create_trigger(trigger_type: str = "speed", **kwargs) -> TriggerStrategy: Requires gpiozero library. """ triggers = { + "hardware": HardwareTriggeredCapture, "speed": SpeedTriggeredCapture, "polling": PollingTrigger, "threshold": ThresholdTrigger, diff --git a/src/openflight/server.py b/src/openflight/server.py index e3a4660ba..ad1c64af1 100644 --- a/src/openflight/server.py +++ b/src/openflight/server.py @@ -2907,12 +2907,15 @@ def start_monitor( Args: port: Serial port for radar mock: Run in mock mode without radar - trigger_type: Trigger strategy (sound, speed, polling) + trigger_type: Trigger strategy (hardware, sound, speed, polling) debug: Enable verbose debug output ops_baud: Target UART baud when the OPS243 is on the GPIO header """ global monitor, mock_mode, mock_swing_speed_mode, radar_config # pylint: disable=global-statement + if trigger_type == "hardware" and sample_rate_ksps != 30: + raise ValueError("Hardware trigger mode requires a 30 ksps sample rate") + # Stop any existing monitor first if monitor is not None: print("[MONITOR] Stopping existing monitor before starting new one") @@ -3593,10 +3596,24 @@ def main(): _add_ballistics_arguments(parser) parser.add_argument( "--trigger", - choices=["polling", "threshold", "speed", "sound"], + choices=["hardware", "polling", "threshold", "speed", "sound"], default="polling", help="Trigger strategy (default: polling)", ) + parser.add_argument( + "--trigger-threshold", + "--speed-trigger-threshold", + dest="trigger_threshold", + type=float, + default=None, + help="Internal or host speed-trigger threshold in mph (hardware default: 25)", + ) + parser.add_argument( + "--trigger-magnitude", + type=int, + default=40, + help="OPS243 internal trigger magnitude SMn, 1-2000 (default: 40)", + ) parser.add_argument( "--swing-speed", action="store_true", @@ -3659,6 +3676,15 @@ def main(): "(default: 16 = 50/50 split, each segment ~4.27ms at 30ksps)" ), ) + parser.add_argument( + "--pre-trigger-segments", + type=int, + default=6, + help=( + "Internal hardware-trigger pre-trigger segments S#n, 0-32 " + "(default: 6; each segment is ~4.27ms at 30ksps)" + ), + ) parser.add_argument( "--sample-rate", type=int, @@ -3932,6 +3958,15 @@ def main(): ) args = parser.parse_args() + if args.trigger_threshold is not None and args.trigger_threshold < 0: + parser.error("--trigger-threshold must be non-negative") + if args.trigger == "hardware" and not 1 <= args.trigger_magnitude <= 2000: + parser.error("--trigger-magnitude must be between 1 and 2000") + if args.trigger == "hardware" and args.sample_rate != 30: + parser.error("--trigger hardware requires --sample-rate 30") + if args.trigger == "hardware" and not 0 <= args.pre_trigger_segments <= 32: + parser.error("--pre-trigger-segments must be between 0 and 32") + # Mount tilt cannot be defaulted safely (a wrong value silently biases the # launch angle), so require it whenever the K-LD7 radars are enabled. if args.kld7 and args.kld7_mount_tilt is None: @@ -4030,9 +4065,19 @@ def main(): set_show_raw_readings(True) print("Raw radar readings display ENABLED - signed speed values will be shown") - # Start the monitor - # Build trigger-specific kwargs (pre_trigger_segments always passed) + # Start the monitor. Keep the sound-trigger option on its legacy flag so + # selecting hardware mode cannot silently change the established path. trigger_kwargs = {"pre_trigger_segments": args.sound_pre_trigger} + if args.trigger == "hardware": + trigger_kwargs = { + "trigger_threshold_mph": ( + args.trigger_threshold if args.trigger_threshold is not None else 25.0 + ), + "trigger_magnitude": args.trigger_magnitude, + "pre_trigger_segments": args.pre_trigger_segments, + } + elif args.trigger == "speed" and args.trigger_threshold is not None: + trigger_kwargs = {"min_trigger_speed_mph": args.trigger_threshold} swing_speed_kwargs = { "trigger_threshold_mph": args.swing_speed_threshold, "max_speed_mph": None if args.swing_speed_max <= 0 else args.swing_speed_max, diff --git a/tests/test_ops243.py b/tests/test_ops243.py index 84e0bb487..89a7f0b18 100644 --- a/tests/test_ops243.py +++ b/tests/test_ops243.py @@ -5,6 +5,7 @@ import time import pytest +import serial from openflight.ops243 import Direction, OPS243Radar, SpeedReading @@ -638,3 +639,149 @@ def test_first_byte_callback_fires_when_capture_starts(self): assert response == b"".join(self._DUMP).decode("ascii") assert events == ["first-byte"] + + +class _InternalTriggerSerial: + """Minimal serial stand-in for internal-trigger command tests.""" + + is_open = True + + def __init__(self, fail_write=False): + self.writes = [] + self.fail_write = fail_write + + @property + def in_waiting(self): + return 0 + + def reset_input_buffer(self): + pass + + def write(self, data): + if self.fail_write: + raise serial.SerialTimeoutException("radar busy") + self.writes.append(data) + return len(data) + + def flush(self): + pass + + +class TestInternalSpeedTrigger: + """Focused tests for the OPS243 board-managed speed trigger.""" + + @staticmethod + def _radar(serial_obj): + radar = OPS243Radar.__new__(OPS243Radar) + radar.serial = serial_obj + return radar + + def test_configuration_uses_gc_trigger_order_and_six_pre_segments(self, monkeypatch): + """Internal trigger setup must restore GC-reset settings in order.""" + radar = self._radar(_InternalTriggerSerial()) + commands = [] + monkeypatch.setattr(time, "sleep", lambda _seconds: None) + monkeypatch.setattr( + radar, + "_send_command", + lambda command: commands.append(command) or "", + ) + + radar.configure_for_internal_speed_trigger( + trigger_threshold_mph=25, + pre_trigger_segments=6, + trigger_magnitude=40, + sample_rate_ksps=30, + ) + + assert commands == [ + "PI", + "GC", + "S=30", + "US", + "P0", + "S(", + "X=2", + "R-", + "R>25", + "OJ", + "OM", + "W0", + "S#6", + ] + assert radar.serial.writes == [b"ST-25\r", b"ST-25\r", b"SM40\r"] + assert not {"GS", "PA", "S#0"} & set(commands) + + @pytest.mark.parametrize( + ("kwargs", "message"), + [ + ({"trigger_threshold_mph": -1}, "non-negative"), + ({"trigger_magnitude": 0}, "between 1 and 2000"), + ({"trigger_magnitude": 2001}, "between 1 and 2000"), + ({"sample_rate_ksps": 25}, "30 ksps"), + ], + ) + def test_configuration_validates_hardware_requirements(self, kwargs, message): + """Unsafe threshold, magnitude, and sample-rate values fail early.""" + radar = self._radar(_InternalTriggerSerial()) + + with pytest.raises(ValueError, match=message): + radar.configure_for_internal_speed_trigger(**kwargs) + + def test_rearm_uses_gc_and_restores_cached_settings(self, monkeypatch): + """A completed dump is re-armed with GC without PA or S#0.""" + radar = self._radar(_InternalTriggerSerial()) + radar._internal_speed_trigger_config = (25.0, 6, 40, 30) + commands = [] + monkeypatch.setattr(time, "sleep", lambda _seconds: None) + monkeypatch.setattr(radar, "_drain_rearm_serial", lambda: None) + monkeypatch.setattr( + radar, + "_send_command", + lambda command: commands.append(command) or "", + ) + + assert radar.rearm_internal_speed_trigger() is True + assert radar.serial.writes == [b"GC", b"ST-25\r", b"SM40\r"] + assert commands == [ + "S=30", + "US", + "P0", + "S(", + "X=2", + "R-", + "R>25", + "OJ", + "OM", + "W0", + "S#6", + ] + assert not {"PI", "GS", "PA", "S#0"} & set(commands) + + def test_rearm_recovers_from_serial_timeout_without_discarding_capture(self, monkeypatch): + """A busy radar reports a retryable re-arm failure instead of raising.""" + radar = self._radar(_InternalTriggerSerial(fail_write=True)) + radar._internal_speed_trigger_config = (25.0, 6, 40, 30) + monkeypatch.setattr(time, "sleep", lambda _seconds: None) + monkeypatch.setattr(radar, "_drain_rearm_serial", lambda: None) + + assert radar.rearm_internal_speed_trigger() is False + assert radar._hardware_trigger_recovery_required is True + + def test_failed_rearm_discards_stale_output_before_next_dump(self): + """Recovery must ignore trailing UART records before the next I/Q dump.""" + stale = b'{"speed":-12.0}\r\n' + radar = self._radar( + _ScheduledSerial( + [ + (0.0, stale), + (0.05, b"".join(TestWaitForHardwareTrigger._DUMP)), + ] + ) + ) + radar._hardware_trigger_recovery_required = True + + response = radar.wait_for_hardware_trigger(timeout=1.0) + + assert response == b"".join(TestWaitForHardwareTrigger._DUMP).decode("ascii") + assert radar._hardware_trigger_recovery_required is False diff --git a/tests/test_rolling_buffer.py b/tests/test_rolling_buffer.py index 61ff3f3c7..323e0768f 100644 --- a/tests/test_rolling_buffer.py +++ b/tests/test_rolling_buffer.py @@ -12,6 +12,7 @@ from openflight.launch_monitor import ClubType, Shot from openflight.rolling_buffer import ( + HardwareTriggeredCapture, ImpactEstimate, IQCapture, ManualTrigger, @@ -20,6 +21,7 @@ RollingBufferProcessor, SpeedReading, SpeedTimeline, + SpeedTriggeredCapture, SpinCandidate, SpinResult, ThresholdTrigger, @@ -541,6 +543,121 @@ def test_invalid_trigger_type(self): with pytest.raises(ValueError): create_trigger("invalid_type") + def test_create_hardware_trigger_with_opt_in_defaults(self): + """The new hardware strategy is registered without changing the factory default.""" + trigger = create_trigger("hardware") + + assert isinstance(trigger, HardwareTriggeredCapture) + assert trigger.trigger_threshold_mph == 25.0 + assert trigger.trigger_magnitude == 40 + assert trigger.pre_trigger_segments == 6 + assert trigger.sample_rate_ksps == 30 + assert isinstance(create_trigger(), SpeedTriggeredCapture) + + def test_monitor_configures_hardware_trigger_on_connect(self): + """Hardware mode delegates its board setup to the OPS243 driver.""" + from openflight.rolling_buffer.monitor import RollingBufferMonitor + + monitor = RollingBufferMonitor( + trigger_type="hardware", + trigger_threshold_mph=31, + trigger_magnitude=55, + pre_trigger_segments=20, + ) + monitor.radar = MagicMock() + + assert monitor.connect() is True + + monitor.radar.configure_for_internal_speed_trigger.assert_called_once_with( + trigger_threshold_mph=31, + pre_trigger_segments=20, + trigger_magnitude=55, + sample_rate_ksps=30, + ) + + def test_hardware_trigger_requires_30_ksps(self): + """The tested internal-trigger waveform is only valid at 30 ksps.""" + with pytest.raises(ValueError, match="30 ksps"): + create_trigger("hardware", sample_rate_ksps=25) + + +class TestHardwareTriggeredCapture: + """Acceptance and false-trigger behavior for board-triggered captures.""" + + @staticmethod + def _capture(): + return IQCapture( + sample_time=0.0, + trigger_time=0.1, + i_samples=[2048], + q_samples=[2048], + ) + + def test_accepts_ball_capture_and_rearms_after_parsing(self): + """A valid board dump is parsed before GC re-arm and returned.""" + radar = MagicMock() + radar.wait_for_hardware_trigger.return_value = '{"Q": [1]}' + radar.last_hardware_trigger_first_byte_timestamp = 12345.678 + processor = MagicMock() + capture = self._capture() + processor.parse_capture.return_value = capture + processor.process_standard.return_value = SpeedTimeline( + readings=[SpeedReading(100.0, 900.0, 68.0, "outbound")], + sample_rate_hz=937.5, + ) + + trigger = HardwareTriggeredCapture() + result = trigger.wait_for_trigger(radar, processor, timeout=1.0) + + assert result is capture + processor.parse_capture.assert_called_once_with( + '{"Q": [1]}', + first_byte_timestamp=12345.678, + ) + radar.rearm_internal_speed_trigger.assert_called_once_with(30) + + def test_rejects_false_trigger_but_still_rearms(self): + """A board trigger with no qualifying outbound ball speed is discarded.""" + radar = MagicMock() + radar.wait_for_hardware_trigger.return_value = '{"Q": [1]}' + processor = MagicMock() + processor.parse_capture.return_value = self._capture() + processor.process_standard.return_value = SpeedTimeline([], 937.5) + + trigger = HardwareTriggeredCapture() + + assert trigger.wait_for_trigger(radar, processor, timeout=1.0) is None + radar.rearm_internal_speed_trigger.assert_called_once_with(30) + assert trigger.drain_diagnostics()[0]["reason"] == "no_ball_speed" + + def test_rearms_after_malformed_dump(self): + """Malformed board output cannot leave the internal trigger idle.""" + radar = MagicMock() + radar.wait_for_hardware_trigger.return_value = "not-json" + processor = MagicMock() + processor.parse_capture.return_value = None + + trigger = HardwareTriggeredCapture() + + assert trigger.wait_for_trigger(radar, processor, timeout=1.0) is None + radar.rearm_internal_speed_trigger.assert_called_once_with(30) + assert trigger.drain_diagnostics()[0]["reason"] == "parse_failed" + + def test_retains_valid_capture_when_rearm_reports_busy_radar(self): + """A re-arm failure is recoverable and does not discard the received shot.""" + radar = MagicMock() + radar.wait_for_hardware_trigger.return_value = '{"Q": [1]}' + radar.rearm_internal_speed_trigger.return_value = False + processor = MagicMock() + capture = self._capture() + processor.parse_capture.return_value = capture + processor.process_standard.return_value = SpeedTimeline( + readings=[SpeedReading(100.0, 900.0, 68.0, "outbound")], + sample_rate_hz=937.5, + ) + + assert HardwareTriggeredCapture().wait_for_trigger(radar, processor) is capture + class TestSoundTriggerTimestampPropagation: """Tests for hardware trigger timestamp propagation.""" diff --git a/tests/test_server.py b/tests/test_server.py index e21dcea94..b8e0bd27f 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -2935,3 +2935,126 @@ def test_every_api_supported_baud_is_accepted(self, good): a stricter check would reject a legitimate fallback to 115200, which the flag's own help text tells operators to use.""" assert good in UART_BAUD_COMMANDS + + +class TestHardwareTriggerPlumbing: + """Server-side forwarding and validation for the opt-in trigger mode.""" + + def test_server_cli_forwards_hardware_flags_and_preserves_sound_alias( + self, monkeypatch + ): + """Argparse selects the new kwargs only for hardware mode.""" + captured = {} + monkeypatch.setattr( + sys, + "argv", + [ + "openflight-server", + "--mock", + "--no-camera", + "--no-logging", + "--trigger", + "hardware", + "--trigger-threshold", + "31", + "--trigger-magnitude", + "52", + "--pre-trigger-segments", + "20", + ], + ) + monkeypatch.setattr( + server_module, + "start_monitor", + lambda **kwargs: captured.update(kwargs), + ) + monkeypatch.setattr(server_module, "load_sim_config", lambda: []) + monkeypatch.setattr(server_module, "build_connectors", lambda *args, **kwargs: []) + monkeypatch.setattr(server_module, "init_session_logger", lambda **kwargs: None) + monkeypatch.setattr(server_module, "_cleanup_hardware_for_shutdown", lambda: None) + monkeypatch.setattr(server_module.socketio, "run", lambda *args, **kwargs: None) + + server_module.main() + + assert captured["trigger_type"] == "hardware" + assert captured["sample_rate_ksps"] == 30 + assert captured["trigger_kwargs"] == { + "trigger_threshold_mph": 31.0, + "trigger_magnitude": 52, + "pre_trigger_segments": 20, + } + + captured.clear() + monkeypatch.setattr( + sys, + "argv", + [ + "openflight-server", + "--mock", + "--no-camera", + "--no-logging", + "--trigger", + "sound", + "--sound-pre-trigger", + "18", + ], + ) + server_module.main() + + assert captured["trigger_type"] == "sound" + assert captured["trigger_kwargs"] == {"pre_trigger_segments": 18} + + def test_start_monitor_forwards_hardware_trigger_kwargs(self, monkeypatch): + """The server passes threshold, magnitude, split, and sample rate through.""" + captured = {} + + class FakeMonitor: + def __init__(self, **kwargs): + captured.update(kwargs) + + def connect(self): + captured["connected"] = True + + def start(self, **kwargs): + captured["started"] = kwargs + + def stop(self): + captured["stopped"] = True + + def disconnect(self): + captured["disconnected"] = True + + monkeypatch.setattr("openflight.rolling_buffer.RollingBufferMonitor", FakeMonitor) + monkeypatch.setattr(server_module, "monitor", None) + monkeypatch.setattr(server_module, "get_session_logger", lambda: None) + + server_module.start_monitor( + port="/dev/ops", + trigger_type="hardware", + sample_rate_ksps=30, + trigger_kwargs={ + "trigger_threshold_mph": 25.0, + "trigger_magnitude": 40, + "pre_trigger_segments": 6, + }, + ) + + assert captured["port"] == "/dev/ops" + assert captured["trigger_type"] == "hardware" + assert captured["sample_rate_ksps"] == 30 + assert captured["trigger_threshold_mph"] == 25.0 + assert captured["trigger_magnitude"] == 40 + assert captured["pre_trigger_segments"] == 6 + assert captured["connected"] is True + + server_module.stop_monitor() + + def test_start_monitor_rejects_non_30_ksps_hardware_mode(self, monkeypatch): + """Hardware mode must not start with an untested sample rate.""" + monkeypatch.setattr(server_module, "monitor", None) + + with pytest.raises(ValueError, match="30 ksps"): + server_module.start_monitor( + trigger_type="hardware", + sample_rate_ksps=25, + ) diff --git a/tests/test_start_kiosk.py b/tests/test_start_kiosk.py index e1d91eb31..7e44db134 100644 --- a/tests/test_start_kiosk.py +++ b/tests/test_start_kiosk.py @@ -63,6 +63,49 @@ def test_iwr6843_enables_ti_launch_pipeline_with_production_defaults(): assert "--kld7" not in command +def test_hardware_trigger_is_opt_in_and_uses_six_pre_trigger_segments(): + """The existing sound startup remains the default; hardware flags are explicit.""" + default_command = _dry_run().stdout.strip() + hardware_command = _dry_run("--trigger", "hardware").stdout.strip() + + assert "--trigger sound" in default_command + assert "--trigger hardware" in hardware_command + assert "--trigger-threshold 25" in hardware_command + assert "--trigger-magnitude 40" in hardware_command + assert "--pre-trigger-segments 6" in hardware_command + assert "--sound-pre-trigger" not in hardware_command + + +def test_hardware_trigger_overrides_are_forwarded_without_changing_sound_alias(): + """Hardware-only names forward their values while sound keeps its legacy flag.""" + hardware = _dry_run( + "--trigger", + "hardware", + "--trigger-threshold", + "31", + "--trigger-magnitude", + "52", + "--pre-trigger-segments", + "20", + ).stdout + sound = _dry_run("--sound-pre-trigger", "18").stdout + + assert "--trigger-threshold 31" in hardware + assert "--trigger-magnitude 52" in hardware + assert "--pre-trigger-segments 20" in hardware + assert "--sound-pre-trigger" not in hardware + assert "--trigger sound" in sound + assert "--sound-pre-trigger 18" in sound + + +def test_speed_trigger_keeps_its_existing_threshold_without_an_override(): + """The hardware default threshold must not alter the existing speed path.""" + command = _dry_run("--trigger", "speed").stdout + + assert "--trigger speed" in command + assert "--trigger-threshold" not in command + + def test_iwr6843_overrides_are_forwarded(): result = _dry_run( "--iwr6843", From c97803707214f0d9386671999622770beb62aed8 Mon Sep 17 00:00:00 2001 From: OminousPanda110 Date: Tue, 18 Aug 2026 14:03:42 -0700 Subject: [PATCH 2/5] fix(ops243): require tested firmware for hardware trigger --- README.md | 2 +- docs/CHANGELOG.md | 3 ++- docs/hardware-trigger.md | 9 +++++++++ docs/rolling_buffer_spin_detection.md | 3 ++- src/openflight/ops243.py | 15 +++++++++++++++ tests/test_ops243.py | 16 +++++++++++++++- 6 files changed, 44 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 88cbfb040..b7d44dee2 100644 --- a/README.md +++ b/README.md @@ -60,7 +60,7 @@ See the **[Parts List](docs/PARTS.md)** for everything you need with purchase li ### 2. Wire it up -Follow the **[Sound Trigger Wiring Guide](docs/sound-trigger-wiring.md)** to connect the SEN-14262 to the OPS243-A. The (deprecated) K-LD7 modules connect via USB — no wiring needed. The OPS243 also has an opt-in internal speed-trigger path; see the **[Internal Hardware Trigger Guide](docs/hardware-trigger.md)**. +Follow the **[Sound Trigger Wiring Guide](docs/sound-trigger-wiring.md)** to connect the SEN-14262 to the OPS243-A. The (deprecated) K-LD7 modules connect via USB — no wiring needed. The OPS243 also has an opt-in internal speed-trigger path, which requires OPS243-A firmware v1.3.1; see the **[Internal Hardware Trigger Guide](docs/hardware-trigger.md)**. **Adding the IWR6843 angle radar?** The Pi cannot power both radars over USB, so the OPS243 moves to the Pi's GPIO UART header while the TI board takes the USB diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 04301f764..e660e09b2 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -12,7 +12,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 the radar's internal speed trigger with 25 mph, magnitude 40, and the tested `S#6` split at 30 ksps. Captures are parsed and the radar is re-armed with `GC` after each dump. The existing sound-trigger default and - `--sound-pre-trigger` behavior are unchanged. See + `--sound-pre-trigger` behavior are unchanged. Hardware mode requires and + validates OPS243-A firmware v1.3.1. See [OPS243 Internal Hardware Trigger](hardware-trigger.md). - **System Prerequisites:** Documented missing binary dependencies (`swig`, `liblgpio-dev`, `python3-dev`) required prior to executing `./scripts/setup/setup.sh`. - **Environment Reload Guidance:** Added instructions for reloading terminal environment variables (`source ~/.bashrc`) when installed dependencies or scripts (`setup.sh`, `start-kiosk.sh`) are not recognized in the current terminal session. diff --git a/docs/hardware-trigger.md b/docs/hardware-trigger.md index 63fcb1bab..766fe7e20 100644 --- a/docs/hardware-trigger.md +++ b/docs/hardware-trigger.md @@ -2,6 +2,15 @@ OpenFlight’s `hardware` trigger mode lets the OPS243-A decide when a rolling-buffer capture starts. It is opt-in; the kiosk default remains the existing SEN-14262 sound trigger. +## OPS243 firmware prerequisite + +Hardware-trigger mode requires **OPS243-A firmware v1.3.1**, the release used for +the validation sessions. The driver queries `?V` before sending the internal +trigger setup commands and fails fast if the reported version is different or +unavailable. Update the physical OPS243 using the manufacturer’s firmware +procedure before running `--trigger hardware`; this application does not flash +the radar. + ## Methodology The host configures the radar once and then waits for the radar’s completed rolling-buffer dump. The host does not poll speed reports or send `S!` for each shot: diff --git a/docs/rolling_buffer_spin_detection.md b/docs/rolling_buffer_spin_detection.md index 6d0fac828..b6a378f15 100644 --- a/docs/rolling_buffer_spin_detection.md +++ b/docs/rolling_buffer_spin_detection.md @@ -33,7 +33,8 @@ re-entering it. See [Sound Trigger Wiring](sound-trigger-wiring.md) for the recommended direct hardware trigger. The OPS243 internal speed trigger is available as a separate, opt-in capture -strategy. It uses the tested 30 ksps configuration and defaults to `S#6`; see +strategy. It requires OPS243-A firmware v1.3.1, uses the tested 30 ksps +configuration, and defaults to `S#6`; see the [Internal Hardware Trigger guide](hardware-trigger.md) for its command ordering, re-arm behavior, and Raspberry Pi validation checklist. The sound path and its defaults remain unchanged. diff --git a/src/openflight/ops243.py b/src/openflight/ops243.py index 697804c24..e72b1f0a3 100644 --- a/src/openflight/ops243.py +++ b/src/openflight/ops243.py @@ -162,6 +162,7 @@ class OPS243Radar: # Default serial settings per datasheet DEFAULT_BAUD = 57600 DEFAULT_TIMEOUT = 1.0 + REQUIRED_INTERNAL_TRIGGER_FIRMWARE = "1.3.1" # Target rate on the J3 UART. At 230,400 a dump moves in ~1.8s; the # 19,200 factory default would take 21s and miss every shot. @@ -784,6 +785,18 @@ def get_firmware_version(self) -> str: except json.JSONDecodeError: return response + def validate_internal_trigger_firmware(self) -> str: + """Require the OPS243 firmware release validated for internal triggering.""" + version = self._probe_firmware_version() + required = self.REQUIRED_INTERNAL_TRIGGER_FIRMWARE + if version != required: + detected = f"v{version}" if version else "no response" + raise RuntimeError( + f"Internal hardware trigger requires OPS243 firmware v{required}; " + f"detected {detected}. Update the OPS243 before using --trigger hardware." + ) + return version + def set_units(self, unit: SpeedUnit): """ Set speed output units. @@ -1762,6 +1775,8 @@ def configure_for_internal_speed_trigger( if sample_rate_ksps != 30: raise ValueError("Internal speed trigger requires a 30 ksps sample rate") + self.validate_internal_trigger_firmware() + pre_trigger_segments = max(0, min(32, pre_trigger_segments)) signed_threshold = self._format_internal_trigger_threshold(threshold) self._speed_read_buffer = "" diff --git a/tests/test_ops243.py b/tests/test_ops243.py index 89a7f0b18..91c16a377 100644 --- a/tests/test_ops243.py +++ b/tests/test_ops243.py @@ -681,10 +681,15 @@ def test_configuration_uses_gc_trigger_order_and_six_pre_segments(self, monkeypa radar = self._radar(_InternalTriggerSerial()) commands = [] monkeypatch.setattr(time, "sleep", lambda _seconds: None) + + def send_command(command): + commands.append(command) + return '{"Version":"1.3.1"}' if command == "?V" else "" + monkeypatch.setattr( radar, "_send_command", - lambda command: commands.append(command) or "", + send_command, ) radar.configure_for_internal_speed_trigger( @@ -695,6 +700,7 @@ def test_configuration_uses_gc_trigger_order_and_six_pre_segments(self, monkeypa ) assert commands == [ + "?V", "PI", "GC", "S=30", @@ -728,6 +734,14 @@ def test_configuration_validates_hardware_requirements(self, kwargs, message): with pytest.raises(ValueError, match=message): radar.configure_for_internal_speed_trigger(**kwargs) + def test_configuration_rejects_unvalidated_ops243_firmware(self, monkeypatch): + """Internal triggering must refuse firmware other than the tested release.""" + radar = self._radar(_InternalTriggerSerial()) + monkeypatch.setattr(radar, "_probe_firmware_version", lambda: "1.3.0") + + with pytest.raises(RuntimeError, match="requires OPS243 firmware v1.3.1"): + radar.configure_for_internal_speed_trigger() + def test_rearm_uses_gc_and_restores_cached_settings(self, monkeypatch): """A completed dump is re-armed with GC without PA or S#0.""" radar = self._radar(_InternalTriggerSerial()) From 05a3aed705d8e28b1eb98cfd2751af154b1ae65c Mon Sep 17 00:00:00 2001 From: OminousPanda110 Date: Tue, 18 Aug 2026 14:22:08 -0700 Subject: [PATCH 3/5] feat(ops243): lower hardware trigger magnitude default --- docs/CHANGELOG.md | 2 +- docs/hardware-trigger.md | 7 ++++--- scripts/start-kiosk.sh | 2 +- src/openflight/ops243.py | 2 +- src/openflight/rolling_buffer/trigger.py | 2 +- src/openflight/server.py | 4 ++-- tests/test_rolling_buffer.py | 2 +- tests/test_start_kiosk.py | 2 +- 8 files changed, 12 insertions(+), 11 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index e660e09b2..a4816cf78 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - **Opt-in OPS243 internal hardware trigger.** `--trigger hardware` configures - the radar's internal speed trigger with 25 mph, magnitude 40, and the tested + the radar's internal speed trigger with 25 mph, magnitude 25, and the tested `S#6` split at 30 ksps. Captures are parsed and the radar is re-armed with `GC` after each dump. The existing sound-trigger default and `--sound-pre-trigger` behavior are unchanged. Hardware mode requires and diff --git a/docs/hardware-trigger.md b/docs/hardware-trigger.md index 766fe7e20..2d7f14000 100644 --- a/docs/hardware-trigger.md +++ b/docs/hardware-trigger.md @@ -24,14 +24,15 @@ The host configures the radar once and then waits for the radar’s completed ro 6. Wait for the board-triggered dump, parse the I/Q payload, and reject it if it has no outbound ball-speed reading at or above 35 mph. 7. Re-arm with `GC`, restore the cached settings, and wait for the buffer to fill again. A serial write timeout keeps the capture and reports a retryable re-arm failure instead of discarding the shot. -The implementation intentionally excludes the optional `SZ` board-processing stream, SigMF/replay tooling, clock synchronization, and UI changes. Those are separate concerns and are not required for this focused trigger path. +The implementation is intentionally limited to the OPS243 trigger path; the +experimental analysis and UI work remain outside this focused change. ## Defaults and command | Setting | Hardware-mode default | |---|---:| | Trigger threshold | 25 mph | -| Trigger magnitude | 40 (`SM40`) | +| Trigger magnitude | 25 (`SM25`) | | Pre-trigger split | 6 segments (`S#6`) | | Sample rate | 30 ksps (required) | | Minimum accepted outbound ball speed | 35 mph | @@ -42,7 +43,7 @@ Run the mode directly with: openflight-server \ --trigger hardware \ --trigger-threshold 25 \ - --trigger-magnitude 40 \ + --trigger-magnitude 25 \ --pre-trigger-segments 6 \ --sample-rate 30 ``` diff --git a/scripts/start-kiosk.sh b/scripts/start-kiosk.sh index 8aa469006..3f14f91bb 100755 --- a/scripts/start-kiosk.sh +++ b/scripts/start-kiosk.sh @@ -25,7 +25,7 @@ TRIGGER="sound" # Default: hardware sound trigger (SEN-14262 → HOST_INT) SOUND_PRE_TRIGGER="" TRIGGER_THRESHOLD=25 TRIGGER_THRESHOLD_SET=false -TRIGGER_MAGNITUDE=40 +TRIGGER_MAGNITUDE=25 PRE_TRIGGER_SEGMENTS=6 BUFFER_SPLIT="" IWR6843=false diff --git a/src/openflight/ops243.py b/src/openflight/ops243.py index e72b1f0a3..78d07f821 100644 --- a/src/openflight/ops243.py +++ b/src/openflight/ops243.py @@ -1751,7 +1751,7 @@ def configure_for_internal_speed_trigger( self, trigger_threshold_mph: float = 25.0, pre_trigger_segments: int = 6, - trigger_magnitude: int = 40, + trigger_magnitude: int = 25, sample_rate_ksps: int = 30, ): """Configure the OPS243's internal speed-triggered rolling buffer. diff --git a/src/openflight/rolling_buffer/trigger.py b/src/openflight/rolling_buffer/trigger.py index 9246624dd..88484d078 100644 --- a/src/openflight/rolling_buffer/trigger.py +++ b/src/openflight/rolling_buffer/trigger.py @@ -1156,7 +1156,7 @@ def __init__( trigger_threshold_mph: float = 25.0, min_ball_speed_mph: float = 35.0, pre_trigger_segments: int = 6, - trigger_magnitude: int = 40, + trigger_magnitude: int = 25, sample_rate_ksps: int = 30, ): super().__init__(pre_trigger_segments=pre_trigger_segments) diff --git a/src/openflight/server.py b/src/openflight/server.py index ad1c64af1..a72ef607e 100644 --- a/src/openflight/server.py +++ b/src/openflight/server.py @@ -3611,8 +3611,8 @@ def main(): parser.add_argument( "--trigger-magnitude", type=int, - default=40, - help="OPS243 internal trigger magnitude SMn, 1-2000 (default: 40)", + default=25, + help="OPS243 internal trigger magnitude SMn, 1-2000 (default: 25)", ) parser.add_argument( "--swing-speed", diff --git a/tests/test_rolling_buffer.py b/tests/test_rolling_buffer.py index 323e0768f..029587ed8 100644 --- a/tests/test_rolling_buffer.py +++ b/tests/test_rolling_buffer.py @@ -549,7 +549,7 @@ def test_create_hardware_trigger_with_opt_in_defaults(self): assert isinstance(trigger, HardwareTriggeredCapture) assert trigger.trigger_threshold_mph == 25.0 - assert trigger.trigger_magnitude == 40 + assert trigger.trigger_magnitude == 25 assert trigger.pre_trigger_segments == 6 assert trigger.sample_rate_ksps == 30 assert isinstance(create_trigger(), SpeedTriggeredCapture) diff --git a/tests/test_start_kiosk.py b/tests/test_start_kiosk.py index 7e44db134..d8e70e03f 100644 --- a/tests/test_start_kiosk.py +++ b/tests/test_start_kiosk.py @@ -71,7 +71,7 @@ def test_hardware_trigger_is_opt_in_and_uses_six_pre_trigger_segments(): assert "--trigger sound" in default_command assert "--trigger hardware" in hardware_command assert "--trigger-threshold 25" in hardware_command - assert "--trigger-magnitude 40" in hardware_command + assert "--trigger-magnitude 25" in hardware_command assert "--pre-trigger-segments 6" in hardware_command assert "--sound-pre-trigger" not in hardware_command From 30db85d2ec78fd124f506bdbe4c6e4aae635e4d7 Mon Sep 17 00:00:00 2001 From: JP Marhefka <37427123+JPMarhefka@users.noreply.github.com> Date: Tue, 18 Aug 2026 15:14:18 -0700 Subject: [PATCH 4/5] Revise Raspberry Pi retest checklist for clarity Updated the Raspberry Pi retest checklist to clarify merging requirements and added a note about regression testing the sound trigger. --- docs/hardware-trigger.md | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/docs/hardware-trigger.md b/docs/hardware-trigger.md index 2d7f14000..e1cb0ee35 100644 --- a/docs/hardware-trigger.md +++ b/docs/hardware-trigger.md @@ -64,16 +64,12 @@ The internal trigger does not depend on the SEN-14262 sound edge. Existing sound ## Raspberry Pi retest checklist -Before treating a PR as ready for review, run the hardware path on the target Pi and record the observations in the PR body: +Before treating a PR as ready for merge, run the hardware path on the target Pi and record the observations in the PR body: -```bash -scripts/start-kiosk.sh --trigger hardware --radar-port /dev/ttyAMA0 -``` - -Record representative shots, including slow and fast swings; deliberate noise or nearby-impact false triggers; observed trigger-to-capture latency; whether each accepted capture produced a shot; and the observed `S#6` pre/post split. Also run the unchanged sound path with the same representative shots: +Also run the unchanged sound path with the same representative shots: ```bash scripts/start-kiosk.sh --trigger sound ``` -The hardware test is incomplete until both modes are checked on the Pi. The Mac development environment can validate command ordering, parsing, re-arm recovery, and CLI forwarding, but it cannot verify the OPS243 electrical and firmware behavior. +I was unable to regression test the sound trigger. From 766b3bb28b1b725b18cfd7c7181e87a169e733b6 Mon Sep 17 00:00:00 2001 From: OminousPanda110 Date: Fri, 21 Aug 2026 16:12:38 -0700 Subject: [PATCH 5/5] fix(server): isolate sound trigger options --- src/openflight/server.py | 17 +++++++++++------ tests/test_server.py | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 6 deletions(-) diff --git a/src/openflight/server.py b/src/openflight/server.py index c9b4d016d..447441aef 100644 --- a/src/openflight/server.py +++ b/src/openflight/server.py @@ -4177,10 +4177,11 @@ def main(): set_show_raw_readings(True) print("Raw radar readings display ENABLED - signed speed values will be shown") - # Start the monitor. Keep the sound-trigger option on its legacy flag so - # selecting hardware mode cannot silently change the established path. - trigger_kwargs = {"pre_trigger_segments": args.sound_pre_trigger} - if args.trigger == "hardware": + # Build trigger kwargs per strategy so sound-only settings cannot change + # the defaults of speed, polling, or threshold capture. + if args.trigger == "sound": + trigger_kwargs = {"pre_trigger_segments": args.sound_pre_trigger} + elif args.trigger == "hardware": trigger_kwargs = { "trigger_threshold_mph": ( args.trigger_threshold if args.trigger_threshold is not None else 25.0 @@ -4188,8 +4189,12 @@ def main(): "trigger_magnitude": args.trigger_magnitude, "pre_trigger_segments": args.pre_trigger_segments, } - elif args.trigger == "speed" and args.trigger_threshold is not None: - trigger_kwargs = {"min_trigger_speed_mph": args.trigger_threshold} + elif args.trigger == "speed": + trigger_kwargs = {} + if args.trigger_threshold is not None: + trigger_kwargs["min_trigger_speed_mph"] = args.trigger_threshold + else: + trigger_kwargs = {} swing_speed_kwargs = { "trigger_threshold_mph": args.swing_speed_threshold, "max_speed_mph": None if args.swing_speed_max <= 0 else args.swing_speed_max, diff --git a/tests/test_server.py b/tests/test_server.py index c6490afb3..024dc26c1 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -3151,6 +3151,42 @@ def test_server_cli_forwards_hardware_flags_and_preserves_sound_alias( assert captured["trigger_type"] == "sound" assert captured["trigger_kwargs"] == {"pre_trigger_segments": 18} + @pytest.mark.parametrize("trigger", ["speed", "polling", "threshold"]) + def test_server_cli_does_not_reuse_sound_pre_trigger_for_other_triggers( + self, monkeypatch, trigger + ): + """Non-sound strategies keep their constructor pre-trigger defaults.""" + captured = {} + monkeypatch.setattr( + sys, + "argv", + [ + "openflight-server", + "--mock", + "--no-camera", + "--no-logging", + "--trigger", + trigger, + "--sound-pre-trigger", + "16", + ], + ) + monkeypatch.setattr( + server_module, + "start_monitor", + lambda **kwargs: captured.update(kwargs), + ) + monkeypatch.setattr(server_module, "load_sim_config", lambda: []) + monkeypatch.setattr(server_module, "build_connectors", lambda *args, **kwargs: []) + monkeypatch.setattr(server_module, "init_session_logger", lambda **kwargs: None) + monkeypatch.setattr(server_module, "_cleanup_hardware_for_shutdown", lambda: None) + monkeypatch.setattr(server_module.socketio, "run", lambda *args, **kwargs: None) + + server_module.main() + + assert captured["trigger_type"] == trigger + assert captured["trigger_kwargs"] == {} + def test_start_monitor_forwards_hardware_trigger_kwargs(self, monkeypatch): """The server passes threshold, magnitude, split, and sample rate through.""" captured = {}