Refactor Solver/Integrator flow to use dataclasses, combine camera/aligned position estimates into ImuDeadReckoning

[brickbots]

Summary

Reworks the Positioning data path (solver.py → integrator.py → consumers) off the legacy solved dict and onto typed dataclasses, then sharpens the boundaries between the solver, the integrator, and the IMU dead-reckoner. The change lands in three layers:

Reviewers

There is a lot of stuff here, but the main areas for input are the shape of the dataclasses in PiFinder/types/positioning.py and the flow of data from solver.py->integrator.py.

1. Dataclass migration — kill the solved dict (220a6484)

End-to-end adoption of the dataclasses in PiFinder/types/positioning.py:

• PointingEstimate replaces the solved dict everywhere. Pointing is modelled as a 2 × 2 matrix — two axes (camera, aligned) × two states (solve, estimate) — reached as pointing.<axis>.<state>.<RA|Dec|Roll>.
• The integrator owns the long-lived PointingEstimate; its solve cells survive failed plate-solves so IMU dead-reckoning keeps producing aligned estimates.
• shared_state.solution() / set_solution() carry PointingEstimate and default to an empty instance, so all ~15 consumers (UI, server, pos_server, nearby, …) read through the access shape without None checks.
• Alignment queues carry typed commands/responses (AlignOnRaDec / AlignCancel / ReloadSqmCalibration, AlignedResult), dispatched via isinstance().
• Rename solve_pixel → target_pixel across code, default_config.json, and shared_state. No config-migration logic — users re-align to write the new key.
• Deletes dead code (solver_main.py, get_initialized_solved_dict(), the legacy dict bridge methods).
• Adds matched_centroids / matched_stars to PointingEstimate so the SQM calibration UI can replay SQM from cached published solutions.

2. Collapse the two-axis IDR into one dual-axis instance (9a2ee904)

• ImuDeadReckoning now handles both axes in one instance. solve(camera, aligned, q_x2imu) captures the static q_cam2aligned alongside the drifting q_eq2x; predict() composes the camera prediction with q_cam2aligned and returns both as a (camera, aligned) tuple. reset() clears both.
• integrator.py drops the idr_camera / idr_aligned pair for a single idr.
• The IDR stays a pure math primitive: RaDecRoll in, RaDecRoll out — no import from PiFinder.types.positioning.
• imu_dead_reckoning_legacy.py is retained in-tree as the equivalence reference until we're comfortable removing it in a follow-up.

3. Split the solver→integrator message into a SolveResult DTO (913e5d08)

The migration in (1) had the solver build the same PointingEstimate the integrator owns and publishes — forcing it to set estimate == solve cells and emit a hollow PointingEstimate on failure. This layer separates the wire message from the published aggregate:

• New SolveResult = SuccessfulSolve | FailedSolve, a typed DTO on solver_queue. The integrator dispatches via isinstance() and is the sole builder/owner of PointingEstimate.
• SuccessfulSolve carries flat per-axis Pointings (no solve/estimate split — the solver never IMU-progresses) plus a single solve_time; the integrator fans these into both cells and assigns solve_time / cam_solve_time.
• The inline failed-solve branch is promoted to _apply_failed_solve so tests exercise the real integrator path.
• Rationale captured in ADR-0003; vocabulary added to positioning/CONTEXT.md.

Also in this layer: silence a spurious invalid value encountered in isnan RuntimeWarning — np.isnan() on the IDR's NaN-quaternion sentinel tripped numpy's FP-invalid flag. Replaced with a per-component math.isnan helper (_quat_has_nan), clearing all 25 unit-test warnings.

Docs (fb92dcd0 + this work)

docs/ax/positioning.md and docs/ax/positioning/CONTEXT.md updated to the new vocabulary; ADR-0003 added. The canonical glossary (CONTEXT.md) and the vocabulary decision (ADR-0001) remain authoritative for terms.

Test plan

• nox -s lint type_hints smoke_tests unit_tests — all green (214 unit tests pass; 219 unit + smoke), 0 warnings.
• Run on hardware: confirm dead-reckoning continues across failed solves and the aligned-axis offset is preserved under IMU motion.
• Run alignment flow: confirm the IDR picks up the alignment offset on the next solve and pointing.aligned.estimate reflects it.

Additional Notes for reviewers

• imu_dead_reckoning_legacy.py is intentionally retained as the equivalence-test reference; removal is a planned follow-up.
• A stale handoff.md scratch doc that had slipped into the branch was removed in this PR.

🤖 Generated with Claude Code

[brickbots]

@TakKanekoGit Here's a partially simplified ImuDeadReckoning class + a whole lot of refactoring around it to make the solver->integrator flow much more understandable. Hopefully this overall simplification helps everyone, but it's a ton to review.

I'm pretty sure about everything but the math in ImuDeadReckoning - Looking at the changes is a bit hard to follow, but the class is pretty small now. I think I preserved the camera center to alignment point logic/math properly and apply it correctly, but I'm still very much not across the quaternion transform maths.

What should be happening is that all the estimation is being done from a camera_center basis (the camera pointing), the delta between the camera and aligned solves is computed every time there is a new image solve, and the aligned estimate is derived from the camera estimate not calculated from scratch which should help with the missing roll from Tetra3 for the target_pixel solution.

Whew! I'm going to test this under the stars to verify this all, but I think it should work 🤞

[TakKanekoGit]

It's a big change! I've just started the review but must dash off. I'll hopefully have time tomorrow (sorry about that).

Good luck with the star testing! Have you tested it in demo mode? I often managed to catch errors with ImuDeadReckoning() using the demo mode.

[TakKanekoGit]

shared_state.set_solve_state() appears in pairs with shared_state.set_solution(). Would it make sense to incorporate setting the solve_state into the latter to enforce that this is set?

[brickbots]

Good catch — they were a hand-maintained pair and they're actually the same fact: solve_state is exactly solution().has_pointing() 👍

I went with the spirit of your suggestion but had set_solution derive the flag rather than hardcode it — self.__solve_state = v.has_pointing()

I kept it as a cached bool rather than making solve_state() compute on read, because the UI polls it every frame and reading the bool avoids round-tripping the whole PointingEstimate across the manager proxy.

[TakKanekoGit]

Sounds great!

[brickbots]

@mrosseel This is a pretty big change in the data models flowing through the solver->integrator and then used by the rest of the system. It will definitely impact you telemetry work, so wanted to flag you here so you could have a heads up and provide any input. Hoping to merge this soon so avoid creating more potential conflicts with new code 👍

[brickbots]

Just tested this briefly under the stars... It seems to work just like the current code in main, which is nice.

Both this branch and main are just AWESOME. Fast, smooth chart updates, works regardless of what weird angle I put the PiFinder in, just amazing work @TakKanekoGit

I did find one bug in this version that I'll fix up. It sometimes reverts to saying 'no solve' even though it's had one and was updating it with the IMU. I'm sure it's something simple leaking through, maybe related to the derivation of the solve_state when the solution is updated 🤷‍♂️

[TakKanekoGit]

Hi @brickbots. This looks fantastic. Very nice. The algorithm looks all fine. I've added a few very minor comments.

[TakKanekoGit]

Roll isn't defined for aligned so do we want to make this explicit via some RA/Dec type or setting roll to None?

[brickbots]

Hmmm.. Roll is generated for align when it is calculated from the estimated camera pointing via the q_cam2aligned so it is available... not sure how accurate it is across the sky though.

In order to generate q_cam2aligned we need a matched ra/dec/roll for camera and aligned. The one for camera comes directly from the solver, but the solver only produces ra/dec for the target_pixel so I'm just copying the image center (camera) roll to the target_pixel (aligned) value, which is bound to be problematic at times.

In practice the aligned ra/dec can't be more than ~5 degrees from the center value where we have a ground truth roll. This can make a pretty big different around the celestial poles, but most of the time the two roll values will be pretty close.

Are suggesting that we don't use roll at all outside the solver/integrator? I could support this, but we need it for the chart and eyepiece image orientation. For the chart, we've had a discussion of orientation wrt the screen (zenith up or EQ oriented) so we could do without it there, but we do need it for the eyepiece image orientation and this will vary based on the roll of the scope which is well captured by the solver roll 🤔

[TakKanekoGit]

Hmmm.. Roll is generated for align when it is calculated from the estimated camera pointing via the q_cam2aligned so it is available... not sure how accurate it is across the sky though.

If we're being super-precise, we can't define the roll at pointing (scope) and we shouldn't generate it lest the knowledge gets lost and roll at pointing gets abused 😁.

In order to generate q_cam2aligned we need a matched ra/dec/roll for camera and aligned. The one for camera comes directly from the solver, but the solver only produces ra/dec for the target_pixel so I'm just copying the image center (camera) roll to the target_pixel (aligned) value, which is bound to be problematic at times.

I think that comes from my Maths but it might be incorrect. I'll have a think about the proper way to do this.

In practice the aligned ra/dec can't be more than ~5 degrees from the center value where we have a ground truth roll. This can make a pretty big different around the celestial poles, but most of the time the two roll values will be pretty close.

Yes, it probably doesn't actually cause any problems in practice.

Are suggesting that we don't use roll at all outside the solver/integrator? I could support this, but we need it for the chart and eyepiece image orientation. For the chart, we've had a discussion of orientation wrt the screen (zenith up or EQ oriented) so we could do without it there, but we do need it for the eyepiece image orientation and this will vary based on the roll of the scope which is well captured by the solver roll 🤔

Yes. Roll is only needed to hand-over between solve and IMU.

I didn't know about the eyepiece image. Are the image orientations controlled by whether the mount is AltAz or EQ? I think we shouldn't use roll here because if the PiFinder is mounted at an odd angle, the roll won't reflect the image orientation. We should probably do the same thing as we do for chart orientation. There could be a corner case when you look near zenith with AltAz, though...

[TakKanekoGit]

I've included a couple of functions in the comments below that will enable you to calculate the RA, Dec at the target properly.

[TakKanekoGit]

Should also have a timestamp attribute so that we can use this for solve_time in the integrator when it uses the IMU sample for dead-reckoning.

[brickbots]

Yep! I'm adding this in here and taking the opportunity to remove the dictionary passing for imu data 👍 This bloats this PR even more, but is much better long-term

[brickbots]

@TakKanekoGit Thank you for the second round of review. I've committed all my updates so please take a look and resolve or add new comments to the conversations.

I've not tested the "no solve" bug fix under the stars yet. I'm on a work trip until Friday so that will have to wait for me, but anyone who wants pull this branch and test is very welcome!

[mrosseel]

@mrosseel This is a pretty big change in the data models flowing through the solver->integrator and then used by the rest of the system. It will definitely impact you telemetry work, so wanted to flag you here so you could have a heads up and provide any input. Hoping to merge this soon so avoid creating more potential conflicts with new code 👍

probably best if I wait to see what comes out of it and then merge with telemetry branch, which should be up to date with main as of now.

[mrosseel]

Hi, telemetry branch has the fix and should be up to date with main

…

On Tue, 26 May 2026, 19:37 Tak Kaneko, ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In python/PiFinder/integrator.py <#429 (comment)>: > - # Set up dead-reckoning tracking by the IMU: - imu_dead_reckoning = ImuDeadReckoning(cfg.get_option("screen_direction")) - # imu_dead_reckoning.set_cam2scope_alignment(q_scope2cam) # TODO: Enable when q_scope2cam is available from alignment + # Single IMU dead-reckoner handling both axes. Seeded with the + # (camera, aligned) pair at each successful plate-solve. + idr = ImuDeadReckoning(screen_direction) Hi @brickbots <https://github.com/brickbots> -- I have a function to estimate this (q_imu2cam) but I paused it until the issue with the invalid solves are fixed and I can record some new telemetry data (perhaps this PR might even fix it?). @mrosseel <https://github.com/mrosseel> has the details on the root cause. Hopefully, we'll be able to include it with the next release. — Reply to this email directly, view it on GitHub <#429?email_source=notifications&email_token=AALFXXX4ZXLLOJ7HGQMLP3D44XI7FA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTIMZWGYYDQMZVGA22M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#discussion_r3305707765>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AALFXXVFAOIL7F47IIV3HX344XI7FAVCNFSM6AAAAACZKB5HXCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHM2DGNRWGA4DGNJQGU> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[TakKanekoGit]

