From adbd567c45807b72727d94edc6067a4e18e671f4 Mon Sep 17 00:00:00 2001 From: Dawn Date: Thu, 20 Aug 2026 20:31:59 +0200 Subject: [PATCH] feat: MotorMoveGroup semantics for both Set Energy rows The energy setpoint applies via the Change Energy button, not Enter, so it gets the full motor state machine instead of the typing-pending color: neutral tracks the readback, a user edit stages pending (and is the only thing that enables the button), the click turns moving, and readback arrival within tol returns to neutral. New SpinMoveState in motor_move_group.py adapts the pattern to a QDoubleSpinBox+button pair (colors ride the movestate QSS via the spin's internal QLineEdit); replaces the hand-rolled pending-until-click in the exp-config row and covers the Beamline setup row that had no color at all. The server's 0.0 detector-unavailable energy is kept out of the readback feed so it cannot clamp the spin to the range minimum. Co-Authored-By: Claude Fable 5 --- .../gui/panels/data_collection_settings.py | 37 +++++------ src/aare/gui/panels/monochromator_panel.py | 11 ++++ src/aare/gui/widgets/motor_move_group.py | 65 ++++++++++++++++++- .../unit/gui/test_data_collection_settings.py | 18 +++-- tests/unit/gui/test_monochromator_panel.py | 45 +++++++++++++ 5 files changed, 147 insertions(+), 29 deletions(-) diff --git a/src/aare/gui/panels/data_collection_settings.py b/src/aare/gui/panels/data_collection_settings.py index b3a57737..c9c471d2 100644 --- a/src/aare/gui/panels/data_collection_settings.py +++ b/src/aare/gui/panels/data_collection_settings.py @@ -19,12 +19,18 @@ from PySide6.QtWidgets import ( from aare.gui.panels.file_path_panel import FilePathPanel from aare.gui.panels.fluorescence_data_collection import FluorescenceDataCollectionPanel from aare.gui.panels.manual_sample_panel import ManualSamplePanel -from aare.gui.panels.monochromator_panel import ENERGY_MAX_KEV, ENERGY_MIN_KEV, ENERGY_RANGE_TIP +from aare.gui.panels.monochromator_panel import ( + ENERGY_AT_TARGET_TOL_KEV, + ENERGY_MAX_KEV, + ENERGY_MIN_KEV, + ENERGY_RANGE_TIP, +) from aare.gui.panels.raster_data_collection import RasterDataCollectionPanel from aare.gui.panels.rotation_data_collection import RotationDataCollectionPanel from aare.gui.panels.smart_rotation_panel import SimpleRotationSettingsPanel from aare.gui.scan_logic.raster_grid_manager import RasterGridManager from aare.gui.styles import BANNER_TAB_GAP +from aare.gui.widgets.motor_move_group import SpinMoveState from aare.gui.widgets.title_label import TitleLabel, tighten_column @@ -116,13 +122,14 @@ class DataCollectionSettings(QFrame): self.energy_spin.setToolTip(ENERGY_RANGE_TIP) self.energy_spin.setSingleStep(0.1) self.energy_spin.setValue(12.0) - # Pending color (same movestate QSS as the value fields) from edit - # until Change Energy is clicked — this row's apply gate is the - # button, not Enter, so the color marks "not sent yet". - self._energy_sent = self.energy_spin.value() - self.energy_spin.valueChanged.connect(self._on_energy_spin_changed) self.change_energy_button = QPushButton("Change Energy", parent=self) self.change_energy_button.clicked.connect(self._emit_change_energy) + # MotorMoveGroup semantics for the button-gated spin, same as the + # Beamline setup row: neutral tracks the readback, edit stages + # pending, the click turns moving until arrival. + self._energy_state = SpinMoveState( + self.energy_spin, self.change_energy_button, tol=ENERGY_AT_TARGET_TOL_KEV, parent=self + ) energy_row = QWidget(self) energy_layout = QHBoxLayout(energy_row) energy_layout.setContentsMargins(0, 0, 0, 0) @@ -202,24 +209,8 @@ class DataCollectionSettings(QFrame): page.setSizePolicy(QSizePolicy.Policy.Preferred, vertical) self._stack.adjustSize() - def _set_energy_pending(self, pending: bool): - # movestate lives on the spinbox's internal QLineEdit so the existing - # QLineEdit[movestate="pending"] theme rules match without new QSS. - box = self.energy_spin.lineEdit() - state = "pending" if pending else "" - if box.property("movestate") != state: - box.setProperty("movestate", state) - box.style().unpolish(box) - box.style().polish(box) - - @Slot(float) - def _on_energy_spin_changed(self, value: float): - self._set_energy_pending(value != self._energy_sent) - @Slot() def _emit_change_energy(self): - self._energy_sent = self.energy_spin.value() - self._set_energy_pending(False) self.change_energy.emit(float(self.energy_spin.value()) * 1000.0) @Slot() @@ -239,6 +230,8 @@ class DataCollectionSettings(QFrame): text = "— / —" else: text = f"{energy:.3f} keV / {s.diffraction.wavelength_angstrom:.4f} Å" + # 0.0 stays out: syncing the fallback would clamp the spin to min + self._energy_state.update_actual(energy) # Guarded: runs per DAQ tick (2 Hz), skip the repaint when unchanged. if self.current_energy_label.text() != text: self.current_energy_label.setText(text) diff --git a/src/aare/gui/panels/monochromator_panel.py b/src/aare/gui/panels/monochromator_panel.py index 15710743..8d817f6e 100644 --- a/src/aare/gui/panels/monochromator_panel.py +++ b/src/aare/gui/panels/monochromator_panel.py @@ -3,6 +3,7 @@ from PySide6.QtCore import Signal, Slot from PySide6.QtWidgets import QDoubleSpinBox, QGridLayout, QLabel, QPushButton, QWidget from aare.gui.styles import THEME_SUNRISE, status_colors +from aare.gui.widgets.motor_move_group import SpinMoveState from aare.gui.widgets.title_label import TitleLabel # TODO: placeholder range (was an arbitrary 1-30; nothing downstream validates — @@ -10,6 +11,9 @@ from aare.gui.widgets.title_label import TitleLabel # limits with the Beamline Scientist. Shared by both Set Energy rows. ENERGY_MIN_KEV = 4.0 ENERGY_MAX_KEV = 20.0 +# "arrived" window for the moving->neutral transition; per-hardware knob like +# MotorMoveGroup's tol (the mono never lands exactly on the setpoint) +ENERGY_AT_TARGET_TOL_KEV = 0.01 ENERGY_RANGE_TIP = ( f"Minimum: {ENERGY_MIN_KEV:.3f} keV\nMaximum: {ENERGY_MAX_KEV:.3f} keV\n" "Range to be confirmed with Beamline Scientist" @@ -62,6 +66,11 @@ class MonochromatorPanel(QWidget): self.change_energy_button = QPushButton("Change Energy", parent=self) self.change_energy_button.clicked.connect(self._emit_change_energy) grid_layout.addWidget(self.change_energy_button, 3, 2) + # MotorMoveGroup semantics for the button-gated spin: neutral tracks + # the readback, edit stages pending, click turns moving until arrival. + self._energy_state = SpinMoveState( + self.energy_spin, self.change_energy_button, tol=ENERGY_AT_TARGET_TOL_KEV, parent=self + ) # Fast shutter row: status left, Open/Close buttons right — same # rich-text scheme as the status bar flag so the two readouts match. @@ -126,6 +135,8 @@ class MonochromatorPanel(QWidget): text = "— / —" else: text = f"{energy:.3f} keV / {status.diffraction.wavelength_angstrom:.4f} Å" + # 0.0 stays out: syncing the fallback would clamp the spin to min + self._energy_state.update_actual(energy) # Guarded: runs per DAQ tick (2 Hz), skip the repaint when unchanged. if self.current_energy_label.text() != text: self.current_energy_label.setText(text) diff --git a/src/aare/gui/widgets/motor_move_group.py b/src/aare/gui/widgets/motor_move_group.py index 2248ad48..05e3c5c7 100644 --- a/src/aare/gui/widgets/motor_move_group.py +++ b/src/aare/gui/widgets/motor_move_group.py @@ -17,11 +17,74 @@ not registered keep their old behavior. """ from PySide6.QtCore import QEvent, QObject, QPoint, Qt, Signal, Slot -from PySide6.QtWidgets import QPushButton, QToolTip +from PySide6.QtWidgets import QDoubleSpinBox, QPushButton, QToolTip from aare.gui.widgets.number_line_edit import NumberLineEdit +class SpinMoveState(QObject): + """MotorMoveGroup's state machine for a single QDoubleSpinBox whose apply + gate is a button (the Set Energy rows): neutral tracks the readback, a + user edit stages pending (orange) and enables the button, the click turns + moving (green), and arrival within tol returns to neutral. Colors ride the + same movestate QSS via the spinbox's internal QLineEdit, so no new + styling. The caller keeps its own clicked connection for the actual send; + this object only tracks state.""" + + def __init__(self, spin: QDoubleSpinBox, button: QPushButton, tol: float, parent=None): + super().__init__(parent) + self._spin = spin + self._button = button + # "at target" window: the mono never lands exactly on the setpoint + self._tol = tol + self._state = "" # "" (neutral) | "pending" | "moving" + self._target = 0.0 + self._actual: float | None = None + # valueChanged fires for our own readback sync too; the flag is how a + # user edit is told apart (a spinbox has no textEdited-only signal for + # arrows/wheel). + self._syncing = False + spin.valueChanged.connect(self._on_value_changed) + button.clicked.connect(self._on_applied) + button.setEnabled(False) + + @Slot(float) + def _on_value_changed(self, value: float): + if self._syncing: + return + # editing back to the current readback cancels the pending change + if self._actual is not None and abs(value - self._actual) <= self._tol: + self._set_state("") + else: + self._set_state("pending") + + @Slot() + def _on_applied(self): + self._target = self._spin.value() + self._set_state("moving") + + @Slot(float) + def update_actual(self, value: float): + """Feed the readback. Neutral spins track it; pending/moving keep + showing the user's target until the move completes.""" + self._actual = value + if self._state == "moving" and abs(value - self._target) <= self._tol: + self._set_state("") + if self._state == "": + self._syncing = True + self._spin.setValue(value) + self._syncing = False + + def _set_state(self, state: str): + box = self._spin.lineEdit() + if box.property("movestate") != state: + box.setProperty("movestate", state) + box.style().unpolish(box) + box.style().polish(box) + self._state = state + self._button.setEnabled(state == "pending") + + class MotorMoveGroup(QObject): # {name: target} for every box that was pending when Move was clicked applied = Signal(dict) diff --git a/tests/unit/gui/test_data_collection_settings.py b/tests/unit/gui/test_data_collection_settings.py index 7d400c4e..32ac0170 100644 --- a/tests/unit/gui/test_data_collection_settings.py +++ b/tests/unit/gui/test_data_collection_settings.py @@ -350,24 +350,30 @@ def test_db_override_source_toggle_clears_pending(qapp, qtbot): assert w.value == 200.0 -def test_energy_spin_pending_until_change_energy(settings_panel): +def test_energy_spin_motor_move_semantics(settings_panel, daq_status_factory): # same placeholder limits as the Beamline setup row (shared constants) assert settings_panel.energy_spin.minimum() == 4.0 assert settings_panel.energy_spin.maximum() == 20.0 assert "Beamline Scientist" in settings_panel.energy_spin.toolTip() + # Same MotorMoveGroup wiring as the Beamline setup row (full state walk + # tested there); here: stage -> pending, button sends -> moving, readback + # arrival -> neutral. box = settings_panel.energy_spin.lineEdit() + assert not settings_panel.change_energy_button.isEnabled() + + settings_panel.update_daq_status(daq_status_factory()) # readback 12.0 keV + assert settings_panel.energy_spin.value() == 12.0 + settings_panel.energy_spin.setValue(12.4) assert box.property("movestate") == "pending" + assert settings_panel.change_energy_button.isEnabled() sent = [] settings_panel.change_energy.connect(sent.append) settings_panel.change_energy_button.click() assert sent and sent[-1] == pytest.approx(12400.0) - assert box.property("movestate") == "" + assert box.property("movestate") == "moving" - # dialing back to the last sent value clears without the button - settings_panel.energy_spin.setValue(12.5) - assert box.property("movestate") == "pending" - settings_panel.energy_spin.setValue(12.4) + settings_panel._energy_state.update_actual(12.398) assert box.property("movestate") == "" diff --git a/tests/unit/gui/test_monochromator_panel.py b/tests/unit/gui/test_monochromator_panel.py index 5c4bf1f5..463756ab 100644 --- a/tests/unit/gui/test_monochromator_panel.py +++ b/tests/unit/gui/test_monochromator_panel.py @@ -1,3 +1,5 @@ +import pytest + from aare.gui.panels.monochromator_panel import MonochromatorPanel @@ -46,3 +48,46 @@ def test_energy_spin_placeholder_limits(qtbot): assert panel.energy_spin.minimum() == 4.0 assert panel.energy_spin.maximum() == 20.0 assert "Beamline Scientist" in panel.energy_spin.toolTip() + + +def test_energy_spin_motor_move_semantics(qtbot, daq_status_factory): + # Same protection UX as MotorMoveGroup: the spin stages, only the button + # sends, and the color walks neutral -> pending -> moving -> neutral. + panel = MonochromatorPanel() + qtbot.addWidget(panel) + box = panel.energy_spin.lineEdit() + sent = [] + panel.change_energy.connect(sent.append) + assert not panel.change_energy_button.isEnabled() # nothing staged yet + + status = daq_status_factory() + panel.update_daq_status(status) # readback 12.0 keV syncs the neutral spin + assert panel.energy_spin.value() == 12.0 + assert not box.property("movestate") + + panel.energy_spin.setValue(12.4) # user staging + assert box.property("movestate") == "pending" + assert panel.change_energy_button.isEnabled() + + panel.energy_spin.setValue(12.0) # back to the readback cancels + assert box.property("movestate") == "" + assert not panel.change_energy_button.isEnabled() + + panel.energy_spin.setValue(12.4) + panel.change_energy_button.click() + assert sent and sent[-1] == pytest.approx(12400.0) # keV -> eV on emit + assert box.property("movestate") == "moving" + assert not panel.change_energy_button.isEnabled() + + # travelling: the spin keeps showing the target, and the server's 0.0 + # detector-unavailable fallback must not disturb the state either + status.diffraction = status.diffraction.model_copy(update={"energy_keV": 0.0}) + panel.update_daq_status(status) + assert panel.energy_spin.value() == 12.4 + assert box.property("movestate") == "moving" + + # arrival within tol returns to neutral and readback tracking resumes + status.diffraction = status.diffraction.model_copy(update={"energy_keV": 12.396}) + panel.update_daq_status(status) + assert box.property("movestate") == "" + assert panel.energy_spin.value() == pytest.approx(12.396)