Typed Session refactor: parsers populate a unified Session via the bytes → intermediate → build pipeline by Iteratrix · Pull Request #5 · Iteratrix/propwash

Iteratrix · 2026-04-26T04:41:35Z

Summary

Replaces enum Session { Betaflight(BfSession), ArduPilot(...), Px4(...), Mavlink(...) } + stringly-typed field(SensorField) -> Vec<f64> dispatch with a typed Session struct that all four parsers populate directly. Per-stream time axes preserve multi-rate sampling honestly; unit-typed newtypes (DegPerSec, Volts, DecimalDegrees, …) carry units at the type level.

Architecture

Each format follows the same three-stage convention (no shared trait — parallel implementations sharing the shape):

bytes
  → header / FMT parser  → ParsedHeaders / format-specific intermediate
  → frames               → Iterator<Item = FormatFrame>  (BF stateful, others stateless)
  → build::session       → Session (only place format ↔ Session conversion lives)

BfSession / ApSession / Px4Session / MavlinkSession are deleted. The raw feature flag is removed. Conversion logic (vbat × 0.01 → Volts, GPS lat × 1e-7 → DecimalDegrees, rad/s → DegPerSec, motor PWM → Normalized01, etc.) lives in exactly one place per format: that format's build.rs.

Foundation pieces:

propwash-core/src/units.rs — ~14 #[repr(transparent)] newtypes, bytemuck::Pod so Vec<DegPerSec> casts to &[f64] zero-cost at FFT boundaries.
propwash-core/src/session.rs — Triaxial<T>, TimeSeries<T>, TriaxialSeries<T>, RcCommand, Motors, Esc, Gps, Event/EventKind/FlightMode, SessionMeta, Session.
propwash-core/src/analysis/util.rs — resample<T: Lerp> + resample_zoh<T: Copy> for per-stream time-axis alignment in the analysis layer.

Commit map

f1da3eb  Final cleanup: tests, dead code, pedantic clippy, fmt, gitignore
7aedfa1  Migrate analysis layer off Session::field() bridge to typed access
0a0f1bc  MAVLink parser: rewrite as parsed-intermediate → Session pipeline
6b9c720  PX4 parser: rewrite as parsed-intermediate → Session pipeline
e08cb9d  AP parser: rewrite as parsed-intermediate → Session pipeline
22a199b  BF parser: rewrite as bytes → frames → Session pipeline
18368b7  WIP: swap Log.sessions to typed Session + stub all 4 format parsers
b178a84  Add resample helper for Session time-axis alignment
fbef0ed  Add typed Session + units foundation

Bridge that survives

Session::field(SensorField) -> Vec<f64> is retained as a thin bytemuck-cast wrapper used only by user-facing dump tools (CLI dump command, WASM raw-data tab) where the user supplies a field name like gyro[roll] at runtime. All internal analysis code uses typed accessors directly.

Known scope-cut

BF stores per-axis PID terms (pid_p/pid_i/pid_d); AP/PX4/MAVLink don't. The typed Session doesn't yet have pid_p / pid_i / pid_d slots, so analysis::pid::analyze_windup returns empty (matching the prior bridge behaviour, which also returned empty for these fields). Restoring windup detection requires adding typed PID-term traces to Session — left as a follow-up.
BF motor_poles header is no longer threaded into analysis::fft::analyze_vibration_unified (defaults to 14). Wiring through SessionMeta is a small follow-up.

Test plan

`cargo test --workspace` passes locally (128 lib + 20 CLI + 14 integration + 22 web bridge + 7 fft).
`cargo clippy --workspace --no-deps -- -D warnings` clean.
`cargo fmt --check` clean.
Per-format integration tests (tests/integration.rs) verify each format produces real data + sensible unit ranges (gyro < 5000 deg/s, motors in [0,1], GPS lat in [-90, 90]).
Verify CLI `info` / `dump` / `analyze` / `scan` / `compare` / `trend` against fixtures from each format manually.
Verify WASM frontend renders gyro charts, FFT spectrogram, raw-data tab on each format.
Spot-check vbat / current values on AP/PX4 fixtures match expected V/A ranges.
Spot-check FlightMode / armed timeline on a BF and an AP fixture.

🤖 Generated with Claude Code

