diff --git a/docs/plans/ids-camera-manual-exposure.md b/docs/plans/ids-camera-manual-exposure.md index b05cf79c..fced10cd 100644 --- a/docs/plans/ids-camera-manual-exposure.md +++ b/docs/plans/ids-camera-manual-exposure.md @@ -1,8 +1,8 @@ -# Plan: Add manual exposure / auto-gain toggle control to IDSCamera +# Plan: Manual exposure / auto-gain control for IDSCamera + xrayeye widget knobs -Status: **planned, not yet implemented** (`IDSCamera` is used in production during -beamtimes; implementation should happen in its own change, reviewed and tested outside -a live beamtime). +Status: **planned, not yet implemented** (`IDSCamera` and the `OMNY_XRayEye` GUI widget +are both used in production during beamtimes; implementation should happen as its own +change, reviewed and tested outside a live beamtime). ## Context @@ -16,67 +16,147 @@ not wired up or exposed to BEC clients: - `Camera.set_auto_gain(enable)` — `base_integration/camera.py:237-246` (auto-gain toggle) None of these three are called anywhere, and none are on `IDSCamera.USER_ACCESS`, so a -BEC client can't reach them today. The sibling `AlliedVisionAravisCamera` class already -has the equivalent pattern wired up for `exposure_time`/`gain` as plain -`get_x`/`set_x` USER_ACCESS methods (`allied_vision_aravis_camera.py:222-238`) — this -plan mirrors that same style for IDS, plus adds the auto-exposure/auto-gain enable -toggles IDS has (and Allied Vision doesn't). +BEC client can't reach them today. + +`OMNY_XRayEye` (`csaxs_bec/bec_widgets/widgets/xray_eye/x_ray_eye.py`) is the GUI panel +for the flomni/LamNI/OMNY X-ray-eye alignment camera (`cam_xeye`, an `IDSCamera` +device). It should get exposure/auto-gain control knobs too, using the same +cached/event-driven pattern it already uses for its "Camera running" toggle, rather than +polling the device with RPC calls. Per user decisions (2026-09-11): cover exposure control + the auto-gain enable/disable -toggle only (no manual gain *value* — the driver has no SDK call for that yet); expose -everything as plain `USER_ACCESS` methods (matching the existing Allied Vision pattern, -not a new `Signal`); and leave connect-time default behavior untouched (no new -constructor/config kwarg — the camera keeps whatever state `is_ResetToDefault()` leaves -it in unless a client explicitly calls the new methods). +toggle only (no manual gain *value* — the driver has no SDK call for that yet); leave +connect-time default behavior untouched (no new constructor/config kwarg — the camera +keeps whatever state `is_ResetToDefault()` leaves it in unless a client/GUI explicitly +calls the new methods); and be careful to use cached readings wherever possible rather +than polling the hardware for GUI display. + +## Design: `Cpt(Signal, kind=Kind.config)`, not plain methods only + +`OMNY_XRayEye`'s existing "Camera running" toggle is the template for a control knob +that both *writes* device state and *reflects* it back without polling: + +- **Write**: `self.dev.get(CAMERA[0]).live_mode_enabled.put(enabled)` (`x_ray_eye.py:882`) + — RPC `.put()` on an ophyd `Cpt(Signal, kind=Kind.config)` on the device + (`ids_camera.py:59-65`). +- **Read (cached/event-driven, no polling)**: the widget subscribes once, at + construction, to `MessageEndpoints.device_read_configuration(CAMERA[0])` + (`x_ray_eye.py:293-295`) — BEC pushes **all** `Kind.config` signals of the device + together in one message whenever any of them changes. `getting_camera_status` + (`x_ray_eye.py:886-892`) reads `data["signals"]["cam_xeye_live_mode_enabled"]["value"]` + and updates the toggle via the `blockSignals(True)/.checked = .../blockSignals(False)` + idiom (to avoid re-triggering the write handler). + +This matches the repo's established convention for Signal-backed device state +(confirmed against `slit_control.py`, `sample_storage.py`, `tomo_params.py`: +`read(cached=True)` for polled numeric readbacks, and +`bec_dispatcher.connect_slot(..., device_read_configuration(...))` for `Kind.config` +signals, which is cheaper still — zero RPC round-trips, pure pub/sub). So exposure_time +and the two auto-enable flags should be `Cpt(Signal, kind=Kind.config)` on `IDSCamera` +(not plain methods only), with thin `USER_ACCESS` wrapper methods kept on top for +ipython/scripted use — mirroring how `start_live_mode()`/`stop_live_mode()` are just +wrappers around `live_mode_enabled.put()`. **No code should poll `get_exposure_time()` / +`get_gain()` RPCs from a `QTimer` loop.** ## Changes ### 1. `csaxs_bec/devices/ids_cameras/ids_camera.py` -Add a new `############## Exposure / Gain ##############` section (mirroring -`allied_vision_aravis_camera.py:222-238`), placed after the live-mode/ROI methods and -before `############## User Interface Methods ##############`: +Add three new components, next to `live_mode_enabled` (`ids_camera.py:59-65`): ```python -def get_exposure_time(self) -> float: - """Get the current exposure time in milliseconds.""" - return self.cam.exposure_time +exposure_time = Cpt( + Signal, name="exposure_time", value=0.0, + doc="Camera exposure time (ms).", kind=Kind.config, +) +auto_exposure_enabled = Cpt( + Signal, name="auto_exposure_enabled", value=True, + doc="Enable/disable auto-exposure (auto-shutter).", kind=Kind.config, +) +auto_gain_enabled = Cpt( + Signal, name="auto_gain_enabled", value=True, + doc="Enable/disable auto-gain.", kind=Kind.config, +) +``` -def set_exposure_time(self, value: float) -> None: - """Set the exposure time in milliseconds. Does not itself disable auto-exposure -- - call set_auto_exposure_enabled(False) first, or the driver will keep overriding it.""" +In `__init__`, alongside the existing `live_mode_enabled.subscribe(...)` +(`ids_camera.py:127`): + +```python +self.exposure_time.subscribe(self._on_exposure_time_changed, run=False) +self.auto_exposure_enabled.subscribe(self._on_auto_exposure_enabled_changed, run=False) +self.auto_gain_enabled.subscribe(self._on_auto_gain_enabled_changed, run=False) +``` + +New callbacks + thin `USER_ACCESS` wrappers, in a new +`############## Exposure / Gain ##############` section (placed after the live-mode/ROI +methods and before `############## User Interface Methods ##############`): + +```python +def _on_exposure_time_changed(self, *args, value, **kwargs): self.cam.exposure_time = value +def _on_auto_exposure_enabled_changed(self, *args, value, **kwargs): + self.cam.set_auto_shutter(bool(value)) + +def _on_auto_gain_enabled_changed(self, *args, value, **kwargs): + self.cam.set_auto_gain(bool(value)) + +def get_exposure_time(self) -> float: + """Get the current exposure time (ms), from the cached Signal value.""" + return float(self.exposure_time.get()) + +def set_exposure_time(self, value: float) -> None: + """Set the exposure time (ms). Does not itself disable auto-exposure -- + call set_auto_exposure_enabled(False) first, or the driver will keep overriding it.""" + self.exposure_time.put(value) + def set_auto_exposure_enabled(self, enable: bool) -> None: - """Enable or disable the camera's auto-exposure (auto-shutter).""" - self.cam.set_auto_shutter(enable) + self.auto_exposure_enabled.put(bool(enable)) def set_auto_gain_enabled(self, enable: bool) -> None: - """Enable or disable the camera's auto-gain.""" - self.cam.set_auto_gain(enable) + self.auto_gain_enabled.put(bool(enable)) ``` (Confirm the SDK's exposure unit — `is_Exposure`/`IS_EXPOSURE_CMD_*` is documented in milliseconds; adjust the docstring if inspection shows otherwise.) -Add the four new method names to `USER_ACCESS` (`ids_camera.py:67-76`): +Add the four wrapper names to `USER_ACCESS` (`ids_camera.py:67-76`): `"get_exposure_time"`, `"set_exposure_time"`, `"set_auto_exposure_enabled"`, `"set_auto_gain_enabled"`. +In `on_connected()` (`ids_camera.py:283-288`), after `self.cam.on_connect()`, seed the +`exposure_time` signal from the real hardware value once, so the GUI shows a real number +immediately on connect instead of the `0.0` placeholder: + +```python +self.exposure_time.put(self.cam.exposure_time) +``` + +(This round-trips through `_on_exposure_time_changed`, which writes the same value back +to the driver — a harmless one-time no-op write, same idiom as the existing +`self.live_mode_enabled.put(bool(self._inputs.get("live_mode", False)))` line right +below it.) `auto_exposure_enabled`/`auto_gain_enabled` are **not** seeded from hardware — +there is no SDK query for current auto-shutter/auto-gain state in this driver (only +enable-setters), so, like `live_mode_enabled`, they start at their declared default +(`True`, matching the uEye SDK's typical post-reset default) and only reflect reality +once explicitly set through this API. + No changes needed in `base_integration/camera.py` — `exposure_time`, `set_auto_shutter`, `set_auto_gain` already exist there exactly as needed. ### 2. `csaxs_bec/devices/sim/sim_cameras.py` `SimIDSCamera` replaces `self.cam` with `_SimIDSBackend` (line 211), which currently has -no `exposure_time` attribute and no `set_auto_shutter`/`set_auto_gain` methods — calling -the new `IDSCamera` methods against a sim device would raise `AttributeError`. Add to +no `exposure_time` attribute and no `set_auto_shutter`/`set_auto_gain` methods — +`on_connected()`'s `self.exposure_time.put(self.cam.exposure_time)` and the new +subscribe callbacks would raise `AttributeError` against `SimIDSCamera` otherwise. Add to `_SimIDSBackend` (mirroring `_SimAlliedVisionBackend.exposure_time`, lines 397-403): ```python def __init__(self, ...): ... - self._exposure_time = 10000.0 # ms, matches _SimAlliedVisionBackend's default + self._exposure_time = 10000.0 # ms self._auto_exposure = True self._auto_gain = True @@ -97,15 +177,11 @@ def set_auto_gain(self, enable: bool): ### 3. Tests — `tests/tests_devices/test_ids_camera.py` -Mirror `test_allied_vision_aravis_camera.py:124-139`'s pass-through style (that file's -fixture mocks the whole `camera.cam`, same as `test_ids_camera.py`'s fixture), e.g.: - ```python def test_get_set_exposure_time(ids_camera): - ids_camera.cam.exposure_time = 5000.0 - assert ids_camera.get_exposure_time() == 5000.0 ids_camera.set_exposure_time(1234.5) assert ids_camera.cam.exposure_time == 1234.5 + assert ids_camera.get_exposure_time() == 1234.5 def test_set_auto_exposure_enabled(ids_camera): ids_camera.set_auto_exposure_enabled(False) @@ -114,20 +190,141 @@ def test_set_auto_exposure_enabled(ids_camera): def test_set_auto_gain_enabled(ids_camera): ids_camera.set_auto_gain_enabled(False) ids_camera.cam.set_auto_gain.assert_called_once_with(False) + +def test_on_connected_seeds_exposure_time(ids_camera): + ids_camera.cam.on_connect = mock.Mock() + ids_camera.cam.exposure_time = 4200.0 + ids_camera.on_connected() + assert ids_camera.get_exposure_time() == 4200.0 ``` +## Phase 2: GUI control knobs in `OMNY_XRayEye` + +### 4. `csaxs_bec/bec_widgets/widgets/xray_eye/x_ray_eye.py` + +**Imports**: add `QDoubleSpinBox` to the `qtpy.QtWidgets` import block (`x_ray_eye.py:15-29`) +— only `QSpinBox` is imported today. + +**New widgets in `_init_ui`**, extending the existing `switch_grid` (`x_ray_eye.py:368-404`) +with a 3rd row (same `QGridLayout` the shutter/camera-running and smear toggles already +share, so columns keep lining up): + +```python +self.auto_exposure_label = QLabel("Auto exposure", parent=self) +self.auto_exposure_toggle = ToggleSwitch(parent=self) +self.auto_exposure_toggle.checked = True +self.auto_exposure_toggle.enabled.connect(self.auto_exposure_enabled_changed) + +self.auto_gain_label = QLabel("Auto gain", parent=self) +self.auto_gain_toggle = ToggleSwitch(parent=self) +self.auto_gain_toggle.checked = True +self.auto_gain_toggle.enabled.connect(self.auto_gain_enabled_changed) + +switch_grid.addWidget(self.auto_exposure_label, 2, 1, _right_vcenter) +switch_grid.addWidget(self.auto_exposure_toggle, 2, 2, Qt.AlignmentFlag.AlignVCenter) +switch_grid.addWidget(self.auto_gain_label, 2, 3, _right_vcenter) +switch_grid.addWidget(self.auto_gain_toggle, 2, 4, Qt.AlignmentFlag.AlignVCenter) +``` + +An exposure-time spinbox, added as its own small form near the step-size form +(`x_ray_eye.py:424-439`), mirroring that form's `QGridLayout` structure: + +```python +exposure_form = QGridLayout() +self.exposure_time_spin = QDoubleSpinBox(parent=self) +self.exposure_time_spin.setRange(0.01, 1000.0) # ms; confirm real driver range +self.exposure_time_spin.setDecimals(2) +self.exposure_time_spin.setSuffix(" ms") +self.exposure_time_spin.setEnabled(False) # auto-exposure starts enabled +self.exposure_time_spin.editingFinished.connect(self.exposure_time_submitted) +exposure_form.addWidget(QLabel("Exposure time", parent=self), 0, 0) +exposure_form.addWidget(self.exposure_time_spin, 0, 1) +self.control_panel_layout.addLayout(exposure_form) +``` + +Using `editingFinished` (fires once, on Enter/focus-loss) rather than `valueChanged` +(fires on every arrow-button click/scroll tick) is the throttling choice here — the +"don't hammer the device" concern applied to a manual-entry control instead of a poll +loop. + +**New handlers**, alongside `camera_running_enabled`/`opening_shutter` +(`x_ray_eye.py:875-906`): + +```python +@SafeSlot(bool) +def auto_exposure_enabled_changed(self, enabled: bool): + if self._manual_toggle_blocked_by_queue(): + logger.warning("Ignoring auto-exposure toggle while scan queue is busy.") + return + self.auto_exposure_toggle.blockSignals(True) + self.dev.get(CAMERA[0]).auto_exposure_enabled.put(enabled) + self.auto_exposure_toggle.checked = enabled + self.exposure_time_spin.setEnabled(not enabled) + self.auto_exposure_toggle.blockSignals(False) + +@SafeSlot(bool) +def auto_gain_enabled_changed(self, enabled: bool): + if self._manual_toggle_blocked_by_queue(): + logger.warning("Ignoring auto-gain toggle while scan queue is busy.") + return + self.auto_gain_toggle.blockSignals(True) + self.dev.get(CAMERA[0]).auto_gain_enabled.put(enabled) + self.auto_gain_toggle.checked = enabled + self.auto_gain_toggle.blockSignals(False) + +def exposure_time_submitted(self): + self.dev.get(CAMERA[0]).exposure_time.put(self.exposure_time_spin.value()) +``` + +**Extend the existing `getting_camera_status`** (`x_ray_eye.py:886-892`) — no new Redis +subscription needed, since `exposure_time`/`auto_exposure_enabled`/`auto_gain_enabled` +are `Kind.config` signals on the same device, so they already arrive in the same +`device_read_configuration(CAMERA[0])` message `live_mode_enabled` uses: + +```python +@SafeSlot(dict, dict) +def getting_camera_status(self, data, meta): + signals = data.get("signals") + live_mode_enabled = signals.get(f"{CAMERA[0]}_live_mode_enabled").get("value") + self.camera_running_toggle.blockSignals(True) + self.camera_running_toggle.checked = live_mode_enabled + self.camera_running_toggle.blockSignals(False) + + auto_exp = signals.get(f"{CAMERA[0]}_auto_exposure_enabled") + if auto_exp is not None: + enabled = bool(auto_exp.get("value")) + self.auto_exposure_toggle.blockSignals(True) + self.auto_exposure_toggle.checked = enabled + self.exposure_time_spin.setEnabled(not enabled) + self.auto_exposure_toggle.blockSignals(False) + + auto_gain = signals.get(f"{CAMERA[0]}_auto_gain_enabled") + if auto_gain is not None: + self.auto_gain_toggle.blockSignals(True) + self.auto_gain_toggle.checked = bool(auto_gain.get("value")) + self.auto_gain_toggle.blockSignals(False) + + exposure_time = signals.get(f"{CAMERA[0]}_exposure_time") + if exposure_time is not None: + self.exposure_time_spin.blockSignals(True) + self.exposure_time_spin.setValue(float(exposure_time.get("value"))) + self.exposure_time_spin.blockSignals(False) +``` + +The `is not None` guards are defensive (harmless if this ever runs against an older +`IDSCamera` revision without these signals) and cost nothing. + ## Verification -- Run the existing + new unit tests: `pytest tests/tests_devices/test_ids_camera.py -v` - (and `test_allied_vision_aravis_camera.py` to confirm no regressions from the shared - pattern). -- Manual/integration check against real hardware (if available during a beamtime/test - session): from a BEC client, on a connected IDS camera device (e.g. `dev.cam_xeye`): - ```python - dev.cam_xeye.set_auto_exposure_enabled(False) - dev.cam_xeye.set_exposure_time(20000) # e.g. 20 ms - print(dev.cam_xeye.get_exposure_time()) - dev.cam_xeye.set_auto_gain_enabled(False) - ``` - and confirm the live image brightness responds to the manual exposure value instead of - drifting on its own. +- Unit tests: `pytest tests/tests_devices/test_ids_camera.py -v`. GUI widget tests (if + any exist under `tests/` for `x_ray_eye.py` — check before implementing) should cover + `getting_camera_status` updating the three new widgets from a synthetic message dict, + and `exposure_time_submitted`/`auto_*_enabled_changed` calling the right RPC `.put()`. +- Manual/simulation check: run against `SimIDSCamera` (no real hardware needed) — + confirm toggling "Auto exposure" off enables the spinbox, entering a value + Enter (or + focus-loss) calls `exposure_time.put(...)`, and the value round-trips back through + `getting_camera_status` into the spinbox display without any added polling. +- Manual/integration check on real hardware (during a non-live test session): open the + xrayeye widget against a connected `cam_xeye`, toggle auto-exposure off, set an + exposure time, and confirm the live image brightness responds and stays stable + (doesn't drift back, confirming auto-exposure is actually off).