[cfremling]

On main, flexure_set runs in parallel with the slew, so flexure is compensated for the previous pointing, and it is never refreshed during an NEXP>1 series or for a REPEAT.

flexure_set now runs for every exposure, in a detached thread that nothing waits for:

• as each exposure is initiated, after any slew, acquisition (automatic, manual or none) and target offset;
• at each shutter close that another sub-exposure follows, for every expose command with NEXP>1, from camerad's shutter report (not a pause). The shutter closes between every two integrations, also with frame transfer. INREADOUT is not used: with frame transfer two readouts can give it one edge;
• for a REPEAT, once camerad is ready.

A flexured failure is broadcast and no longer stops the sequence. camerad records FLEX_X/Y/Z at the end of each exposure, with SHUTTIME, since the compensation can change after an exposure starts. An exposure shorter than the rest of the flexure actuator move (a 4-channel move takes 1.6 s) records the earlier positions; only a blocking compensation would avoid that.

Check: "flexure compensator set for ..." once per expose command, plus once per extra sub-exposure.

The previous version was tested on sky using branch cf/summit-20261030 on 2026-11-03 (20 re-applies between sub-exposures); this version is not yet tested on sky.

Jira: PAL-152.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

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

Review	Status	Commit	Review trigger
📝 Code Review	⚠️ Failed 2026-11-05T18:56:03.112886Z We couldn’t complete this review. Comment @codex review to try again.	239adf5	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]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

[astronomerdave]

Since claude wrote the code, I asked claude to review the PR:

---

The problem it fixes is real

On main, flexure_set runs alongside move_to_target, so it computes the correction before the telescope reaches the new target. flexured only refreshes its telemetry copy when that copy is more than 10 s old (main:flexured/flexure_interface.h:158-167). The correction is absolute: the configured default position plus a computed delta (main:flexured/flexure_interface.cpp:463-469). So re-applying it between sub-exposures is safe and doesn't add up over repeated calls.

Findings on its own merits

1. New way to hang on an NEXP=1 exposure (most serious). At :550, the EXPOSE wait is now cleared only by a PIXELCOUNT message from the first half of a readout. Nothing else clears it on the normal path, and the wait at :970-973 has no timeout. If the first PIXELCOUNT the sequencer receives shows ≥50%, the sequence stalls until someone aborts. That can happen if UDP drops the early messages, or if a fast binned readout reports in coarse steps.

• camerad sends each PIXELCOUNT from its own detached thread (main:camerad/astrocam.cpp:241-245), so even their order isn't guaranteed.
• The stated reason, "the tail of the previous target's readout," only arises with #508's early target pick. In #507 alone, the sequencer waits for READY before moving on (:1004-1007). This is #508's change shipped inside #507.

2. Readout counting mixes two message channels. It is armed by camerad's pubsub status (:59-66) and completed by the UDP PIXELCOUNT strings (:562-564). The comment at sequence.h:440 says reports arrive in publish order, but that only holds within the pubsub stream.

• camerad already publishes INREADOUT in the same message as SHUTTEROPEN (main:camerad/astrocam.cpp:36,39), from its state monitor (:376-380). That flag is set at the start of every readout (:2241-2242, :317-318).
• Counting rising edges of INREADOUT would use one ordered stream, with no string parsing and no 50% guess.
• Cost: INREADOUT means "any controller is reading out or doing a frame transfer" (astrocam.h:792-799), and it doesn't give the "at least half a readout left" margin. The PR's check after the move (:2658) already covers an overlap with the next exposure, so the margin isn't essential.

3. The telemetry check in flexure_reapply tests the sequencer's copy, not flexured's. It checks the sequencer's tcsinfo (:2638), but flexured computes from its own copy, which can be up to 10 s old (main:flexured/flexure_interface.h:161). While the telescope is tracking, that is harmless. The check just doesn't guarantee what its message implies.

4. The actuators can move after the next shutter opens. If a readout is shorter than the flexured move, the actuators keep moving into the first seconds of the next sub-exposure. The PR logs this as a warning and carries on (:2656-2659). I can't verify how long the move takes; the PR's "~11 s" is an assumption, unverified.

5. Possible lost time per target. For targets the telescope slews to, the flexure move now overlaps only the slit move and the offset settle (:911, :937). Any part of the move longer than those is added before every exposure. Before, it was hidden behind the slew.

6. Abort can now block for up to 20 s instead of 5 s (:2882).

7. Dead error branches. flexure_set either returns NO_ERROR or throws (:2542-2581). So the != NO_ERROR branches after the new join (:937) and in the REPEAT path (:3039) can never run.

8. Merge process. The PR asks for "Create a merge commit", but the repo allows squash merges only (allow_merge_commit:false, checked via the GitHub API). Accepting it means changing repo settings so a stacked PR can merge. The alternative is to squash #507 and have #508 rebased onto it.

Against the design (seq.txt)

• Moving flexure out of the parallel worker block for slewed targets is right on correctness. "Proper order" (seq.txt:116-119) requires it. It does cost some of the efficiency the block exists for.
• An option that keeps the parallel block intact: start flexure_set with the other workers, have it wait for move_to_target to finish, then compensate during ACAM and fine acquisition. Join it before the exposure, as the PR already does.
  • Gained: the move time is hidden again.
  • Lost: the correction is computed one acquisition (about a minute or two) earlier. The NEXP>1 re-apply partly covers that.
  • This depends on the flexure actuators not affecting ACAM or slicecam images. That is an assumption, unverified; you know the optics.
• The between-sub-exposure re-apply has the sequencer inferring camerad's internal phase from progress strings. That's a weak form of orchestration. Using camerad's published INREADOUT state (finding 2) fits "one daemon per subsystem, sequencer orchestrates" better.

What I'd ask the author for

• Drop or move to #508 the :550 gate.
• Count readouts from INREADOUT edges.
• Say how long the flexured move takes, so the lost time per target (finding 5) can be judged.

[cfremling]

Thanks for the review. Rewritten as one commit. Per finding:

1. The EXPOSE-wait gate was not needed; removed. The EXPOSE wait is unchanged.

2. INREADOUT edges fail with frame transfer: the next integration starts as soon as the transfer ends, so back-to-back readouts keep INREADOUT true and two readouts give one edge. The re-apply is made instead at each shutter close camerad reports (not a pause), the event that ends an integration. The shutter closes between every two integrations, also with frame transfer, so nothing merges. No progress strings are parsed, and nothing waits on it: a missed report skips one re-apply.

3. Telemetry check removed; flexured checks its own telemetry.

4-6. flexure_set now runs detached and nothing waits for it, so no time is lost; abort is 5 s again. Measured: a 4-channel comp takes 1.6 s, the slit move 5.5-5.8 s, a readout ~21.5 s (2x3 binning). Where nothing covers the flexure actuator move it can overlap the start of an exposure (intended). camerad now records FLEX_X/Y/Z at the end of each exposure, with SHUTTIME.

7. flexure_set returns ERROR instead of throwing; a flexured failure no longer stops the sequence (deliberate).

8. Normal squash merge.

Worker + join alternative: the pointing is final only when the exposure is initiated. Acquisition can be manual, failed or off, and the telescope can still move after move_to_target; with no acquisition, a join would also delay the exposure.

Not yet tested on sky in this form. Should not be on main until tested. A second Claude 'review' of this could be useful although likely to identify mostly non-issues (or even more useful at this stage, a human review).