refactor(ophyd-validation): Allow option to keep device visible after successful validation

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)

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
             )

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