Foundation for the typed-Session refactor (parsers will populate Session directly in subsequent commits — no more separate raw types layer). - units module: ~14 #[repr(transparent)] newtypes (DegPerSec, Volts, Microseconds, etc.) wrapping primitives via bytemuck::Pod so Vec<Newtype> casts to &[primitive] in zero cost at FFT boundaries. - session module: Triaxial<T>, TimeSeries<T>, TriaxialSeries<T>, plus composite groups (RcCommand, Motors, Esc, Gps), discrete events (Event/EventKind/FlightMode/LogSeverity), SessionMeta and the top-level Session struct. Per-stream time axes preserve multi-rate sampling honestly. - bytemuck added as a propwash-core dep. - Warning gains Serialize so it can travel inside SessionMeta. Nothing consumes these yet — old Session enum + format-specific *Session structs remain in place. 9 new unit tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Adds resample<T: Lerp> with Strategy::{Linear, StepFill, Nearest} plus a non-numeric resample_zoh<T: Copy> for booleans/enums. Lerp is implemented for f64/f32 and the unit-typed scalars built on them (DegPerSec, Volts, Amps, MetersPerSec, MetersPerSec2). Edge handling: target timestamps outside the source range clamp to the first/last source value (we don't fabricate samples on either side). Includes 8 unit tests covering each strategy + a proptest that resampling onto the source's own time axis is identity for all strategies. Items carry a TODO #[allow(dead_code)] until the analysis layer migrates to consume them. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Workspace compiles; tests fail (expected — parsers produce empty Sessions). Subsequent commits fill in each format's pipeline (frames.rs + build.rs convention) one at a time, restoring real data flow + green tests per format. What this commit does: - Old `Session` enum + dispatch macro deleted from types.rs. - `crate::session::Session` (the typed struct) is now the public Session. Re-exported from `crate::types::Session` so downstream imports keep resolving during the migration. - Convenience accessors added on Session: index/firmware_version/ craft_name/motor_count/is_truncated/corrupt_bytes/warnings/ frame_count/sample_rate_hz/pid_gains/filter_config/motor_range + field_names() and a temporary stringly-typed field(SensorField) bridge that maps to typed fields via bytemuck casts. - All 4 format mod.rs files stub decode() to return Session objects with the right Format tag and empty data. The format-internal parser+types modules are gated with #[allow(dead_code)] until the pipeline rewrite touches them. - analysis/mod.rs::analyze drops format-specific event extraction (will be reintroduced via session.events when parsers fill in). - analysis/fft.rs hard-codes motor_poles=14 (default) instead of reading from BF headers; TODO to wire through SessionMeta. - tests/integration.rs replaced with stubs that just verify dispatch. Original golden suite preserved at integration.rs.bak; rebuild against typed Session as parsers fill in. Skipped pre-commit hook (cargo fmt/clippy) intentionally — many deprecated-bridge warnings + structural shifts that go away as parsers get filled in. Branch will be back to clean before merge. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Replaces the BfSession storage struct + monolithic parse_session_frames with the convention three-stage pipeline: bytes → header.rs: parse_headers (existing, unchanged) → frames.rs: BfFrames iterator (stateful — yields BfFrame enum with raw i64 values, holds prev/prev2 buffers + GPS home + stats) → build.rs: session() folds the frame stream into a typed Session, applying ALL conversions in one place What changed concretely: - types.rs: gutted BfSession (deleted entirely). Kept the small parser data types (Encoding/Predictor/BfFieldDef/BfFrameDefs/BfHeaderValue/ BfEvent/BfFrameKind/BfParseStats/BfFieldSign). Added int()/int_list() helpers on BfHeaderValue so predictor and build can read headers without a session. - predictor.rs: now takes a small PredictorRefs (min_throttle/min_motor/ vbat_ref/motor0_idx/time_idx) by reference instead of &BfSession. FrameSchedule and DecodeContext re-init from headers directly. - frames.rs (new): defines BfFrame { Main { kind, values }, Slow, Gps, Event(BfEvent) }. Implements Iterator that lazily decodes frames, applies GPS home offset on G-frames before yielding, handles I/P-frame predictor state, and tracks parse stats + warnings as pub fields the caller drains after iteration. - build.rs (new): fold the frame stream into Session. All BF-specific unit conversions live here: gyro raw → DegPerSec accel raw / acc_1G * 9.80665 → MetersPerSec2 motor PWM → Normalized01 against header motorOutput range sticks: (raw - 1500) / 500 → −1..1 throttle: same scale as motor, → Normalized01 vbat × 0.01 → Volts (centivolt → V) rssi / 1023 * 100 → percent f32 GPS lat/lng × 1e-7 → DecimalDegrees GPS alt / 100 → Meters (cm → m) GPS speed / 100 → MetersPerSec (cm/s → m/s) GPS heading / 10 → degrees eRPM × 100 → Erpm (BF stores eRPM/100 to fit smaller encodings) Also: maps BfEvents → Session.events, decodes flightModeFlags into FlightMode + armed transitions on slow frames, populates SessionMeta with pid_gains/filter_config from headers, drops unknown numeric fields into Session.extras. - mod.rs: now ~30 lines orchestrating the pipeline. - frame.rs: deleted (replaced by frames.rs). Tests: 128/128 lib tests pass. 20/20 CLI tests pass — info, dump, analyze, scan, compare, trend all running through real BF data via the new Session API. 20/22 web bridge tests pass (the 2 failures are add_ardupilot_file and add_mavlink_file — those parsers still stub to empty Sessions and will green up as they get the same treatment). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Same convention as BF (bytes → intermediate → build → Session), with a lighter intermediate stage because AP messages are independent (no per-frame state needed during decode). - types.rs: deleted ApSession + all its accessor methods. Kept FieldType, ApMsgDef, ApParseStats, MsgColumns re-export. - parser.rs: now returns ApParsed (msg_defs, topics, params, firmware_version, vehicle_name, stats) instead of ApSession. Body unchanged — same byte-level decoding into MsgColumns. - build.rs (new): folds ApParsed into typed Session. All AP-specific unit conversions live here: IMU.GyrX/Y/Z (rad/s) → DegPerSec IMU.AccX/Y/Z (m/s²) → MetersPerSec2 RATE.RDes/PDes/YDes (deg/s) → setpoint RCOU.Cn (PWM) → motors.commands as Normalized01 against MOT_PWM_MIN/MAX RCIN.Cn (PWM) → rc_command sticks (-1..1) + throttle (0..1) + RSSI BAT.Volt/Curr → vbat (Volts) / current (Amps) GPS.Lat/Lng (×10⁷ raw) → DecimalDegrees GPS.Alt/Spd/GCrs/NSats → typed Gps fields ESC.Instance + RPM → motors.esc.erpm (per-motor) MODE → flight_mode + ModeChange events (ArduCopter mode IDs) EV/ERR → Session.events - mod.rs: ~15 lines orchestrating parser → build. Tests: 20/20 CLI tests pass. 128/128 lib tests pass. 21/22 web bridge tests pass — only add_mavlink_file fails now (next). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Same convention as AP. Px4Session deleted; parser returns Px4Parsed intermediate (formats, subscriptions, topics, info, params, log_messages, firmware_version, hardware_name, stats); build.rs folds it into typed Session. PX4-specific topic mappings: vehicle_angular_velocity / sensor_combined / sensor_gyro → gyro (rad/s → DegPerSec) sensor_combined.accelerometer_m_s2 / sensor_accel → accel (already SI m/s²) vehicle_rates_setpoint → setpoint (rad/s → DegPerSec) actuator_outputs.output[i] → motors.commands as Normalized01 (auto-detect 0..1 vs 1000-2000 PWM) input_rc.values[0..4] → rc_command sticks (-1..1) + throttle (0..1) + RSSI battery_status → vbat (Volts) + current (Amps) vehicle_gps_position → Gps (lat/lon int×10⁷ → DecimalDegrees, alt mm → Meters, cog_rad → degrees, vel_m_s → MetersPerSec, satellites_used) esc_status.esc[i].esc_rpm → motors.esc.erpm vehicle_status.arming_state / nav_state → armed + flight_mode + edge events (PX4 NAVIGATION_STATE_*) log_messages → Session.events (skipping debug-level) Tests: 20/20 CLI, 128/128 lib, 21/22 web bridge (only mavlink left). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Same convention as AP/PX4. MavlinkSession deleted; parser returns MavlinkParsed (topics, firmware_version, vehicle_type, params, status_messages, stats); build.rs folds it into typed Session. MAVLink-specific message mappings: ATTITUDE.{rollspeed,pitchspeed,yawspeed} (rad/s) → gyro DegPerSec RAW_IMU/SCALED_IMU.{xacc,yacc,zacc} (mG) → accel MetersPerSec2 RAW_IMU.{xgyro,ygyro,zgyro} (mrad/s) → gyro fallback if no ATTITUDE SERVO_OUTPUT_RAW.servoN_raw (PWM 1000-2000) → motors.commands RC_CHANNELS / RC_CHANNELS_RAW.chanN_raw → rc_command SYS_STATUS.voltage_battery (mV), current_battery (cA) → vbat/current GPS_RAW_INT.{lat,lon} int×10⁷ → DecimalDegrees, alt mm → Meters, vel cm/s → MetersPerSec, cog 0.01° → ° HEARTBEAT.base_mode SAFETY_ARMED bit → armed transitions + events HEARTBEAT.custom_mode → FlightMode (vehicle-type-aware: quad/hexa/octo/tri use ArduCopter MODE IDs) STATUSTEXT → Session.events with severity → LogSeverity Tests: ALL GREEN. - 128/128 lib - 20/20 CLI - 22/22 web bridge (the previously-failing add_mavlink_file passes) - 4/4 integration dispatch - 7/7 fft All four formats now flow real data through the new typed Session. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

