Skip to content

feat: add altitude-based air density to ballistics model and test coverage - #169

Open
Kayd-Craig wants to merge 7 commits into
open-flight:mainfrom
Kayd-Craig:feature/altitude-air-density
Open

feat: add altitude-based air density to ballistics model and test coverage#169
Kayd-Craig wants to merge 7 commits into
open-flight:mainfrom
Kayd-Craig:feature/altitude-air-density

Conversation

@Kayd-Craig

@Kayd-Craig Kayd-Craig commented Jul 30, 2026

Copy link
Copy Markdown

What does this PR do?

Adds altitude-aware air density to the ballistics model so carry calculations
are accurate at elevation, and expands test coverage across two undertested
modules. Users can set their altitude in the Debug Panel tuning section with
a m/ft toggle.

Why was this required?

The ballistics model hardcoded sea-level air density (1.225 kg/m³) for all
carry calculations. At Denver (1609m) air density is ~85% of sea level; at
high-altitude courses like Leadville, CO (3094m) it drops to ~74%, producing
meaningfully wrong carry estimates for players at elevation.

The session_logger and rolling_buffer/trigger modules had significant untested
paths that reduced confidence in regressions going undetected.

Automated tests

101 new tests across three files:

tests/test_session_logger.py — 47 new tests covering previously untested methods:
log_connection, log_accepted_reading, log_shot optional fields, log_camera_data,
log_config_change, log_sim_send, log_sim_status, log_sim_player, log_iq_reading,
log_iq_blocks, log_trigger_event, properties (session_id, raw_path),
init_session_logger.

tests/test_trigger_strategies.py — 51 new tests covering TriggerStrategy base
(drain_diagnostics, _append_diagnostic), ManualTrigger, PollingTrigger,
ThresholdTrigger, SpeedTriggeredCapture, SoundTrigger clock sync helpers,
and SoundTrigger.wait_for_trigger edge paths.

tests/test_ballistics.py — 11 new tests covering air_density_at_altitude()
(sea level, Denver, Leadville, negative altitude clamped) and simulate()
with altitude_m parameter.

Manual (human) testing

Tested in mock mode:

  • Verified altitude slider appears in Debug Panel tuning section
  • Toggled between m and ft — display updates correctly, server always
    receives metres
  • Set altitude to 1609 (Denver) — confirmed carry distance increases
    vs sea level for same shot inputs
  • Set altitude to 0 — confirmed carry matches default sea-level output
  • All 1267 tests pass locally

Note: ESLint pre-commit hook failed locally due to environment issue
(hook running from home directory instead of ui/). npm run lint and
npm run format:check from ui/ both pass cleanly.

Checklist

  • Single feature/fix — this PR is scoped to one thing with a clear story above
  • Automated tests included — new or updated tests cover this change
  • Manual testing described — I documented what I verified by hand above
  • Python tests pass (uv run pytest tests/ -v)
  • Pylint passes (uv run pylint src/openflight/ --fail-under=9)
  • Ruff passes (uv run ruff check src/openflight/)
  • UI builds (cd ui && npm run build)
  • UI lint passes (cd ui && npm run lint)
  • Updated docs or CHANGELOG if needed
  • No unrelated changes mixed in

- log_connection: device/port/baud, optional firmware and radc_available
- log_accepted_reading: entry content and stats increment
- log_shot: optional fields (angle_source, club_angle_deg, club_path_deg,
  spin_axis_deg, pipeline_ms) included/omitted correctly
- log_camera_data: full entry and None angles
- log_config_change: default and custom source
- log_sim_send: full entry and None values fallback
- log_sim_status: connected/retrying states
- log_sim_player: entry content
- log_iq_reading: full entry, cfar flag
- log_iq_blocks: block count, empty list
- log_trigger_event: accepted/rejected, stats tracking
- Properties: session_id and raw_path before/after start
- init_session_logger: instance creation, global logger registration
Consolidate new tests into tests/test_session_logger.py
rather than keeping them in a separate file.
Cover previously untested trigger paths without hardware:
- TriggerStrategy base: drain_diagnostics, _append_diagnostic fields
- ManualTrigger: request_trigger, reset, wait_for_trigger fire/timeout
- PollingTrigger: defaults, activity detection, timeout, None capture, exceptions
- ThresholdTrigger: defaults, reset, threshold detection, inbound rejection, timeout
- SpeedTriggeredCapture: defaults, reset, last_trigger_speed property
- SoundTrigger._clock_sync_last_read_host_time: host_after, host_mid, edge cases
- SoundTrigger._clock_sync_age_s: age calculation, missing host time
- SoundTrigger._clock_sync_quality: all valid/invalid branches
- SoundTrigger._clock_sync_summary_for_log: non-dict, summary fields
- SoundTrigger.wait_for_trigger: timeout, parse failure, false trigger rejection
Add air_density_at_altitude() using the ISA tropospheric formula,
valid from sea level to ~11 km (covers all realistic golf venues).
At Denver (1609m) density is ~85% of sea level; at high-altitude
courses like Leadville (3094m) it drops to ~74%, adding meaningful
carry distance that the previous sea-level constant missed.

Add optional altitude_m parameter to simulate() that overrides
air_density when provided. Existing call sites are unaffected —
the parameter defaults to None and behaviour is unchanged.

Add 11 tests covering:
- ISA density at sea level, Denver, Leadville, negative altitude
- altitude_m increases carry vs sea level
- altitude_m overrides explicit air_density argument
- altitude_m=None leaves air_density param untouched
@Kayd-Craig
Kayd-Craig requested a review from jewbetcha as a code owner July 30, 2026 18:52
Users can now set altitude in the Debug Panel tuning section with a
m/ft toggle. The value is always stored internally in metres and sent
to the server, but displays and accepts input in whichever unit the
user selects. Conversion is handled entirely in the UI.

- radar_config: add altitude_m field (default 0 = sea level)
- set_radar_config: handle altitude_m updates from UI
- simulate(): pass altitude_m from radar_config
- RadarConfig type: add altitude_m field
- useDebugStore: initialise altitude_m to 0
- DebugPanel: add AltitudeControl with m/ft toggle (0-4500m / 0-14764ft)
@Kayd-Craig Kayd-Craig changed the title Add altitude-based air density to ballistics model Add test coverage for session_logger and trigger strategies, and altitude-based air density to ballistics Jul 30, 2026
@jewbetcha

jewbetcha commented Aug 1, 2026

Copy link
Copy Markdown
Member

How are users setting these new changes to the ballistic model?

Edit: nvm, saw UI slider there

@jewbetcha

Copy link
Copy Markdown
Member

Couple CI failures, can you fix? then good to merge

@Kayd-Craig Kayd-Craig changed the title Add test coverage for session_logger and trigger strategies, and altitude-based air density to ballistics feat(ballistics): add altitude-based air density model and expand test coverage Aug 1, 2026
@Kayd-Craig Kayd-Craig changed the title feat(ballistics): add altitude-based air density model and expand test coverage feat: add altitude-based air density to ballistics model and test coverage Aug 1, 2026
@Kayd-Craig

Copy link
Copy Markdown
Author

CI fixes pushed — ESLint pre-commit hook working directory corrected.
All checks passing locally (pytest, pylint, ruff, npm run lint, npm run format:check).
Ready for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants