sequencerd: skip re-acquisition when the same target is repeated#457
sequencerd: skip re-acquisition when the same target is repeated#457cfremling wants to merge 4 commits into
Conversation
When the observer presses GO on the target already being observed (a repeat), the sequencer should NOT re-acquire -- it should proceed straight to the "continue" prompt and expose. On main it re-ran the full ACAM astrometric acquire (and fine acquire) every time. Cause: move_to_target already skips the telescope move on a repeat (matches last_ra_hms/last_dec_dms), but do_acam_acquire and do_slicecam_fineacquire are called unconditionally in the run loop -- there is no matching same-target guard on acquisition. (The vestigial `last_target` member was written but never read; the only same-target logic ever present was a commented-out *move* guard.) Fix: detect a repeat by comparing target coordinates to the last target we ACQUIRED (new last_acquire_ra_hms/last_acquire_dec_dms, set after a completed acquisition). On a repeat, skip ACAM acquire, fine acquire, and the science re-offset (target_offset), then wait for the observer to continue before exposing. Guiding state is intentionally NOT consulted -- whether ACAM is still guiding is left to the observer. `clearlasttarget` clears the new state to force a fresh acquisition on the same target. Note: the science re-offset is also skipped on a repeat. is_fineacquire_locked is telemetry-driven and would otherwise be stale-true on a repeat (move_to_target returns before do_acam_stop), so target_offset would double-apply the science offset. Skipping it on a repeat avoids that. Not compiled in this environment -- please build on the instrument. Edits mirror the existing move_to_target coordinate compare/assign, so types are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![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: b41774acb9
ℹ️ 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".
| if ( !repeat_target && this->is_fineacquire_locked.load() && | ||
| this->target_offset() == ERROR ) { |
There was a problem hiding this comment.
When the next observation row has the same RA/DECL but different OFFSET_RA/OFFSET_DEC (for example an offset/dither sequence), repeat_target is true because it only compares coordinates, so this new guard skips target_offset() and exposes at the previous offset. Those offsets are per-target fields read from the database and target_offset() applies the current row's values, so the repeat shortcut should not suppress the offset unless the offset values also match the cached acquisition state.
Useful? React with 👍 / 👎.
Per review feedback: the repeat-target guard keyed only on RA/DEC, so two target-list entries sharing the same acquisition coordinates but with DIFFERENT science offsets (e.g. one offset star used to reach several science targets) or a different slit angle would be wrongly treated as a repeat and skipped. Those are NOT the same target and must be fully re-acquired (ACAM acquire, move, slit off-target, fine acquire if enabled, science offset). Extend the repeat key to require equal offset_ra, offset_dec and slitangle in addition to RA/DEC; remember all five after a completed acquisition. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Updated per review feedback (thanks @codex): the repeat-target key now also requires equal offset_ra, offset_dec, and slitangle, not just RA/DEC. Two list entries that share acquisition coordinates but differ in science offset (e.g. one offset star reused for several science targets) or slit angle are correctly treated as different targets and fully re-acquired (ACAM acquire, move, slit off-target, fine-acquire if enabled, science offset). Identity is remembered after a completed acquisition; |
|
To use Codex here, create an environment for this repo. |
…cience) On a repeat the slit must not move at all -- it should stay at the science (EXPOSE) position. The VSM_ACQUIRE worker already self-skips on a repeat (it returns early when target coords match last_ra_hms/last_dec_dms), so the slit was never moved to ACQUIRE. But the post-acquire block still called slit_set(VSM_EXPOSE) unconditionally, which re-commands the slit (and broadcasts "moving slit") even though it is already at EXPOSE. Gate the whole post-acquire block (science offset + slit_set EXPOSE) on !repeat_target so that on a repeat the slit receives no command and stays put at the science position. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ope offset) Correction to the previous commit, which skipped slit_set(VSM_EXPOSE) entirely on a repeat. slit_set(VSM_EXPOSE) sets BOTH the slit width (target.slitwidth_req) and the EXPOSE offset, so skipping it would also drop a slit-width change made for the repeat exposure. The EXPOSE offset is unchanged on a repeat, so re-asserting it does not translate the slit (stays at the science position) -- only the width is (re)applied. So: keep slit_set(VSM_EXPOSE) unconditional (honors width changes, no translation) and gate only target_offset (the telescope science offset) on !repeat_target to avoid double-applying it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
Issue
When the observer presses GO on the target already being observed (a repeat of the same target), the sequencer re-runs the full ACAM astrometric acquisition (and fine acquire). Observed on-sky 2026-06-17. The expected behavior is that a repeat does no re-acquisition and proceeds straight to the continue prompt to start the exposure.
Cause
move_to_targetalready treats a repeat correctly — it skips the telescope move when the coordinates match the last target (last_ra_hms/last_dec_dms) and returns beforedo_acam_stop. But the run loop callsdo_acam_acquire()anddo_slicecam_fineacquire()unconditionally for every non-cal target — there is no matching same-target guard on acquisition. So the move is skipped but the acquisition is not.The
last_targetmember is declared, written twice, and never read (vestigial); git history shows the only same-target code that ever existed was a commented-out move guard — the acquire path was never guarded.Intended behavior
On a repeat of the same target (same coordinates as the last target we acquired):
clearlasttargetforces a fresh acquisition on the same target.Fix
last_acquire_ra_hms/last_acquire_dec_dms, set after a completed acquisition (distinct from the move guard'slast_ra_hms/last_dec_dmsto avoid the move-runs-before-acquire ordering trap).repeat_target= coordinates equal the last acquired target.target_offset(science offset), keepslit_set(VSM_EXPOSE), andwait_for_userbefore exposing.clearlasttargetclears the new state.Why the science offset is also skipped on a repeat:
is_fineacquire_lockedis telemetry-driven and would be stale-true on a repeat (move_to_targetreturns beforedo_acam_stop), sotarget_offset()would double-apply the science offset. Skipping it on a repeat prevents that.Testing
Not compiled in this environment — please build on the instrument. The new compares/assignments mirror the existing
move_to_targetcoordinate logic exactly (samemysqlx::stringtypes), so the change is type-equivalent to working code. Suggested check: GO a target → acquires; GO the same target again → logs"repeat of same target -- skipping re-acquisition", no acam/fine activity, prompts continue;clearlasttargetthen GO → re-acquires.🤖 Generated with Claude Code