Sequencer: stop fineacquire on timeout; lock user moves during ACAM acquire#453
Sequencer: stop fineacquire on timeout; lock user moves during ACAM acquire#453cfremling wants to merge 2 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8778c60016
ℹ️ 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".
| ( was_running && !this->is_fineacquire_running.load() ) || | ||
| (use_timeout && std::chrono::steady_clock::now() > timeout_time); | ||
| }); | ||
| } // release fineacquire_mtx before do_slicecam_stop() below (it locks the same mutex) |
There was a problem hiding this comment.
This unlock lets handletopic_slicecamd() change is_fineacquire_locked/is_fineacquire_running before the outcome checks below, even though those values are deliberately written under fineacquire_mtx for waiters. If a forced slicecamd status publish wakes this wait because the deadline expired, and then a late lock publish is handled before line 200 runs, this function can return success without issuing the stop even though the acquisition already timed out; the previous code held the mutex until the outcome was classified. Capture the wake reason/state while the lock is still held, then release it only before calling do_slicecam_stop().
Useful? React with 👍 / 👎.
When do_slicecam_fineacquire times out it returned TIMEOUT but left slicecamd's fineacquire loop running. If the observer continues past the warning, that lingering loop keeps sending guide-goal offsets to ACAM during the science exposure (perturbing it). new-target (move_to_target) and user abort (abort_process) already stop fineacquire, but neither covers this window. On timeout, call do_slicecam_stop() so the loop is stopped immediately. The outcome (cancelled/locked/timed_out) is classified while still holding fineacquire_mtx -- those flags are written under that mutex by handletopic_slicecamd(), so reading them after releasing could misclassify a timed-out run as success (and skip the stop) if a late publish arrived. The lock is released only before do_slicecam_stop() (which locks the same mutex).
A manual telescope move during ACAM acquisition (while it is converging on the
target) disrupts the acquisition. Reject such moves while ACAM's mode is
"acquiring" (TARGET_ACQUIRE/ACQUIRE_HERE); allow them while guiding or stopped.
- slicecam put_on_slit: the single entry for every GUI move that uses
"scam putonslit" -- jog LEFT/RIGHT/UP/DOWN, put-on-slit and put-on-guider on
both the ACAM and SCAM ds9 GUIs. It queries acamd's mode directly and rejects
when acquiring. (Guard placed in put_on_slit, not offset_acam_goal, to avoid
conflicting with the in-flight put-on-slit-routing PRs.)
- acam put_on_slit: the "offset guider" button ("acam putonslit"); checks the
local acquire_mode and rejects when acquiring.
cfremling
force-pushed
the
fix/sequencer-acquire-timeout
branch
from
8778c60 to
907f233
Compare
cfremling
changed the title
1 participant
Two small, related acquisition-robustness fixes.
1. Stop slicecam fineacquire on timeout (
sequence_acquisition.cpp)do_slicecam_fineacquirereturnedTIMEOUTbut left slicecamd's fineacquire loop running. If the observer continues past the warning, that lingering loop keeps sending guide-goal offsets to ACAM during the science exposure, perturbing it.move_to_target(new target) andabort_process(user abort) already stop fineacquire, but neither covers this window. On timeout it now callsdo_slicecam_stop().The outcome (
cancelled/locked/timed_out) is classified while still holdingfineacquire_mtx— those flags are written under that mutex byhandletopic_slicecamd(), so reading them after releasing could misclassify a timed-out run as success (and skip the stop) if a late publish arrived. The lock is released only beforedo_slicecam_stop()(which locks the same mutex). (addresses the P2 review note.)2. Lock out user-initiated moves while ACAM is acquiring (
slicecam/acam put_on_slit)A manual telescope move during ACAM acquisition disrupts the convergence. Moves are now rejected while ACAM's mode is
acquiring(TARGET_ACQUIRE/ACQUIRE_HERE); allowed while guiding or stopped.slicecam put_on_slit— the single entry for every GUI move that usesscam putonslit: jog LEFT/RIGHT/UP/DOWN, put-on-slit, and put-on-guider on both the ACAM and SCAM ds9 GUIs. Queries acamd's mode directly and rejects when acquiring. Placed input_on_slit(notoffset_acam_goal) to stay conflict-free with the in-flight put-on-slit-routing PRs.acam put_on_slit— the "offset guider" button (acam putonslit); checks the localacquire_mode.Notes
acquisition_timeoutitself already fires (via slicecamd's 60 s temperature heartbeat onTopic::SLICECAMDwaking the cv); this PR adds the missing stop on timeout, not the timeout itself.🤖 Generated with Claude Code