diff --git a/bec_widgets/utils/forms_from_types/forms.py b/bec_widgets/utils/forms_from_types/forms.py index ece7e769..db5ad7de 100644 --- a/bec_widgets/utils/forms_from_types/forms.py +++ b/bec_widgets/utils/forms_from_types/forms.py @@ -171,8 +171,9 @@ class TypedForm(BECWidget, QWidget): class PydanticModelForm(TypedForm): - metadata_updated = Signal(dict) - metadata_cleared = Signal(NoneType) + form_data_updated = Signal(dict) + form_data_cleared = Signal(NoneType) + validity_proc = Signal(bool) def __init__( self, @@ -204,7 +205,7 @@ class PydanticModelForm(TypedForm): self._validity = CompactPopupWidget() self._validity.compact_view = True # type: ignore - self._validity.label = "Metadata validity" # type: ignore + self._validity.label = "Validity" # type: ignore self._validity.compact_show_popup.setIcon( material_icon(icon_name="info", size=(10, 10), convert_to_pixmap=False) ) @@ -264,16 +265,18 @@ class PydanticModelForm(TypedForm): def validate_form(self, *_) -> bool: """validate the currently entered metadata against the pydantic schema. If successful, returns on metadata_emitted and returns true. - Otherwise, emits on metadata_cleared and returns false.""" + Otherwise, emits on form_data_cleared and returns false.""" try: metadata_dict = self.get_form_data() self._md_schema.model_validate(metadata_dict) self._validity.set_global_state("success") self._validity_message.setText("No errors!") - self.metadata_updated.emit(metadata_dict) + self.form_data_updated.emit(metadata_dict) + self.validity_proc.emit(True) return True except ValidationError as e: self._validity.set_global_state("emergency") self._validity_message.setText(str(e)) - self.metadata_cleared.emit(None) + self.form_data_cleared.emit(None) + self.validity_proc.emit(False) return False diff --git a/bec_widgets/widgets/control/scan_control/scan_control.py b/bec_widgets/widgets/control/scan_control/scan_control.py index 4537d30f..cf114ee5 100644 --- a/bec_widgets/widgets/control/scan_control/scan_control.py +++ b/bec_widgets/widgets/control/scan_control/scan_control.py @@ -169,8 +169,8 @@ class ScanControl(BECWidget, QWidget): self.layout.addWidget(self._metadata_form) self._metadata_form.update_with_new_scan(self.comboBox_scan_selection.currentText()) self.scan_selected.connect(self._metadata_form.update_with_new_scan) - self._metadata_form.metadata_updated.connect(self.update_scan_metadata) - self._metadata_form.metadata_cleared.connect(self.update_scan_metadata) + self._metadata_form.form_data_updated.connect(self.update_scan_metadata) + self._metadata_form.form_data_cleared.connect(self.update_scan_metadata) self._metadata_form.validate_form() def populate_scans(self): diff --git a/bec_widgets/widgets/services/device_browser/device_browser.py b/bec_widgets/widgets/services/device_browser/device_browser.py index 0f454e95..795e7d17 100644 --- a/bec_widgets/widgets/services/device_browser/device_browser.py +++ b/bec_widgets/widgets/services/device_browser/device_browser.py @@ -5,6 +5,7 @@ from functools import partial from bec_lib.callback_handler import EventType from bec_lib.logger import bec_logger from bec_lib.messages import ConfigAction +from bec_qthemes import material_icon from pyqtgraph import SignalProxy from qtpy.QtCore import QSize, Signal from qtpy.QtWidgets import QListWidget, QListWidgetItem, QVBoxLayout, QWidget @@ -14,6 +15,9 @@ from bec_widgets.utils.bec_widget import BECWidget from bec_widgets.utils.error_popups import SafeSlot from bec_widgets.utils.ui_loader import UILoader from bec_widgets.widgets.services.device_browser.device_item import DeviceItem +from bec_widgets.widgets.services.device_browser.device_item.device_config_dialog import ( + DeviceConfigDialog, +) from bec_widgets.widgets.services.device_browser.util import map_device_type_to_icon logger = bec_logger.logger @@ -49,6 +53,8 @@ class DeviceBrowser(BECWidget, QWidget): EventType.DEVICE_UPDATE, self.on_device_update ) self.device_update.connect(self.update_device_list) + self.ui.add_button.clicked.connect(self._create_add_dialog) + self.ui.add_button.setIcon(material_icon("add", size=(20, 20), convert_to_pixmap=False)) self.init_device_list() self.update_device_list() @@ -63,6 +69,10 @@ class DeviceBrowser(BECWidget, QWidget): layout.addWidget(self.ui) self.setLayout(layout) + def _create_add_dialog(self): + dialog = DeviceConfigDialog(parent=self, device=None, action="add") + dialog.open() + def on_device_update(self, action: ConfigAction, content: dict) -> None: """ Callback for device update events. Triggers the device_update signal. diff --git a/bec_widgets/widgets/services/device_browser/device_browser.ui b/bec_widgets/widgets/services/device_browser/device_browser.ui index 1082d7db..9528b6cf 100644 --- a/bec_widgets/widgets/services/device_browser/device_browser.ui +++ b/bec_widgets/widgets/services/device_browser/device_browser.ui @@ -29,6 +29,31 @@ + + + + + 0 + + + 0 + + + 0 + + + 0 + + + + + ... + + + + + + diff --git a/bec_widgets/widgets/services/device_browser/device_item/device_config_dialog.py b/bec_widgets/widgets/services/device_browser/device_item/device_config_dialog.py index ce026a70..6422699b 100644 --- a/bec_widgets/widgets/services/device_browser/device_item/device_config_dialog.py +++ b/bec_widgets/widgets/services/device_browser/device_item/device_config_dialog.py @@ -1,4 +1,5 @@ from ast import literal_eval +from typing import Literal from bec_lib.atlas_models import Device as DeviceConfigModel from bec_lib.config_helper import CONF as DEVICE_CONF_KEYS @@ -32,20 +33,26 @@ class _CommSignals(QObject): class _CommunicateUpdate(QRunnable): - def __init__(self, config_helper: ConfigHelper, device: str, config: dict) -> None: + def __init__(self, config_helper: ConfigHelper, device: str, config: dict, action: str) -> None: super().__init__() self.config_helper = config_helper self.device = device self.config = config + self.action = action self.signals = _CommSignals() @SafeSlot() def run(self): try: + if (dev_name := self.device or self.config.get("name")) is None: + raise ValueError("Must be updating a device or be supplied a name for a new device") + req_args = { + "action": self.action, + "config": {dev_name: self.config}, + "wait_for_response": False, + } timeout = self.config_helper.suggested_timeout_s(self.config) - RID = self.config_helper.send_config_request( - action="update", config={self.device: self.config}, wait_for_response=False - ) + RID = self.config_helper.send_config_request(**req_args) logger.info("Waiting for config reply") reply = self.config_helper.wait_for_config_reply(RID, timeout=timeout) self.config_helper.handle_update_reply(reply, RID, timeout) @@ -65,6 +72,7 @@ class DeviceConfigDialog(BECWidget, QDialog): parent=None, device: str | None = None, config_helper: ConfigHelper | None = None, + action: Literal["update", "add"] = "update", **kwargs, ): """A dialog to edit the configuration of a device in BEC. Generated from the pydantic model @@ -76,12 +84,14 @@ class DeviceConfigDialog(BECWidget, QDialog): config_helper (ConfigHelper | None): a ConfigHelper object for communication with Redis, will be created if necessary. action (Literal["update", "add"]): the action which the form should perform on application or acceptance. """ + self._initial_config = {} super().__init__(parent=parent, **kwargs) self._config_helper = config_helper or ConfigHelper( self.client.connector, self.client._service_name ) self.threadpool = QThreadPool() self._device = device + self._action = action self.setWindowTitle(f"Edit config for: {device}") self._container = QStackedLayout() self._container.setStackingMode(QStackedLayout.StackAll) @@ -94,11 +104,18 @@ class DeviceConfigDialog(BECWidget, QDialog): user_warning.setWordWrap(True) user_warning.setStyleSheet("QLabel { color: red; }") self._layout.addWidget(user_warning) + self.get_bec_shortcuts() self._add_form() + if self._action == "update": + self._form._validity.setVisible(False) + else: + self._form._validity.setVisible(True) + self._form.validity_proc.connect(self.enable_buttons_for_validity) self._add_overlay() self._add_buttons() self.setLayout(self._container) + self._form.validate_form() self._overlay_widget.setVisible(False) def _add_form(self): @@ -108,11 +125,15 @@ class DeviceConfigDialog(BECWidget, QDialog): self._layout.addWidget(self._form) for row in self._form.enumerate_form_widgets(): - if row.label.property("_model_field_name") in DEVICE_CONF_KEYS.NON_UPDATABLE: + if ( + row.label.property("_model_field_name") in DEVICE_CONF_KEYS.NON_UPDATABLE + and self._action == "update" + ): row.widget._set_pretty_display() - self._fetch_config() - self._fill_form() + if self._action == "update" and self._device in self.dev: + self._fetch_config() + self._fill_form() self._container.addWidget(self._form_widget) def _add_overlay(self): @@ -129,16 +150,15 @@ class DeviceConfigDialog(BECWidget, QDialog): self._container.addWidget(self._overlay_widget) def _add_buttons(self): - button_box = QDialogButtonBox( + self.button_box = QDialogButtonBox( QDialogButtonBox.Apply | QDialogButtonBox.Ok | QDialogButtonBox.Cancel ) - button_box.button(QDialogButtonBox.Apply).clicked.connect(self.apply) - button_box.accepted.connect(self.accept) - button_box.rejected.connect(self.reject) - self._layout.addWidget(button_box) + self.button_box.button(QDialogButtonBox.Apply).clicked.connect(self.apply) + self.button_box.accepted.connect(self.accept) + self.button_box.rejected.connect(self.reject) + self._layout.addWidget(self.button_box) def _fetch_config(self): - self._initial_config = {} if ( self.client.device_manager is not None and self._device in self.client.device_manager.devices @@ -163,37 +183,42 @@ class DeviceConfigDialog(BECWidget, QDialog): } return diff - @SafeSlot() + @SafeSlot(bool) + def enable_buttons_for_validity(self, valid: bool): + self.button_box.button(QDialogButtonBox.Apply).setEnabled(valid) + self.button_box.button(QDialogButtonBox.Ok).setEnabled(valid) + + @SafeSlot(popup_error=True) def apply(self): - self._process_update_action() + self._process_action() self.applied.emit() - @SafeSlot() + @SafeSlot(popup_error=True) def accept(self): - self._process_update_action() + self._process_action() return super().accept() - def _process_update_action(self): + def _process_action(self): updated_config = self.updated_config() - if (device_name := updated_config.get("name")) == "": - logger.warning("Can't create a device with no name!") - elif set(updated_config.keys()) & set(DEVICE_CONF_KEYS.NON_UPDATABLE): - logger.info( - f"Removing old device {self._device} and adding new device {device_name or self._device} with modified config: {updated_config}" - ) + if self._action == "add": + if (name := updated_config.get("name")) in self.dev: + raise ValueError( + f"Can't create a new device with the same name as already existing device {name}!" + ) + self._proc_device_config_change(updated_config) else: - self._update_device_config(updated_config) + if updated_config == {}: + logger.info("No changes made to device config") + return + self._proc_device_config_change(updated_config) - def _update_device_config(self, config: dict): - if self._device is None: - return - if config == {}: - logger.info("No changes made to device config") - return + def _proc_device_config_change(self, config: dict): logger.info(f"Sending request to update device config: {config}") self._start_waiting_display() - communicate_update = _CommunicateUpdate(self._config_helper, self._device, config) + communicate_update = _CommunicateUpdate( + self._config_helper, self._device, config, self._action + ) communicate_update.signals.error.connect(self.update_error) communicate_update.signals.done.connect(self.update_done) self.threadpool.start(communicate_update) @@ -201,8 +226,9 @@ class DeviceConfigDialog(BECWidget, QDialog): @SafeSlot() def update_done(self): self._stop_waiting_display() - self._fetch_config() - self._fill_form() + if self._action == "update": + self._fetch_config() + self._fill_form() @SafeSlot(Exception, popup_error=True) def update_error(self, e: Exception): @@ -247,7 +273,8 @@ def main(): # pragma: no cover def _show_dialog(*_): nonlocal dialog if dialog is None: - dialog = DeviceConfigDialog(device=device.text()) + kwargs = {"device": dev} if (dev := device.text()) else {"action": "add"} + dialog = DeviceConfigDialog(**kwargs) dialog.accepted.connect(accept) dialog.rejected.connect(_destroy_dialog) dialog.open() diff --git a/bec_widgets/widgets/services/device_browser/device_item/device_config_form.py b/bec_widgets/widgets/services/device_browser/device_item/device_config_form.py index 6c49afae..eb812b0b 100644 --- a/bec_widgets/widgets/services/device_browser/device_item/device_config_form.py +++ b/bec_widgets/widgets/services/device_browser/device_item/device_config_form.py @@ -42,6 +42,7 @@ class DeviceConfigForm(PydanticModelForm): if theme is None: theme = get_theme_name() self.setStyleSheet(styles.pretty_display_theme(theme)) + self._validity.setVisible(False) def get_form_data(self): """Get the entered metadata as a dict.""" diff --git a/tests/unit_tests/test_device_config_form_dialog.py b/tests/unit_tests/test_device_config_form_dialog.py index 866ad749..f554f6be 100644 --- a/tests/unit_tests/test_device_config_form_dialog.py +++ b/tests/unit_tests/test_device_config_form_dialog.py @@ -2,7 +2,10 @@ from unittest.mock import MagicMock, patch import pytest from bec_lib.atlas_models import Device as DeviceConfigModel +from PySide6.QtWidgets import QPushButton +from qtpy.QtWidgets import QDialogButtonBox, QLineEdit +from bec_widgets.utils.forms_from_types.items import StrFormItem from bec_widgets.widgets.services.device_browser.device_item.device_config_dialog import ( DeviceConfigDialog, ) @@ -16,83 +19,114 @@ _BASIC_CONFIG = { @pytest.fixture -def dialog(qtbot): - """Fixture to create a DeviceConfigDialog instance.""" +def mock_client(): mock_device = MagicMock(_config=DeviceConfigModel.model_validate(_BASIC_CONFIG).model_dump()) mock_client = MagicMock() mock_client.device_manager.devices = {"test_device": mock_device} - dialog = DeviceConfigDialog(device="test_device", config_helper=MagicMock(), client=mock_client) - qtbot.addWidget(dialog) - return dialog + return mock_client -def test_initialization(dialog): - assert dialog._device == "test_device" - assert dialog._container.count() == 2 +@pytest.fixture +def update_dialog(mock_client, qtbot): + """Fixture to create a DeviceConfigDialog instance.""" + update_dialog = DeviceConfigDialog( + device="test_device", config_helper=MagicMock(), client=mock_client + ) + qtbot.addWidget(update_dialog) + return update_dialog -def test_fill_form(dialog): - with patch.object(dialog._form, "set_data") as mock_set_data: - dialog._fill_form() +@pytest.fixture +def add_dialog(mock_client, qtbot): + """Fixture to create a DeviceConfigDialog instance.""" + add_dialog = DeviceConfigDialog( + device=None, config_helper=MagicMock(), client=mock_client, action="add" + ) + qtbot.addWidget(add_dialog) + return add_dialog + + +def test_initialization(update_dialog): + assert update_dialog._device == "test_device" + assert update_dialog._container.count() == 2 + + +def test_fill_form(update_dialog): + with patch.object(update_dialog._form, "set_data") as mock_set_data: + update_dialog._fill_form() mock_set_data.assert_called_once_with(DeviceConfigModel.model_validate(_BASIC_CONFIG)) -def test_updated_config(dialog): +def test_updated_config(update_dialog): """Test that updated_config returns the correct changes.""" - dialog._initial_config = {"key1": "value1", "key2": "value2"} + update_dialog._initial_config = {"key1": "value1", "key2": "value2"} with patch.object( - dialog._form, "get_form_data", return_value={"key1": "value1", "key2": "new_value"} + update_dialog._form, "get_form_data", return_value={"key1": "value1", "key2": "new_value"} ): - updated = dialog.updated_config() + updated = update_dialog.updated_config() assert updated == {"key2": "new_value"} -def test_apply(dialog): - with patch.object(dialog, "_process_update_action") as mock_process_update: - dialog.apply() +def test_apply(update_dialog): + with patch.object(update_dialog, "_process_action") as mock_process_update: + update_dialog.apply() mock_process_update.assert_called_once() -def test_accept(dialog): +def test_accept(update_dialog): with ( - patch.object(dialog, "_process_update_action") as mock_process_update, + patch.object(update_dialog, "_process_action") as mock_process_update, patch("qtpy.QtWidgets.QDialog.accept") as mock_parent_accept, ): - dialog.accept() + update_dialog.accept() mock_process_update.assert_called_once() mock_parent_accept.assert_called_once() -def test_waiting_display(dialog, qtbot): +def test_waiting_display(update_dialog, qtbot): with ( - patch.object(dialog._spinner, "start") as mock_spinner_start, - patch.object(dialog._spinner, "stop") as mock_spinner_stop, + patch.object(update_dialog._spinner, "start") as mock_spinner_start, + patch.object(update_dialog._spinner, "stop") as mock_spinner_stop, ): - dialog.show() - dialog._start_waiting_display() - qtbot.waitUntil(dialog._overlay_widget.isVisible, timeout=100) + update_dialog.show() + update_dialog._start_waiting_display() + qtbot.waitUntil(update_dialog._overlay_widget.isVisible, timeout=100) mock_spinner_start.assert_called_once() mock_spinner_stop.assert_not_called() - dialog._stop_waiting_display() - qtbot.waitUntil(lambda: not dialog._overlay_widget.isVisible(), timeout=100) + update_dialog._stop_waiting_display() + qtbot.waitUntil(lambda: not update_dialog._overlay_widget.isVisible(), timeout=100) mock_spinner_stop.assert_called_once() -def test_update_cycle(dialog, qtbot): +def test_update_cycle(update_dialog, qtbot): update = {"enabled": False, "readoutPriority": "baseline", "deviceTags": {"tag"}} def _mock_send(action="update", config=None, wait_for_response=True, timeout_s=None): - dialog.client.device_manager.devices["test_device"]._config = config["test_device"] # type: ignore + update_dialog.client.device_manager.devices["test_device"]._config = config["test_device"] # type: ignore - dialog._config_helper.send_config_request = MagicMock(side_effect=_mock_send) - for item in dialog._form.enumerate_form_widgets(): + update_dialog._config_helper.send_config_request = MagicMock(side_effect=_mock_send) + for item in update_dialog._form.enumerate_form_widgets(): if (val := update.get(item.label.property("_model_field_name"))) is not None: item.widget.setValue(val) - assert dialog.updated_config() == update - dialog.apply() - qtbot.waitUntil(lambda: dialog._config_helper.send_config_request.call_count == 1, timeout=100) + assert update_dialog.updated_config() == update + update_dialog.apply() + qtbot.waitUntil( + lambda: update_dialog._config_helper.send_config_request.call_count == 1, timeout=100 + ) - dialog._config_helper.send_config_request.assert_called_with( + update_dialog._config_helper.send_config_request.assert_called_with( action="update", config={"test_device": update}, wait_for_response=False ) + + +def test_add_form_init_without_name(add_dialog, qtbot): + assert (name_widget := add_dialog._form.widget_dict.get("name")) is not None + assert isinstance(name_widget, StrFormItem) + assert name_widget.getValue() is None + + +def test_add_form_validates_and_disables_on_init(add_dialog, qtbot): + assert (ok_button := add_dialog.button_box.button(QDialogButtonBox.Ok)) is not None + assert isinstance(ok_button, QPushButton) + assert not ok_button.isEnabled()