Skip to content

sequencerd: expose once acamd has made the target offset - #502

Closed
cfremling wants to merge 5 commits into
feat/sequencer-nexp-stop-abortfrom
feat/sequencer-wait-for-target-offset
Closed

cfremling wants to merge 5 commits into
feat/sequencer-nexp-stop-abortfrom
feat/sequencer-wait-for-target-offset

Conversation

@cfremling

@cfremling cfremling commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator

A target offset sent while ACAM guides only changes acamd's goal; the guide loop's next correction makes the move. The sequencer exposed a fixed OFFSET_SETTLE_SEC (3 s) after sending it, which could be before the telescope had moved. It now waits until acamd's status shows GOALSHIFT_PENDING raised after the offset was sent and cleared again, bounded at 30 s (it then exposes anyway, with a warning); a cancel ends the wait. OFFSET_SETTLE_SEC becomes an extra wait after the move, default 0.

Depends on #479 (acamd publishes GOALSHIFT_PENDING) and #497 (cleared only once the correction for the new goal has been made); #497 is merged into this branch. Base branch feat/sequencer-nexp-stop-abort; this PR's change is the last commit.

Jira: PAL-171. Tested on sky 2026-10-28/29 UT using branch cf/summit-20261020, 18 offset-star targets in 10:27–12:34 UT: acamd reported the offset made 5.6–9.2 s after it was sent for 15 of them and 12.9, 16.9 and 22.8 s for the other 3; the 30 s bound never fired and no exposure was taken on the offset star.

🤖 Generated with Claude Code

cfremling and others added 5 commits August 13, 2026 23:38
While ACAM guides, a fine-acquire correction only re-points acamd's
goal; the guide loop applies it at its next executed correction. The
settle was timed from the send, so with short slicecam exposures the
next sample median could be built from pre-move frames and the same
correction applied twice (UT 2026-08-06, BD+28d4211: exposed 2.6 arcsec
off slit).

acamd tracks the request: offsetgoal marks the goal change pending; the
guide loop clears it when it executes a correction computed from the
changed goal (the pending state is captured before the goal is read, so
a correction already in flight can never clear it). The flag is
published; do_fineacquire waits for pending false on a status newer
than its send before settling. A stop interrupts the wait; timeout
FINE_ACQUIRE_MOVE_TIMEOUT (default 60 s, 0 disables) proceeds on the
old timer. Also fixes the offset_acam_goal log line, which printed
degrees labelled arcsec on the guiding path.

Co-Authored-By: Claude Fable 5 <[email protected]>
After a correction has been applied, one frame already inside the goal is
accepted without waiting for the full sample set. A frame can only accept,
never reject: larger readings fall through to the existing median logic
unchanged. SETTLE_FRAMES config default 2 -> 0; the wall-clock settle after
the confirmed goal change covers the move.
A deliberate goal offset (the star-to-target offset after fine
acquisition, put-on-slit, the GUI offset) armed a one-shot allowance
that any sent correction cleared. A guide correction whose blocking
pt_offset was in flight when the offset arrived cleared it as it
completed, and the next frame's correction to the new goal was refused
at the guiding cap, so the exposure was taken on the offset star. The
cap selection also required acquire_mode == TARGET_GUIDE, which is a
race of its own.

Goal changes now carry a generation, and each correction is classified
by the goal it was computed from: guide, fine-acquire, or deliberate
offset. Only a deliberate-offset correction may use the larger
ACQUIRE_TCS_MAX_PUTONSLIT_OFFSET cap; once sent it has made the change
and clears goalshift_pending, unless a newer change arrived meanwhile.
A correction computed from an older goal can neither use nor spend a
newer goal's allowance.

acamd.cfg.in: ACQUIRE_OFFSET_THRESHOLD=1.0, the value the deployed
configuration uses.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
…ations' into feat/sequencer-wait-for-target-offset
A target offset sent while ACAM guides only changes acamd's goal; the
guide loop's next correction makes the move. The sequencer exposed a
fixed OFFSET_SETTLE_SEC (3 s) after sending it, which could be before
the telescope had moved. It now waits until acamd's status shows the
goal change raised after the offset was sent and cleared again
(GOALSHIFT_PENDING), bounded at 30 s, after which it exposes anyway
with a warning; a cancel ends the wait. OFFSET_SETTLE_SEC is now an
extra wait after the move and defaults to 0.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-29T14:18:29.545466Z a9f4f4a PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a9f4f4a7a3

ℹ️ 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".

Comment on lines +619 to +622
bool seen_pending = false;
const bool applied = this->acam_cv.wait_for( lock,
std::chrono::duration<double>( this->fineacquire_state.move_timeout_sec ),
[this, send_time, &seen_pending]() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Record pending transitions in the subscriber before waiting

If the subscriber processes both the pending and cleared statuses before this waiter evaluates its predicate, acam_goalshift_pending retains only false and the local seen_pending can never become true. This can happen before offset_acam_goal() returns or when both notifications arrive before the waiting thread is rescheduled. A successfully completed correction then stalls fine acquisition and frame grabbing for the full default 60 seconds, potentially on multiple correction cycles. Record the raised and cleared publish times in handletopic_acamd, as the sequencer does, so completion remains observable after the transitions have passed.

Useful? React with 👍 / 👎.

Comment thread acamd/acam_interface.cpp
const int attempts = this->target.attempts;
const std::string filter = this->motion.get_current_filtername();
const std::string cover = this->motion.get_current_coverpos();
const bool goalshift_pending = this->target.goalshift_pending.load();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Serialize status capture through publication

When a snapshot or frame-grab thread publishes status concurrently with offset_goal, it can capture goalshift_pending=false here, then pause while offset_goal raises the flag and publishes true. If the first publisher resumes afterward, it publishes its stale false with a newer PUBTIME. The new sequencer predicate interprets that pair as a completed move and can expose before the guide loop has applied the offset. Common::PubSub::publish locks only the socket send, not this state capture or timestamp creation. Serialize the entire status capture/publication operation so an older captured state cannot masquerade as a later completion.

Useful? React with 👍 / 👎.

@cfremling

Copy link
Copy Markdown
Collaborator Author

Combined into #503 with the changes it depends on, so that every open PR applies to main on its own; its change is one commit of #503. Closing.

@cfremling cfremling closed this Oct 29, 2026
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