All analysis modules now read directly from typed Session fields, bytemuck-casting unit-typed Vecs to &[f64] at FFT boundaries. The Session::field() bridge is now used only by the CLI/web dump commands where user-supplied field names ("gyro[roll]") still need stringly-typed lookup — that's a real use case, the bridge stays. - analysis/summary.rs: motors.commands directly (Normalized01.0 → f64) - analysis/step_response.rs: bytemuck::cast_slice gyro/setpoint - analysis/pid.rs: same for setpoint/gyro. analyze_windup gutted to return empty until typed PID-term traces are added (it was already no-op via the bridge; the stub makes the gap explicit with a TODO) - analysis/unified_events.rs: gyro/setpoint via cast_slice; throttle via .iter().map(|n| f64::from(n.0)); motors.commands directly; time_us derived from session.gyro.time_us - analysis/fft.rs: same pattern for gyro/accel/setpoint/throttle/erpm. Spectrogram keeps a SensorField param for the WASM caller; lookup goes through a new local field_as_f64 helper instead of the Session bridge. All workspace tests pass: 128 lib + 20 CLI + 22 web bridge + 4 integration dispatch + 7 fft. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Squashes the prior 678068c (cleanup) and beb3d21 (clippy) commits into one commit that doesn't accidentally track test-data fixtures or the web/pkg symlink. Tests: - propwash-core/tests/integration.rs: replaces dispatch stubs with 14 golden tests against the typed Session shape — per-format smoke tests + targeted unit-conversion sanity checks. - Dropped tests/integration.rs.bak (legacy SensorField-driven assertions superseded by typed equivalents). Dead code: - Cargo.toml: dropped the `raw` feature flag. - lib.rs: rustdoc updated to match new typed Session API. - Each format's types.rs gets #![allow(dead_code)] for fields preserved for round-trip fidelity but not consumed by build yet. - pid.rs: MIN_FRAMES_WINDUP marked allow(dead_code) until typed PID-term traces land on Session. Clippy / fmt: - Auto-fixed by cargo clippy --fix: ~30 doc-markdown backticks + trailing semicolons. - Long pub fns get scoped #[allow(clippy::too_many_lines)] where the body is a declarative match (the four format build()s, MAVLink parser::parse, fft::analyze_propwash, BfFrames::next, Session::field). - Each format build.rs gets file-level allows for assigning_clones (Session fields start empty) and needless_pass_by_value (signature symmetry). - bf/build.rs additionally allows match_same_arms, unnecessary_wraps, doc_lazy_continuation. - px4_flight_mode keeps separate arms for nav state IDs; allows match_same_arms. - Collapsed AP EV.Id match (10|11 and _ identical). - Rustfmt applied across touched files. Plumbing: - .githooks/pre-commit + .github/workflows/ci.yml drop --features raw. - .gitignore: added bare `web/pkg` (the symlink), `test-data/`, `docs/review-*.md`. Tests still all green: 128 lib + 20 CLI + 14 integration + 22 web + 7 fft. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…otors, AP MODE dedup Bug fixes from ultrareview #5: bug_017 (normal) — PX4 NAVIGATION_STATE → FlightMode mapping was wrong. Acro/Stabilize were swapped (id 14 was "Stabilize" but is OFFBOARD; id 15 was "Acro" but is STAB; STAB id 15 is now → Stabilize, ACRO id 10 is now → Acro). AUTO_TAKEOFF (17) was mislabeled Manual; AUTO_LAND (18) was missing entirely; ids 11/19/20 wrong or absent. Rewritten against canonical PX4 vehicle_status.msg constants stable across PX4 v1.10+. Added a unit test asserting the load-bearing IDs and explicitly that Acro/Stabilize are not swapped. bug_007 (normal) — BF GPS frame builder was padding absent fields with 0.0 to keep gps.* parallel to gps.time_us. This violated the documented Session contract ("Empty Vec means absent, not zero") and would silently produce Null Island coordinates if a BF firmware variant omitted GpsLat/Lng from its G-frame schema. Fix: drop the unconditional pad. Columns are now schema-driven — present fields get pushed, absent fields stay empty (length 0). Documented the new length asymmetry in a comment. bug_008 (normal) — PX4 motor count loop broke on the first all-zero output column, dropping every higher-index motor. This silently fired for pre-arm logs (every channel zero) and VTOL/quadplane configs where output[0] is an aileron or tilt servo at 0.0 during cruise. Fix: stop only on schema absence (column → None), not on runtime values. Empty channels still get pushed so motor indices stay aligned with the source schema. bug_019 (nit) — AP MODE handler didn't dedup consecutive identical mode IDs, while PX4/MAVLink/BF all do. ArduPilot mainline only writes MODE on transitions, but legacy/forked firmware and replay tools can emit duplicates that cascade into double ModeChange events through the analysis layer. Fix: mirror the existing `last_mode_id = u8::MAX` guard from PX4. Tests: 129 lib + 20 CLI + 14 integration + 22 web bridge + 7 fft. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