@mrosseel This is a pretty big change in the data models flowing through the solver->integrator and then used by the rest of the system. It will definitely impact you telemetry work, so wanted to flag you here so you could have a heads up and provide any input. Hoping to merge this soon so avoid creating more potential conflicts with new code 👍

probably best if I wait to see what comes out of it and then merge with telemetry branch, which should be up to date with main as of now.

@mrosseel -- Just to let you know, I'm working on changes to imu_pi.py to use gyro measurements directly which will probably conflict with your telemetry PR. I'll probably need your help to fix the merge conflicts when we get to it.

[TakKanekoGit]

I think these two function will solve the issue with roll being undefined at the target. The first one needs to be called once after alignment. The second one needs to be called after IMU predict. You could also use it after solve or you could take the target RA/Dec solved by tetra3.

I hope that helps to clear things up? I haven't tested it thoroughly but it's probably easiest to do sky testing.

# Do this once after alignment

def get_camera_to_target_vector(
        q_eq2cam: quaternion,  # Orientation of camera frame relative to the eq frame
        ra_target, dec_target  # RA/Dec of target [rad]  TODO: Use RaDec?
        ) -> np.ndarray:
    """
    Returns the pointing of the target relative the boresight (z_cam axis) of
    the camera frame as a unit vector. This is fixed until the target pixel
    is re-aligned.
    """
    # Target vector in equatorial frame  TODO: Move this to a method in RaDec?
    v_eq_target = np.array([
        np.cos(dec_target) * np.cos(ra_target),
        np.cos(dec_target) * np.sin(ra_target),
        np.sin(dec_target)
    ])
    q_eq_target = quaternion.from_vector_part(v_eq_target)  # Pure quaternion

    # Rotate into camera frame
    q_cam_target = q_eq2cam * q_eq_target * q_eq2cam.conjugate()  # Pure quaternion
    v_cam_target = q_cam_target.vec

    return v_cam_target / np.linalg.norm(v_cam_target)  # Unit vector of target relative to the camera frame

# Do this after every IMU dead-reckoning

def get_target_radec(
        q_eq2cam: quaternion,  # Orientation of camera frame relative to the eq frame
        v_cam_target: np.ndarray  # Unit vector of target in camera frame 
        ):
    """
    Get the target RA/Dec from q_eq2cam.
    """
    v_cam_target /= np.linalg.norm(v_cam_target)
    q_cam_target = quaternion.from_vector_part(v_cam_target)  # Pure quaternion

    # Rotate target into equatorial frame
    q_eq_target = q_eq2cam.conjugate() * q_cam_target * q_eq2cam  # Pure quaternion

    # Convert to RA/Dec  TODO: Move this to a method in RaDec?
    dec_target = np.arcsin(q_eq_target.z)
    ra_target = np.arctan2(q_eq_target.y, q_eq_target.x)
    ra_target = ra_target % (2.0 * np.pi)

    return ra_target, dec_target  # [rad]

[brickbots]

Hi @TakKanekoGit Sorry, I'm a bit confused here... this seems very similar to what is already implemented, just describing the delta between camera and align in a different space. The only difference functional I see is that the aligned pointing estimate currently does return a roll, which may be less accurate than the one for camera, but could still be useful.

Please let me know if I'm missing a key difference here as I'm not great with reasoning about the math involved and not realize the change you're proposing

WRT to Roll....
We can migrate away from the integrator provided roll in the future, figuring out better solutions for each use case:

Chart
You've already come up with a nice plan here, just need to implement it 🚀

Eyepiece image orientation
This may not be possible in all cases without user calibration, but the roll of the sky relative to the camera does provide really useful information here, and the current system uses it well to fairly accurately orient object images with the correct rotation, if you are using an alt-az newtonian scope and the PiFinder is positioned upright. This is not at all a universal solution, but covers enough of the users right now to be something I don't want to just drop.

The universal solution may require a one-time calibration from the user to work out the roll orientation of the camera vs. their scope optics, but after that the roll of the solution will still be required to properly orient the eyepiece image, so I think we're going to continue to need the roll out of the solver/integrator for both camera and aligned.

Since your Quaternion improvements and the refactor in this PR does not regress the roll accuracy from the current naive alt-az estimation, I think we can get this out into the world and then come back and improve the aligned roll estimate in some way if possible.