feat: (#495) add devices through browser

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

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):

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.

bec_widgets/widgets/services/device_browser/device_browser.ui

@@ -29,6 +29,31 @@
           </property>
          </widget>
         </item>
+        <item>
+         <widget class="QGroupBox" name="button_box">
+          <layout class="QHBoxLayout" name="horizontalLayout">
+           <property name="leftMargin">
+            <number>0</number>
+           </property>
+           <property name="topMargin">
+            <number>0</number>
+           </property>
+           <property name="rightMargin">
+            <number>0</number>
+           </property>
+           <property name="bottomMargin">
+            <number>0</number>
+           </property>
+           <item>
+            <widget class="QToolButton" name="add_button">
+             <property name="text">
+              <string>...</string>
+             </property>
+            </widget>
+           </item>
+          </layout>
+         </widget>
+        </item>
        </layout>
       </item>
       <item>

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()

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."""

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()