From d01d00339fce715e87d18331331c0b420dbbb779 Mon Sep 17 00:00:00 2001 From: wyzula-jan Date: Wed, 13 May 2026 10:34:13 +0200 Subject: [PATCH] refactor(device_input): consolidation of device/signal combobox logic; docstings added --- bec_widgets/utils/filter_io.py | 77 +++++- .../device_combobox/device_combobox.py | 182 +++++++++---- .../signal_combobox/signal_combobox.py | 245 +++++++++++++----- tests/unit_tests/test_device_input_base.py | 26 +- tests/unit_tests/test_device_input_widgets.py | 9 + tests/unit_tests/test_device_signal_input.py | 160 +++++++++++- tests/unit_tests/test_filter_io.py | 24 +- 7 files changed, 572 insertions(+), 151 deletions(-) diff --git a/bec_widgets/utils/filter_io.py b/bec_widgets/utils/filter_io.py index 607e8595..e6237aa7 100644 --- a/bec_widgets/utils/filter_io.py +++ b/bec_widgets/utils/filter_io.py @@ -2,7 +2,10 @@ from __future__ import annotations +from contextlib import nullcontext + from bec_lib.logger import bec_logger +from qtpy.QtCore import QSignalBlocker, QStringListModel from qtpy.QtWidgets import QComboBox from typeguard import TypeCheckError @@ -11,25 +14,63 @@ from bec_widgets.utils.ophyd_kind_util import Kind logger = bec_logger.logger -def replace_combobox_items(combo_box: QComboBox, items: list[str | tuple]) -> None: - """Replace all combobox entries with strings or ``(text, data)`` tuples.""" - combo_box.clear() - for item in items: - if isinstance(item, str): - combo_box.addItem(item) - else: - combo_box.addItem(*item) +def replace_combobox_items( + combo_box: QComboBox, + items: list[str | tuple], + *, + preserve_current_text: bool = False, + block_signals: bool = False, +) -> None: + """Replace all combobox entries. + + Args: + combo_box: Combobox whose entries should be replaced. + items: Entries to add. String entries are added as display text. Tuple entries are + passed to ``QComboBox.addItem`` as ``(text, data)``. + preserve_current_text: If True, restore the combobox text after replacing the items. + block_signals: If True, block combobox signals while the items are replaced. + """ + current_text = combo_box.currentText() + signal_blocker = QSignalBlocker(combo_box) if block_signals else nullcontext() + with signal_blocker: + combo_box.clear() + for item in items: + if isinstance(item, str): + combo_box.addItem(item) + else: + combo_box.addItem(*item) + if preserve_current_text: + combo_box.setCurrentText(current_text) -def combobox_contains_text(combo_box: QComboBox, text: str) -> bool: - """Return whether *text* is present as visible combobox text.""" - return any(combo_box.itemText(i) == text for i in range(combo_box.count())) +def restore_default_combobox_completer(combo_box: QComboBox) -> None: + """Restore Qt's default editable-combobox completer. + + Args: + combo_box: Editable combobox whose default completer should be restored. + """ + if combo_box.completer() is not None and combo_box.completer().model() == combo_box.model(): + return + current_text = combo_box.currentText() + combo_box.setEditable(False) + combo_box.setEditable(True) + combo_box.setCurrentText(current_text) def signal_items_for_kind( *, kind: Kind, signal_filter: set[Kind], device_info: dict, device_name: str ) -> list[tuple[str, dict]]: - """Build display entries for signals matching a BEC signal kind.""" + """Build display entries for signals matching a BEC signal kind. + + Args: + kind: Signal kind to collect. + signal_filter: Enabled signal kinds. + device_info: Signal metadata from the BEC device info dictionary. + device_name: Name of the device owning the signals. + + Returns: + Combobox entries as ``(display_text, signal_info)`` tuples. + """ items: list[tuple[str, dict]] = [] for signal_name, signal_info in device_info.items(): if kind not in signal_filter or signal_info.get("kind_str") != kind.name: @@ -54,7 +95,17 @@ def signal_items_for_kind( def get_bec_signals_for_classes( *, client, signal_class_filter: str | list[str], ndim_filter: int | list[int] | None = None ) -> list[tuple[str, str, dict]]: - """Return BEC signals filtered by signal class and optional dimensionality.""" + """Return BEC signals filtered by signal class and optional dimensionality. + + Args: + client: BEC client that provides ``device_manager.get_bec_signals``. + signal_class_filter: Signal class name or class names passed to the device manager. + ndim_filter: Optional dimensionality filter. If provided, only signals whose + ``describe.signal_info.ndim`` is in this value are returned. + + Returns: + Tuples of ``(device_name, signal_name, signal_config)`` for matching signals. + """ if not client or not hasattr(client, "device_manager"): return [] diff --git a/bec_widgets/widgets/control/device_input/device_combobox/device_combobox.py b/bec_widgets/widgets/control/device_input/device_combobox/device_combobox.py index 268ddbb7..df9e1e3c 100644 --- a/bec_widgets/widgets/control/device_input/device_combobox/device_combobox.py +++ b/bec_widgets/widgets/control/device_input/device_combobox/device_combobox.py @@ -1,3 +1,5 @@ +"""Editable combobox for selecting BEC devices.""" + from __future__ import annotations import enum @@ -14,13 +16,17 @@ from bec_widgets.utils.bec_connector import ConnectionConfig from bec_widgets.utils.bec_widget import BECWidget from bec_widgets.utils.colors import get_accent_colors from bec_widgets.utils.error_popups import SafeProperty, SafeSlot -from bec_widgets.utils.filter_io import get_bec_signals_for_classes, replace_combobox_items +from bec_widgets.utils.filter_io import ( + get_bec_signals_for_classes, + replace_combobox_items, + restore_default_combobox_completer, +) logger = bec_logger.logger class BECDeviceFilter(enum.Enum): - """Filter for BEC device classes.""" + """Device class filters accepted by :class:`DeviceComboBox`.""" DEVICE = "Device" POSITIONER = "Positioner" @@ -29,6 +35,20 @@ class BECDeviceFilter(enum.Enum): class DeviceInputConfig(ConnectionConfig): + """Serializable configuration for :class:`DeviceComboBox`. + + Attributes: + device_filter: Enabled device class filters as ``BECDeviceFilter.value`` strings. + readout_filter: Enabled readout priority filters as ``ReadoutPriority.value`` strings. + devices: Explicit device names shown by the combobox. + default: Device selected by default. + arg_name: Optional argument name used by scan/input widgets. + apply_filter: Whether the combobox should refresh devices from the BEC device manager. + signal_class_filter: Signal class names used to restrict listed devices. + autocomplete: Whether to use the explicit completer model instead of Qt's default + editable-combobox completer. + """ + device_filter: list[str] = Field(default_factory=list) readout_filter: list[str] = Field(default_factory=list) devices: list[str] = Field(default_factory=list) @@ -41,6 +61,17 @@ class DeviceInputConfig(ConnectionConfig): @field_validator("device_filter") @classmethod def check_device_filter(cls, value): + """Validate configured device class filters. + + Args: + value: Device class filter values from the persisted widget configuration. + + Returns: + The validated filter values. + + Raises: + ValueError: If any configured filter is not a valid ``BECDeviceFilter`` value. + """ valid_filters = [entry.value for entry in BECDeviceFilter] for device_filter in value: if device_filter not in valid_filters: @@ -52,6 +83,17 @@ class DeviceInputConfig(ConnectionConfig): @field_validator("readout_filter") @classmethod def check_readout_filter(cls, value): + """Validate configured readout priority filters. + + Args: + value: Readout priority filter values from the persisted widget configuration. + + Returns: + The validated filter values. + + Raises: + ValueError: If any configured filter is not a valid ``ReadoutPriority`` value. + """ valid_filters = [entry.value for entry in ReadoutPriority] for readout_filter in value: if readout_filter not in valid_filters: @@ -62,20 +104,23 @@ class DeviceInputConfig(ConnectionConfig): class DeviceComboBox(BECWidget, QComboBox): - """ - Editable combobox for BEC device input. + """Editable combobox for selecting a BEC device. Args: - parent: Parent widget. - client: BEC client object. - config: Device input configuration. - gui_id: GUI ID. - device_filter: Device class filter from BECDeviceFilter. - readout_priority_filter: Readout priority filter from ReadoutPriority. - available_devices: Explicit list of devices. Passing this disables automatic filtering. - default: Default device name. - arg_name: Argument name used by scan/input widgets. - signal_class_filter: Only show devices with signals of these classes. + parent: Optional parent widget. + client: Optional BEC client object. + config: Device input configuration as a ``DeviceInputConfig`` instance or dictionary. + gui_id: Optional GUI identifier. + device_filter: Device class filter or filters from ``BECDeviceFilter``. + readout_priority_filter: Readout priority filter or filters from ``ReadoutPriority``. + available_devices: Explicit device names to show. Passing this disables automatic + BEC filtering. + default: Device name selected during initialization. + arg_name: Optional argument name used by scan/input widgets. + signal_class_filter: Signal class names used to restrict listed devices. + autocomplete: If True, use the explicit line-edit style completer. If False, keep + Qt's default editable-combobox completion behavior. + **kwargs: Additional keyword arguments passed to ``BECWidget``. """ ICON_NAME = "list_alt" @@ -186,13 +231,25 @@ class DeviceComboBox(BECWidget, QComboBox): @staticmethod def _process_config(config: DeviceInputConfig | dict | None) -> DeviceInputConfig: + """Normalize user-provided configuration. + + Args: + config: Existing configuration, configuration dictionary, or None. + + Returns: + A validated ``DeviceInputConfig`` instance. + """ if config is None: return DeviceInputConfig(widget_class="DeviceComboBox") return DeviceInputConfig.model_validate(config) @SafeSlot(str) def set_device(self, device: str): - """Set the current device if it is valid for the current filters.""" + """Set the current device if it is valid for the current filters. + + Args: + device: Device name to select. + """ if self.validate_device(device): self.setCurrentText(device) self.config.default = device @@ -204,7 +261,6 @@ class DeviceComboBox(BECWidget, QComboBox): @SafeSlot() def update_devices_from_filters(self): """Refresh the available device list from current device/readout/signal filters.""" - current_device = self.currentText() self.config.device_filter = [entry.value for entry in self.device_filter] self.config.readout_filter = [entry.value for entry in self.readout_filter] self.config.signal_class_filter = self.signal_class_filter @@ -215,13 +271,14 @@ class DeviceComboBox(BECWidget, QComboBox): devices = [device for device in devices if self._check_device_filter(device)] devices = [device for device in devices if self._check_readout_filter(device)] self.devices = [device.name for device in devices] - if current_device: - self.setCurrentText(current_device) - self.check_validity(current_device) @SafeSlot(list) def set_available_devices(self, devices: list[str]): - """Use an explicit device list and disable automatic BEC filtering.""" + """Use an explicit device list and disable automatic BEC filtering. + + Args: + devices: Device names to show in the combobox. + """ self.apply_filter = False self.devices = devices @@ -374,10 +431,9 @@ class DeviceComboBox(BECWidget, QComboBox): def autocomplete(self, value: bool) -> None: self.config.autocomplete = value if value: - completer = QCompleter(self._completer_model, self) - self.setCompleter(completer) + self.setCompleter(QCompleter(self._completer_model, self)) else: - self._restore_default_completer() + restore_default_combobox_completer(self) @property def device_filter(self) -> list[BECDeviceFilter]: @@ -405,7 +461,12 @@ class DeviceComboBox(BECWidget, QComboBox): def set_device_filter( self, filter_selection: BECDeviceFilter | str | list[BECDeviceFilter | str] ): - """Enable one or more device class filters.""" + """Enable one or more device class filters. + + Args: + filter_selection: Filter or filters to enable. Strings must match + ``BECDeviceFilter.value``. + """ for device_filter in self._as_list(filter_selection): normalized = self._normalize_device_filter(device_filter) if normalized is None: @@ -416,7 +477,12 @@ class DeviceComboBox(BECWidget, QComboBox): def set_readout_priority_filter( self, filter_selection: ReadoutPriority | str | list[ReadoutPriority | str] ): - """Enable one or more readout priority filters.""" + """Enable one or more readout priority filters. + + Args: + filter_selection: Readout priority filter or filters to enable. Strings must match + ``ReadoutPriority.value``. + """ for readout_filter in self._as_list(filter_selection): normalized = self._normalize_readout_filter(readout_filter) if normalized is None: @@ -427,7 +493,12 @@ class DeviceComboBox(BECWidget, QComboBox): self._set_readout_filter_enabled(normalized, True) def on_device_update(self, action: str, content: dict) -> None: - """Refresh filters when the BEC device configuration changes.""" + """Refresh filters when the BEC device configuration changes. + + Args: + action: Device update action emitted by BEC. + content: Device update payload. Currently unused. + """ if action in ["add", "remove", "reload"]: self.device_config_update.emit() @@ -438,12 +509,20 @@ class DeviceComboBox(BECWidget, QComboBox): super().cleanup() def get_current_device(self) -> object: - """Return the current BEC device object.""" + """Return the current BEC device object. + + Returns: + Device object for the current combobox text. + """ return self.get_device_object(self._device_name_from_text(self.currentText())) @Slot(str) def check_validity(self, input_text: str) -> None: - """Validate current text and update visual state.""" + """Validate current text and update visual state. + + Args: + input_text: Current combobox text. + """ if self.validate_device(input_text): self._is_valid_input = True self.device_selected.emit(input_text) @@ -455,7 +534,15 @@ class DeviceComboBox(BECWidget, QComboBox): self.setStyleSheet("border: 1px solid red;") def validate_device(self, device: str | None) -> bool: - """Validate a device against the current filtered device selection.""" + """Validate a device against the current filtered device selection. + + Args: + device: Device name or displayed device text to validate. + + Returns: + True if the device exists in the current BEC device manager and is present in the + filtered combobox list. + """ if not device: return False device_name = self._device_name_from_text(device) @@ -463,7 +550,17 @@ class DeviceComboBox(BECWidget, QComboBox): return device_name in self.devices and device_name in all_devices def get_device_object(self, device: str) -> object: - """Return a device object by name.""" + """Return a device object by name. + + Args: + device: Device name. + + Returns: + BEC device object. + + Raises: + ValueError: If the device is not available in the device manager. + """ dev = getattr(self.dev, device, None) if dev is None: raise ValueError( @@ -506,13 +603,11 @@ class DeviceComboBox(BECWidget, QComboBox): ) -> bool: if not self.device_filter: return True - return any(isinstance(device, self._device_handler[entry]) for entry in self.device_filter) + return all(isinstance(device, self._device_handler[entry]) for entry in self.device_filter) def _check_readout_filter( self, device: Device | BECSignal | ComputedSignal | Positioner ) -> bool: - if not self.readout_filter: - return True return device.readout_priority in self.readout_filter def _filter_devices_by_signal_class( @@ -527,23 +622,10 @@ class DeviceComboBox(BECWidget, QComboBox): return [device for device in devices if device.name in allowed_devices] def _replace_items(self, devices: list[str]): - current_text = self.currentText() - replace_combobox_items(self, devices) - self._update_completer_model(devices) - if self._set_first_element_as_empty: - self.insertItem(0, "") - self.setCurrentText(current_text) - - def _update_completer_model(self, items: list[str]) -> None: - self._completer_model.setStringList(items) - - def _restore_default_completer(self) -> None: - if self.completer() is not None and self.completer().model() == self.model(): - return - current_text = self.currentText() - self.setEditable(False) - self.setEditable(True) - self.setCurrentText(current_text) + items = [""] + devices if self._set_first_element_as_empty else devices + replace_combobox_items(self, items, preserve_current_text=True, block_signals=True) + self._completer_model.setStringList(devices) + self.check_validity(self.currentText()) def _device_name_from_text(self, text: str) -> str: index = self.findText(text) diff --git a/bec_widgets/widgets/control/device_input/signal_combobox/signal_combobox.py b/bec_widgets/widgets/control/device_input/signal_combobox/signal_combobox.py index 6af2946d..296b7d3c 100644 --- a/bec_widgets/widgets/control/device_input/signal_combobox/signal_combobox.py +++ b/bec_widgets/widgets/control/device_input/signal_combobox/signal_combobox.py @@ -1,8 +1,11 @@ +"""Editable combobox for selecting BEC device signals.""" + from __future__ import annotations from bec_lib.callback_handler import EventType from bec_lib.device import Signal as BECSignal from bec_lib.logger import bec_logger +from pydantic import Field from qtpy.QtCore import Property, QSize, QStringListModel, Qt, Signal, Slot from qtpy.QtWidgets import QComboBox, QCompleter, QSizePolicy @@ -12,6 +15,7 @@ from bec_widgets.utils.error_popups import SafeProperty, SafeSlot from bec_widgets.utils.filter_io import ( get_bec_signals_for_classes, replace_combobox_items, + restore_default_combobox_completer, signal_items_for_kind, ) from bec_widgets.utils.ophyd_kind_util import Kind @@ -20,35 +24,50 @@ logger = bec_logger.logger class SignalComboBoxConfig(ConnectionConfig): - """Configuration for SignalComboBox.""" + """Serializable configuration for :class:`SignalComboBox`. - signal_filter: list[str] | None = None - signal_class_filter: list[str] | None = None + Attributes: + signal_filter: Enabled signal kind filters as ``Kind.name`` strings. + signal_class_filter: Signal class names used to build the signal list from BEC. + ndim_filter: Optional dimensionality filter for class-based signal lists. + default: Signal selected by default. + arg_name: Optional argument name used by scan/input widgets. + device: Device name used to scope kind-based signal filtering. + signals: Signal names available after filtering. + autocomplete: Whether to use the explicit completer model instead of Qt's default + editable-combobox completer. + """ + + signal_filter: list[str] = Field(default_factory=list) + signal_class_filter: list[str] = Field(default_factory=list) ndim_filter: int | list[int] | None = None default: str | None = None arg_name: str | None = None device: str | None = None - signals: list[str] | None = None + signals: list[str] = Field(default_factory=list) autocomplete: bool = False class SignalComboBox(BECWidget, QComboBox): - """ - Editable combobox for selecting BEC device signals. + """Editable combobox for selecting a signal from a BEC device. Args: - parent: Parent widget. - client: BEC client object. - config: Signal combobox configuration. - gui_id: GUI ID. - device: Device name to filter signals from. - signal_filter: Signal kind filters from Kind. - signal_class_filter: Signal classes to show. - ndim_filter: Dimensionality filter for signal-class based lists. - default: Default signal name. - arg_name: Argument name used by scan/input widgets. - store_signal_config: Whether to store signal config in item data. - require_device: If True, signals are only shown/validated when a device is set. + parent: Optional parent widget. + client: Optional BEC client object. + config: Signal combobox configuration as a ``SignalComboBoxConfig`` instance or + dictionary. + gui_id: Optional GUI identifier. + device: Device name used to scope kind-based signal filtering. + signal_filter: Signal kind filter or filters from ``Kind``. + signal_class_filter: Signal class names used to build a class-based signal list. + ndim_filter: Dimensionality filter for class-based signal lists. + default: Signal selected during initialization. + arg_name: Optional argument name used by scan/input widgets. + store_signal_config: Whether to store each signal config in the item data. + require_device: If True, class-based signal filtering requires a valid selected device. + autocomplete: If True, use the explicit line-edit style completer. If False, keep + Qt's default editable-combobox completion behavior. + **kwargs: Additional keyword arguments passed to ``BECWidget``. """ USER_ACCESS = ["set_signal", "set_device", "signals", "get_signal_name"] @@ -134,13 +153,25 @@ class SignalComboBox(BECWidget, QComboBox): @staticmethod def _process_config(config: SignalComboBoxConfig | dict | None) -> SignalComboBoxConfig: + """Normalize user-provided configuration. + + Args: + config: Existing configuration, configuration dictionary, or None. + + Returns: + A validated ``SignalComboBoxConfig`` instance. + """ if config is None: return SignalComboBoxConfig(widget_class="SignalComboBox") return SignalComboBoxConfig.model_validate(config) @SafeSlot(str) def set_signal(self, signal: str): - """Set the current signal if it is available in the combobox.""" + """Set the current signal if it is available in the combobox. + + Args: + signal: Signal display text, object name, or component name to select. + """ display_text = self._display_text_for_signal(signal) if display_text is None: logger.warning( @@ -152,12 +183,18 @@ class SignalComboBox(BECWidget, QComboBox): @SafeSlot(str) def set_device(self, device: str | None): - """Set the device that scopes kind-based signal filtering.""" - if not self.validate_device(device): - self._device = None - else: - self._device = device + """Set the device that scopes kind-based signal filtering. + + Args: + device: Device name to use for signal filtering. Invalid or empty values clear + the current device and signal selection. + """ + previous_device = self._device + valid_device = device if self.validate_device(device) else None + self._device = valid_device self.config.device = self._device + if valid_device is None or valid_device != previous_device: + self.setCurrentText("") self.update_signals_from_filters() @SafeSlot() @@ -165,7 +202,12 @@ class SignalComboBox(BECWidget, QComboBox): def update_signals_from_filters( self, content: dict | None = None, metadata: dict | None = None ): - """Refresh available signals from the current device and filters.""" + """Refresh available signals from the current device and filters. + + Args: + content: Optional callback payload from BEC device updates. Currently unused. + metadata: Optional callback metadata from BEC device updates. Currently unused. + """ self.config.signal_filter = [kind.name for kind in self.signal_filter] if self._signal_class_filter: @@ -297,10 +339,9 @@ class SignalComboBox(BECWidget, QComboBox): def autocomplete(self, value: bool) -> None: self.config.autocomplete = value if value: - completer = QCompleter(self._completer_model, self) - self.setCompleter(completer) + self.setCompleter(QCompleter(self._completer_model, self)) else: - self._restore_default_completer() + restore_default_combobox_completer(self) @property def signals(self) -> list[str | tuple[str, dict]]: @@ -335,7 +376,12 @@ class SignalComboBox(BECWidget, QComboBox): return self.get_signal_name() def set_filter(self, filter_selection: Kind | str | list[Kind | str] | None): - """Enable one or more signal kind filters.""" + """Enable one or more signal kind filters. + + Args: + filter_selection: Filter or filters to enable. Strings must match ``Kind`` member + names. + """ if filter_selection is None: return filters = filter_selection if isinstance(filter_selection, list) else [filter_selection] @@ -346,11 +392,22 @@ class SignalComboBox(BECWidget, QComboBox): self.update_signals_from_filters() def get_available_filters(self) -> list[Kind]: - """Return available signal kind filters.""" + """Return available signal kind filters. + + Returns: + Signal kinds supported by the combobox. + """ return [Kind.hinted, Kind.normal, Kind.config] def get_device_object(self, device: str) -> object | None: - """Return a BEC device object by name.""" + """Return a BEC device object by name. + + Args: + device: Device name. + + Returns: + Device object if it exists in the device manager, otherwise None. + """ dev = getattr(self.dev, device, None) if dev is None: logger.warning(f"Device {device} not found in devicemanager.") @@ -358,7 +415,18 @@ class SignalComboBox(BECWidget, QComboBox): return dev def validate_device(self, device: str | None, raise_on_false: bool = False) -> bool: - """Validate that a device exists in the current device manager.""" + """Validate that a device exists in the current device manager. + + Args: + device: Device name to validate. + raise_on_false: If True, raise instead of returning False for missing devices. + + Returns: + True if the device exists in the current device manager. + + Raises: + ValueError: If ``raise_on_false`` is True and the device is missing. + """ if device in self.dev: return True if raise_on_false: @@ -366,11 +434,27 @@ class SignalComboBox(BECWidget, QComboBox): return False def validate_signal(self, signal: str) -> bool: - """Validate a signal by display text, object name, or component name.""" + """Validate a signal by display text, object name, or component name. + + Args: + signal: Signal display text, object name, or component name. + + Returns: + True if the signal is present in the current filtered signal list. + """ + if not signal: + return False return self._display_text_for_signal(signal) is not None def set_to_obj_name(self, obj_name: str) -> bool: - """Select the item whose signal config has the given object name.""" + """Select the item whose signal config has the given object name. + + Args: + obj_name: Signal object name to select. + + Returns: + True if a matching signal was selected. + """ index = self._find_signal_index(obj_name) if index < 0: return False @@ -378,7 +462,11 @@ class SignalComboBox(BECWidget, QComboBox): return True def set_to_first_enabled(self) -> bool: - """Select the first enabled item.""" + """Select the first enabled item. + + Returns: + True if an enabled item was found and selected. + """ for index in range(self.count()): item = self.model().item(index) if item is not None and item.isEnabled(): @@ -387,7 +475,12 @@ class SignalComboBox(BECWidget, QComboBox): return False def get_signal_name(self) -> str: - """Return the selected signal object name when available.""" + """Return the selected signal object name when available. + + Returns: + Signal object name from item data, or the current display text when no item data + is available. + """ current_text = self.currentText() index = self._find_signal_index(current_text) if index < 0: @@ -399,14 +492,24 @@ class SignalComboBox(BECWidget, QComboBox): return current_text def get_signal_config(self) -> dict | None: - """Return the selected signal config if item-data storage is enabled.""" + """Return the selected signal config if item-data storage is enabled. + + Returns: + Selected signal configuration dictionary, or None when item-data storage is disabled + or the current item has no configuration. + """ if not self._store_signal_config: return None signal_info = self.itemData(self.currentIndex()) return signal_info if isinstance(signal_info, dict) else None def update_signals_from_signal_classes(self, ndim_filter: int | list[int] | None = None): - """Refresh signals from device_manager.get_bec_signals for class-based filtering.""" + """Refresh signals from ``device_manager.get_bec_signals`` for class-based filtering. + + Args: + ndim_filter: Optional dimensionality filter overriding the configured value for + this refresh. + """ if not self._signal_class_filter: return @@ -423,8 +526,8 @@ class SignalComboBox(BECWidget, QComboBox): ndim_filter=self.config.ndim_filter, ) - self.clear() - self._signals = [] + combo_items: list[str | tuple[str, dict]] = [] + item_tooltips: dict[int, str] = {} for device_name, signal_name, signal_config in signals: if self._device and device_name != self._device: continue @@ -436,21 +539,19 @@ class SignalComboBox(BECWidget, QComboBox): continue if self._store_signal_config: - self.addItem(signal_name, signal_config) + combo_items.append((signal_name, signal_config)) else: - self.addItem(signal_name) + combo_items.append(signal_name) - self._signals.append(signal_name) storage_name = signal_config.get("storage_name", "") if storage_name: - self.setItemData(self.count() - 1, storage_name, Qt.ItemDataRole.ToolTipRole) + item_tooltips[len(combo_items) - 1] = storage_name - self.config.signals = [ - entry if isinstance(entry, str) else entry[0] for entry in self._signals - ] - self._update_completer_model(self.config.signals) - if self._set_first_element_as_empty and self.count() > 0 and self.itemText(0) != "": - self.insertItem(0, "") + self.signals = combo_items + tooltip_offset = 1 if self._set_first_element_as_empty and self.count() else 0 + for item_index, tooltip in item_tooltips.items(): + self.setItemData(item_index + tooltip_offset, tooltip, Qt.ItemDataRole.ToolTipRole) + self.check_validity(self.currentText()) @SafeSlot() def reset_selection(self): @@ -461,12 +562,20 @@ class SignalComboBox(BECWidget, QComboBox): @SafeSlot(str) def on_text_changed(self, text: str): - """Validate the current text when edited or selected.""" + """Validate the current text when edited or selected. + + Args: + text: Current combobox text. + """ self.check_validity(text) @Slot(str) def check_validity(self, input_text: str) -> None: - """Validate current text and update visual state.""" + """Validate current text and update visual state. + + Args: + input_text: Current combobox text. + """ if self._signal_class_filter: is_valid = not (self._require_device and not self._device) and self.validate_signal( input_text @@ -513,12 +622,17 @@ class SignalComboBox(BECWidget, QComboBox): self._config_signals = config self.signals = self._hinted_signals + self._normal_signals + self._config_signals self._insert_group_headers() + self.check_validity(self.currentText()) - def _replace_signal_items(self): - replace_combobox_items(self, self._signals) - self._update_completer_model(self._signal_display_texts(self._signals)) - if self._set_first_element_as_empty and self.count() > 0 and self.itemText(0) != "": - self.insertItem(0, "") + def _replace_signal_items(self, items: list[str | tuple[str, dict]] | None = None): + combo_items = self._signals if items is None else items + display_items = [""] + combo_items if self._set_first_element_as_empty else combo_items + replace_combobox_items( + self, display_items, preserve_current_text=bool(self.currentText()), block_signals=True + ) + self._completer_model.setStringList( + [entry if isinstance(entry, str) else entry[0] for entry in combo_items] + ) def _insert_group_headers(self): offset = ( @@ -550,6 +664,8 @@ class SignalComboBox(BECWidget, QComboBox): @staticmethod def _signal_info_matches(signal_info: dict, signal: str) -> bool: + if not signal: + return False return signal in { signal_info.get("obj_name"), signal_info.get("component_name"), @@ -566,21 +682,6 @@ class SignalComboBox(BECWidget, QComboBox): return item_index return -1 - @staticmethod - def _signal_display_texts(signals: list[str | tuple[str, dict]]) -> list[str]: - return [entry[0] if isinstance(entry, tuple) else entry for entry in signals] - - def _update_completer_model(self, items: list[str]) -> None: - self._completer_model.setStringList(items) - - def _restore_default_completer(self) -> None: - if self.completer() is not None and self.completer().model() == self.model(): - return - current_text = self.currentText() - self.setEditable(False) - self.setEditable(True) - self.setCurrentText(current_text) - if __name__ == "__main__": # pragma: no cover from qtpy.QtWidgets import QApplication, QVBoxLayout, QWidget diff --git a/tests/unit_tests/test_device_input_base.py b/tests/unit_tests/test_device_input_base.py index 0b73be3e..47dc9ebf 100644 --- a/tests/unit_tests/test_device_input_base.py +++ b/tests/unit_tests/test_device_input_base.py @@ -1,6 +1,6 @@ from unittest import mock -from bec_lib.device import ReadoutPriority +from bec_lib.device import Positioner, ReadoutPriority from bec_widgets.widgets.control.device_input.device_combobox.device_combobox import ( BECDeviceFilter, @@ -86,6 +86,30 @@ def test_device_combobox_properties(qtbot, mocked_client): assert ReadoutPriority.ON_REQUEST in widget.readout_filter +def test_device_combobox_multiple_device_filters_match_main_intersection(qtbot, mocked_client): + widget = create_widget(qtbot=qtbot, widget=DeviceComboBox, client=mocked_client) + + widget.filter_to_device = True + widget.filter_to_positioner = True + + assert widget.devices + assert all(isinstance(getattr(widget.dev, device), Positioner) for device in widget.devices) + assert "eiger" not in widget.devices + + +def test_device_combobox_empty_readout_filter_matches_main_empty_selection(qtbot, mocked_client): + widget = create_widget(qtbot=qtbot, widget=DeviceComboBox, client=mocked_client) + + widget.readout_monitored = False + widget.readout_baseline = False + widget.readout_async = False + widget.readout_continuous = False + widget.readout_on_request = False + + assert widget.readout_filter == [] + assert widget.devices == [] + + def test_device_combobox_signal_class_filter(qtbot, mocked_client): mocked_client.device_manager.get_bec_signals = mock.MagicMock( return_value=[ diff --git a/tests/unit_tests/test_device_input_widgets.py b/tests/unit_tests/test_device_input_widgets.py index 2a7e8c31..c99fa173 100644 --- a/tests/unit_tests/test_device_input_widgets.py +++ b/tests/unit_tests/test_device_input_widgets.py @@ -95,6 +95,15 @@ def test_device_input_combobox_autocomplete(qtbot, mocked_client): assert widget.completer().model() == widget.model() +def test_device_input_combobox_refresh_preserves_manual_text(device_input_combobox): + device_input_combobox.setCurrentText("manual_device") + + device_input_combobox.update_devices_from_filters() + + assert device_input_combobox.currentText() == "manual_device" + assert device_input_combobox.is_valid_input is False + + def test_get_device_from_input_combobox_init(device_input_combobox): device_input_combobox.setCurrentIndex(0) device_text = device_input_combobox.currentText() diff --git a/tests/unit_tests/test_device_signal_input.py b/tests/unit_tests/test_device_signal_input.py index 904b600e..71389273 100644 --- a/tests/unit_tests/test_device_signal_input.py +++ b/tests/unit_tests/test_device_signal_input.py @@ -8,7 +8,10 @@ from bec_widgets.widgets.control.device_input.device_combobox.device_combobox im BECDeviceFilter, DeviceComboBox, ) -from bec_widgets.widgets.control.device_input.signal_combobox.signal_combobox import SignalComboBox +from bec_widgets.widgets.control.device_input.signal_combobox.signal_combobox import ( + SignalComboBox, + SignalComboBoxConfig, +) from .client_mocks import mocked_client from .conftest import create_widget @@ -18,6 +21,10 @@ class FakeSignal(Signal): """Fake signal used by SignalComboBox tests.""" +def signal_names(signals): + return [entry[0] if isinstance(entry, tuple) else entry for entry in signals] + + @pytest.fixture def device_signal_combobox(qtbot, mocked_client): widget = create_widget(qtbot=qtbot, widget=SignalComboBox, client=mocked_client) @@ -49,6 +56,19 @@ def test_signal_combobox_init(device_signal_combobox): assert device_signal_combobox.completer().model() == device_signal_combobox.model() +def test_signal_combobox_config_defaults_are_independent_lists(): + config_a = SignalComboBoxConfig(widget_class="SignalComboBox") + config_b = SignalComboBoxConfig(widget_class="SignalComboBox") + + config_a.signal_filter.append("hinted") + config_a.signal_class_filter.append("AsyncSignal") + config_a.signals.append("sig") + + assert config_b.signal_filter == [] + assert config_b.signal_class_filter == [] + assert config_b.signals == [] + + def test_signal_combobox_autocomplete(qtbot, mocked_client): widget = create_widget( qtbot=qtbot, widget=SignalComboBox, client=mocked_client, autocomplete=True @@ -117,6 +137,34 @@ def test_signal_combobox(qtbot, device_signal_combobox): assert device_signal_combobox._hinted_signals == [("fake_signal", {})] +def test_linked_device_combobox_updates_signal_combobox_on_each_text_change( + qtbot, test_device_signal_combo +): + device, signal = test_device_signal_combo + device.currentTextChanged.connect(signal.set_device) + + emitted_device_texts: list[str] = [] + device.currentTextChanged.connect(emitted_device_texts.append) + + device.setCurrentText("samx") + assert signal.device == "samx" + assert signal.currentText() == "samx (readback)" + + device.setCurrentText("sa") + + assert emitted_device_texts[-1] == "sa" + assert signal.device == "" + assert signal.signals == [] + assert signal.currentText() == "" + assert signal.is_valid_input is False + + device.setCurrentText("samx") + + assert emitted_device_texts[-1] == "samx" + assert signal.device == "samx" + assert [entry[0] for entry in signal.signals] == ["samx (readback)", "setpoint", "velocity"] + + def test_device_signal_input_base_cleanup(qtbot, mocked_client): with mock.patch.object(mocked_client.callbacks, "remove"): widget = SignalComboBox(client=mocked_client) @@ -224,11 +272,107 @@ def test_signal_combobox_signal_class_filter_by_device(qtbot, mocked_client): device="samx", ) - assert widget.signals == ["samx_readback_async"] + assert signal_names(widget.signals) == ["samx_readback_async"] assert widget.signal_class_filter == ["AsyncSignal"] widget.set_device("samy") - assert widget.signals == ["samy_readback_async"] + assert signal_names(widget.signals) == ["samy_readback_async"] + + +def test_signal_combobox_signal_class_filter_selects_by_metadata(qtbot, mocked_client): + """Class-based signal lists should support obj_name/component_name lookup.""" + mocked_client.device_manager.get_bec_signals = mock.MagicMock( + return_value=[ + ( + "eiger", + "image", + { + "obj_name": "eiger_image", + "component_name": "det.image", + "signal_class": "PreviewSignal", + "describe": {"signal_info": {"ndim": 2}}, + }, + ) + ] + ) + widget = create_widget( + qtbot=qtbot, + widget=SignalComboBox, + client=mocked_client, + signal_class_filter=["PreviewSignal"], + ndim_filter=[2], + device="eiger", + ) + + assert widget.validate_signal("eiger_image") is True + assert widget.validate_signal("det.image") is True + assert widget.set_to_obj_name("eiger_image") is True + assert widget.currentText() == "image" + + widget.set_signal("det.image") + + assert widget.currentText() == "image" + + +def test_signal_combobox_signal_class_update_revalidates_selected_signal(qtbot, mocked_client): + """Signal-class rebuilds should validate after items and signal metadata are in sync.""" + mocked_client.device_manager.get_bec_signals = mock.MagicMock( + return_value=[ + ( + "eiger", + "img", + { + "obj_name": "img", + "signal_class": "PreviewSignal", + "describe": {"signal_info": {"ndim": 2}}, + }, + ) + ] + ) + widget = create_widget( + qtbot=qtbot, + widget=SignalComboBox, + client=mocked_client, + signal_class_filter=["PreviewSignal"], + ndim_filter=[2], + require_device=True, + ) + + widget.set_device("eiger") + + assert widget.currentText() == "img" + assert widget.is_valid_input is True + + +def test_signal_combobox_signal_class_refresh_preserves_manual_text(qtbot, mocked_client): + mocked_client.device_manager.get_bec_signals = mock.MagicMock( + return_value=[ + ( + "eiger", + "img", + { + "obj_name": "img", + "signal_class": "PreviewSignal", + "describe": {"signal_info": {"ndim": 2}}, + }, + ) + ] + ) + widget = create_widget( + qtbot=qtbot, + widget=SignalComboBox, + client=mocked_client, + signal_class_filter=["PreviewSignal"], + ndim_filter=[2], + require_device=True, + ) + + widget.set_device("eiger") + widget.setCurrentText("manual_signal") + widget.update_signals_from_signal_classes() + + assert widget.currentText() == "manual_signal" + assert widget.is_valid_input is False def test_signal_class_filter_setter_clears_to_kind_filters(qtbot, mocked_client): @@ -243,7 +387,7 @@ def test_signal_class_filter_setter_clears_to_kind_filters(qtbot, mocked_client) signal_class_filter=["AsyncSignal"], device="samx", ) - assert widget.signals == ["samx_readback_async"] + assert signal_names(widget.signals) == ["samx_readback_async"] widget.signal_class_filter = [] samx = widget.dev.samx @@ -266,7 +410,7 @@ def test_signal_class_filter_setter_none_reverts_to_kind_filters(qtbot, mocked_c signal_class_filter=["AsyncSignal"], device="samx", ) - assert widget.signals == ["samx_readback_async"] + assert signal_names(widget.signals) == ["samx_readback_async"] widget.signal_class_filter = None samx = widget.dev.samx @@ -333,12 +477,12 @@ def test_signal_combobox_class_kind_ndim_filters(qtbot, mocked_client): ) # Default kinds are hinted + normal, ndim=1, device=samx - assert widget.signals == ["sig1"] + assert signal_names(widget.signals) == ["sig1"] # Enable config kinds and widen ndim to include sig2 widget.include_config_signals = True widget.ndim_filter = 2 - assert widget.signals == ["sig2"] + assert signal_names(widget.signals) == ["sig2"] def test_signal_combobox_require_device_validation(qtbot, mocked_client): @@ -366,7 +510,7 @@ def test_signal_combobox_require_device_validation(qtbot, mocked_client): assert widget.signals == [] widget.set_device("samx") - assert widget.signals == ["sig1"] + assert signal_names(widget.signals) == ["sig1"] resets: list[str] = [] widget.signal_reset.connect(lambda: resets.append("reset")) diff --git a/tests/unit_tests/test_filter_io.py b/tests/unit_tests/test_filter_io.py index 9bd0b7c4..0de54e9b 100644 --- a/tests/unit_tests/test_filter_io.py +++ b/tests/unit_tests/test_filter_io.py @@ -1,10 +1,6 @@ from qtpy.QtWidgets import QComboBox -from bec_widgets.utils.filter_io import ( - combobox_contains_text, - get_bec_signals_for_classes, - replace_combobox_items, -) +from bec_widgets.utils.filter_io import get_bec_signals_for_classes, replace_combobox_items from bec_widgets.widgets.dap.dap_combo_box.dap_combo_box import DapComboBox from .client_mocks import mocked_client @@ -20,8 +16,6 @@ def test_replace_combobox_items(qtbot, mocked_client): assert widget.itemText(0) == "testA" assert widget.itemText(1) == "testB" assert widget.itemData(1) == {"payload": True} - assert combobox_contains_text(widget, "testA") is True - assert combobox_contains_text(widget, "missing") is False def test_get_bec_signals_for_classes_ndim_filter(mocked_client): @@ -46,3 +40,19 @@ def test_replace_combobox_items_empty(qtbot): replace_combobox_items(widget, []) assert widget.count() == 0 + + +def test_replace_combobox_items_preserves_text_and_blocks_signals(qtbot): + widget = QComboBox() + qtbot.addWidget(widget) + widget.setEditable(True) + widget.addItems(["old", "other"]) + widget.setCurrentText("typed") + emitted: list[str] = [] + widget.currentTextChanged.connect(emitted.append) + + replace_combobox_items(widget, ["new"], preserve_current_text=True, block_signals=True) + + assert widget.currentText() == "typed" + assert widget.itemText(0) == "new" + assert emitted == []