From 91959e82de8586934af3ebb5aaa0923930effc51 Mon Sep 17 00:00:00 2001 From: appel_c Date: Thu, 24 Oct 2024 14:01:18 +0200 Subject: [PATCH] refactor: do not flush selection upon receiving config update; allow widgetIO to receive kwargs to be able to use get_value to receive string instead of int for QComboBox --- bec_widgets/{test_utils => tests}/__init__.py | 0 .../client_mocks.py => tests/utils.py} | 17 +++--- bec_widgets/utils/widget_io.py | 21 ++++--- .../widgets/base_classes/device_input_base.py | 33 +++++----- .../device_combobox/device_combobox.py | 61 ++++++++++++++++++- .../device_line_edit/device_line_edit.py | 24 ++++++++ .../signal_line_edit/signal_line_edit.py | 26 +++++++- tests/unit_tests/client_mocks.py | 2 +- tests/unit_tests/test_client_utils.py | 2 +- tests/unit_tests/test_device_input_base.py | 8 +-- tests/unit_tests/test_device_input_widgets.py | 1 - tests/unit_tests/test_device_signal_input.py | 23 ++++--- 12 files changed, 168 insertions(+), 50 deletions(-) rename bec_widgets/{test_utils => tests}/__init__.py (100%) rename bec_widgets/{test_utils/client_mocks.py => tests/utils.py} (95%) diff --git a/bec_widgets/test_utils/__init__.py b/bec_widgets/tests/__init__.py similarity index 100% rename from bec_widgets/test_utils/__init__.py rename to bec_widgets/tests/__init__.py diff --git a/bec_widgets/test_utils/client_mocks.py b/bec_widgets/tests/utils.py similarity index 95% rename from bec_widgets/test_utils/client_mocks.py rename to bec_widgets/tests/utils.py index dc6035f5..759d0e5c 100644 --- a/bec_widgets/test_utils/client_mocks.py +++ b/bec_widgets/tests/utils.py @@ -1,11 +1,9 @@ from unittest.mock import MagicMock -from bec_lib.client import BECClient from bec_lib.device import Device as BECDevice from bec_lib.device import Positioner as BECPositioner from bec_lib.device import ReadoutPriority from bec_lib.devicemanager import DeviceContainer -from bec_lib.redis_connector import RedisConnector class FakeDevice(BECDevice): @@ -80,6 +78,8 @@ class FakePositioner(BECPositioner): super().__init__(name=name) # self.limits = limits if limits is not None else [0.0, 0.0] self.read_value = read_value + self.setpoint_value = read_value + self.motor_is_moving_value = 0 self._enabled = enabled self._limits = limits self._readout_priority = readout_priority @@ -101,6 +101,11 @@ class FakePositioner(BECPositioner): "velocity": {"kind_str": "2"}, # config } } + self.signals = { + self.name: {"value": self.read_value}, + f"{self.name}_setpoint": {"value": self.setpoint_value}, + f"{self.name}_motor_is_moving": {"value": self.motor_is_moving_value}, + } @property def readout_priority(self): @@ -139,7 +144,7 @@ class FakePositioner(BECPositioner): Args: fake_value(float): Desired fake value """ - self.signals[self.name]["value"] = fake_value + self.read_value = fake_value def describe(self) -> dict: """ @@ -157,11 +162,7 @@ class FakePositioner(BECPositioner): self.read_value = value def read(self): - return { - self.name: {"value": self.read_value}, - f"{self.name}_setpoint": {"value": self.read_value}, - f"{self.name}_motor_is_moving": {"value": 0}, - } + return self.signals def set_limits(self, limits): self.limits = limits diff --git a/bec_widgets/utils/widget_io.py b/bec_widgets/utils/widget_io.py index d2a85fac..111ef9f6 100644 --- a/bec_widgets/utils/widget_io.py +++ b/bec_widgets/utils/widget_io.py @@ -1,5 +1,6 @@ # pylint: disable=no-name-in-module from abc import ABC, abstractmethod +from typing import Literal from qtpy.QtWidgets import ( QApplication, @@ -20,7 +21,7 @@ class WidgetHandler(ABC): """Abstract base class for all widget handlers.""" @abstractmethod - def get_value(self, widget: QWidget): + def get_value(self, widget: QWidget, **kwargs): """Retrieve value from the widget instance.""" @abstractmethod @@ -31,7 +32,7 @@ class WidgetHandler(ABC): class LineEditHandler(WidgetHandler): """Handler for QLineEdit widgets.""" - def get_value(self, widget: QLineEdit) -> str: + def get_value(self, widget: QLineEdit, **kwargs) -> str: return widget.text() def set_value(self, widget: QLineEdit, value: str) -> None: @@ -41,7 +42,9 @@ class LineEditHandler(WidgetHandler): class ComboBoxHandler(WidgetHandler): """Handler for QComboBox widgets.""" - def get_value(self, widget: QComboBox) -> int: + def get_value(self, widget: QComboBox, as_string: bool = False, **kwargs) -> int | str: + if as_string is True: + return widget.currentText() return widget.currentIndex() def set_value(self, widget: QComboBox, value: int | str) -> None: @@ -54,7 +57,7 @@ class ComboBoxHandler(WidgetHandler): class TableWidgetHandler(WidgetHandler): """Handler for QTableWidget widgets.""" - def get_value(self, widget: QTableWidget) -> list: + def get_value(self, widget: QTableWidget, **kwargs) -> list: return [ [ widget.item(row, col).text() if widget.item(row, col) else "" @@ -73,7 +76,7 @@ class TableWidgetHandler(WidgetHandler): class SpinBoxHandler(WidgetHandler): """Handler for QSpinBox and QDoubleSpinBox widgets.""" - def get_value(self, widget): + def get_value(self, widget, **kwargs): return widget.value() def set_value(self, widget, value): @@ -83,7 +86,7 @@ class SpinBoxHandler(WidgetHandler): class CheckBoxHandler(WidgetHandler): """Handler for QCheckBox widgets.""" - def get_value(self, widget): + def get_value(self, widget, **kwargs): return widget.isChecked() def set_value(self, widget, value): @@ -93,7 +96,7 @@ class CheckBoxHandler(WidgetHandler): class LabelHandler(WidgetHandler): """Handler for QLabel widgets.""" - def get_value(self, widget): + def get_value(self, widget, **kwargs): return widget.text() def set_value(self, widget, value): @@ -114,7 +117,7 @@ class WidgetIO: } @staticmethod - def get_value(widget, ignore_errors=False): + def get_value(widget, ignore_errors=False, **kwargs): """ Retrieve value from the widget instance. @@ -124,7 +127,7 @@ class WidgetIO: """ handler_class = WidgetIO._find_handler(widget) if handler_class: - return handler_class().get_value(widget) # Instantiate the handler + return handler_class().get_value(widget, **kwargs) # Instantiate the handler if not ignore_errors: raise ValueError(f"No handler for widget type: {type(widget)}") return None diff --git a/bec_widgets/widgets/base_classes/device_input_base.py b/bec_widgets/widgets/base_classes/device_input_base.py index 59aac898..37c947a8 100644 --- a/bec_widgets/widgets/base_classes/device_input_base.py +++ b/bec_widgets/widgets/base_classes/device_input_base.py @@ -2,10 +2,10 @@ from __future__ import annotations import enum -from bec_lib.callback_handler import EventType -from bec_lib.device import ComputedSignal, Device, Positioner, ReadoutPriority, Signal +from bec_lib.device import ComputedSignal, Device, Positioner, ReadoutPriority +from bec_lib.device import Signal as BECSignal from bec_lib.logger import bec_logger -from qtpy.QtCore import Property, Slot +from qtpy.QtCore import Property, Signal, Slot from bec_widgets.utils import ConnectionConfig from bec_widgets.utils.bec_widget import BECWidget @@ -43,7 +43,7 @@ class DeviceInputBase(BECWidget): _device_handler = { BECDeviceFilter.DEVICE: Device, BECDeviceFilter.POSITIONER: Positioner, - BECDeviceFilter.SIGNAL: Signal, + BECDeviceFilter.SIGNAL: BECSignal, BECDeviceFilter.COMPUTED_SIGNAL: ComputedSignal, } @@ -72,9 +72,6 @@ class DeviceInputBase(BECWidget): self._device_filter = [] self._readout_filter = [] self._devices = [] - self.bec_dispatcher.client.callbacks.register( - EventType.DEVICE_UPDATE, self.update_devices_from_filters - ) ### QtSlots ### @@ -92,15 +89,13 @@ class DeviceInputBase(BECWidget): else: logger.warning(f"Device {device} is not in the filtered selection.") - @Slot(dict, dict) @Slot() - def update_devices_from_filters( - self, content: dict | None = None, metadata: dict | None = None - ): + def update_devices_from_filters(self): """Update the devices based on the current filter selection in self.device_filter and self.readout_filter. If apply_filter is False, it will not apply the filters, store the filter settings and return. """ + current_device = WidgetIO.get_value(widget=self, as_string=True) self.config.device_filter = self.device_filter self.config.readout_filter = self.readout_filter if self.apply_filter is False: @@ -111,6 +106,7 @@ class DeviceInputBase(BECWidget): # Filter based on readout priority devs = [dev for dev in devs if self._check_readout_filter(dev)] self.devices = [device.name for device in devs] + self.set_device(current_device) @Slot(list) def set_available_devices(self, devices: list[str]): @@ -153,6 +149,8 @@ class DeviceInputBase(BECWidget): def default(self, value: str): if self.validate_device(value) is False: return + self.config.default = value + WidgetIO.set_value(widget=self, value=value) @Property(bool) def apply_filter(self): @@ -343,7 +341,9 @@ class DeviceInputBase(BECWidget): for entry in filters: setattr(self, entry, True) - def _check_device_filter(self, device: Device | Signal | ComputedSignal | Positioner) -> bool: + def _check_device_filter( + self, device: Device | BECSignal | ComputedSignal | Positioner + ) -> bool: """Check if filter for device type is applied or not. Args: @@ -351,7 +351,9 @@ class DeviceInputBase(BECWidget): """ return all(isinstance(device, self._device_handler[entry]) for entry in self.device_filter) - def _check_readout_filter(self, device: Device | Signal | ComputedSignal | Positioner) -> bool: + def _check_readout_filter( + self, device: Device | BECSignal | ComputedSignal | Positioner + ) -> bool: """Check if filter for readout priority is applied or not. Args: @@ -373,7 +375,7 @@ class DeviceInputBase(BECWidget): dev = getattr(self.dev, device.lower(), None) if dev is None: raise ValueError( - f"Device {device} is not found in devicemanager {self.dev} as enabled device." + f"Device {device} is not found in the device manager {self.dev} as enabled device." ) return dev @@ -384,6 +386,7 @@ class DeviceInputBase(BECWidget): Args: device(str): Device to validate. """ - if device in self.devices: + all_devs = [dev.name for dev in self.dev.enabled_devices] + if device in self.devices and device in all_devs: return True return False diff --git a/bec_widgets/widgets/device_combobox/device_combobox.py b/bec_widgets/widgets/device_combobox/device_combobox.py index 9400f146..78c841ff 100644 --- a/bec_widgets/widgets/device_combobox/device_combobox.py +++ b/bec_widgets/widgets/device_combobox/device_combobox.py @@ -1,5 +1,7 @@ +from bec_lib.callback_handler import EventType from bec_lib.device import ReadoutPriority -from qtpy.QtCore import QSize +from qtpy.QtCore import QSize, Signal, Slot +from qtpy.QtGui import QPainter, QPaintEvent, QPen from qtpy.QtWidgets import QComboBox, QSizePolicy from bec_widgets.utils.colors import get_accent_colors @@ -26,6 +28,9 @@ class DeviceComboBox(DeviceInputBase, QComboBox): ICON_NAME = "list_alt" + device_selected = Signal(str) + device_config_update = Signal() + def __init__( self, parent=None, @@ -47,6 +52,7 @@ class DeviceComboBox(DeviceInputBase, QComboBox): self.arg_name = arg_name self.setSizePolicy(QSizePolicy.Preferred, QSizePolicy.Fixed) self.setMinimumSize(QSize(100, 0)) + self._callback_id = None self._is_valid_input = False self._accent_colors = get_accent_colors() # We do not consider the config that is passed here, this produced problems @@ -74,6 +80,28 @@ class DeviceComboBox(DeviceInputBase, QComboBox): # Set default device if passed if default is not None: self.set_device(default) + self._callback_id = self.bec_dispatcher.client.callbacks.register( + EventType.DEVICE_UPDATE, self.on_device_update + ) + self.device_config_update.connect(self.update_devices_from_filters) + self.currentTextChanged.connect(self.check_validity) + self.check_validity(self.currentText()) + + def on_device_update(self, action: str, content: dict) -> None: + """ + Callback for device update events. Triggers the device_update signal. + + Args: + action (str): The action that triggered the event. + content (dict): The content of the config update. + """ + if action in ["add", "remove", "reload"]: + self.device_config_update.emit() + + def cleanup(self): + """Cleanup the widget.""" + if self._callback_id is not None: + self.bec_dispatcher.client.callbacks.remove(self._callback_id) def get_current_device(self) -> object: """ @@ -85,6 +113,36 @@ class DeviceComboBox(DeviceInputBase, QComboBox): dev_name = self.currentText() return self.get_device_object(dev_name) + def paintEvent(self, event: QPaintEvent) -> None: + """Extend the paint event to set the border color based on the validity of the input. + + Args: + event (PySide6.QtGui.QPaintEvent) : Paint event. + """ + # logger.info(f"Received paint event: {event} in {self.__class__}") + super().paintEvent(event) + + if self._is_valid_input is False and self.isEnabled() is True: + painter = QPainter(self) + pen = QPen() + pen.setWidth(2) + pen.setColor(self._accent_colors.emergency) + painter.setPen(pen) + painter.drawRect(self.rect().adjusted(1, 1, -1, -1)) + painter.end() + + @Slot(str) + def check_validity(self, input_text: str) -> None: + """ + Check if the current value is a valid device name. + """ + if self.validate_device(input_text) is True: + self._is_valid_input = True + self.device_selected.emit(input_text.lower()) + else: + self._is_valid_input = False + self.update() + if __name__ == "__main__": # pragma: no cover # pylint: disable=import-outside-toplevel @@ -99,6 +157,7 @@ if __name__ == "__main__": # pragma: no cover layout = QVBoxLayout() widget.setLayout(layout) combo = DeviceComboBox() + combo.devices = ["samx", "dev1", "dev2", "dev3", "dev4"] layout.addWidget(combo) widget.show() app.exec_() diff --git a/bec_widgets/widgets/device_line_edit/device_line_edit.py b/bec_widgets/widgets/device_line_edit/device_line_edit.py index 669d05af..c348b476 100644 --- a/bec_widgets/widgets/device_line_edit/device_line_edit.py +++ b/bec_widgets/widgets/device_line_edit/device_line_edit.py @@ -1,3 +1,4 @@ +from bec_lib.callback_handler import EventType from bec_lib.device import ReadoutPriority from bec_lib.logger import bec_logger from qtpy.QtCore import QSize, Signal, Slot @@ -29,6 +30,7 @@ class DeviceLineEdit(DeviceInputBase, QLineEdit): """ device_selected = Signal(str) + device_config_update = Signal() ICON_NAME = "edit_note" @@ -46,6 +48,7 @@ class DeviceLineEdit(DeviceInputBase, QLineEdit): default: str | None = None, arg_name: str | None = None, ): + self._callback_id = None self._is_valid_input = False self._accent_colors = get_accent_colors() super().__init__(client=client, config=config, gui_id=gui_id) @@ -84,7 +87,28 @@ class DeviceLineEdit(DeviceInputBase, QLineEdit): # Set default device if passed if default is not None: self.set_device(default) + self._callback_id = self.bec_dispatcher.client.callbacks.register( + EventType.DEVICE_UPDATE, self.on_device_update + ) + self.device_config_update.connect(self.update_devices_from_filters) self.textChanged.connect(self.check_validity) + self.check_validity(self.text()) + + def on_device_update(self, action: str, content: dict) -> None: + """ + Callback for device update events. Triggers the device_update signal. + + Args: + action (str): The action that triggered the event. + content (dict): The content of the config update. + """ + if action in ["add", "remove", "reload"]: + self.device_config_update.emit() + + def cleanup(self): + """Cleanup the widget.""" + if self._callback_id is not None: + self.bec_dispatcher.client.callbacks.remove(self._callback_id) def get_current_device(self) -> object: """ diff --git a/bec_widgets/widgets/signal_line_edit/signal_line_edit.py b/bec_widgets/widgets/signal_line_edit/signal_line_edit.py index b6e9e9c2..36027fbe 100644 --- a/bec_widgets/widgets/signal_line_edit/signal_line_edit.py +++ b/bec_widgets/widgets/signal_line_edit/signal_line_edit.py @@ -1,3 +1,4 @@ +from bec_lib.device import Positioner from ophyd import Kind from qtpy.QtCore import QSize, Signal, Slot from qtpy.QtGui import QPainter, QPaintEvent, QPen @@ -61,7 +62,8 @@ class SignalLineEdit(DeviceSignalInputBase, QLineEdit): self.set_device(device) if default is not None: self.set_signal(default) - self.textChanged.connect(self.check_validity) + self.textChanged.connect(self.validate_device) + self.validate_device(self.text()) def get_current_device(self) -> object: """ @@ -96,12 +98,30 @@ class SignalLineEdit(DeviceSignalInputBase, QLineEdit): """ if self.validate_signal(input_text) is True: self._is_valid_input = True - if self.validate_device(self.device) is True: - self.device_signal_changed.emit(input_text) + self.on_text_changed(input_text) else: self._is_valid_input = False self.update() + @Slot(str) + def on_text_changed(self, text: str): + """Slot for text changed. If a device is selected and the signal is changed and valid it emits a signal. + For a positioner, the readback value has to be renamed to the device name. + + Args: + text (str): Text in the combobox. + """ + print("test") + if self.validate_device(self.device) is False: + return + if self.validate_signal(text) is False: + return + if text == "readback" and isinstance(self.get_device_object(self.device), Positioner): + device_signal = self.device + else: + device_signal = f"{self.device}_{text}" + self.device_signal_changed.emit(device_signal) + if __name__ == "__main__": # pragma: no cover # pylint: disable=import-outside-toplevel diff --git a/tests/unit_tests/client_mocks.py b/tests/unit_tests/client_mocks.py index f86b55bc..15885fb5 100644 --- a/tests/unit_tests/client_mocks.py +++ b/tests/unit_tests/client_mocks.py @@ -5,7 +5,7 @@ import fakeredis import pytest from bec_lib.redis_connector import RedisConnector -from bec_widgets.test_utils.client_mocks import DEVICES, DMMock, FakePositioner, Positioner +from bec_widgets.tests.utils import DEVICES, DMMock, FakePositioner, Positioner def fake_redis_server(host, port): diff --git a/tests/unit_tests/test_client_utils.py b/tests/unit_tests/test_client_utils.py index 29999887..49ff7dcc 100644 --- a/tests/unit_tests/test_client_utils.py +++ b/tests/unit_tests/test_client_utils.py @@ -4,7 +4,7 @@ import pytest from bec_widgets.cli.client import BECFigure from bec_widgets.cli.client_utils import BECGuiClientMixin, _start_plot_process -from bec_widgets.test_utils.client_mocks import FakeDevice +from bec_widgets.tests.utils import FakeDevice @pytest.fixture diff --git a/tests/unit_tests/test_device_input_base.py b/tests/unit_tests/test_device_input_base.py index 97e5d438..759e08c1 100644 --- a/tests/unit_tests/test_device_input_base.py +++ b/tests/unit_tests/test_device_input_base.py @@ -3,9 +3,8 @@ from unittest import mock import pytest from bec_lib.device import ReadoutPriority from qtpy.QtWidgets import QWidget -from typeguard import TypeCheckError -from bec_widgets.test_utils.client_mocks import FakePositioner +from bec_widgets.tests.utils import FakePositioner from bec_widgets.widgets.base_classes.device_input_base import BECDeviceFilter, DeviceInputBase from .client_mocks import mocked_client @@ -26,8 +25,9 @@ def device_input_base(qtbot, mocked_client): """Fixture with mocked FilterIO and WidgetIO""" with mock.patch("bec_widgets.utils.filter_io.FilterIO.set_selection"): with mock.patch("bec_widgets.utils.widget_io.WidgetIO.set_value"): - widget = create_widget(qtbot=qtbot, widget=DeviceInputWidget, client=mocked_client) - yield widget + with mock.patch("bec_widgets.utils.widget_io.WidgetIO.get_value"): + widget = create_widget(qtbot=qtbot, widget=DeviceInputWidget, client=mocked_client) + yield widget def test_device_input_base_init(device_input_base): diff --git a/tests/unit_tests/test_device_input_widgets.py b/tests/unit_tests/test_device_input_widgets.py index 7d941776..7cc32e46 100644 --- a/tests/unit_tests/test_device_input_widgets.py +++ b/tests/unit_tests/test_device_input_widgets.py @@ -35,7 +35,6 @@ def test_device_input_combobox_init(device_input_combobox): assert device_input_combobox.client is not None assert isinstance(device_input_combobox, DeviceComboBox) assert device_input_combobox.config.widget_class == "DeviceComboBox" - assert device_input_combobox.config.default is None assert device_input_combobox.devices == [ "samx", "samy", diff --git a/tests/unit_tests/test_device_signal_input.py b/tests/unit_tests/test_device_signal_input.py index 2dffe993..82374aa3 100644 --- a/tests/unit_tests/test_device_signal_input.py +++ b/tests/unit_tests/test_device_signal_input.py @@ -90,23 +90,32 @@ def test_device_signal_set_device(device_signal_base): assert device_signal_base.signals == ["readback", "setpoint", "velocity"] -def test_signal_combobox(device_signal_combobox): +def test_signal_combobox(qtbot, device_signal_combobox): """Test the signal_combobox""" - device_signal_combobox._signals == [] + container = [] + + def test_cb(input): + container.append(input) + + device_signal_combobox.device_signal_changed.connect(test_cb) + assert device_signal_combobox._signals == [] device_signal_combobox.include_normal_signals = True device_signal_combobox.include_hinted_signals = True device_signal_combobox.include_config_signals = True - device_signal_combobox.signals == [] + assert device_signal_combobox.signals == [] device_signal_combobox.set_device("samx") - device_signal_combobox.signals == ["readback", "setpoint", "velocity"] + assert device_signal_combobox.signals == ["readback", "setpoint", "velocity"] + qtbot.wait(100) + assert container == ["samx"] def test_signal_lineeidt(device_signal_line_edit): """Test the signal_combobox""" - device_signal_line_edit._signals == [] + + assert device_signal_line_edit._signals == [] device_signal_line_edit.include_normal_signals = True device_signal_line_edit.include_hinted_signals = True device_signal_line_edit.include_config_signals = True - device_signal_line_edit.signals == [] + assert device_signal_line_edit.signals == [] device_signal_line_edit.set_device("samx") - device_signal_line_edit.signals == ["readback", "setpoint", "velocity"] + assert device_signal_line_edit.signals == ["readback", "setpoint", "velocity"]