Skip to content

Telemetry recording and replay with pointing math refactor#411

Open
mrosseel wants to merge 18 commits into
brickbots:mainfrom
mrosseel:telemetry
Open

Telemetry recording and replay with pointing math refactor#411
mrosseel wants to merge 18 commits into
brickbots:mainfrom
mrosseel:telemetry

Conversation

@mrosseel
Copy link
Copy Markdown
Collaborator

Summary

  • Add telemetry recording and replay system (telemetry.py, telemetry_list.py UI, menu entries)
  • Extract pointing math and telemetry into separate modules from integrator.py (slims integrator from ~600 lines to ~335)
  • Enhance telemetry with raw IMU data, target tracking, replay fixes
  • Add integrator drift integration tests and telemetry unit tests

Test plan

  • nox -s unit_tests passes (includes new test_telemetry.py and test_integrator_drift.py)
  • nox -s lint passes
  • nox -s type_hints passes
  • Record a telemetry session on real hardware and verify replay produces matching pointing output
  • Verify integrator behavior unchanged against a known session (regression check after pointing extraction)

🤖 Generated with Claude Code

mrosseel and others added 12 commits March 5, 2026 16:29
Replays synthetic telemetry through real ImuDeadReckoning and
integrator wrapper functions, measuring dead-reckoning error vs
ground truth. Three tests: stationary drift, slew tracking, and
solve correction reset.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Telemetry recorder captures solve and IMU events with timestamps
- Menu entries for telemetry start/stop/replay
- Integrator wired to record solves and IMU readings
- UI list screen for telemetry sessions

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Split integrator.py into three focused modules:
- pointing.py: coordinate math (IMU dead-reckoning, plate-solve
  integration, roll/constellation/altaz finalization)
- telemetry.py: TelemetryManager facade (recording, replay,
  command dispatch, image saving, replay event handling)
- integrator.py: main loop and queue plumbing only

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- 55 unit tests covering TelemetryRecorder, TelemetryPlayer,
  TelemetryManager, and pointing.py functions
- Fix test_integrator_drift.py import from PiFinder.integrator
  to PiFinder.pointing after refactor

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…and analysis tools

- Record raw gyro and accelerometer readings in IMU events
- Record target changes (name, RA/Dec, Alt/Az) for time-to-target analysis
- Fix replay: solve_time, solve_source, imu_pos, failed solves, mount_type header
- Reset integrator state after replay ends
- Move location to separate .location sidecar file for privacy
- Reduce file size with stationary IMU decimation (10x) and float rounding
- Add telemetry analysis scripts: session visualizer, drift analysis,
  truss flex analysis, IMU free-run drift visualization
- Include sample recording (session_20260309.jsonl, location scrubbed)
- 67 unit tests covering all new functionality

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Single conflict in python/PiFinder/integrator.py: telemetry already
refactored the helper functions (update_plate_solve_and_imu, update_imu,
set_cam2scope_alignment, get_roll_by_mount_type) into pointing.py.

Kept telemetry's refactor; dropped the duplicate helpers that upstream
modified in place. Upstream-only additions (get_alt_az, get_constellation,
pointing_updated flag) and the imu_time tweak in update_imu are NOT
incorporated here — apply them to pointing.py as a follow-up if wanted.
Upstream's astro_coords refactor (brickbots#420) moved RaDecRoll from
pointing_model/astro_coords.py to types/coordinates.py and reworked the
API: constructor now takes (ra, dec, roll, deg=...) and replaces
set_from_deg/get_deg with set(...) and get(deg=True).

Resolution:
- integrator.py: kept HEAD; dropped duplicate helpers (now in pointing.py)
- pointing.py: updated import path and ported all RaDecRoll calls to the
  new API (set_from_deg/get_deg → constructor / get(deg=True))
- integrator_classic.py: accepted upstream deletion (brickbots#421 dropped classic
  integrator); telemetry's only change was a trivial **kwargs signature
The clear-on-failure block in solver() reset solved["RA"]/Dec/Matches
but left camera_center and camera_solve pinned to the last successful
solve across every failure path.

Integrator-side downstream is unaffected (integrator.py:96 only merges
new position data when RA is not None), but stale camera_center leaks
into anything reading 'solved' from the solver's perspective. Completes
the existing 'otherwise old values persist' clearing block.
mrosseel added 2 commits May 22, 2026 03:42
solver.py imports tetra3 at module scope, which transitively pulled the
submodule into any caller of get_initialized_solved_dict — including
integrator.py and the integration-drift tests. CI's checkout step does
not init submodules, so tests/test_integrator_drift.py failed to collect:

  PiFinder/solver.py:28: ModuleNotFoundError: No module named 'tetra3'

Move the pure-data dict factory to a tiny new module PiFinder/solved.py.
Update solver.py, integrator.py, and test_integrator_drift.py to import
from there. No runtime behavior change.
Copy link
Copy Markdown
Contributor

@TakKanekoGit TakKanekoGit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! I just reviewed the part I'm familiar with (imu_pi.py). I use telemetry a lot so it'll be great to have it in main.

Comment thread python/PiFinder/imu_pi.py
imu_data["status"] = imu.calibration

# TODO: move_start and move_end don't seem to be used?
# Read raw sensor data for telemetry
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it make more sense to do this in imu.update() when the quaternions are read out?

Comment thread python/PiFinder/imu_pi.py Outdated
# TODO: move_start and move_end don't seem to be used?
# Read raw sensor data for telemetry
try:
imu_data["gyro"] = imu.sensor.gyro
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current version doesn't read gyro and accelerometer so we might get different behaviour, what with read/write, if we also start sampling additional data from the IMU.

When I previously analysed the gyro recorded with an earlier version of telemetry, it wasn't very usable. That might be due to the BNO055 IMU because I also got poor performance in GYRONLY mode.

Would it be an idea to make the default just to read in the quaternions and to have a configuration somewhere to enable reading in gyro and acceleration as an option?

Port telemetry record/replay onto the refactored Solver/Integrator flow
(upstream brickbots#429): SolveResult DTOs on solver_queue, integrator-owned
PointingEstimate, ImuSample on shared_state.

- integrator.py: take upstream's dataclass integrator, re-add telemetry
  hooks. Replay now converts recorded events back into SolveResult /
  ImuSample messages and feeds them through the same _apply_successful_solve
  / _apply_failed_solve / _advance_with_imu paths as live data (the old
  branch duplicated integrator logic inside TelemetryManager).
- telemetry.py: recorder consumes SolveResult/ImuSample; player produces
  them. JSONL field names unchanged, so existing recordings stay replayable.
- types/positioning.py: ImuSample gains gyro/accel fields for raw-sensor
  telemetry; imu_pi.py populates them.
- Drop pointing.py and solved.py: superseded by types/positioning.py and
  the upstream integrator (incl. the legacy roll-by-mount-type logic that
  upstream replaced with chart-side handling).
- solver.py: take upstream as-is (get_initialized_solved_dict is gone).
- Tests ported to the new types; drift tests now exercise the real
  integrator apply/advance functions.
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