From 04a30ea04c80038b3a54b92403c7e579daf23350 Mon Sep 17 00:00:00 2001 From: appel_c Date: Thu, 8 Jan 2026 09:21:11 +0100 Subject: [PATCH] refactor(ophyd-validation): Allow option to keep device visible after successful validation --- .../device_form_dialog.py | 62 ++++++++------- .../ophyd_validation/ophyd_validation.py | 77 +++++++++++++++---- tests/unit_tests/test_device_manager_view.py | 9 ++- 3 files changed, 101 insertions(+), 47 deletions(-) diff --git a/bec_widgets/applications/views/device_manager_view/device_manager_dialogs/device_form_dialog.py b/bec_widgets/applications/views/device_manager_view/device_manager_dialogs/device_form_dialog.py index c5fc18b3..08bb81f0 100644 --- a/bec_widgets/applications/views/device_manager_view/device_manager_dialogs/device_form_dialog.py +++ b/bec_widgets/applications/views/device_manager_view/device_manager_dialogs/device_form_dialog.py @@ -52,6 +52,10 @@ class DeviceManagerOphydValidationDialog(QtWidgets.QDialog): # Load and apply configuration config = config or {} + device_name = config.get("name", None) + if device_name: + self.device_manager_ophyd_test.add_device_to_keep_visible_after_validation(device_name) + self.device_manager_ophyd_test.change_device_configs([config], True, True) # Dialog Buttons: equal size, stacked horizontally @@ -332,15 +336,17 @@ class DeviceFormDialog(QtWidgets.QDialog): ): """Handle completion of validation.""" try: - if DeviceModel.model_validate(device_config) == DeviceModel.model_validate( - self._validation_result[0] + if ( + DeviceModel.model_validate(device_config) + == DeviceModel.model_validate(self._validation_result[0]) + and connection_status == ConnectionStatus.UNKNOWN.value ): - config_status = self._validation_result[1] + # Config unchanged, we can reuse previous connection status. Only do this if the new + # connection status is UNKNOWN as the current validation should not test the connection. connection_status = self._validation_result[2] - validation_msg = self._validation_result[3] except Exception: logger.debug( - f"Device config validation changed for config: {device_config} compared to previous validation. Using status from recent validation ." + f"Device config validation changed for config: {device_config} compared to previous validation. Using status from recent validation." ) self._validation_result = (device_config, config_status, connection_status, validation_msg) if self._wait_dialog is not None: @@ -351,7 +357,6 @@ class DeviceFormDialog(QtWidgets.QDialog): def _add_config(self): """ - Adding a config will always run a validation check of the config without a connection test. We will check if tests have already run, and reuse the information in case they also tested the connection to the device. """ @@ -371,33 +376,34 @@ class DeviceFormDialog(QtWidgets.QDialog): # We will show a wait dialog while this is happening, and compare the results with the last known validation results. # If the config is unchanged, we will use the connection status results from the last validation. self._wait_dialog = self._create_validation_dialog() + ophyd_validation: OphydValidation | None = None + try: + ophyd_validation = self._create_and_run_ophyd_validation(config) - ophyd_validation = self._create_and_run_ophyd_validation(config) + # NOTE If dialog was already closed, this means that a validation callback was already received + # which closed the dialog. In this case, we skip exec to avoid deadlock. With the singleShot above, + # this should not happen, but we keep the check for safety. + if self._wait_dialog is not None: + self._wait_dialog.exec() # This will block until the validation is complete - # NOTE If dialog was already close, this means that a validation callback was already received - # which closed the dialog. In this case, we skip exec to avoid deadlock. With the singleShot above, - # this should not happen, but we keep the check for safety. - if self._wait_dialog is not None: - self._wait_dialog.exec() # This will block until the validation is complete + config, config_status, connection_status, validation_msg = self._validation_result - config, config_status, connection_status, validation_msg = self._validation_result + if config_status == ConfigStatus.INVALID.value: + msg_box = self._create_warning_message_box( + "Invalid Device Configuration", + f"Device configuration is invalid. Last known validation message:\n\nErrors:\n{self._validation_result[3]}", + ) + msg_box.exec() + return - if config_status == ConfigStatus.INVALID.value: - msg_box = self._create_warning_message_box( - "Invalid Device Configuration", - f"Device configuration is invalid. Last known validation message:\n\nErrors:\n{self._validation_result[3]}", + self.accepted_data.emit( + config, config_status, connection_status, validation_msg, self._old_device_name ) - msg_box.exec() - ophyd_validation.close() - ophyd_validation.deleteLater() - return - - self.accepted_data.emit( - config, config_status, connection_status, validation_msg, self._old_device_name - ) - self.accept() - ophyd_validation.close() - ophyd_validation.deleteLater() + self.accept() + finally: + if ophyd_validation is not None: + ophyd_validation.close() + ophyd_validation.deleteLater() def _create_warning_message_box(self, title: str, text: str) -> QtWidgets.QMessageBox: msg_box = QtWidgets.QMessageBox(self) diff --git a/bec_widgets/widgets/control/device_manager/components/ophyd_validation/ophyd_validation.py b/bec_widgets/widgets/control/device_manager/components/ophyd_validation/ophyd_validation.py index eb2acfd9..af2c3058 100644 --- a/bec_widgets/widgets/control/device_manager/components/ophyd_validation/ophyd_validation.py +++ b/bec_widgets/widgets/control/device_manager/components/ophyd_validation/ophyd_validation.py @@ -312,6 +312,7 @@ class OphydValidation(BECWidget, QtWidgets.QWidget): def __init__(self, parent=None, client=None, hide_legend: bool = False): super().__init__(parent=parent, client=client, theme_update=True) self._running_ophyd_tests = False + self._keep_visible_after_validation: list[str] = [] if not READY_TO_TEST: self.setDisabled(True) self.thread_pool_manager = None @@ -339,6 +340,25 @@ class OphydValidation(BECWidget, QtWidgets.QWidget): self._main_layout.addWidget(legend_widget) self._thread_pool_poll_loop() + def add_device_to_keep_visible_after_validation(self, device_name: str) -> None: + """Add a device name to the list of devices to keep visible after validation. + + Args: + device_name (str): Name of the device to keep visible. + """ + if device_name not in self._keep_visible_after_validation: + self._keep_visible_after_validation.append(device_name) + + def remove_device_to_keep_visible_after_validation(self, device_name: str) -> None: + """Remove a device name from the list of devices to keep visible after validation. + + Args: + device_name (str): Name of the device to remove. + """ + if device_name in self._keep_visible_after_validation: + self._keep_visible_after_validation.remove(device_name) + self._remove_device(device_name) + def apply_theme(self, theme: str): """Apply the current theme to the widget.""" self._colors = get_accent_colors() @@ -507,6 +527,8 @@ class OphydValidation(BECWidget, QtWidgets.QWidget): connect (bool, optional): Whether to attempt connection during validation. Defaults to False. force_connect (bool, optional): Whether to force connection during validation. Defaults to False. timeout (float, optional): Timeout for connection attempt. Defaults to 5.0. + skip_validation (bool, optional): Whether to skip validation for the added devices. Defaults to False. + keep_device_item_in_list (bool, optional): Whether to keep the device item in the list after validation in success case. """ if not READY_TO_TEST: logger.error("Cannot change device configs: dependencies not available.") @@ -535,6 +557,20 @@ class OphydValidation(BECWidget, QtWidgets.QWidget): "Device already in session.", ) ) + if device_name in self._keep_visible_after_validation: + self._add_device_config( + cfg, + connect=connect, + force_connect=force_connect, + timeout=timeout, + skip_validation=True, + ) + self._on_device_test_completed( + cfg, + ConfigStatus.VALID.value, + ConnectionStatus.CONNECTED.value, + "Device already in session.", + ) self._remove_device_config(cfg) continue if not self._device_already_exists(cfg.get("name")): # New device case @@ -596,7 +632,12 @@ class OphydValidation(BECWidget, QtWidgets.QWidget): return device_name in self.list_widget def _add_device_config( - self, device_config: dict[str, Any], connect: bool, force_connect: bool, timeout: float + self, + device_config: dict[str, Any], + connect: bool, + force_connect: bool, + timeout: float, + skip_validation: bool = False, ) -> None: device_name = device_config.get("name") # Check if device is in redis session with same config, if yes don't even bother testing.. @@ -611,22 +652,28 @@ class OphydValidation(BECWidget, QtWidgets.QWidget): ) widget.request_rerun_validation.connect(self._on_request_rerun_validation) self.list_widget.add_widget_item(device_name, widget) - self.__delayed_submit_test(widget, connect, force_connect, timeout) + if skip_validation is False: + self.__delayed_submit_test(widget, connect, force_connect, timeout) - def _remove_device_config(self, device_config: dict[str, Any]) -> None: - device_name = device_config.get("name") - if not device_name: - logger.error(f"Device config missing 'name': {device_config}. Cannot remove device.") - return + def _remove_device(self, device_name: str) -> None: if not self._device_already_exists(device_name): logger.debug( f"Device with name {device_name} not found in OphydValidation, can't remove it." ) return + if device_name in self._keep_visible_after_validation: + logger.debug( + f"Device with name {device_name} is set to be kept visible after validation, not removing it." + ) + return if self.thread_pool_manager: self.thread_pool_manager.clear_device_in_queue(device_name) self.list_widget.remove_widget_item(device_name) + def _remove_device_config(self, device_config: dict[str, Any]) -> None: + device_name = device_config.get("name") + self._remove_device(device_name) + @SafeSlot(str, dict, bool, bool, float) def _on_request_rerun_validation( self, @@ -712,16 +759,6 @@ class OphydValidation(BECWidget, QtWidgets.QWidget): if not self._device_already_exists(device_name): logger.debug(f"Received test result for unknown device {device_name}. Ignoring.") return - if config_status == ConfigStatus.VALID.value and connection_status in [ - ConnectionStatus.CONNECTED.value, - ConnectionStatus.CAN_CONNECT.value, - ]: - # Validated successfully, remove item from running list - self.list_widget.remove_widget_item(device_name) - self.validation_completed.emit( - device_config, config_status, connection_status, error_message - ) - return widget = self.list_widget.get_widget(device_name) if widget: widget.on_validation_finished( @@ -729,6 +766,12 @@ class OphydValidation(BECWidget, QtWidgets.QWidget): config_status=config_status, connection_status=connection_status, ) + if config_status == ConfigStatus.VALID.value and connection_status in [ + ConnectionStatus.CONNECTED.value, + ConnectionStatus.CAN_CONNECT.value, + ]: + # Validated successfully, remove item from running list + self._remove_device(device_name) self.validation_completed.emit( device_config, config_status, connection_status, error_message ) diff --git a/tests/unit_tests/test_device_manager_view.py b/tests/unit_tests/test_device_manager_view.py index 39e235df..e822d1e9 100644 --- a/tests/unit_tests/test_device_manager_view.py +++ b/tests/unit_tests/test_device_manager_view.py @@ -652,7 +652,7 @@ class TestDeviceManagerView: mock_add.assert_called_once() def test_run_validate_connection_action_connected( - self, device_manager_display_widget: DeviceManagerDisplayWidget, device_configs: dict + self, device_manager_display_widget: DeviceManagerDisplayWidget, device_configs: dict, qtbot ): """Test run validate connection action is connected.""" dm_view = device_manager_display_widget @@ -661,7 +661,12 @@ class TestDeviceManagerView: dm_view.ophyd_test_view, "change_device_configs" ) as mock_change_configs: # First, add device configs to the table - dm_view.device_table_view.add_device_configs(device_configs) + with qtbot.waitSignal(dm_view.device_table_view.device_configs_changed) as sig_blocker: + dm_view.device_table_view.add_device_configs(device_configs) + cfgs, added, skip_validation = sig_blocker.args + assert cfgs == device_configs + assert added is True + assert skip_validation is False mock_change_configs.assert_called_once_with( device_configs=device_configs, added=True, skip_validation=False ) # Configs were added