Repository navigation
Capture plan for any detector (bp.count); Phantom arms in prepare and waits for the event trigger; GUI tab - #92
Open
Anthony Sligar (sligara7) wants to merge 2 commits into
Conversation
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved GUI input, frame-selection, and acquisition-cleanup defects affect capture correctness.
Review effort: Balanced
Findings: 4
Open (4)
What changed in this PR
Adds operator-triggered Phantom capture to the beamline plans and Tomography GUI. The trigger button remains a follow-up.
Changes:
- Moves trigger and readout waits into a background acquisition task.
- Adds
phantom_capture, profile exposure, and a GUI tab. - Updates mock tests for asynchronous acquisition and cancellation.
| File | Description |
|---|---|
| tests/detectors/test_phantom.py | Tests nonblocking arming and readout cancellation. |
| src/hextools/profiles/collection.py | Exposes the capture plan by name. |
| src/hextools/gui/__main__.py | Adds the Phantom Capture tab. |
| src/hextools/detectors/phantom.py | Separates arming from background readout and adds cleanup. |
| src/hextools/capture.py | Introduces the operator-triggered capture plan. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+536
to
+537
| readout, self._readout = self._readout, None | ||
| await readout |
Comment on lines
+583
to
+588
| if self._readout is not None: | ||
| readout, self._readout = self._readout, None | ||
| readout.cancel() | ||
| with contextlib.suppress(asyncio.CancelledError): | ||
| await readout | ||
| await super().ensure_stopped() |
Anthony Sligar (sligara7)
changed the title
|
|
||
|
|
||
| def capture( | ||
| detectors: list[StandardDetector], |
Comment on lines
+701
to
+703
| def _prepare_then_read(dets, take_reading=None): | ||
| for det in dets: | ||
| yield from bps.prepare(det, info, group="capture_prepare") |
Comment on lines
+703
to
+705
| yield from bps.prepare(det, info, group="capture_prepare") | ||
| yield from bps.wait(group="capture_prepare") | ||
| yield from bps.one_shot(dets, take_reading=take_reading) |
…r; GUI tab Step 1 of the HEX GUI event-trigger work (ports the device change and phantom_capture plan from f19ea8d on planrunner-readiness, without the queueserver helper, which is step 3). PhantomAcquireLogic.start_acquiring now arms and returns once the camera waits for its trigger; the trigger/post-trigger/download waits run in the background, collected by wait_for_idle and cancelled by ensure_stopped. phantom_capture (hextools.capture) arms in free-run and waits in complete for the event trigger. The profile exposes it by name and the GUI's Tomography view gets a 'Phantom Capture' tab. Verification so far: mock tests - the two new arm/stop tests pass; the four Phantom tests that fail on main (60acf66) fail the same way here. Not yet run in the GUI or on the simulator. Assisted-by: copilot:claude-opus-4-8
Replaces the Phantom-only phantom_capture (src/hextools/capture.py, removed) with capture(detectors, num_images, exposure_time, timeout, sample_name) in hextools.tomography.flyscans. It is bp.count with a per_shot that prepares each detector for one event of num_images frames after count's staging (staging clears an earlier prepare), then takes one reading. A Phantom arms in trigger() and waits for its event trigger; other detectors take the frames straight away. Times are plain floats, so the GUI renders them as numbers, not device selectors. GUI tab renamed 'Capture'. Verified with mock tests: tests/tomography/test_capture.py runs capture on ophyd-async's SimBlobDetector and checks one event holding num_images frames; it failed (no capture) before this change. The 4 Phantom test failures are unchanged from the PR's previous commit. Assisted-by: copilot:claude-opus-4-8
Anthony Sligar (sligara7)
force-pushed
the
hex-gui-phantom-event-trigger
branch
from
October 6, 2026 20:52
1989089 to
6cb5f1c
Compare
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Phantom cleanup defects and the mismatch between promised and selected frames remain unresolved.
Review effort: Balanced
Findings: 3
Open (8)
Comment on lines
+583
to
+588
| if self._readout is not None: | ||
| readout, self._readout = self._readout, None | ||
| readout.cancel() | ||
| with contextlib.suppress(asyncio.CancelledError): | ||
| await readout | ||
| await super().ensure_stopped() |
| # in per_shot, after staging and before the reading. | ||
| def _prepare_then_read(dets, take_reading=None): | ||
| for det in dets: | ||
| yield from bps.prepare(det, info, group="capture_prepare") |
Comment on lines
+536
to
+537
| readout, self._readout = self._readout, None | ||
| await readout |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Step 1 of an operator event-trigger button in the HEX GUI. The Phantom is often triggered by the operator while watching the live image; this PR adds the plan and device behaviour the button will act on. The button itself comes in a follow-up.
Changes
PhantomAcquireLogic.start_acquiringnow arms the camera (Acquire without waiting for the put, which only completes when acquisition ends) and returns once it reports waiting for a trigger. The trigger / post-trigger / download waits run in a background task, collected bywait_for_idle(errors surface there) and cancelled byensure_stopped. ophyd-async callsstart_acquiringfrompreparefor externally triggered detectors, where waiting for the trigger hungprepare. One behaviour serves a PandA-triggered fly scan and an operator-triggered capture.hextools.tomography.flyscans.capture(any ophyd-async detector, not only the Phantom): abp.countthat prepares each detector for one event ofnum_imagesframes and takes one reading. A Phantom arms intrigger()and waits for its event trigger (timeout, default 600 s), then downloads the firstnum_imagespost-trigger frames; other detectors take the frames straight away. The prepare runs inper_shot, aftercountstages the detectors, because staging clears an earlier prepare. Times are plainfloats, so the GUI renders them as number fields.captureby name, and the GUI's Tomography view gets a Capture tab (detector list accepts any detector).Ported from earlier work on
planrunner-readiness(f19ea8d), without its queueserversend_event_triggerhelper (a later step). An earlier revision of this PR had a Phantom-only fly-basedphantom_captureinsrc/hextools/capture.py; that file is gone.How it was verified: mock
tests/tomography/test_capture.py:captureon ophyd-async'sSimBlobDetectorproduces one event whose image holdsnum_imagesframes. Observed failing beforecaptureexisted.tests/detectors/test_phantom.py: the two new tests pass (start_acquiringreturns before the trigger;ensure_stoppedcancels a readout still waiting for its trigger).main(60acf66) fail the same way here (arm_success,wait_for_idle_timeout,wait_for_idle_success,detector_full_stack), socapturehas no passing end-to-end Phantom mock test yet. No new lint errors on the files touched.Notes
devices-and-mock-tests), which carries other Phantom fixes; whichever merges second will need a merge inphantom.pyand its tests.mainitself (same test failures; type check cannot see the GUI packages), so the checks here will be red for the same reasons.Assisted-by: copilot:claude-opus-4-8