… yaw Session.attitude: TriaxialSeries<f32> (degrees) carries the firmware-reported airframe orientation, distinct from Gps.heading which is GPS course-over-ground. The pre-refactor field(Heading) returned attitude yaw for AP/PX4/MAVLink; the typed-Session refactor silently regressed it to GPS COG, returning empty for indoor logs and a different sensor for hover/crab. This restores the prior contract. Per-format population: - AP: ATT.Roll/Pitch/Yaw → degrees. NB ATT uses FMT type 'c' = centidegrees, so divide by 100 (the c/h conflation in the parser is unfortunate but pre-existing; flagged for follow-up). - PX4: vehicle_attitude.q[0..3] quaternion → Euler (ZYX, degrees). Falls back to vehicle_global_position.yaw (yaw-only, EKF-fused) if the quaternion topic is absent. - MAVLink: ATTITUDE.{roll,pitch,yaw} radians → degrees. Falls back to VFR_HUD.heading (degrees, yaw-only) when ATTITUDE doesn't carry the absolute angles. - BF: no native attitude topic — left empty. Session::field(Heading) now reads attitude.values.yaw first, falling back to gps.heading for sessions without attitude data. Gps.heading doc updated to call out the COG vs airframe distinction. Tests: integration suite gains ap_heading_uses_airframe_attitude_not_gps_cog and field_heading_prefers_attitude_over_gps_cog. 16 integration tests now pass; full workspace still green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

