From 7c1363c855ff142f01c519da167af767ba8df0ea Mon Sep 17 00:00:00 2001 From: menzel Date: Mon, 31 Aug 2026 16:44:41 +0200 Subject: [PATCH] fix(canon): warn when stream_start="stage" can never fire The scan server excludes async devices with readoutPriority on_request or continuous from its stage instruction, while pre_scan, complete and unstage still go to every enabled device. Such a device therefore never sees on_stage, so stream_start="stage" starts no worker, buffers no frame, and reports nothing wrong -- the first symptom is an empty grab long after the scan. The example device config ships readoutPriority: on_request alongside stream_start as a documented option, so the broken combination is reachable without doing anything unusual. The device now warns about it at connect, and the config, README and commissioning guide say so. The readout priority is only knowable through the device manager, which exists on the device server but not in a bare instantiation, so failing to determine it is treated as "nothing to say" rather than an error. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01KLnmUurqcNd1FiDY5M2uZr --- csaxs_bec/device_configs/canon_crn300.yaml | 7 ++- csaxs_bec/devices/canon/COMMISSIONING.md | 2 +- csaxs_bec/devices/canon/README.md | 6 ++ csaxs_bec/devices/canon/canon_ptz.py | 39 +++++++++++- tests/tests_devices/test_canon_device.py | 70 ++++++++++++++++++++++ 5 files changed, 121 insertions(+), 3 deletions(-) diff --git a/csaxs_bec/device_configs/canon_crn300.yaml b/csaxs_bec/device_configs/canon_crn300.yaml index 1ee750f0..d7e1f9e7 100644 --- a/csaxs_bec/device_configs/canon_crn300.yaml +++ b/csaxs_bec/device_configs/canon_crn300.yaml @@ -29,7 +29,12 @@ ptz_cam: # and this firmware implements no MJPEG endpoint. acquisition_mode: "jpeg_poll" poll_rate: 2.0 # [Hz] jpeg_poll only - stream_start: "off" # off | connect | stage (when to start acquisition) + # off | connect | stage (when to start acquisition) + # NOTE "stage" only works for a device that actually takes part in staging. The + # scan server excludes readoutPriority on_request/continuous devices from its stage + # instruction, so "stage" together with the readoutPriority below is a silent + # no-op -- the device warns about it at connect. Use "connect" for those. + stream_start: "off" buffer_maxlen: 64 # ring-buffer depth (compressed frames) # still_url: http://192.168.0.100/-wvhttp-01-/image.cgi # jpeg_poll URL override deviceTags: diff --git a/csaxs_bec/devices/canon/COMMISSIONING.md b/csaxs_bec/devices/canon/COMMISSIONING.md index 1d78ea4e..68d13e53 100644 --- a/csaxs_bec/devices/canon/COMMISSIONING.md +++ b/csaxs_bec/devices/canon/COMMISSIONING.md @@ -82,7 +82,7 @@ behaviour is what you are implicitly testing in Stage 1–3. |------|------|-----------------|--------| | `wait_for_connection()` | on device load | Open XC session (`open.cgi`) **and claim control** (`claim.cgi`), then wait for signals | 🟡 [R1](#r1)[R2](#r2)[R6b](#r6b) | | `on_connected()` | after connect | Log camera identity; seed pan/tilt/zoom/focus readbacks; start stream if `stream_start: connect` | 🟡 [R7](#r7) | -| `on_stage()` | scan start | Start stream if `stream_start: stage` | 🟢 | +| `on_stage()` | scan start | Start stream if `stream_start: stage` -- **not called at all** for `readoutPriority: on_request`/`continuous`, which the scan server excludes from staging; the device warns at connect | 🟢 | | `on_unstage()` | scan end | No-op (stream persists across scans by design) | 🟢 | | `on_stop()` | abort | Halt all motion (best-effort) | 🟡 [R9](#r9) | | `on_destroy()` | device teardown | Stop stream worker; yield control + close session (`yield.cgi`/`close.cgi`) | 🟡 [R2](#r2) | diff --git a/csaxs_bec/devices/canon/README.md b/csaxs_bec/devices/canon/README.md index a7bc5e37..4c4da426 100644 --- a/csaxs_bec/devices/canon/README.md +++ b/csaxs_bec/devices/canon/README.md @@ -230,6 +230,12 @@ Set `stream_start: connect` (or `stage`) in `deviceConfig` to start acquisition automatically. Buffering the compressed bytes (not decoded arrays) keeps memory small and spends no CPU decoding frames nobody grabs; decoding happens only on grab. +> **`stage` needs a staged device.** The scan server excludes devices with +> `readoutPriority: on_request` or `continuous` from its stage instruction, so +> `on_stage` never runs for them and `stream_start: stage` quietly does nothing. +> The example config ships `on_request`; use `stream_start: connect` there. The +> device logs a warning at connect if the combination is configured. + Frame *delivery into a scan* is the remaining Phase-2 work. The intent is the area-detector convention — file-writer references via `SUB_FILE_EVENT` for full frames, `PreviewSignal` for live preview. Only the `preview` component is declared today; nothing diff --git a/csaxs_bec/devices/canon/canon_ptz.py b/csaxs_bec/devices/canon/canon_ptz.py index c6869cb4..92bef32f 100644 --- a/csaxs_bec/devices/canon/canon_ptz.py +++ b/csaxs_bec/devices/canon/canon_ptz.py @@ -353,9 +353,46 @@ class CanonPTZCamera(PSIDeviceBase): logger.warning(f"{self.name}: initial readback for {ax.name} failed: {exc}") if self._stream_start == "connect": self.start_stream() + self._warn_if_stage_start_unreachable() + + # NOTE The scan server filters async devices with these readout priorities out of + # its stage instruction (bec_server ScanStubs.stage), while pre_scan, complete and + # unstage still go to every enabled device. A device with one of these priorities + # therefore never sees on_stage, and stream_start="stage" would silently do nothing. + UNSTAGED_READOUT_PRIORITIES = ("on_request", "continuous") + + def _warn_if_stage_start_unreachable(self) -> None: + """Warn when stream_start="stage" can never fire for this device. + + The combination is reachable straight from the example device config, and it + fails silently: no worker is started, no frame is ever buffered, and the first + symptom is an empty grab long after the scan. Reading the readout priority + needs the device manager, which is only populated on the device server, so a + failure to determine it is not itself an error. + """ + if self._stream_start != "stage": + return + try: + priority = str(self.device_manager.devices[self.name].readout_priority) + except Exception: # pylint: disable=broad-except + return + # ReadoutPriority is a str-enum, so str() may render as either "on_request" or + # "ReadoutPriority.ON_REQUEST" depending on the Python version. + if not priority.lower().endswith(self.UNSTAGED_READOUT_PRIORITIES): + return + logger.warning( + f"{self.name}: stream_start='stage' has no effect with " + f"readoutPriority='{priority}' -- devices with that priority are excluded " + "from staging, so on_stage never runs and no frames are acquired. Use " + "stream_start='connect', or give the device a staged readout priority." + ) def on_stage(self) -> None: - """Ensure the persistent stream is running if configured to start at stage.""" + """Ensure the persistent stream is running if configured to start at stage. + + Only reached when the device takes part in staging; see + :meth:`_warn_if_stage_start_unreachable`. + """ if self._stream_start == "stage": self.start_stream() diff --git a/tests/tests_devices/test_canon_device.py b/tests/tests_devices/test_canon_device.py index c8563a0a..865782be 100644 --- a/tests/tests_devices/test_canon_device.py +++ b/tests/tests_devices/test_canon_device.py @@ -245,3 +245,73 @@ def test_real_transport_reads_credentials_from_env(): def test_destroy_closes_transport(cam): cam.destroy() assert not cam.transport.connected + + +def _dm_with_priority(priority): + """Minimal device-manager stand-in exposing one device's readout priority.""" + entry = mock.MagicMock() + entry.readout_priority = priority + dm = mock.MagicMock() + dm.devices = {"canon_test": entry} + return dm + + +@pytest.mark.parametrize("priority", ["on_request", "continuous", "ReadoutPriority.ON_REQUEST"]) +def test_stage_start_warns_when_device_is_never_staged(priority): + """stream_start='stage' is a silent no-op for devices excluded from staging.""" + device = CanonPTZCamera( + name="canon_test", + simulation=True, + stream_start="stage", + device_manager=_dm_with_priority(priority), + ) + try: + with mock.patch("csaxs_bec.devices.canon.canon_ptz.logger") as mock_logger: + device._warn_if_stage_start_unreachable() + assert mock_logger.warning.call_count == 1 + assert "stream_start='stage' has no effect" in mock_logger.warning.call_args[0][0] + finally: + device.destroy() + + +@pytest.mark.parametrize("priority", ["async", "monitored", "baseline"]) +def test_stage_start_is_silent_for_staged_priorities(priority): + """A device that does get staged must not be warned about.""" + device = CanonPTZCamera( + name="canon_test", + simulation=True, + stream_start="stage", + device_manager=_dm_with_priority(priority), + ) + try: + with mock.patch("csaxs_bec.devices.canon.canon_ptz.logger") as mock_logger: + device._warn_if_stage_start_unreachable() + mock_logger.warning.assert_not_called() + finally: + device.destroy() + + +@pytest.mark.parametrize("stream_start", ["off", "connect"]) +def test_no_stage_warning_for_other_stream_start_modes(stream_start): + """The warning is specific to stream_start='stage'.""" + device = CanonPTZCamera( + name="canon_test", + simulation=True, + stream_start=stream_start, + device_manager=_dm_with_priority("on_request"), + ) + try: + with mock.patch("csaxs_bec.devices.canon.canon_ptz.logger") as mock_logger: + device._warn_if_stage_start_unreachable() + mock_logger.warning.assert_not_called() + finally: + device.destroy() + + +def test_stage_start_check_survives_missing_device_manager(): + """Off the device server there is no device manager; that is not an error.""" + device = CanonPTZCamera(name="canon_test", simulation=True, stream_start="stage") + try: + device._warn_if_stage_start_unreachable() # must not raise + finally: + device.destroy()