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()