From d7ee8ef03166bd903f758dcaf1af76fe78bd8226 Mon Sep 17 00:00:00 2001 From: menzel Date: Tue, 1 Sep 2026 15:11:47 +0200 Subject: [PATCH] fix(falcon): set the layout the IOC can actually resolve, and verify it _initialize_detector_backend put a bare "layout.xml" into the HDF5 plugin. The IOC resolves that relative to its own working directory, where no such file exists: the cSAXS SITORO IOC ships the layout as cfg/layout.xml (installed at /ioc/X12SA-CPCL-FALCONX1/cfg/layout.xml), and that is what the IOC configures at startup. BEC was overwriting a correct value with a broken one on every device init. The failure was silent and badly signposted. An unreadable layout does not raise; the plugin simply refuses to open the output file later, reporting "Error opening file ..., status=-1" with the actual cause buried in XMLErrorMsg_RBV. Because a caget taken before a BEC device reload showed the IOC's own valid value, the two readings disagreed and the layout looked innocent. The default is now cfg/layout.xml, overridable per deployment with an xml_file_name key in deviceConfig ("" selects the plugin's built-in layout), and on_connected reads XMLValid_RBV back and logs an error naming the file and the IOC's message if the layout was rejected. The existing on_connected test asserted the broken value; it now asserts the default constant. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01KLnmUurqcNd1FiDY5M2uZr --- csaxs_bec/devices/epics/falcon_csaxs.py | 56 +++++++++++++++++++++- tests/tests_devices/test_falcon_csaxs.py | 59 +++++++++++++++++++++++- 2 files changed, 112 insertions(+), 3 deletions(-) diff --git a/csaxs_bec/devices/epics/falcon_csaxs.py b/csaxs_bec/devices/epics/falcon_csaxs.py index 23a27cf3..521aad58 100644 --- a/csaxs_bec/devices/epics/falcon_csaxs.py +++ b/csaxs_bec/devices/epics/falcon_csaxs.py @@ -19,6 +19,13 @@ from csaxs_bec.devices.utils.utils import fetch_scan_info 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 +82,17 @@ class FalconcSAXS(PSIDeviceBase, FalconControl): file_event = Cpt(FileEventSignal, name="file_event") + def __init__(self, *args, xml_file_name: str | None = None, **kwargs): + """ + Args: + 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__(*args, **kwargs) + def on_init(self) -> None: """Initialize Falcon Sitoro detector""" self._lock = threading.RLock() @@ -82,6 +100,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 +259,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 +279,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..97ce39b0 100644 --- a/tests/tests_devices/test_falcon_csaxs.py +++ b/tests/tests_devices/test_falcon_csaxs.py @@ -15,6 +15,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 +85,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 +304,59 @@ 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()