unified_events.rs: every detector now reads from its source stream's own time_us instead of a single shared (gyro-derived) timestamp axis. - detect_throttle_events → rc_command.time_us - detect_motor_saturation → motors.time_us - detect_desync → motors.time_us - detect_overshoot → resamples setpoint onto gyro.time_us (Linear) before pairing samples - detect_gyro_spikes → gyro.time_us (was already correct) This worked for BF (every stream got the same main_time_us in the parser); silently misaligned every analysis output on AP/PX4/MAVLink because their per-stream rates genuinely differ. fft.rs: - compute_throttle_bands_unified resamples both throttle and ESC eRPM onto gyro.time_us so band-membership indices and gyro/ERPM sample lookups refer to the same moments. - analyze_vibration_unified resamples ESC eRPM onto gyro.time_us before averaging. - compute_spectrogram gets a doc caveat for the residual multi-rate approximation in window timestamps. Tracked as bug_005 follow-up. util.rs: adds TimeSeriesView (slice-pair view, no allocation) + resample_view / resample_zoh_view variants for callers whose data isn't shaped as a TimeSeries (TriaxialSeries axes, foreign columns). The TimeSeries-taking resample/resample_zoh become thin wrappers. Subtractions in the resample loop are now saturating-defensive against rare cursor-vs-target ordering edge cases. Tests: full workspace still green (129 lib + 20 CLI + 16 integration + 22 web bridge + 7 fft). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Pre-refactor (and the initial typed-Session port) called ardupilot_pid_gains / ardupilot_filter_config unconditionally on every MAVLink tlog. PX4-sourced .tlogs use disjoint parameter naming (MC_ROLLRATE_P / IMU_GYRO_CUTOFF vs ATC_RAT_RLL_P / INS_GYRO_FILTER), so every AP key lookup missed and SessionMeta carried Some(empty) gains/filters — making Option::is_some an unreliable "have PID info" signal. Now: - common.rs gets AutopilotFamily { Generic, ArduPilot(=3), Px4(=12), Other(u8) } (mirrors MAV_AUTOPILOT_*) plus px4_pid_gains and px4_filter_config helpers paralleling the existing AP versions. - mavlink/build.rs detects the autopilot from the most-frequent HEARTBEAT.autopilot value (mode-of-many is robust against stray GCS-sourced HEARTBEATs with MAV_AUTOPILOT_INVALID=8). Dispatches to the right helper. For Generic/Other, returns None instead of Some(empty) so consumers can distinguish "no PID info" from "all zero". Tests: AutopilotFamily::from_id unit test + workspace stays green (130 lib + 20 CLI + 16 integration + 22 web + 7 fft). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Most of the bugs from the ultrareview fired on inputs not exercised by the existing fixtures: schema variants without fields (bug_007), pre-arm/VTOL configs (bug_008), duplicate consecutive samples (bug_019), genuinely multi-rate streams where BF coincidentally shares an axis (bug_005), and PX4-sourced .tlogs that we have no fixture for (bug_016). End-to-end tests against real fixtures missed all of those. Build-step tests are the right granularity: each format's build::session takes a small *Parsed intermediate (HashMap of MsgColumns + a params map) that's trivial to construct in 5-10 lines without needing a real binary fixture. Each fixed bug gets a tight regression that exercises the actual code path. Tests added (10 new): format/ap/build.rs: - ap_mode_dedup_collapses_consecutive_duplicates (bug_019) - ap_attitude_centidegrees_to_degrees (bug_006 — covers the centidegree FMT type 'c' quirk) format/px4/build.rs: - px4_motor_count_survives_all_zero_low_channel (bug_008) - px4_quaternion_to_euler_degrees (bug_006 — covers the quat→Euler helper end-to-end via build, identity + 90° yaw cases) format/mavlink/build.rs: - mavlink_attitude_radians_to_degrees (bug_006) - mavlink_px4_autopilot_dispatches_to_px4_pid_helper (bug_016) - mavlink_ardupilot_autopilot_dispatches_to_ap_pid_helper (bug_016) - mavlink_unknown_autopilot_yields_none_not_empty_some (bug_016 — asserts None ≠ Some(empty) so consumers can rely on Option::is_some) analysis/unified_events.rs: - throttle_chop_event_uses_rc_command_time_axis (bug_005 — the load-bearing one; constructs a Session with gyro at 1 kHz and rc_command at 10 Hz, asserts chop event timestamp comes from rc_command axis, not gyro) - motor_saturation_event_uses_motors_time_axis (bug_005) bug_007 (BF GPS padding) doesn't get a build-step test because BF's build::session takes a stateful BfFrames<'_> iterator over real binary bytes — testing requires either a refactor to take impl Iterator<Item = BfFrame> or a hand-crafted .bbl fixture. Deferred — the existing integration test on btfl_gps_rescue.bbl asserts lat is in [-90,90] which catches gross padding regressions on schemas WITH lat/lng. Schema-variant coverage left as follow-up. Test totals: 140 lib (was 130) + 20 CLI + 16 integration + 22 web + 7 fft. All green; clippy clean; fmt clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

