From 69ed67714e1f40896f56c90c3660d1d4407ef98a Mon Sep 17 00:00:00 2001 From: sligara7 Date: Tue, 27 Oct 2026 16:42:54 -0400 Subject: [PATCH 1/2] Add move_slits plan and a Slits tab in the GUI Moves a slit pair by its center line and gap, as the slits are usually moved: only the values given move, a center move keeps the gap, a gap move is symmetric about the center, and relative=True shifts from the current positions. Gaps move before centers since both drive the same blades. Relative moves read the blade readbacks: bps.rd on the derived gap/center signals raises TypeError (readbacks are not locatable). Assisted-by: copilot:claude-opus-5-5 --- docs/cli.md | 9 +++ docs/profile.md | 2 +- src/hextools/gui/__main__.py | 9 ++- .../photon_delivery_system/__init__.py | 3 +- src/hextools/photon_delivery_system/slits.py | 65 +++++++++++++++++++ tests/gui/test_plan_widget_slits.py | 40 ++++++++++++ tests/photon_delivery_system/test_slits.py | 55 +++++++++++++++- tests/test_profile_namespace.py | 1 + 8 files changed, 180 insertions(+), 4 deletions(-) create mode 100644 tests/gui/test_plan_widget_slits.py diff --git a/docs/cli.md b/docs/cli.md index d6f3078..4f40077 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -121,6 +121,15 @@ RE(bps.mv(sample_tower.x1, 1, sample_tower.y, 2)) # several motors at once RE(bps.mvr(sample_tower.x1, 0.5)) # relative move ``` +Slits (`a_slits`, `f_slits`, `edxd_slits`) move by the center line and gap of each +blade pair; only the values given move, and a center move keeps the gap: + +```python +RE(move_slits(a_slits, horizontal_center=0.5)) # shift the pair +RE(move_slits(a_slits, horizontal_gap=2.0, vertical_gap=1.0)) # open about the center +RE(move_slits(f_slits, vertical_center=-0.1, relative=True)) # nudge by 0.1 mm +``` + Every filter defined in the filter configuration file is placed in the namespace by name (e.g. `filter1_upstream`) and can be moved with `bps.mv`. diff --git a/docs/profile.md b/docs/profile.md index cb6c002..f18e792 100644 --- a/docs/profile.md +++ b/docs/profile.md @@ -42,7 +42,7 @@ The import block pulls in three kinds of names: `PhantomDetector`, `GeRMDetector`, `NSLS2StorageRing`, filters). - **Plans** — everything a user should be able to run at the prompt: `bp` and `bps` (bluesky's built-in plans and stubs), `move_motor`, - `change_energy`, `change_beam_mode`, the tomography plans (`tomo_flyscan`, + `move_slits`, `change_energy`, `change_beam_mode`, the tomography plans (`tomo_flyscan`, `tomo_alignment_scan`, `tomo_1d_step_scan`, `tomo_2d_step_scan`, `tomo_nd_step_scan`) and the EDXD plans (`edxd_scan`, `edxd_count`, `edxd_grid_scan`, `edxd_2theta_tilt`, `edxd_calib_scan`, diff --git a/src/hextools/gui/__main__.py b/src/hextools/gui/__main__.py index 2718200..da6f7ff 100644 --- a/src/hextools/gui/__main__.py +++ b/src/hextools/gui/__main__.py @@ -96,7 +96,13 @@ SampleTower, move_motor, ) -from hextools.photon_delivery_system import DCLM, Shutter, Slits, change_energy +from hextools.photon_delivery_system import ( + DCLM, + Shutter, + Slits, + change_energy, + move_slits, +) from hextools.photon_delivery_system.dclm import change_beam_mode from hextools.settings import ( apply_device_settings, @@ -393,6 +399,7 @@ def __init__(self, re_client: RunEngineClient | RunEngine, *args, **kwargs): # IPython vs. Queue Server). plan_tabs = QTabWidget() plan_tabs.addTab(QtPlanWidget(re_client, move_motor), "Motors") + plan_tabs.addTab(QtPlanWidget(re_client, move_slits), "Slits") plan_tabs.addTab(QtPlanWidget(re_client, change_beam_mode), "Change Beam Mode") plan_tabs.addTab(QtPlanWidget(re_client, change_energy), "Change Energy") plan_tabs.addTab(QtPlanWidget(re_client, reset_scan_id), "Reset Scan ID") diff --git a/src/hextools/photon_delivery_system/__init__.py b/src/hextools/photon_delivery_system/__init__.py index 0b51490..fee3e53 100644 --- a/src/hextools/photon_delivery_system/__init__.py +++ b/src/hextools/photon_delivery_system/__init__.py @@ -9,7 +9,7 @@ ensure_shutter_open, ensure_shutter_state, ) -from .slits import Slits +from .slits import Slits, move_slits __all__ = [ "Shutter", @@ -18,6 +18,7 @@ "load_filters", "FilterPosition", "Slits", + "move_slits", "ensure_shutter_closed", "ensure_shutter_open", "ensure_shutter_state", diff --git a/src/hextools/photon_delivery_system/slits.py b/src/hextools/photon_delivery_system/slits.py index 2910eda..eec4e3b 100644 --- a/src/hextools/photon_delivery_system/slits.py +++ b/src/hextools/photon_delivery_system/slits.py @@ -3,6 +3,7 @@ import asyncio from functools import partial +import bluesky.plan_stubs as bps from ophyd_async.core import ( AsyncMovable, StandardReadable, @@ -198,3 +199,67 @@ async def set( ] async for update in forward_watcher_updates(centers, self.name, combine=False): yield update + + +def move_slits( + slits: Slits, + horizontal_center: float | None = None, + vertical_center: float | None = None, + horizontal_gap: float | None = None, + vertical_gap: float | None = None, + relative: bool = False, +): + """Move a pair of slit blades by their center line and gap. + + Only the values given move; the others stay where they are. A center move + shifts both blades of the pair together, keeping the gap, and a gap move + opens or closes both blades about the center line. + + Parameters + ---------- + slits : Slits + The slits to move. + horizontal_center : float, optional + Center of the inboard/outboard pair, in mm. + vertical_center : float, optional + Center of the bottom/top pair, in mm. + horizontal_gap : float, optional + Gap between the inboard and outboard blades, in mm. + vertical_gap : float, optional + Gap between the bottom and top blades, in mm. + relative : bool, default False + Treat the given values as shifts from the current positions. + + Raises + ------ + ValueError + If no center or gap is given. + """ + horizontal = (slits.inboard, slits.outboard) + vertical = (slits.bottom, slits.top) + # A gap and a center on one axis drive the same two blades, so gaps go first + stages = [ + [ + (slits.horizontal_gap, horizontal_gap, horizontal, True), + (slits.vertical_gap, vertical_gap, vertical, True), + ], + [ + (slits.horizontal_center, horizontal_center, horizontal, False), + (slits.vertical_center, vertical_center, vertical, False), + ], + ] + if all(value is None for stage in stages for _, value, _, _ in stage): + raise ValueError("Give at least one center or gap to move the slits.") + for stage in stages: + args = [] + for signal, value, (low_motor, high_motor), is_gap in stage: + if value is None: + continue + if relative: + # bps.rd cannot read the derived gap/center: readbacks aren't locatable + low = yield from bps.rd(low_motor.user_readback) + high = yield from bps.rd(high_motor.user_readback) + value += high - low if is_gap else (low + high) / 2 + args += [signal, value] + if args: + yield from bps.mv(*args) diff --git a/tests/gui/test_plan_widget_slits.py b/tests/gui/test_plan_widget_slits.py new file mode 100644 index 0000000..ccab5c7 --- /dev/null +++ b/tests/gui/test_plan_widget_slits.py @@ -0,0 +1,40 @@ +import os + +import pytest + +os.environ.setdefault("QT_QPA_PLATFORM", "offscreen") +pytest.importorskip("qtpy") +pytest.importorskip("bluesky_widgets") + +from bluesky import RunEngine # noqa: E402 +from IPython.core.interactiveshell import InteractiveShell # noqa: E402 +from qtpy.QtWidgets import QApplication # noqa: E402 + +from hextools.gui.plan_widget import QtPlanWidget # noqa: E402 +from hextools.photon_delivery_system import Slits, move_slits # noqa: E402 + + +def test_slits_form_offers_device_optional_centers_gaps_and_relative(): + app = QApplication.instance() or QApplication([]) + InteractiveShell.instance() + widget = QtPlanWidget(RunEngine({}, call_returns_result=True), move_slits) + try: + scalars = { + name: (kind, required) for name, kind, _, required in widget._scalar_fields + } + for name in ( + "horizontal_center", + "vertical_center", + "horizontal_gap", + "vertical_gap", + ): + assert scalars[name] == (float, False) + assert scalars["relative"] == (bool, False) + devices = {field["name"]: field for field in widget._device_fields} + assert ( + devices["slits"]["types"] == (Slits,) or Slits in devices["slits"]["types"] + ) + finally: + widget.close() + widget.deleteLater() + app.processEvents() diff --git a/tests/photon_delivery_system/test_slits.py b/tests/photon_delivery_system/test_slits.py index d107015..73ffdfb 100644 --- a/tests/photon_delivery_system/test_slits.py +++ b/tests/photon_delivery_system/test_slits.py @@ -3,7 +3,7 @@ from bluesky.run_engine import RunEngine from ophyd_async.core import callback_on_mock_put, init_devices, set_mock_value -from hextools.photon_delivery_system.slits import Slits +from hextools.photon_delivery_system.slits import Slits, move_slits @pytest.fixture @@ -113,3 +113,56 @@ async def test_slits_set_reports_blade_progress_to_watchers(slits: Slits): slits.bottom.name, slits.top.name, } <= set(updates) + + +def _re_slits(blades: tuple[float, float, float, float]) -> Slits: + """Mock slits under the RE loop; blades at (inboard, outboard, bottom, top).""" + with init_devices(mock=True): + slits = Slits("XF:TEST:", 1) # type: ignore + for motor, value in zip(_motors(slits), blades, strict=True): + callback_on_mock_put( + motor.user_setpoint, + lambda value, motor=motor, **_: set_mock_value(motor.user_readback, value), + ) + set_mock_value(motor.user_readback, value) + return slits + + +def _motors(slits: Slits): + return (slits.inboard, slits.outboard, slits.bottom, slits.top) + + +async def _blades(slits: Slits) -> list[float]: + return [await m.user_readback.get_value() for m in _motors(slits)] + + +async def test_move_slits_moves_only_the_given_values(RE: RunEngine): + slits = _re_slits((-1.0, 3.0, -2.0, 2.0)) + + RE(move_slits(slits, horizontal_center=2.0)) + + # Both horizontal blades shift by +1 (gap 4 kept); the vertical pair stays put. + assert await _blades(slits) == pytest.approx([0.0, 4.0, -2.0, 2.0]) + + +async def test_move_slits_sets_gap_before_center(RE: RunEngine): + slits = _re_slits((-1.0, 1.0, 0.0, 0.0)) + + RE(move_slits(slits, horizontal_gap=4.0, horizontal_center=5.0)) + + assert (await _blades(slits))[:2] == pytest.approx([3.0, 7.0]) + + +async def test_move_slits_relative_shifts_from_current(RE: RunEngine): + slits = _re_slits((-1.0, 3.0, -2.0, 2.0)) + + RE(move_slits(slits, vertical_center=0.5, horizontal_gap=-1.0, relative=True)) + + # Horizontal gap 4 -> 3 about center 1; vertical pair shifted up by 0.5. + assert await _blades(slits) == pytest.approx([-0.5, 2.5, -1.5, 2.5]) + + +def test_move_slits_needs_a_value(RE: RunEngine): + slits = _re_slits((0.0, 0.0, 0.0, 0.0)) + with pytest.raises(ValueError, match="at least one"): + RE(move_slits(slits)) diff --git a/tests/test_profile_namespace.py b/tests/test_profile_namespace.py index 0a0b061..004d511 100644 --- a/tests/test_profile_namespace.py +++ b/tests/test_profile_namespace.py @@ -15,6 +15,7 @@ "bps", "bpp", "move_motor", + "move_slits", "edxd_grid_scan", "GeRMStatsDataLogic", "ProgressBarManager", From f556c4e0f1f49738d9f9c2d5cb55248c4afe794d Mon Sep 17 00:00:00 2001 From: sligara7 Date: Tue, 27 Oct 2026 16:51:49 -0400 Subject: [PATCH 2/2] Slits: build gap/center from the motors so plans can locate them The derived gap/center signals took the motors' user_readback signals as raw inputs. A derived signal is only locatable when its raw inputs are, and readback signals are not, so bps.rd and bps.mvr raised TypeError on them. With the Motor devices as raw inputs they locate (the transform still receives readback values), and move_slits uses bps.mv / bps.mvr instead of recomputing the kinematics (Jakub, #115). New test pins bps.rd and bps.mvr on every slit axis; observed failing before the fix. Assisted-by: copilot:claude-opus-5-5 --- src/hextools/photon_delivery_system/slits.py | 45 ++++++++------------ tests/photon_delivery_system/test_slits.py | 23 ++++++++++ 2 files changed, 40 insertions(+), 28 deletions(-) diff --git a/src/hextools/photon_delivery_system/slits.py b/src/hextools/photon_delivery_system/slits.py index eec4e3b..c2206f8 100644 --- a/src/hextools/photon_delivery_system/slits.py +++ b/src/hextools/photon_delivery_system/slits.py @@ -44,32 +44,32 @@ def __init__(self, prefix: str, name: str = ""): partial(self._set_gap, self.inboard, self.outboard), derived_units="mm", derived_precision=3, - low=self.inboard.user_readback, - high=self.outboard.user_readback, + low=self.inboard, + high=self.outboard, ) self.horizontal_center = derived_signal_rw( self._get_center, partial(self._set_center, self.inboard, self.outboard), derived_units="mm", derived_precision=3, - low=self.inboard.user_readback, - high=self.outboard.user_readback, + low=self.inboard, + high=self.outboard, ) self.vertical_gap = derived_signal_rw( self._get_gap, partial(self._set_gap, self.bottom, self.top), derived_units="mm", derived_precision=3, - low=self.bottom.user_readback, - high=self.top.user_readback, + low=self.bottom, + high=self.top, ) self.vertical_center = derived_signal_rw( self._get_center, partial(self._set_center, self.bottom, self.top), derived_units="mm", derived_precision=3, - low=self.bottom.user_readback, - high=self.top.user_readback, + low=self.bottom, + high=self.top, ) super().__init__(name=name) @@ -235,31 +235,20 @@ def move_slits( ValueError If no center or gap is given. """ - horizontal = (slits.inboard, slits.outboard) - vertical = (slits.bottom, slits.top) # A gap and a center on one axis drive the same two blades, so gaps go first stages = [ + [(slits.horizontal_gap, horizontal_gap), (slits.vertical_gap, vertical_gap)], [ - (slits.horizontal_gap, horizontal_gap, horizontal, True), - (slits.vertical_gap, vertical_gap, vertical, True), - ], - [ - (slits.horizontal_center, horizontal_center, horizontal, False), - (slits.vertical_center, vertical_center, vertical, False), + (slits.horizontal_center, horizontal_center), + (slits.vertical_center, vertical_center), ], ] - if all(value is None for stage in stages for _, value, _, _ in stage): + if all(value is None for stage in stages for _, value in stage): raise ValueError("Give at least one center or gap to move the slits.") + move = bps.mvr if relative else bps.mv for stage in stages: - args = [] - for signal, value, (low_motor, high_motor), is_gap in stage: - if value is None: - continue - if relative: - # bps.rd cannot read the derived gap/center: readbacks aren't locatable - low = yield from bps.rd(low_motor.user_readback) - high = yield from bps.rd(high_motor.user_readback) - value += high - low if is_gap else (low + high) / 2 - args += [signal, value] + args = [ + x for signal, value in stage if value is not None for x in (signal, value) + ] if args: - yield from bps.mv(*args) + yield from move(*args) diff --git a/tests/photon_delivery_system/test_slits.py b/tests/photon_delivery_system/test_slits.py index 73ffdfb..dceef77 100644 --- a/tests/photon_delivery_system/test_slits.py +++ b/tests/photon_delivery_system/test_slits.py @@ -124,6 +124,8 @@ def _re_slits(blades: tuple[float, float, float, float]) -> Slits: motor.user_setpoint, lambda value, motor=motor, **_: set_mock_value(motor.user_readback, value), ) + # A motor at rest: bps.mvr moves relative to the setpoint, not the readback + set_mock_value(motor.user_setpoint, value) set_mock_value(motor.user_readback, value) return slits @@ -166,3 +168,24 @@ def test_move_slits_needs_a_value(RE: RunEngine): slits = _re_slits((0.0, 0.0, 0.0, 0.0)) with pytest.raises(ValueError, match="at least one"): RE(move_slits(slits)) + + +@pytest.mark.parametrize("axis, low, high, gap, center", _AXES) +async def test_slits_gap_and_center_work_with_locating_plan_stubs( + RE: RunEngine, axis: str, low: str, high: str, gap: str, center: str +): + # bps.rd and bps.mvr locate() a movable; a derived signal is only locatable + # when its raw inputs are, so the gap/center must be built from the motors. + slits = _re_slits((-1.0, 3.0, -1.0, 3.0)) + readings = {} + + def plan(): + readings["gap"] = yield from bps.rd(getattr(slits, gap)) + readings["center"] = yield from bps.rd(getattr(slits, center)) + yield from bps.mvr(getattr(slits, center), 0.5) + + RE(plan()) + + assert readings == {"gap": pytest.approx(4.0), "center": pytest.approx(1.0)} + assert await getattr(slits, center).get_value() == pytest.approx(1.5) + assert await getattr(slits, gap).get_value() == pytest.approx(4.0)