Query ACAM directly for guide state in "put on slit" routing#450
Query ACAM directly for guide state in "put on slit" routing#450cfremling wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd449fb900
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // offsetgoal -- a no-op when ACAM isn't guiding -- and the telescope never | ||
| // moves ("put on slit didn't work"). | ||
| // | ||
| bool is_guiding = this->is_acam_status_fresh() && this->is_acam_guiding.load(); |
There was a problem hiding this comment.
Do not treat stale ACAM telemetry as not guiding
When ACAM is still guiding but no heartbeat arrives within 10 seconds, this expression makes is_guiding false and the code below sends the offset directly to the TCS instead of updating ACAM's guide goal. This can occur without ACAM stopping: the frame loop skips do_acquire() entirely for each configured nskip_before_acquire frame (acamd/acam_interface.cpp:2767-2771), and skipframes accepts any nonnegative value, so a normal 1-second exposure with skipframes 10 is enough to exceed the freshness window while guiding remains active. Pub/sub interruptions or slow solver processing have the same effect. A stale status should cause the operation to wait, query ACAM, or fail safely rather than assume guiding is off and move the telescope.
Useful? React with 👍 / 👎.
"Put on slit" intermittently failed when guiding was off. slicecam's
offset_acam_goal() decides between updating ACAM's guide goal (guiding) and
sending a PT offset directly to the TCS (not guiding) from a cached pub/sub
flag (is_acam_guiding), populated from ACAM's status broadcasts.
ACAM publishes its status only on change and skips frames between solves, so
the cached flag can read "guiding" while stale, or lag a state change. Trusting
it for this decision is unsafe in both directions: a wrong "not guiding" sends
a PT offset and moves the telescope while ACAM is actively guiding (fighting
the guider); a wrong "guiding" sends a no-op offsetgoal and never moves.
Query acamd directly instead. ACAMD_ACQUIRE with no args returns the current
mode { stopped | acquiring | guiding }, which is authoritative and current, so
the routing decision no longer depends on pub/sub freshness.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
dd449fb to
754ca53
Compare
|
Thanks Codex — this was correct and it changed the approach. The earlier version added a per-frame ACAM heartbeat plus a freshness guard, but as you noted a freshness cache fundamentally cannot distinguish "guiding but stale" from "not guiding": Reworked (force-push |
Problem
"Put on slit" intermittently fails when guiding is off — the telescope doesn't move and the command silently does nothing.
The decision is made inside the daemon:
putonslit.shonly reads ds9 coordinates and sends onescam putonslit. slicecam'soffset_acam_goal()then routes between updating ACAM's guide goal (guiding) and sending a PT offset directly to the TCS (not guiding), based on a cachedis_acam_guidingflag populated from ACAM's pub/sub status.Root cause — the cached guide state can be wrong
ACAM publishes its status only on change, and the frame loop skips frames between solves (
if (_skipframes-- > 0) continue;). So the cached flag can read "guiding" while stale, or lag a state change. Trusting it for this decision is unsafe in both directions:offsetgoaland never moves ("put on slit didn't work").Fix (minimal)
Query acamd directly for its guide state at decision time instead of trusting the cache.
ACAMD_ACQUIREwith no args returns the current mode{ stopped | acquiring | guiding }— authoritative and current — so the routing no longer depends on pub/sub freshness.Single-file change in
slicecam_interface.cpp::offset_acam_goal.Not compiled locally (no local build available).
🤖 Generated with Claude Code