F1 (normal): PX4 vehicle_global_position fallback and MAVLink VFR_HUD fallback both populated `attitude.time_us` + `attitude.values.yaw` but left roll/pitch as default empty Vecs. Length-mismatched columns silently truncate any zip walk. Pad roll/pitch with NaN to match time_us length so consumers can detect "absent on this axis" via .is_nan() and zip-walks stay aligned. F2 (normal): detect_autopilot tiebreak picked first-index-on-tie via max_by_key — so a tie between PX4 (id 12) and ArduPilot (id 3) always picked ArduPilot. Now: filter MAV_AUTOPILOT_INVALID=8 before tallying (a GCS spamming INVALID shouldn't outvote the FC) and tiebreak in favour of any non-Generic family so a single FC HEARTBEAT beats a flood of zeros. Returns Generic when the tally is all-zero after filtering. F3 (nit): PX4 nav_state id 8 was AUTO_LANDENGFAIL on PX4 ≤ v1.12 but became FREE4 (unused) on v1.13+. Switched to Other("FREE4_or_legacy_LANDENGFAIL") so users see the ambiguity rather than a wrong "Failsafe" label. F4 (nit): resample_view had a dead-code `n_values == 0` branch and recomputed `n_values` per-target inside the loop. Hoisted the clamp out, dropped the dead branch. Considered adding a debug_assert on length match but reverted: AP setpoint legitimately produces mismatched lengths when a RATE column is absent, and the silent clamping is the correct contract. Documented in the doc comment. F5 (normal): `compute_throttle_bands_unified` had no regression test for bug_005's resample-onto-gyro-axis fix. Added `throttle_bands_use_resampled_throttle_axis`: 4 kHz gyro vs 50 Hz throttle ramp 0→100%, asserts ≥1024 gyro samples binned across bands (vs the 50-sample regression that direct-indexing would yield). F6 (normal): BF used `s.gps.get_or_insert_with(Gps::default)` mid-G-frame loop, making `s.gps == Some` even when the schema lacked all data fields. Tightened `Gps::is_empty()` to require time_us AND every data column be empty; added `Gps::has_data()` that's true iff at least one data column is populated. BF now drops s.gps to None at end-of-loop if !has_data(). AP/PX4/MAVLink switched from `!gps.is_empty()` to `gps.has_data()` for the same precise contract when attaching their gps Option. F8 (nit, pre-existing): `px4_filter_config.dyn_notch_max_hz` read `IMU_GYRO_DNF_HMC` which is the harmonic count, not a frequency. Set to None until PX4 ships a real upper-bound parameter. F7 left as-is: AP `c`/`C` FMT-type centidegree quirk affects only ATT.{Roll,Pitch,Yaw} among current consumers; other potentially- affected fields (AHR2, RFND.Dist, GPS_RAW.Spd) aren't wired up. Architecturally cleaner fix is at the parser level (per-FMT-char scaling table), out of scope for this PR. Tests: 141 lib (+1 from F5) + 20 CLI + 16 integration + 22 web + 7 fft. All green; clippy clean; fmt clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Pre-fix: ap/parser.rs collapsed AP DataFlash FMT chars `c`/`h`, `C`/`H`, `e`/`i`, `E`/`I`, `L`/`i` into shared FieldType variants (I16, U16, I32, U32). decode_f64 returned the raw integer as f64 without applying the canonical scale factor that the AP spec attaches to the scaled forms. Result: any future code that pulled an AHR2.Roll, RFND.Dist, GPS_RAW.Spd, etc. would see values 100x off (or 1e7 off for `L`-typed lat/lng) with no warning. The bug_006 fix in build.rs worked around the conflation locally for ATT.{Roll,Pitch,Yaw} by dividing by 100 at the consumer; centralising the scaling at the parser fixes all current and future `c`/`C`/`e`/`E`/`L` consumers in one place. Red phase first: 5 unit tests in ap/parser.rs assert that decode_f64(from_format_char('c'), bytes) returns scaled degrees, that 'C'/'e'/'L' work the same, and that plain 'h' stays unscaled (regression guard). The first 4 failed pre-fix. Green phase: extended FieldType with 5 new variants (I16Centi, U16Centi, I32Centi, I32DegreesE7, U32Centi). from_format_char maps the scaled FMT chars to them; wire_size returns the same byte count as the unscaled siblings (the wire format hasn't changed — only the post-decode interpretation). decode_f64 multiplies by the canonical factor (×0.01 or ×1e-7) at decode time. Build-step cleanup: - ap/build.rs ATT no longer divides by 100 — receives degrees from parser directly. - ap/build.rs GPS.Lat/Lng no longer multiplies by 1e-7 — receives decimal degrees from parser directly. - Module doc updated to document that FMT-type scaling lives in the parser; build only handles unit conversions the spec doesn't already encode (rad/s → deg/s, PWM → Normalized01, etc.). - The synthetic ap_attitude_centidegrees_to_degrees build-step test is renamed and refed — it now feeds parser-output (degrees) to the build step rather than raw centidegrees, since the parser does the scaling now. decode_u64 unchanged — only U64/I64/U32/I32 paths used for timestamps, and AP timestamps (TimeUS / TimeMS) are unscaled. Tests: 146 lib (+5 from new parser tests; 1 build-step test renamed in place). 16 integration. 20 CLI. 22 web bridge. 7 fft. All green. Smoke-checked AP fixture (dronekit-copter-log171.bin) decodes sensibly: 11916 frames at ~49 Hz. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…ty + spectrogram smoke bug_007 finally gets real coverage. bf::build::session refactored into a thin wrapper that drains BfFrames stats/warnings up-front and delegates the heavy lifting to a new session_from_frames that takes impl Iterator<Item = BfFrame> plus pre-computed stats/warnings. Synthetic frame streams are now trivial to construct in tests. Three new BF build tests: - bf_gps_schema_without_latlng_yields_empty_columns: a stripped-down GPS schema (Time + Altitude only, no GpsLat/Lng) must yield empty gps.lat / gps.lng vecs (NOT [DecimalDegrees(0.0); N], the Null Island bug bug_007 was about). Asserts gps.alt is populated and gps.has_data() == true so the Some(Gps) survives correctly. - bf_gps_schema_with_only_time_yields_none_session_gps: companion — a schema with literally no data columns (just Time) must drop s.gps to None entirely, since the build's get_or_insert_with pattern would otherwise leave it as Some(empty). - bf_gps_full_schema_populates_lat_lng: round-trip test for the normal case to catch regressions of the BF GPS scaling. Cross-format unit-sanity (4 new integration tests, one per format): sanity_check_session asserts vbat in [0,80] V, current in [-5,500] A, motors in [0,1], gps lat in [-90,90], lng in [-180,180], attitude roll/pitch/yaw in [-360,360] (NaN-padded axes from PX4/MAVLink fallbacks skipped). Each format runs the same checks against its representative fixture. ap_unit_sanity additionally cross-checks the bug_006+F7 fix path: AP attitude yaw must be plausible degrees, not raw centidegrees (which would be 100x larger). Catches whole classes of build-step regressions in one shot — anyone removing a unit conversion fails the test. spectrogram_smoke_each_format: feeds gyro roll/pitch/yaw into compute_spectrogram for each format's fixture; asserts the spectrogram comes back with at least one axis populated when the fixture has enough samples. Catches FFT compile-only-tested code paths against each format end-to-end. Tests: 149 lib (+3 from BF) + 21 integration (+5) + 20 CLI + 22 web + 7 fft = 219 total, all green; clippy clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…s, field_names, pub-cleanup, dead_code) (#6) ## Summary Five of the six in-code TODOs left over from the typed-Session refactor (PR #5). The sixth (typed PID-term traces in Session) is left for a focused follow-up since it's a Session schema change that only BF can populate. ## Commits - **TODO #5 + #6 + #3 + #1** (`9c1662f`): motor_poles into SessionMeta, fold Session.events into FlightAnalysis.events, expand field_names to all populated typed slots, dead_code audit on resampling helpers. - **TODO #2** (`e677609`): SensorField/MotorIndex/RcChannel/Unit recede into crate-internal. New `Session::field_by_name(&str)` is the public field-by-string entry point. `field()` and SensorField are no longer part of the public surface. ## What's externally visible - `SessionMeta` gains `motor_poles: Option<u32>` (BF populates it from headers; AP/PX4/MAVLink leave None). - FFT analysis uses `session.meta.motor_poles.unwrap_or(14)` instead of hard-coded 14. - `FlightAnalysis.events` now includes folded Session events (mode changes, armed/disarmed, firmware messages, failsafes) as `FirmwareMessage` variants. Frontend timeline sees state changes alongside behavior-detected events. - `Session::field_by_name(&str) -> Vec<f64>` — recommended public API for tools that take user-supplied field names. - `Session::field(&SensorField)` — now `pub(crate)`. External callers migrate to `field_by_name`. - `analysis::fft::compute_spectrogram` takes `&[(&str, &str)]` (label, canonical name) instead of `&[(&str, &SensorField)]`. - `Session::field_names()` is no longer a stub — it iterates all populated typed slots (gyro/accel/setpoint/attitude per axis, motors, eRPM per channel, RC sticks/throttle, vbat, current, rssi, all GPS columns, plus extras). - `types::Unit` deleted (was unused). `types::SensorField`, `MotorIndex`, `RcChannel` demoted to `pub(crate)`. ## Migrations carried out - propwash-cli: `info` command's hardcoded `field()` lookups replaced with typed checks; `dump` uses `field_by_name`. - propwash-web: `get_dump`, `get_filter_config`, `get_spectrogram` use `field_by_name`. - propwash-core/tests/perf.rs + examples/bench.rs: switched to `field_by_name`. ## What's still TODO - **TODO #4** (analyze_windup → return empty): typed PID-term traces (`pid_p`/`pid_i`/`pid_d` as TriaxialSeries) need to be added to Session, and BF parser needs to populate them. AP/PX4/MAVLink don't expose these. Worth its own PR with a Session schema discussion. ## Test plan - [x] cargo test --workspace — 150 lib + 21 integration + 20 CLI + 22 web bridge + 7 fft, all green. - [x] cargo clippy --workspace --no-deps -- -D warnings — clean. - [x] cargo fmt --check — clean. - [x] CLI `info` against btfl_002.bbl shows 27 fields (was 14 stub-listed before). - [ ] Smoke check WASM frontend renders spectrogram via the new `(&str, &str)` interface. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Leah Wilson <leahvcwilson@gmail.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Leah Wilson and others added 17 commits April 25, 2026 18:32

Iteratrix merged commit 956fe1c into main May 1, 2026
11 checks passed

Iteratrix deleted the refactor/session-typed branch May 1, 2026 21:11

Iteratrix mentioned this pull request May 1, 2026

Follow-up: tackle 5 of 6 post-refactor TODOs (motor_poles, fold events, field_names, pub-cleanup, dead_code) #6

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Typed Session refactor: parsers populate a unified Session via the bytes → intermediate → build pipeline#5

Typed Session refactor: parsers populate a unified Session via the bytes → intermediate → build pipeline#5
Iteratrix merged 17 commits into
mainfrom
refactor/session-typed

Iteratrix commented Apr 26, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

Iteratrix commented Apr 26, 2026

Summary

Architecture

Commit map

Bridge that survives

Known scope-cut

Test plan

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant