diff --git a/csaxs_bec/devices/epics/falcon_csaxs.py b/csaxs_bec/devices/epics/falcon_csaxs.py index 23a27cf3..d818cf10 100644 --- a/csaxs_bec/devices/epics/falcon_csaxs.py +++ b/csaxs_bec/devices/epics/falcon_csaxs.py @@ -4,6 +4,7 @@ import enum import os import threading import time +from typing import TYPE_CHECKING from typing import Literal from bec_lib.file_utils import get_full_path @@ -17,8 +18,18 @@ from ophyd_devices.interfaces.base_classes.psi_device_base import PSIDeviceBase from csaxs_bec.devices.utils.utils import fetch_scan_info +if TYPE_CHECKING: # pragma: no cover + from bec_lib.devicemanager import DeviceManagerBase, ScanInfo + logger = bec_logger.logger +# NOTE Layout of the written HDF5 file, resolved by the IOC relative to its own working +# directory. The cSAXS SITORO IOC ships it as cfg/layout.xml (installed at +# /ioc/X12SA-CPCL-FALCONX1/cfg/layout.xml). Override per deployment with the +# xml_file_name key in deviceConfig, or set it to "" to fall back to the plugin's +# built-in layout. +DEFAULT_XML_FILE_NAME = "cfg/layout.xml" + class FalconError(Exception): """Base class for exceptions in this module.""" @@ -75,6 +86,36 @@ class FalconcSAXS(PSIDeviceBase, FalconControl): file_event = Cpt(FileEventSignal, name="file_event") + def __init__( + self, + name: str, + prefix: str = "", + scan_info: "ScanInfo | None" = None, + device_manager: "DeviceManagerBase | None" = None, + xml_file_name: str | None = None, + **kwargs, + ): + """ + Every deviceConfig key must appear here by name. The device server builds the + init kwargs by intersecting the config keys with the class signature's named + parameters, so a bare *args/**kwargs signature silently drops 'prefix' and the + device is built with empty PV names. + + Args: + name: device name. + prefix: EPICS prefix, e.g. 'X12SA-SITORO:'. + scan_info: scan info object. + device_manager: BEC device manager. + xml_file_name: HDF5 layout description, as the IOC resolves it. Defaults to + DEFAULT_XML_FILE_NAME; pass "" to use the plugin's built-in layout. + """ + self._xml_file_name_config = ( + DEFAULT_XML_FILE_NAME if xml_file_name is None else xml_file_name + ) + super().__init__( + name=name, prefix=prefix, scan_info=scan_info, device_manager=device_manager, **kwargs + ) + def on_init(self) -> None: """Initialize Falcon Sitoro detector""" self._lock = threading.RLock() @@ -82,6 +123,7 @@ class FalconcSAXS(PSIDeviceBase, FalconControl): self._value_pixel_per_buffer = 20 self._queue_size = 2000 self._full_path = "" + self._xml_file_name = self._xml_file_name_config self.scan_parameters: ScanServerScanInfo | None = None def on_connected(self): @@ -240,8 +282,13 @@ class FalconcSAXS(PSIDeviceBase, FalconControl): # Enable HDF5 plugin self.hdf5.enable.put(1) - # Use layout.xml file for cSAXS Falcon. FIXME:Should be checked if IOC runs on different host. - self.hdf5.xml_file_name.put("layout.xml") + # Layout of the written file. The IOC resolves this relative to its own working + # directory, so a bare "layout.xml" does not resolve -- the cSAXS SITORO IOC keeps + # it under cfg/. An unreadable layout does not fail here: the plugin refuses to + # open the output file later, reporting only "Error opening file ... status=-1", + # so the value is verified against XMLValid_RBV right away. + self.hdf5.xml_file_name.put(self._xml_file_name) + self._check_xml_layout() # TODO Check if lazy open is needed and wanted! self.hdf5.lazy_open.put(1) @@ -255,6 +302,34 @@ class FalconcSAXS(PSIDeviceBase, FalconControl): # Set nd_array mode to 1: This means segmentation into Spectra within EPICS, 1 is activate, 0 is deactivate self.nd_array_mode.put(1) + def _check_xml_layout(self, timeout: float = 1.0) -> bool: + """Warn if the IOC could not read the configured HDF5 layout description. + + Args: + timeout (float): Time in seconds to allow the IOC to parse the file. + + Returns: + bool: True if the IOC reports the layout as valid. + """ + if not self._xml_file_name: + return True # built-in layout requested; nothing to validate + deadline = time.time() + timeout + valid = "" + while time.time() < deadline: + valid = str(self.hdf5.xml_valid.get()) + if valid.lower() in ("yes", "1"): + return True + time.sleep(0.05) + logger.error( + f"{self.name}: the IOC cannot use the HDF5 layout " + f"{self._xml_file_name!r} (XMLValid={valid!r}, " + f"error={self.hdf5.xml_error_msg.get()!r}). Writing will fail with an " + "'Error opening file' until this is corrected. The path is resolved by the " + 'IOC, not by BEC; set xml_file_name in deviceConfig, or "" for the ' + "plugin's built-in layout." + ) + return False + def on_stage(self): """ This method is called when the detector is staged for acquisition. diff --git a/tests/tests_devices/test_falcon_csaxs.py b/tests/tests_devices/test_falcon_csaxs.py index c7ca8950..156544f9 100644 --- a/tests/tests_devices/test_falcon_csaxs.py +++ b/tests/tests_devices/test_falcon_csaxs.py @@ -1,4 +1,5 @@ # pylint: skip-file +import inspect import os import threading from typing import Generator @@ -15,6 +16,7 @@ from ophyd_devices.tests.utils import patched_device from csaxs_bec.devices.epics.falcon_csaxs import ( ACQUIRESTATUS, + DEFAULT_XML_FILE_NAME, FalconcSAXS, FalconError, MappingSource, @@ -84,7 +86,7 @@ def test_falcon_on_connected(mock_det: FalconcSAXS): # Backend default PV values assert falcon.hdf5.enable.get() == "1" # Enabled - assert falcon.hdf5.xml_file_name.get() == "layout.xml" + assert falcon.hdf5.xml_file_name.get() == DEFAULT_XML_FILE_NAME assert falcon.hdf5.lazy_open.get() == "1" # Enabled assert falcon.hdf5.temp_suffix.get() == "" assert falcon.hdf5.queue_size.get() == falcon._queue_size @@ -303,3 +305,88 @@ def test_falcon_prime_restores_config_on_failure(mock_det: FalconcSAXS): assert falcon.pixel_advance_mode.get() == TriggerSource.GATE assert falcon.ignore_gate.get() == 0 assert falcon.pixels_per_buffer.get() == 20 + + +def test_falcon_sets_the_ioc_resolvable_layout(mock_det: FalconcSAXS): + """A bare 'layout.xml' does not resolve for the IOC; cfg/layout.xml does.""" + falcon = mock_det + falcon.hdf5.xml_file_name.put("something-else") + + with mock.patch.object(falcon, "_check_xml_layout", return_value=True): + falcon._initialize_detector_backend() + + assert falcon.hdf5.xml_file_name.get() == DEFAULT_XML_FILE_NAME + assert DEFAULT_XML_FILE_NAME == "cfg/layout.xml" + + +def test_falcon_xml_file_name_from_device_config(): + """deviceConfig can point at a different layout, or disable it with ''.""" + with patched_device( + FalconcSAXS, + name="falcon", + prefix="X12SA-SITORO:", + device_manager=DMMock(), + xml_file_name="", + _mock_pv_initial_value=1, + ) as dev: + try: + assert dev._xml_file_name == "" + # Nothing to validate when the built-in layout is requested. + assert dev._check_xml_layout(timeout=0.01) is True + finally: + dev.destroy() + + +def test_falcon_warns_when_the_ioc_rejects_the_layout(mock_det: FalconcSAXS): + """An unreadable layout is reported at connect, not as a write error much later.""" + falcon = mock_det + + with ( + mock.patch.object(falcon.hdf5.xml_valid, "get", return_value="No"), + mock.patch.object(falcon.hdf5.xml_error_msg, "get", return_value="cannot be opened"), + mock.patch("csaxs_bec.devices.epics.falcon_csaxs.logger") as mock_logger, + ): + assert falcon._check_xml_layout(timeout=0.05) is False + assert mock_logger.error.call_count == 1 + assert "layout" in mock_logger.error.call_args[0][0].lower() + + +def test_falcon_accepts_a_valid_layout_silently(mock_det: FalconcSAXS): + """A layout the IOC accepts produces no error.""" + falcon = mock_det + + with ( + mock.patch.object(falcon.hdf5.xml_valid, "get", return_value="Yes"), + mock.patch("csaxs_bec.devices.epics.falcon_csaxs.logger") as mock_logger, + ): + assert falcon._check_xml_layout(timeout=0.05) is True + mock_logger.error.assert_not_called() + + +def test_falcon_init_names_every_device_config_key(): + """The device server drops config keys that are not named parameters. + + bec_server's devicemanager builds the init kwargs by intersecting the deviceConfig + keys with the class signature's named parameters. A *args/**kwargs signature + therefore silently discards 'prefix', and every PV is built without it -- which + presents as 'failed to connect to all signals' rather than as a config error. + """ + params = inspect.signature(FalconcSAXS).parameters + for key in ("name", "prefix", "scan_info", "device_manager", "xml_file_name"): + assert key in params, f"{key} must be a named parameter or the device server drops it" + + +def test_falcon_keeps_the_configured_prefix(): + """A prefix passed as the device server passes it must reach the signals.""" + with patched_device( + FalconcSAXS, + name="falcon", + prefix="X12SA-SITORO:", + device_manager=DMMock(), + _mock_pv_initial_value=1, + ) as dev: + try: + assert dev.prefix == "X12SA-SITORO:" + assert dev.hdf5.file_path.pvname.startswith("X12SA-SITORO:") + finally: + dev.destroy()