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 c0453f93..c5fc18b3 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 @@ -288,6 +288,24 @@ class DeviceFormDialog(QtWidgets.QDialog): wait_dialog.setMinimumDuration(0) return wait_dialog + def _create_and_run_ophyd_validation(self, config: dict[str, Any]) -> OphydValidation: + """Run ophyd validation test on the current device configuration.""" + ophyd_validation = OphydValidation(parent=self) + ophyd_validation.validation_completed.connect(self._handle_validation_result) + ophyd_validation.multiple_validations_completed.connect( + self._handle_devices_already_in_session_results + ) + + # NOTE Use singleShot here to ensure that the signal is emitted after all other scheduled + # tasks in the event loop are processed. This avoids potential deadlocks. In particular, + # this is relevant for the _wait_dialog exec which opens a modal dialog during validation + # and therefore must not have the signal emitted immediately in the same event loop iteration. + # Otherwise, the callback may be scheduled before the dialog is shown resulting in a deadlock. + QtCore.QTimer.singleShot( + 0, lambda: ophyd_validation.change_device_configs([config], True, False) + ) + return ophyd_validation + @SafeSlot(list) def _handle_devices_already_in_session_results( self, validation_results: _ValidationResultIter @@ -354,20 +372,7 @@ class DeviceFormDialog(QtWidgets.QDialog): # 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() - ophyd_validation.validation_completed.connect(self._handle_validation_result) - ophyd_validation.multiple_validations_completed.connect( - self._handle_devices_already_in_session_results - ) - - # NOTE Use singleShot here to ensure that the signal is emitted after all other scheduled - # tasks in the event loop are processed. This avoids potential deadlocks. In particular, - # this is relevant for the _wait_dialog exec which opens a modal dialog during validation - # and therefore must not have the signal emitted immediately in the same event loop iteration. - # Otherwise, the callback may be scheduled before the dialog is shown resulting in a deadlock. - QtCore.QTimer.singleShot( - 0, lambda: ophyd_validation.change_device_configs([config], True, False) - ) + ophyd_validation = self._create_and_run_ophyd_validation(config) # 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, diff --git a/bec_widgets/applications/views/device_manager_view/device_manager_display_widget.py b/bec_widgets/applications/views/device_manager_view/device_manager_display_widget.py index 34031264..15b7f0c1 100644 --- a/bec_widgets/applications/views/device_manager_view/device_manager_display_widget.py +++ b/bec_widgets/applications/views/device_manager_view/device_manager_display_widget.py @@ -104,7 +104,7 @@ class DeviceManagerDisplayWidget(DockAreaWidget): self.ophyd_test_view.multiple_validations_completed, (self.device_table_view.update_multiple_device_validations,), ), - (self.request_ophyd_validation, (self.ophyd_test_view.device_table_config_changed,)), + (self.request_ophyd_validation, (self.ophyd_test_view.change_device_configs,)), ( self.device_table_view.device_configs_changed, (self.ophyd_test_view.device_table_config_changed,), diff --git a/tests/unit_tests/test_device_manager_components.py b/tests/unit_tests/test_device_manager_components.py index 9964a396..a690d059 100644 --- a/tests/unit_tests/test_device_manager_components.py +++ b/tests/unit_tests/test_device_manager_components.py @@ -575,9 +575,10 @@ class TestDeviceTable: device_table.remove_device_configs([sample_devices[0]]) # Verify signal emission - emitted_configs, added = blocker.args + emitted_configs, added, skip_validation = blocker.args assert len(emitted_configs) == 1 assert added is False + assert skip_validation is True # Verify row was removed assert device_table.table.rowCount() == 1 diff --git a/tests/unit_tests/test_device_manager_view.py b/tests/unit_tests/test_device_manager_view.py index 220fa8cc..e0609d93 100644 --- a/tests/unit_tests/test_device_manager_view.py +++ b/tests/unit_tests/test_device_manager_view.py @@ -184,48 +184,67 @@ class TestDeviceManagerViewDialogs: assert config["name"] == "TestDevice" assert config["deviceClass"] == "ophyd.EpicsSignal" assert config["deviceConfig"]["read_pv"] == "X25DA-ES1-MOT:GET" - # Set the validation results, assume that test was running - dialog.config_validation_result = ( - dialog._device_config_template.get_config_fields(), - ConfigStatus.VALID.value, - 0, - "", - ) - with mock.patch.object(dialog, "_create_warning_message_box") as mock_warning_box: - with qtbot.waitSignal(dialog.accepted_data) as sig_blocker: - qtbot.mouseClick(dialog.add_btn, QtCore.Qt.LeftButton) - config, _, _, _, _ = sig_blocker.args - mock_warning_box.assert_not_called() - # Called with config_status invalid should show warning - dialog.config_validation_result = ( - dialog._device_config_template.get_config_fields(), - ConfigStatus.INVALID.value, - 0, - "", - ) - with mock.patch.object(dialog, "_create_warning_message_box") as mock_warning_box: - qtbot.mouseClick(dialog.add_btn, QtCore.Qt.LeftButton) - mock_warning_box.assert_called_once() + # Test now to add the device config with different validation results + # For this we have to mock the additional ophyd_validation checks + with ( + mock.patch.object(dialog, "_create_validation_dialog") as mock_create_dialog, + mock.patch.object(dialog, "_create_and_run_ophyd_validation") as mock_create_validation, + ): - # Set to random config without name - - random_config = {"deviceClass": "Unknown"} - dialog.set_device_config(random_config) - dialog.config_validation_result = ( - dialog._device_config_template.get_config_fields(), - 0, - 0, - "", - ) - assert group_combo.currentText() == "CustomDevice" - assert variant_combo.currentText() == "CustomDevice" - with mock.patch.object(dialog, "_create_warning_message_box") as mock_warning_box: - qtbot.mouseClick(dialog.add_btn, QtCore.Qt.LeftButton) - mock_warning_box.assert_called_once_with( - "Invalid Device Name", - f"Device is invalid, can not be empty with spaces. Please provide a valid name. {dialog._device_config_template.get_config_fields().get('name', '')!r} ", + # Set the validation results, assume that test was running + dialog.config_validation_result = ( + dialog._device_config_template.get_config_fields(), + ConfigStatus.VALID.value, + 0, + "", ) + with mock.patch.object(dialog, "_create_warning_message_box") as mock_warning_box: + with qtbot.waitSignal(dialog.accepted_data) as sig_blocker: + qtbot.mouseClick(dialog.add_btn, QtCore.Qt.LeftButton) + config, _, _, _, _ = sig_blocker.args + mock_warning_box.assert_not_called() + + mock_create_dialog.assert_called_once() + mock_create_validation.assert_called_once() + mock_create_dialog.reset_mock() + mock_create_validation.reset_mock() + + # Called with config_status invalid should show warning + dialog.config_validation_result = ( + dialog._device_config_template.get_config_fields(), + ConfigStatus.INVALID.value, + 0, + "", + ) + with mock.patch.object(dialog, "_create_warning_message_box") as mock_warning_box: + qtbot.mouseClick(dialog.add_btn, QtCore.Qt.LeftButton) + mock_warning_box.assert_called_once() + mock_create_dialog.assert_called_once() + mock_create_validation.assert_called_once() + mock_create_dialog.reset_mock() + mock_create_validation.reset_mock() + + # Set to random config without name + + random_config = {"deviceClass": "Unknown"} + dialog.set_device_config(random_config) + dialog.config_validation_result = ( + dialog._device_config_template.get_config_fields(), + 0, + 0, + "", + ) + assert group_combo.currentText() == "CustomDevice" + assert variant_combo.currentText() == "CustomDevice" + with mock.patch.object(dialog, "_create_warning_message_box") as mock_warning_box: + qtbot.mouseClick(dialog.add_btn, QtCore.Qt.LeftButton) + mock_warning_box.assert_called_once_with( + "Invalid Device Name", + f"Device is invalid, can not be empty with spaces. Please provide a valid name. {dialog._device_config_template.get_config_fields().get('name', '')!r} ", + ) + mock_create_dialog.assert_not_called() + mock_create_validation.assert_not_called() def test_device_status_item(self, device_config: dict, qtbot): """Test the DeviceStatusItem widget.""" @@ -642,7 +661,9 @@ class TestDeviceManagerView: ) as mock_change_configs: # First, add device configs to the table dm_view.device_table_view.add_device_configs(device_configs) - assert mock_change_configs.call_args[0][1] is True # Configs were added + mock_change_configs.assert_called_once_with( + device_configs=device_configs, added=True, skip_validation=False + ) # Configs were added mock_change_configs.reset_mock() # Trigger the validate connection action without selection, should validate all @@ -650,7 +671,9 @@ class TestDeviceManagerView: "rerun_validation" ].action.action.triggered.emit() assert len(mock_change_configs.call_args[0][0]) == len(device_configs) - assert mock_change_configs.call_args[0][1:] == (True, True) # Configs were not added + mock_change_configs.assert_called_once_with( + device_configs, True, True + ) # Configs were added with connect=True mock_change_configs.reset_mock() # Select a single row and trigger again, should only validate that one