From 46e50017f81ba36bccbdc7cfb6ba16591ea6120c Mon Sep 17 00:00:00 2001 From: Alex Dewar Date: Wed, 8 Nov 2023 13:47:14 +0000 Subject: [PATCH 01/18] Only send device list when device.list.request message received We don't want to send it unconditionally when the main window loads. --- finesse/gui/device_view.py | 5 +++-- finesse/hardware/__init__.py | 8 -------- finesse/hardware/manage_devices.py | 8 +++++++- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/finesse/gui/device_view.py b/finesse/gui/device_view.py index b6f0f7b8..e22a5ae7 100644 --- a/finesse/gui/device_view.py +++ b/finesse/gui/device_view.py @@ -274,8 +274,9 @@ def __init__(self) -> None: self.setSizePolicy(QSizePolicy.Policy.Minimum, QSizePolicy.Policy.Fixed) self.setLayout(QVBoxLayout()) - # pubsub topics - pub.subscribe(self._on_device_list, "device.list") + # Retrieve the list of device plugins + pub.subscribe(self._on_device_list, "device.list.response") + pub.sendMessage("device.list.request") def _on_device_list( self, device_types: Mapping[DeviceBaseTypeInfo, Sequence[DeviceTypeInfo]] diff --git a/finesse/hardware/__init__.py b/finesse/hardware/__init__.py index 937dd746..ddf68b95 100644 --- a/finesse/hardware/__init__.py +++ b/finesse/hardware/__init__.py @@ -18,17 +18,11 @@ from finesse.config import NUM_TEMPERATURE_MONITOR_CHANNELS, TEMPERATURE_MONITOR_TOPIC from finesse.hardware import data_file_writer # noqa: F401 -from finesse.hardware.device import get_device_types from finesse.hardware.plugins.temperature import get_temperature_monitor_instance _opus: OPUSInterface -def _broadcast_device_types() -> None: - """Broadcast the available device types via pubsub.""" - pub.sendMessage("device.list", device_types=get_device_types()) - - def _try_get_temperatures() -> Sequence | None: """Try to read the current temperatures from the temperature monitor. @@ -67,8 +61,6 @@ def _init_hardware(): _opus = OPUSInterface() - _broadcast_device_types() - def _stop_hardware(): global _opus diff --git a/finesse/hardware/manage_devices.py b/finesse/hardware/manage_devices.py index 96671c7f..d99daa3d 100644 --- a/finesse/hardware/manage_devices.py +++ b/finesse/hardware/manage_devices.py @@ -7,7 +7,7 @@ from pubsub import pub from finesse.device_info import DeviceInstanceRef -from finesse.hardware.device import Device +from finesse.hardware.device import Device, get_device_types _devices: dict[DeviceInstanceRef, Device] = {} @@ -114,8 +114,14 @@ def _close_all_devices() -> None: _devices.clear() +def _broadcast_device_types() -> None: + """Broadcast the available device types via pubsub.""" + pub.sendMessage("device.list.response", device_types=get_device_types()) + + pub.subscribe(_open_device, "device.open") pub.subscribe(_close_device, "device.close") pub.subscribe(_on_device_error, "device.error") +pub.subscribe(_broadcast_device_types, "device.list.request") pub.subscribe(_close_all_devices, "window.closed") From f09202d92b8b1718fba17f9ae646a42c31d5d6c8 Mon Sep 17 00:00:00 2001 From: Alex Dewar Date: Thu, 9 Nov 2023 11:11:55 +0000 Subject: [PATCH 02/18] Move manual device management into separate dialog Closes #395. --- finesse/gui/device_view.py | 47 ++++++++++++----- .../gui/hardware_set/hardware_sets_view.py | 52 ++++++++++++++++--- finesse/gui/main_window.py | 11 ++-- 3 files changed, 82 insertions(+), 28 deletions(-) diff --git a/finesse/gui/device_view.py b/finesse/gui/device_view.py index e22a5ae7..13e9f68d 100644 --- a/finesse/gui/device_view.py +++ b/finesse/gui/device_view.py @@ -2,7 +2,7 @@ import logging from collections.abc import Mapping, Sequence from dataclasses import dataclass -from typing import Any, cast +from typing import AbstractSet, Any, cast from pubsub import pub from PySide6.QtWidgets import ( @@ -93,6 +93,7 @@ def __init__( description: str, instance: DeviceInstanceRef, device_types: Sequence[DeviceTypeInfo], + is_connected: bool, ) -> None: """Create a new DeviceTypeControl. @@ -100,6 +101,7 @@ def __init__( description: A description of the device type instance: The device instance this panel is for device_types: The available devices for this base device type + is_connected: Whether the device is already connected """ if not device_types: raise RuntimeError("At least one device type must be specified") @@ -144,19 +146,23 @@ def __init__( cur_item.widget.show() layout.addWidget(cur_item.widget) - self._open_close_btn = QPushButton("Open") + self._open_close_btn = QPushButton() self._open_close_btn.setSizePolicy( QSizePolicy.Policy.Fixed, QSizePolicy.Policy.Fixed ) self._open_close_btn.clicked.connect(self._on_open_close_clicked) layout.addWidget(self._open_close_btn) + if is_connected: + self._set_device_opened() + else: + self._set_device_closed() # Determine whether the button should be enabled or not self._update_open_btn_enabled_state() # pubsub subscriptions pub.subscribe(self._on_device_opened, f"device.opening.{topic}") - pub.subscribe(self._on_device_closed, f"device.closed.{topic}") + pub.subscribe(self._set_device_closed, f"device.closed.{topic}") pub.subscribe(self._show_error_message, f"device.error.{topic}") def _update_open_btn_enabled_state(self) -> None: @@ -187,6 +193,7 @@ def _on_device_selected(self) -> None: self._update_open_btn_enabled_state() def _get_current_device_type_item(self) -> DeviceTypeItem: + """Get information about the currently selected device type.""" return self._device_combo.currentData() def _get_current_device_and_params( @@ -215,6 +222,16 @@ def _set_combos_enabled(self, enabled: bool) -> None: if widget := self._get_current_device_type_item().widget: widget.setEnabled(enabled) + def _set_device_opened(self) -> None: + """Update the GUI for when the device is opened.""" + self._set_combos_enabled(False) + self._open_close_btn.setText("Close") + + def _set_device_closed(self, **kwargs) -> None: + """Update the GUI for when the device is opened.""" + self._set_combos_enabled(True) + self._open_close_btn.setText("Open") + def _open_device(self) -> None: """Open the currently selected device.""" device_type, device_params = self._get_current_device_and_params() @@ -230,19 +247,12 @@ def _on_device_opened( f"device/{instance.topic}/{class_name}/params", params, ) - - self._set_combos_enabled(False) - self._open_close_btn.setText("Close") + self._set_device_opened() def _close_device(self) -> None: """Close the device.""" close_device(self._device_instance) - def _on_device_closed(self, instance: DeviceInstanceRef) -> None: - """Update the GUI for when the device is closed.""" - self._set_combos_enabled(True) - self._open_close_btn.setText("Open") - def _show_error_message( self, instance: DeviceInstanceRef, error: BaseException ) -> None: @@ -268,11 +278,13 @@ def _on_open_close_clicked(self) -> None: class DeviceControl(QGroupBox): """Allows for viewing and connecting to devices.""" - def __init__(self) -> None: + def __init__(self, connected_devices: AbstractSet[DeviceInstanceRef]) -> None: """Create a new DeviceControl.""" super().__init__("Device control") self.setSizePolicy(QSizePolicy.Policy.Minimum, QSizePolicy.Policy.Fixed) self.setLayout(QVBoxLayout()) + self._connected_devices = connected_devices + """The devices already connected when the control is created.""" # Retrieve the list of device plugins pub.subscribe(self._on_device_list, "device.list.response") @@ -281,22 +293,29 @@ def __init__(self) -> None: def _on_device_list( self, device_types: Mapping[DeviceBaseTypeInfo, Sequence[DeviceTypeInfo]] ) -> None: + """Populate with DeviceTypeControls when a list of devices is received.""" layout = cast(QVBoxLayout, self.layout()) # Group together devices based on their base types (e.g. "stepper motor") for base_type, types in device_types.items(): if not base_type.names_long: + instance = DeviceInstanceRef(base_type.name) layout.addWidget( DeviceTypeControl( - base_type.description, DeviceInstanceRef(base_type.name), types + base_type.description, + instance, + types, + instance in self._connected_devices, ) ) else: for short, long in zip(base_type.names_short, base_type.names_long): + instance = DeviceInstanceRef(base_type.name, short) layout.addWidget( DeviceTypeControl( f"{base_type.description} ({long})", - DeviceInstanceRef(base_type.name, short), + instance, types, + instance in self._connected_devices, ) ) diff --git a/finesse/gui/hardware_set/hardware_sets_view.py b/finesse/gui/hardware_set/hardware_sets_view.py index b5384a0d..2ae9a146 100644 --- a/finesse/gui/hardware_set/hardware_sets_view.py +++ b/finesse/gui/hardware_set/hardware_sets_view.py @@ -1,18 +1,23 @@ """Provides a panel for choosing between hardware sets and (dis)connecting.""" from collections.abc import Mapping -from typing import Any, cast +from typing import AbstractSet, Any, cast from frozendict import frozendict from pubsub import pub +from PySide6.QtCore import Qt from PySide6.QtWidgets import ( QComboBox, + QDialog, + QGridLayout, QGroupBox, - QHBoxLayout, QPushButton, QSizePolicy, + QVBoxLayout, + QWidget, ) from finesse.device_info import DeviceInstanceRef +from finesse.gui.device_view import DeviceControl from finesse.gui.hardware_set.hardware_set import ( HardwareSet, OpenDeviceArgs, @@ -21,6 +26,27 @@ from finesse.settings import settings +class ManageDevicesDialog(QDialog): + """A dialog for manually opening, closing and configuring devices.""" + + def __init__( + self, parent: QWidget, connected_devices: AbstractSet[DeviceInstanceRef] + ) -> None: + """Create a new ManageDevicesDialog. + + Args: + parent: Parent window + connected_devices: Which devices are already connected + """ + super().__init__(parent) + self.setWindowTitle("Manage devices") + self.setModal(True) + + layout = QVBoxLayout() + layout.addWidget(DeviceControl(connected_devices)) + self.setLayout(layout) + + class HardwareSetsControl(QGroupBox): """A panel for choosing between hardware sets and (dis)connecting.""" @@ -61,15 +87,29 @@ def __init__(self) -> None: ) self._disconnect_btn.pressed.connect(self._on_disconnect_btn_pressed) + manage_devices_btn = QPushButton("Manage devices") + manage_devices_btn.pressed.connect(self._show_manage_devices_dialog) + self._manage_devices_dialog: ManageDevicesDialog + # Enable/disable controls self._update_control_state() - layout = QHBoxLayout() - layout.addWidget(self._hardware_sets_combo) - layout.addWidget(self._connect_btn) - layout.addWidget(self._disconnect_btn) + layout = QGridLayout() + layout.addWidget(self._hardware_sets_combo, 0, 0) + layout.addWidget(self._connect_btn, 0, 1) + layout.addWidget(self._disconnect_btn, 0, 2) + layout.addWidget(manage_devices_btn, 1, 0, 1, 3, Qt.AlignmentFlag.AlignHCenter) self.setLayout(layout) + def _show_manage_devices_dialog(self) -> None: + # Create dialog lazily + if not hasattr(self, "_manage_devices_dialog"): + self._manage_devices_dialog = ManageDevicesDialog( + self.window(), {device.instance for device in self._connected_devices} + ) + + self._manage_devices_dialog.show() + def _add_hardware_set(self, hw_set: HardwareSet) -> None: """Add a new hardware set to the combo box.""" labels = { diff --git a/finesse/gui/main_window.py b/finesse/gui/main_window.py index 6aa3fd76..7669a017 100644 --- a/finesse/gui/main_window.py +++ b/finesse/gui/main_window.py @@ -17,7 +17,6 @@ TEMPERATURE_MONITOR_HOT_BB_IDX, ) from finesse.gui.data_file_view import DataFileControl -from finesse.gui.device_view import DeviceControl from finesse.gui.docs_view import DocsViewer from finesse.gui.em27_monitor import EM27Monitor from finesse.gui.hardware_set.hardware_sets_view import HardwareSetsControl @@ -54,9 +53,6 @@ def __init__(self) -> None: # Setup for measure script panel measure_script = ScriptControl() - # Setup for device panel - device_control = DeviceControl() - # Setup for interferometer monitor em27_monitor = EM27Monitor() @@ -64,10 +60,9 @@ def __init__(self) -> None: layout_left.addWidget(hardware_sets, 0, 0, 1, 2) layout_left.addWidget(stepper_motor, 1, 0, 1, 2) - layout_left.addWidget(opus, 2, 0, 1, 2) - layout_left.addWidget(measure_script, 3, 0, 1, 2) - layout_left.addWidget(device_control, 4, 0, 1, 1) - layout_left.addWidget(em27_monitor, 4, 1, 1, 1) + layout_left.addWidget(measure_script, 2, 0, 1, 2) + layout_left.addWidget(opus, 3, 0, 1, 1) + layout_left.addWidget(em27_monitor, 3, 1, 1, 1) layout_right = QGridLayout() From 194c778e745b12a8267861c2273a64dee4a21172 Mon Sep 17 00:00:00 2001 From: Alex Dewar Date: Thu, 9 Nov 2023 11:21:16 +0000 Subject: [PATCH 03/18] Move some device management modules into hardware_set/ --- finesse/gui/{ => hardware_set}/device_connection.py | 0 finesse/gui/{ => hardware_set}/device_view.py | 2 +- finesse/gui/hardware_set/hardware_set.py | 2 +- finesse/gui/hardware_set/hardware_sets_view.py | 2 +- tests/gui/{ => hardware_set}/test_device_connection.py | 2 +- 5 files changed, 4 insertions(+), 4 deletions(-) rename finesse/gui/{ => hardware_set}/device_connection.py (100%) rename finesse/gui/{ => hardware_set}/device_view.py (99%) rename tests/gui/{ => hardware_set}/test_device_connection.py (90%) diff --git a/finesse/gui/device_connection.py b/finesse/gui/hardware_set/device_connection.py similarity index 100% rename from finesse/gui/device_connection.py rename to finesse/gui/hardware_set/device_connection.py diff --git a/finesse/gui/device_view.py b/finesse/gui/hardware_set/device_view.py similarity index 99% rename from finesse/gui/device_view.py rename to finesse/gui/hardware_set/device_view.py index 13e9f68d..0325e9f7 100644 --- a/finesse/gui/device_view.py +++ b/finesse/gui/hardware_set/device_view.py @@ -16,8 +16,8 @@ ) from finesse.device_info import DeviceBaseTypeInfo, DeviceInstanceRef, DeviceTypeInfo -from finesse.gui.device_connection import close_device, open_device from finesse.gui.error_message import show_error_message +from finesse.gui.hardware_set.device_connection import close_device, open_device from finesse.settings import settings diff --git a/finesse/gui/hardware_set/hardware_set.py b/finesse/gui/hardware_set/hardware_set.py index 46d043e6..e444d2f6 100644 --- a/finesse/gui/hardware_set/hardware_set.py +++ b/finesse/gui/hardware_set/hardware_set.py @@ -12,7 +12,7 @@ from frozendict import frozendict from finesse.device_info import DeviceInstanceRef -from finesse.gui.device_connection import close_device, open_device +from finesse.gui.hardware_set.device_connection import close_device, open_device @dataclass(frozen=True) diff --git a/finesse/gui/hardware_set/hardware_sets_view.py b/finesse/gui/hardware_set/hardware_sets_view.py index 2ae9a146..5051c218 100644 --- a/finesse/gui/hardware_set/hardware_sets_view.py +++ b/finesse/gui/hardware_set/hardware_sets_view.py @@ -17,7 +17,7 @@ ) from finesse.device_info import DeviceInstanceRef -from finesse.gui.device_view import DeviceControl +from finesse.gui.hardware_set.device_view import DeviceControl from finesse.gui.hardware_set.hardware_set import ( HardwareSet, OpenDeviceArgs, diff --git a/tests/gui/test_device_connection.py b/tests/gui/hardware_set/test_device_connection.py similarity index 90% rename from tests/gui/test_device_connection.py rename to tests/gui/hardware_set/test_device_connection.py index 83c56a73..b5a9fb66 100644 --- a/tests/gui/test_device_connection.py +++ b/tests/gui/hardware_set/test_device_connection.py @@ -2,7 +2,7 @@ from unittest.mock import MagicMock from finesse.device_info import DeviceInstanceRef -from finesse.gui.device_connection import close_device, open_device +from finesse.gui.hardware_set.device_connection import close_device, open_device def test_open_device(sendmsg_mock: MagicMock) -> None: From 66f02695b14a069b3e6dcbd2f97db663510a8bc1 Mon Sep 17 00:00:00 2001 From: Alex Dewar Date: Thu, 9 Nov 2023 12:01:10 +0000 Subject: [PATCH 04/18] DeviceControl: Auto-select opened device type in combo box Fixes #397. --- finesse/gui/hardware_set/device_view.py | 65 +++++++++++++------ .../gui/hardware_set/hardware_sets_view.py | 4 +- 2 files changed, 47 insertions(+), 22 deletions(-) diff --git a/finesse/gui/hardware_set/device_view.py b/finesse/gui/hardware_set/device_view.py index 0325e9f7..6082f729 100644 --- a/finesse/gui/hardware_set/device_view.py +++ b/finesse/gui/hardware_set/device_view.py @@ -18,6 +18,7 @@ from finesse.device_info import DeviceBaseTypeInfo, DeviceInstanceRef, DeviceTypeInfo from finesse.gui.error_message import show_error_message from finesse.gui.hardware_set.device_connection import close_device, open_device +from finesse.gui.hardware_set.hardware_set import OpenDeviceArgs from finesse.settings import settings @@ -93,7 +94,7 @@ def __init__( description: str, instance: DeviceInstanceRef, device_types: Sequence[DeviceTypeInfo], - is_connected: bool, + connected_device_type: str | None, ) -> None: """Create a new DeviceTypeControl. @@ -101,7 +102,7 @@ def __init__( description: A description of the device type instance: The device instance this panel is for device_types: The available devices for this base device type - is_connected: Whether the device is already connected + connected_device_type: The class name for this device type, if opened """ if not device_types: raise RuntimeError("At least one device type must be specified") @@ -127,16 +128,11 @@ def __init__( # Select the last device that was successfully opened, if there is one topic = instance.topic - if previous_device := settings.value(f"device/{instance.topic}/type"): - try: - idx = next( - i - for i, device_type in enumerate(device_types) - if device_type.class_name == previous_device - ) - self._device_combo.setCurrentIndex(idx) - except StopIteration: - logging.warn(f"Unknown class name: {previous_device}") + previous_device = cast( + str | None, settings.value(f"device/{instance.topic}/type") + ) + if previous_device: + self._select_device(previous_device) self._device_combo.currentIndexChanged.connect(self._on_device_selected) layout.addWidget(self._device_combo) @@ -152,8 +148,8 @@ def __init__( ) self._open_close_btn.clicked.connect(self._on_open_close_clicked) layout.addWidget(self._open_close_btn) - if is_connected: - self._set_device_opened() + if connected_device_type: + self._set_device_opened(connected_device_type) else: self._set_device_closed() @@ -222,10 +218,26 @@ def _set_combos_enabled(self, enabled: bool) -> None: if widget := self._get_current_device_type_item().widget: widget.setEnabled(enabled) - def _set_device_opened(self) -> None: + def _set_device_opened(self, class_name: str) -> None: """Update the GUI for when the device is opened.""" self._set_combos_enabled(False) self._open_close_btn.setText("Close") + self._select_device(class_name) + + def _select_device(self, class_name: str) -> None: + """Select the device from the combo box which matches class_name. + + Todo: Select params too + """ + try: + idx = next( + i + for i in range(self._device_combo.count()) + if self._device_combo.itemData(i).device_type.class_name == class_name + ) + self._device_combo.setCurrentIndex(idx) + except StopIteration: + logging.warn(f"Unknown class_name for opened device: {class_name}") def _set_device_closed(self, **kwargs) -> None: """Update the GUI for when the device is opened.""" @@ -240,14 +252,16 @@ def _open_device(self) -> None: def _on_device_opened( self, instance: DeviceInstanceRef, class_name: str, params: Mapping[str, Any] ) -> None: - """Update the GUI for when the device is successfully opened.""" + """Save the last opened device and update the GUI appropriately.""" settings.setValue(f"device/{instance.topic}/type", class_name) if params: settings.setValue( f"device/{instance.topic}/{class_name}/params", params, ) - self._set_device_opened() + + # Update GUI + self._set_device_opened(class_name) def _close_device(self) -> None: """Close the device.""" @@ -278,7 +292,7 @@ def _on_open_close_clicked(self) -> None: class DeviceControl(QGroupBox): """Allows for viewing and connecting to devices.""" - def __init__(self, connected_devices: AbstractSet[DeviceInstanceRef]) -> None: + def __init__(self, connected_devices: AbstractSet[OpenDeviceArgs]) -> None: """Create a new DeviceControl.""" super().__init__("Device control") self.setSizePolicy(QSizePolicy.Policy.Minimum, QSizePolicy.Policy.Fixed) @@ -290,6 +304,17 @@ def __init__(self, connected_devices: AbstractSet[DeviceInstanceRef]) -> None: pub.subscribe(self._on_device_list, "device.list.response") pub.sendMessage("device.list.request") + def _get_connected_device(self, instance: DeviceInstanceRef) -> str | None: + """Get the class name of the connected device matching instance, if any.""" + try: + return next( + device.class_name + for device in self._connected_devices + if device.instance == instance + ) + except StopIteration: + return None + def _on_device_list( self, device_types: Mapping[DeviceBaseTypeInfo, Sequence[DeviceTypeInfo]] ) -> None: @@ -305,7 +330,7 @@ def _on_device_list( base_type.description, instance, types, - instance in self._connected_devices, + self._get_connected_device(instance), ) ) else: @@ -316,6 +341,6 @@ def _on_device_list( f"{base_type.description} ({long})", instance, types, - instance in self._connected_devices, + self._get_connected_device(instance), ) ) diff --git a/finesse/gui/hardware_set/hardware_sets_view.py b/finesse/gui/hardware_set/hardware_sets_view.py index 5051c218..36a293f0 100644 --- a/finesse/gui/hardware_set/hardware_sets_view.py +++ b/finesse/gui/hardware_set/hardware_sets_view.py @@ -30,7 +30,7 @@ class ManageDevicesDialog(QDialog): """A dialog for manually opening, closing and configuring devices.""" def __init__( - self, parent: QWidget, connected_devices: AbstractSet[DeviceInstanceRef] + self, parent: QWidget, connected_devices: AbstractSet[OpenDeviceArgs] ) -> None: """Create a new ManageDevicesDialog. @@ -105,7 +105,7 @@ def _show_manage_devices_dialog(self) -> None: # Create dialog lazily if not hasattr(self, "_manage_devices_dialog"): self._manage_devices_dialog = ManageDevicesDialog( - self.window(), {device.instance for device in self._connected_devices} + self.window(), self._connected_devices ) self._manage_devices_dialog.show() From 2481a6f93678256f8dade8fcfe0a9893b59d0441 Mon Sep 17 00:00:00 2001 From: Alex Dewar Date: Thu, 9 Nov 2023 12:31:30 +0000 Subject: [PATCH 05/18] Add get_instances_and_descriptions() helper method --- finesse/device_info.py | 16 ++++++++++- finesse/gui/hardware_set/device_view.py | 16 ++--------- tests/gui/test_device_info.py | 36 +++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 15 deletions(-) create mode 100644 tests/gui/test_device_info.py diff --git a/finesse/device_info.py b/finesse/device_info.py index 88adc5ae..575ddca5 100644 --- a/finesse/device_info.py +++ b/finesse/device_info.py @@ -1,7 +1,7 @@ """Provides common dataclasses about devices for using in backend and frontend.""" from __future__ import annotations -from collections.abc import Sequence +from collections.abc import Iterable, Sequence from dataclasses import dataclass from typing import Any @@ -57,6 +57,20 @@ class DeviceBaseTypeInfo: names_long: Sequence[str] """A list of names for this type of device (human readable).""" + def get_instances_and_descriptions(self) -> Iterable[tuple[DeviceInstanceRef, str]]: + """Get instances and descriptions. + + If there are no possible names for the type, then there will only be one + instance, otherwise there will be one per name. + """ + if not self.names_long: + yield DeviceInstanceRef(self.name), self.description + return + + for short, long in zip(self.names_short, self.names_long): + instance = DeviceInstanceRef(self.name, short) + yield instance, f"{self.description} ({long})" + @dataclass(frozen=True) class DeviceInstanceRef: diff --git a/finesse/gui/hardware_set/device_view.py b/finesse/gui/hardware_set/device_view.py index 6082f729..729467c5 100644 --- a/finesse/gui/hardware_set/device_view.py +++ b/finesse/gui/hardware_set/device_view.py @@ -323,24 +323,12 @@ def _on_device_list( # Group together devices based on their base types (e.g. "stepper motor") for base_type, types in device_types.items(): - if not base_type.names_long: - instance = DeviceInstanceRef(base_type.name) + for instance, description in base_type.get_instances_and_descriptions(): layout.addWidget( DeviceTypeControl( - base_type.description, + description, instance, types, self._get_connected_device(instance), ) ) - else: - for short, long in zip(base_type.names_short, base_type.names_long): - instance = DeviceInstanceRef(base_type.name, short) - layout.addWidget( - DeviceTypeControl( - f"{base_type.description} ({long})", - instance, - types, - self._get_connected_device(instance), - ) - ) diff --git a/tests/gui/test_device_info.py b/tests/gui/test_device_info.py new file mode 100644 index 00000000..9cc03fb4 --- /dev/null +++ b/tests/gui/test_device_info.py @@ -0,0 +1,36 @@ +"""Test functionality in device_info.py.""" + +import pytest + +from finesse.device_info import DeviceBaseTypeInfo, DeviceInstanceRef + + +@pytest.mark.parametrize( + "type_info,expected", + ( + ( + DeviceBaseTypeInfo("type_name", "Type description", (), ()), + [(DeviceInstanceRef("type_name"), "Type description")], + ), + ( + DeviceBaseTypeInfo( + "type_name", + "Type description", + ("name1", "name2"), + ("Name 1", "Name 2"), + ), + [ + ( + DeviceInstanceRef("type_name", f"name{i}"), + f"Type description (Name {i})", + ) + for i in range(1, 3) + ], + ), + ), +) +def test_get_instances_and_descriptions( + type_info: DeviceBaseTypeInfo, expected: list[tuple[DeviceInstanceRef, str]] +) -> None: + """Test DeviceBaseTypeInfo's get_devices_and_descriptions() method.""" + assert list(type_info.get_instances_and_descriptions()) == expected From 04c280655d1e6553e77ac8014d2163aabe2850f7 Mon Sep 17 00:00:00 2001 From: Alex Dewar Date: Thu, 9 Nov 2023 12:40:05 +0000 Subject: [PATCH 06/18] Save device settings in HardwareSetsControl cf. DeviceControl --- finesse/gui/hardware_set/device_view.py | 10 +--------- finesse/gui/hardware_set/hardware_sets_view.py | 9 +++++++++ tests/gui/hardware_set/test_hardware_sets_view.py | 8 +++++++- 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/finesse/gui/hardware_set/device_view.py b/finesse/gui/hardware_set/device_view.py index 729467c5..025e4e35 100644 --- a/finesse/gui/hardware_set/device_view.py +++ b/finesse/gui/hardware_set/device_view.py @@ -252,15 +252,7 @@ def _open_device(self) -> None: def _on_device_opened( self, instance: DeviceInstanceRef, class_name: str, params: Mapping[str, Any] ) -> None: - """Save the last opened device and update the GUI appropriately.""" - settings.setValue(f"device/{instance.topic}/type", class_name) - if params: - settings.setValue( - f"device/{instance.topic}/{class_name}/params", - params, - ) - - # Update GUI + """Update the GUI on device open.""" self._set_device_opened(class_name) def _close_device(self) -> None: diff --git a/finesse/gui/hardware_set/hardware_sets_view.py b/finesse/gui/hardware_set/hardware_sets_view.py index 36a293f0..8aabab14 100644 --- a/finesse/gui/hardware_set/hardware_sets_view.py +++ b/finesse/gui/hardware_set/hardware_sets_view.py @@ -177,6 +177,15 @@ def _on_device_opened( self._connected_devices.add( OpenDeviceArgs(instance, class_name, frozendict(params)) ) + + # Remember last opened device + settings.setValue(f"device/{instance.topic}/type", class_name) + if params: + settings.setValue( + f"device/{instance.topic}/{class_name}/params", + params, + ) + self._update_control_state() def _on_device_closed(self, instance: DeviceInstanceRef) -> None: diff --git a/tests/gui/hardware_set/test_hardware_sets_view.py b/tests/gui/hardware_set/test_hardware_sets_view.py index ab2c440d..8dffd4c0 100644 --- a/tests/gui/hardware_set/test_hardware_sets_view.py +++ b/tests/gui/hardware_set/test_hardware_sets_view.py @@ -197,7 +197,10 @@ def test_disconnect_button( update_mock.assert_called_once_with() -def test_on_device_opened(hw_sets: HardwareSetsControl, qtbot) -> None: +@patch("finesse.gui.hardware_set.hardware_sets_view.settings") +def test_on_device_opened( + settings_mock: Mock, hw_sets: HardwareSetsControl, qtbot +) -> None: """Test the _on_device_opened() method.""" device = DEVICES[0] assert not hw_sets._connected_devices @@ -207,6 +210,9 @@ def test_on_device_opened(hw_sets: HardwareSetsControl, qtbot) -> None: ) assert hw_sets._connected_devices == {device} update_mock.assert_called_once_with() + settings_mock.setValue.assert_called_with( + f"device/{device.instance.topic}/type", device.class_name + ) def test_on_device_closed(hw_sets: HardwareSetsControl, qtbot) -> None: From e5df45ab3ebcfc5bb9c399297a80f04da3abb428 Mon Sep 17 00:00:00 2001 From: Alex Dewar Date: Thu, 9 Nov 2023 16:06:39 +0000 Subject: [PATCH 07/18] DeviceControl: Refactor device parameter widget code Make a bespoke class for device parameters and use it. --- finesse/gui/hardware_set/device_view.py | 175 ++++++++++++------------ 1 file changed, 87 insertions(+), 88 deletions(-) diff --git a/finesse/gui/hardware_set/device_view.py b/finesse/gui/hardware_set/device_view.py index 025e4e35..a8f95cbc 100644 --- a/finesse/gui/hardware_set/device_view.py +++ b/finesse/gui/hardware_set/device_view.py @@ -1,7 +1,8 @@ """Provides a control for viewing and connecting to devices.""" +from __future__ import annotations + import logging from collections.abc import Mapping, Sequence -from dataclasses import dataclass from typing import AbstractSet, Any, cast from pubsub import pub @@ -22,68 +23,72 @@ from finesse.settings import settings -@dataclass -class DeviceTypeItem: - """User data associated with a given device type.""" - - device_type: DeviceTypeInfo - widget: QWidget | None - - -def _create_device_widget( - instance: DeviceInstanceRef, device_type: DeviceTypeInfo -) -> QWidget | None: - """Create a widget for the specified device type. - - If there are no parameters, return None. - """ - params = device_type.parameters +class DeviceParametersWidget(QWidget): + """A widget containing controls for setting a device's parameters.""" - # Don't bother making a widget if there are no parameters - if not params: - return None + def __init__(self, device_type: DeviceTypeInfo) -> None: + """Create a new DeviceParametersWidget. - # Previous parameter values are saved if a device opens successfully - previous_param_values = cast( - dict[str, Any] | None, - settings.value(f"device/{instance.topic}/{device_type.class_name}/params"), - ) - - widget = QWidget() - widget.hide() - layout = QHBoxLayout() - layout.setContentsMargins(0, 0, 0, 0) - widget.setLayout(layout) - - # Make a combo box for each parameter - for param in params: - combo = QComboBox() - combo.setSizePolicy(QSizePolicy.Policy.Fixed, QSizePolicy.Policy.Fixed) - - # Keep the "real" value along with its string representation, so that we can - # pass it back to the backend on device open - for value in param.possible_values: - combo.addItem(str(value), value) + Args: + device_type: The device type whose parameters will be used + """ + super().__init__() - curval = None - if ( - previous_param_values - and previous_param_values[param.name] in param.possible_values - ): - curval = previous_param_values[param.name] - elif param.default_value is not None: - curval = param.default_value + self.device_type = device_type + """This value is not used within the class, but is stored for convenience.""" - if curval is not None: - try: - combo.setCurrentIndex(param.possible_values.index(curval)) - except ValueError: - # curval is not in param.possible_values (although it should be) - pass + layout = QHBoxLayout() + layout.setContentsMargins(0, 0, 0, 0) + self.setLayout(layout) - layout.addWidget(combo) + # Make a combo box for each parameter + self._combos: dict[str, QComboBox] = {} + for param in device_type.parameters: + combo = QComboBox() + combo.setSizePolicy(QSizePolicy.Policy.Fixed, QSizePolicy.Policy.Fixed) + + # Keep the "real" value along with its string representation, so that we can + # pass it back to the backend on device open + for value in param.possible_values: + combo.addItem(str(value), value) + + if param.default_value is not None: + combo.setCurrentIndex(param.possible_values.index(param.default_value)) + + layout.addWidget(combo) + self._combos[param.name] = combo + + def set_parameter_value(self, param: str, value: Any) -> None: + """Set the relevant combo box's parameter value.""" + self._combos[param].setCurrentText(str(value)) + + @property + def current_parameter_values(self) -> dict[str, Any]: + """Get all parameters and their current values.""" + return {param: combo.currentData() for param, combo in self._combos.items()} + + @classmethod + def create_with_saved_param_values( + cls, instance: DeviceInstanceRef, device_type: DeviceTypeInfo + ) -> DeviceParametersWidget: + """Create a DeviceParametersWidget using saved values for parameters.""" + widget = cls(device_type) + widget.hide() # will be shown when used + + # Previous parameter values are saved if a device opens successfully. Update the + # combo boxes to these values. + previous_param_values = cast( + dict[str, Any] | None, + settings.value(f"device/{instance.topic}/{device_type.class_name}/params"), + ) + if previous_param_values: + for param, value in previous_param_values.items(): + try: + widget.set_parameter_value(param, value) + except Exception as error: + logging.warn(f"Error while setting param {param}: {error!s}") - return widget + return widget class DeviceTypeControl(QGroupBox): @@ -122,9 +127,14 @@ def __init__( ) # Add names for devices to combo box along with relevant user data + self._device_widgets: list[DeviceParametersWidget] = [] for t in device_types: - user_data = DeviceTypeItem(t, _create_device_widget(instance, t)) - self._device_combo.addItem(t.description, user_data) + widget = DeviceParametersWidget.create_with_saved_param_values(instance, t) + self._device_combo.addItem(t.description, widget) + + # YUCK: We have to keep our own reference to widget, as self._device_combo + # seemingly won't prevent it from being GC'd + self._device_widgets.append(widget) # Select the last device that was successfully opened, if there is one topic = instance.topic @@ -137,10 +147,10 @@ def __init__( self._device_combo.currentIndexChanged.connect(self._on_device_selected) layout.addWidget(self._device_combo) - if (cur_item := self._get_current_device_type_item()) and cur_item.widget: - # Show the combo boxes for the device's parameters - cur_item.widget.show() - layout.addWidget(cur_item.widget) + # Show the combo boxes for the device's parameters + current_widget = self.current_device_type_widget + current_widget.show() + layout.addWidget(current_widget) self._open_close_btn = QPushButton() self._open_close_btn.setSizePolicy( @@ -167,7 +177,7 @@ def _update_open_btn_enabled_state(self) -> None: The "open" button should be disabled if there are no possible values for any of the params. """ - all_params = self._get_current_device_type_item().device_type.parameters + all_params = self.current_device_type_widget.device_type.parameters self._open_close_btn.setEnabled(all(p.possible_values for p in all_params)) def _on_device_selected(self) -> None: @@ -181,42 +191,30 @@ def _on_device_selected(self) -> None: layout.takeAt(1).widget().hide() # Add the widget for the newly selected parameter if needed - if widget := self._get_current_device_type_item().widget: - widget.show() - layout.insertWidget(1, widget) + widget = self.current_device_type_widget + widget.show() + layout.insertWidget(1, widget) # Enable/disable the "open" button self._update_open_btn_enabled_state() - def _get_current_device_type_item(self) -> DeviceTypeItem: + @property + def current_device_type_widget(self) -> DeviceParametersWidget: """Get information about the currently selected device type.""" return self._device_combo.currentData() - def _get_current_device_and_params( + @property + def current_device_and_params( self, ) -> tuple[DeviceTypeInfo, dict[str, Any]]: """Get the current device type and associated parameters.""" - item = self._get_current_device_type_item() - - # The current device widget contains combo boxes with the values - if not item.widget: - # No parameters needed for this device type - return item.device_type, {} - - # Get the parameter values - combos: list[QComboBox] = item.widget.findChildren(QComboBox) - device_params = { - p.name: c.currentData() for p, c in zip(item.device_type.parameters, combos) - } - - return item.device_type, device_params + widget = self.current_device_type_widget + return widget.device_type, widget.current_parameter_values def _set_combos_enabled(self, enabled: bool) -> None: """Set the enabled state of the combo boxes.""" self._device_combo.setEnabled(enabled) - - if widget := self._get_current_device_type_item().widget: - widget.setEnabled(enabled) + self.current_device_type_widget.setEnabled(enabled) def _set_device_opened(self, class_name: str) -> None: """Update the GUI for when the device is opened.""" @@ -235,10 +233,11 @@ def _select_device(self, class_name: str) -> None: for i in range(self._device_combo.count()) if self._device_combo.itemData(i).device_type.class_name == class_name ) - self._device_combo.setCurrentIndex(idx) except StopIteration: logging.warn(f"Unknown class_name for opened device: {class_name}") + self._device_combo.setCurrentIndex(idx) + def _set_device_closed(self, **kwargs) -> None: """Update the GUI for when the device is opened.""" self._set_combos_enabled(True) @@ -246,7 +245,7 @@ def _set_device_closed(self, **kwargs) -> None: def _open_device(self) -> None: """Open the currently selected device.""" - device_type, device_params = self._get_current_device_and_params() + device_type, device_params = self.current_device_and_params open_device(device_type.class_name, self._device_instance, device_params) def _on_device_opened( From e9ee184a4b73dbdf5ccc8c35b3eb6a2508123a14 Mon Sep 17 00:00:00 2001 From: Alex Dewar Date: Thu, 9 Nov 2023 16:23:17 +0000 Subject: [PATCH 08/18] Rename device/* settings and remove unnecessary "instance" in params setting --- finesse/gui/hardware_set/device_view.py | 8 ++++---- finesse/gui/hardware_set/hardware_sets_view.py | 7 ++----- tests/gui/hardware_set/test_hardware_sets_view.py | 12 +++++++++--- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/finesse/gui/hardware_set/device_view.py b/finesse/gui/hardware_set/device_view.py index a8f95cbc..90557b5a 100644 --- a/finesse/gui/hardware_set/device_view.py +++ b/finesse/gui/hardware_set/device_view.py @@ -69,7 +69,7 @@ def current_parameter_values(self) -> dict[str, Any]: @classmethod def create_with_saved_param_values( - cls, instance: DeviceInstanceRef, device_type: DeviceTypeInfo + cls, device_type: DeviceTypeInfo ) -> DeviceParametersWidget: """Create a DeviceParametersWidget using saved values for parameters.""" widget = cls(device_type) @@ -79,7 +79,7 @@ def create_with_saved_param_values( # combo boxes to these values. previous_param_values = cast( dict[str, Any] | None, - settings.value(f"device/{instance.topic}/{device_type.class_name}/params"), + settings.value(f"device/params/{device_type.class_name}"), ) if previous_param_values: for param, value in previous_param_values.items(): @@ -129,7 +129,7 @@ def __init__( # Add names for devices to combo box along with relevant user data self._device_widgets: list[DeviceParametersWidget] = [] for t in device_types: - widget = DeviceParametersWidget.create_with_saved_param_values(instance, t) + widget = DeviceParametersWidget.create_with_saved_param_values(t) self._device_combo.addItem(t.description, widget) # YUCK: We have to keep our own reference to widget, as self._device_combo @@ -139,7 +139,7 @@ def __init__( # Select the last device that was successfully opened, if there is one topic = instance.topic previous_device = cast( - str | None, settings.value(f"device/{instance.topic}/type") + str | None, settings.value(f"device/type/{instance.topic}") ) if previous_device: self._select_device(previous_device) diff --git a/finesse/gui/hardware_set/hardware_sets_view.py b/finesse/gui/hardware_set/hardware_sets_view.py index 8aabab14..42974526 100644 --- a/finesse/gui/hardware_set/hardware_sets_view.py +++ b/finesse/gui/hardware_set/hardware_sets_view.py @@ -179,12 +179,9 @@ def _on_device_opened( ) # Remember last opened device - settings.setValue(f"device/{instance.topic}/type", class_name) + settings.setValue(f"device/type/{instance.topic}", class_name) if params: - settings.setValue( - f"device/{instance.topic}/{class_name}/params", - params, - ) + settings.setValue(f"device/params/{class_name}", params) self._update_control_state() diff --git a/tests/gui/hardware_set/test_hardware_sets_view.py b/tests/gui/hardware_set/test_hardware_sets_view.py index 8dffd4c0..094e0759 100644 --- a/tests/gui/hardware_set/test_hardware_sets_view.py +++ b/tests/gui/hardware_set/test_hardware_sets_view.py @@ -98,7 +98,10 @@ def test_add_hardware_set(hw_sets: HardwareSetsControl, qtbot) -> None: add_mock.assert_called_once_with("Test 1 (3)", hw_set3) -DEVICES = [OpenDeviceArgs.create(f"type{i}", f"class{i}") for i in range(2)] +DEVICES = [ + OpenDeviceArgs.create(f"type{i}", f"class{i}", {"my_param": "my_value"}) + for i in range(2) +] def _get_devices(indexes: Sequence[int]) -> set[OpenDeviceArgs]: @@ -210,8 +213,11 @@ def test_on_device_opened( ) assert hw_sets._connected_devices == {device} update_mock.assert_called_once_with() - settings_mock.setValue.assert_called_with( - f"device/{device.instance.topic}/type", device.class_name + settings_mock.setValue.assert_has_calls( + [ + call(f"device/type/{device.instance.topic}", device.class_name), + call(f"device/params/{device.class_name}", device.params), + ] ) From 9b453d82c2ca207ac2ac76232b8a89f4179e1a0e Mon Sep 17 00:00:00 2001 From: Alex Dewar Date: Thu, 9 Nov 2023 16:41:47 +0000 Subject: [PATCH 09/18] Also update parameter values in GUI when device connects --- finesse/gui/hardware_set/device_view.py | 53 ++++++++++++------------- 1 file changed, 25 insertions(+), 28 deletions(-) diff --git a/finesse/gui/hardware_set/device_view.py b/finesse/gui/hardware_set/device_view.py index 90557b5a..ad205d99 100644 --- a/finesse/gui/hardware_set/device_view.py +++ b/finesse/gui/hardware_set/device_view.py @@ -58,38 +58,33 @@ def __init__(self, device_type: DeviceTypeInfo) -> None: layout.addWidget(combo) self._combos[param.name] = combo + # If there are saved parameter values, load them now + self.load_saved_parameter_values() + def set_parameter_value(self, param: str, value: Any) -> None: """Set the relevant combo box's parameter value.""" self._combos[param].setCurrentText(str(value)) + def load_saved_parameter_values(self) -> None: + """Set the combo boxes' parameter values according to their saved values.""" + params = cast( + dict[str, Any] | None, + settings.value(f"device/params/{self.device_type.class_name}"), + ) + if not params: + return + + for param, value in params.items(): + try: + self.set_parameter_value(param, value) + except Exception as error: + logging.warn(f"Error while setting param {param}: {error!s}") + @property def current_parameter_values(self) -> dict[str, Any]: """Get all parameters and their current values.""" return {param: combo.currentData() for param, combo in self._combos.items()} - @classmethod - def create_with_saved_param_values( - cls, device_type: DeviceTypeInfo - ) -> DeviceParametersWidget: - """Create a DeviceParametersWidget using saved values for parameters.""" - widget = cls(device_type) - widget.hide() # will be shown when used - - # Previous parameter values are saved if a device opens successfully. Update the - # combo boxes to these values. - previous_param_values = cast( - dict[str, Any] | None, - settings.value(f"device/params/{device_type.class_name}"), - ) - if previous_param_values: - for param, value in previous_param_values.items(): - try: - widget.set_parameter_value(param, value) - except Exception as error: - logging.warn(f"Error while setting param {param}: {error!s}") - - return widget - class DeviceTypeControl(QGroupBox): """A set of widgets for choosing a device and its params and connecting to it.""" @@ -129,7 +124,9 @@ def __init__( # Add names for devices to combo box along with relevant user data self._device_widgets: list[DeviceParametersWidget] = [] for t in device_types: - widget = DeviceParametersWidget.create_with_saved_param_values(t) + widget = DeviceParametersWidget(t) + widget.hide() # will be shown when used + self._device_combo.addItem(t.description, widget) # YUCK: We have to keep our own reference to widget, as self._device_combo @@ -223,10 +220,7 @@ def _set_device_opened(self, class_name: str) -> None: self._select_device(class_name) def _select_device(self, class_name: str) -> None: - """Select the device from the combo box which matches class_name. - - Todo: Select params too - """ + """Select the device from the combo box which matches class_name.""" try: idx = next( i @@ -238,6 +232,9 @@ def _select_device(self, class_name: str) -> None: self._device_combo.setCurrentIndex(idx) + # Reload saved parameter values + self._device_widgets[idx].load_saved_parameter_values() + def _set_device_closed(self, **kwargs) -> None: """Update the GUI for when the device is opened.""" self._set_combos_enabled(True) From c86954604f8b035c60d3a568d5e8efc827f07c6c Mon Sep 17 00:00:00 2001 From: Alex Dewar Date: Mon, 13 Nov 2023 10:54:41 +0000 Subject: [PATCH 10/18] DeviceTypeInfo: Use a more sensible order for members Also, make parameters a Sequence cf. list. --- finesse/device_info.py | 6 +++--- finesse/hardware/device.py | 2 +- tests/hardware/test_device.py | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/finesse/device_info.py b/finesse/device_info.py index 575ddca5..cfc3c89b 100644 --- a/finesse/device_info.py +++ b/finesse/device_info.py @@ -36,12 +36,12 @@ def __post_init__(self) -> None: class DeviceTypeInfo: """Description of a device.""" + class_name: str + """The name of the device's class including the module name.""" description: str """A human-readable name for the device.""" - parameters: list[DeviceParameter] + parameters: Sequence[DeviceParameter] """The device parameters.""" - class_name: str - """The name of the device's class including the module name.""" @dataclass(frozen=True) diff --git a/finesse/hardware/device.py b/finesse/hardware/device.py index 197d7f84..6d00ea35 100644 --- a/finesse/hardware/device.py +++ b/finesse/hardware/device.py @@ -99,9 +99,9 @@ def get_device_base_type_info(cls) -> DeviceBaseTypeInfo: def get_device_type_info(cls) -> DeviceTypeInfo: """Get information about this device type.""" return DeviceTypeInfo( + f"{cls.__module__}.{cls.__name__}", cls._device_description, cls.get_device_parameters(), - f"{cls.__module__}.{cls.__name__}", ) diff --git a/tests/hardware/test_device.py b/tests/hardware/test_device.py index 658cefda..670b3840 100644 --- a/tests/hardware/test_device.py +++ b/tests/hardware/test_device.py @@ -106,7 +106,7 @@ class MyDevice(AbstractDevice): MyDevice.add_device_parameters(param) assert MyDevice.get_device_type_info() == DeviceTypeInfo( - "DESCRIPTION", [param], f"{MyDevice.__module__}.MyDevice" + f"{MyDevice.__module__}.MyDevice", "DESCRIPTION", [param] ) From 6c28231c9febbf3dff528626bedebf4f14a5e141 Mon Sep 17 00:00:00 2001 From: Alex Dewar Date: Mon, 13 Nov 2023 17:10:12 +0000 Subject: [PATCH 11/18] DeviceTypeControl: Fix: select device before disabling combos --- finesse/gui/hardware_set/device_view.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/finesse/gui/hardware_set/device_view.py b/finesse/gui/hardware_set/device_view.py index ad205d99..c3fc11bf 100644 --- a/finesse/gui/hardware_set/device_view.py +++ b/finesse/gui/hardware_set/device_view.py @@ -215,9 +215,9 @@ def _set_combos_enabled(self, enabled: bool) -> None: def _set_device_opened(self, class_name: str) -> None: """Update the GUI for when the device is opened.""" + self._select_device(class_name) self._set_combos_enabled(False) self._open_close_btn.setText("Close") - self._select_device(class_name) def _select_device(self, class_name: str) -> None: """Select the device from the combo box which matches class_name.""" From 85a616299dd0bf3e0ccc1aab36c076c9ea5f6c68 Mon Sep 17 00:00:00 2001 From: Alex Dewar Date: Mon, 13 Nov 2023 17:45:55 +0000 Subject: [PATCH 12/18] DeviceTypeControl: Connect signal after device type is selected in constructor This is a harmless bug, which just means that the signal handler is invoked unnecessarily. --- finesse/gui/hardware_set/device_view.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/finesse/gui/hardware_set/device_view.py b/finesse/gui/hardware_set/device_view.py index c3fc11bf..20256625 100644 --- a/finesse/gui/hardware_set/device_view.py +++ b/finesse/gui/hardware_set/device_view.py @@ -141,7 +141,6 @@ def __init__( if previous_device: self._select_device(previous_device) - self._device_combo.currentIndexChanged.connect(self._on_device_selected) layout.addWidget(self._device_combo) # Show the combo boxes for the device's parameters @@ -163,6 +162,8 @@ def __init__( # Determine whether the button should be enabled or not self._update_open_btn_enabled_state() + self._device_combo.currentIndexChanged.connect(self._on_device_selected) + # pubsub subscriptions pub.subscribe(self._on_device_opened, f"device.opening.{topic}") pub.subscribe(self._set_device_closed, f"device.closed.{topic}") From ed869e04410e1e1fa20b2fa033b35f80f0936988 Mon Sep 17 00:00:00 2001 From: Alex Dewar Date: Mon, 13 Nov 2023 17:47:43 +0000 Subject: [PATCH 13/18] DeviceTypeControl: Remove pointless if statement This will never be false. --- finesse/gui/hardware_set/device_view.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/finesse/gui/hardware_set/device_view.py b/finesse/gui/hardware_set/device_view.py index 20256625..80c5629d 100644 --- a/finesse/gui/hardware_set/device_view.py +++ b/finesse/gui/hardware_set/device_view.py @@ -182,11 +182,9 @@ def _on_device_selected(self) -> None: """Swap out the parameter combo boxes for the current device.""" layout = cast(QHBoxLayout, self.layout()) - # If there's already a widget in place, remove it - if layout.count() == 3: - # For some reason we also have to hide the widget else it appears over the - # others - layout.takeAt(1).widget().hide() + # For some reason we also have to hide the widget else it appears over the + # others + layout.takeAt(1).widget().hide() # Add the widget for the newly selected parameter if needed widget = self.current_device_type_widget From 305b6414e62a31e5b27156cd3f89216114393351 Mon Sep 17 00:00:00 2001 From: Alex Dewar Date: Tue, 14 Nov 2023 11:40:15 +0000 Subject: [PATCH 14/18] DeviceTypeControl: Remove pointless property --- finesse/gui/hardware_set/device_view.py | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/finesse/gui/hardware_set/device_view.py b/finesse/gui/hardware_set/device_view.py index 80c5629d..8a9ac117 100644 --- a/finesse/gui/hardware_set/device_view.py +++ b/finesse/gui/hardware_set/device_view.py @@ -199,14 +199,6 @@ def current_device_type_widget(self) -> DeviceParametersWidget: """Get information about the currently selected device type.""" return self._device_combo.currentData() - @property - def current_device_and_params( - self, - ) -> tuple[DeviceTypeInfo, dict[str, Any]]: - """Get the current device type and associated parameters.""" - widget = self.current_device_type_widget - return widget.device_type, widget.current_parameter_values - def _set_combos_enabled(self, enabled: bool) -> None: """Set the enabled state of the combo boxes.""" self._device_combo.setEnabled(enabled) @@ -241,8 +233,12 @@ def _set_device_closed(self, **kwargs) -> None: def _open_device(self) -> None: """Open the currently selected device.""" - device_type, device_params = self.current_device_and_params - open_device(device_type.class_name, self._device_instance, device_params) + widget = self.current_device_type_widget + open_device( + widget.device_type.class_name, + self._device_instance, + widget.current_parameter_values, + ) def _on_device_opened( self, instance: DeviceInstanceRef, class_name: str, params: Mapping[str, Any] From dc473fe4ad372fcc3b779201290d96667650dc29 Mon Sep 17 00:00:00 2001 From: Alex Dewar Date: Tue, 14 Nov 2023 11:55:33 +0000 Subject: [PATCH 15/18] DeviceTypeControl: Fix: Don't try to use idx if error occurs --- finesse/gui/hardware_set/device_view.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/finesse/gui/hardware_set/device_view.py b/finesse/gui/hardware_set/device_view.py index 8a9ac117..f6218d13 100644 --- a/finesse/gui/hardware_set/device_view.py +++ b/finesse/gui/hardware_set/device_view.py @@ -220,11 +220,11 @@ def _select_device(self, class_name: str) -> None: ) except StopIteration: logging.warn(f"Unknown class_name for opened device: {class_name}") + else: + self._device_combo.setCurrentIndex(idx) - self._device_combo.setCurrentIndex(idx) - - # Reload saved parameter values - self._device_widgets[idx].load_saved_parameter_values() + # Reload saved parameter values + self._device_widgets[idx].load_saved_parameter_values() def _set_device_closed(self, **kwargs) -> None: """Update the GUI for when the device is opened.""" From 8f42eaec499b421a9ceed25242fdfe357cd00204 Mon Sep 17 00:00:00 2001 From: Alex Dewar Date: Tue, 14 Nov 2023 12:32:54 +0000 Subject: [PATCH 16/18] Add tests for DeviceControl and associated classes Closes #398. --- tests/gui/hardware_set/test_device_control.py | 75 +++++ .../test_device_parameters_widget.py | 106 +++++++ .../hardware_set/test_device_type_control.py | 263 ++++++++++++++++++ 3 files changed, 444 insertions(+) create mode 100644 tests/gui/hardware_set/test_device_control.py create mode 100644 tests/gui/hardware_set/test_device_parameters_widget.py create mode 100644 tests/gui/hardware_set/test_device_type_control.py diff --git a/tests/gui/hardware_set/test_device_control.py b/tests/gui/hardware_set/test_device_control.py new file mode 100644 index 00000000..b9b5c791 --- /dev/null +++ b/tests/gui/hardware_set/test_device_control.py @@ -0,0 +1,75 @@ +"""Test the DeviceControl class.""" + +from unittest.mock import MagicMock, Mock, patch + +import pytest + +from finesse.device_info import DeviceBaseTypeInfo, DeviceInstanceRef, DeviceTypeInfo +from finesse.gui.hardware_set.device_view import DeviceControl +from finesse.gui.hardware_set.hardware_set import OpenDeviceArgs + +CONNECTED_DEVICES = ( + OpenDeviceArgs.create("stepper_motor", "MyStepperMotor"), + OpenDeviceArgs.create( + "temperature_monitor", + "MyTemperatureMonitor", + {"param1": "value1"}, + ), +) + + +@pytest.fixture +def widget(qtbot) -> DeviceControl: + """Return a DeviceControl fixture.""" + return DeviceControl(set(CONNECTED_DEVICES)) + + +def test_init(sendmsg_mock: MagicMock, subscribe_mock: MagicMock, qtbot) -> None: + """Test the constructor.""" + devices = MagicMock() + widget = DeviceControl(devices) + assert widget._connected_devices is devices + + # Check that the list of devices was requested and the response is listened for + subscribe_mock.assert_called_once_with( + widget._on_device_list, "device.list.response" + ) + sendmsg_mock.assert_called_once_with("device.list.request") + + +@pytest.mark.parametrize( + "instance,expected", + ( + (DeviceInstanceRef("stepper_motor"), CONNECTED_DEVICES[0].class_name), + (DeviceInstanceRef("made_up"), None), + ), +) +def test_get_connected_device( + instance: DeviceInstanceRef, expected: str | None, widget: DeviceControl, qtbot +) -> None: + """Test the _get_connected_device() method.""" + assert widget._get_connected_device(instance) == expected + + +@patch("finesse.gui.hardware_set.device_view.DeviceTypeControl") +def test_on_device_list(widget_mock: Mock, widget: DeviceControl, qtbot) -> None: + """Test the _on_device_list() method.""" + base_type = DeviceBaseTypeInfo("base_type", "Base type", (), ()) + device_types = [ + DeviceTypeInfo("my_class1", "Device 1", []), + DeviceTypeInfo("my_class2", "Device 2", []), + ] + + with patch.object(widget, "layout"): + with patch.object(widget, "_get_connected_device") as connected_mock: + connected_mock.return_value = "connected_device" + widget._on_device_list({base_type: device_types}) + + # In practice, there will be more than one base type, but let's just test + # that the code creates this one widget for ease + widget_mock.assert_called_once_with( + "Base type", + DeviceInstanceRef("base_type"), + device_types, + "connected_device", + ) diff --git a/tests/gui/hardware_set/test_device_parameters_widget.py b/tests/gui/hardware_set/test_device_parameters_widget.py new file mode 100644 index 00000000..c18daea2 --- /dev/null +++ b/tests/gui/hardware_set/test_device_parameters_widget.py @@ -0,0 +1,106 @@ +"""Tests for the DeviceParametersWidget class.""" +from collections.abc import Sequence +from unittest.mock import Mock, patch + +import pytest + +from finesse.device_info import DeviceParameter, DeviceTypeInfo +from finesse.gui.hardware_set.device_view import DeviceParametersWidget + + +@pytest.fixture +def widget(qtbot) -> DeviceParametersWidget: + """A fixture providing a DeviceParametersWidget.""" + return DeviceParametersWidget( + DeviceTypeInfo("my_class", "My Device", [DeviceParameter("my_param", range(2))]) + ) + + +@pytest.mark.parametrize( + "params", + ( + # no params + (), + # one param + (DeviceParameter("my_param", ("value1", "value2")),), + # two params + ( + DeviceParameter("my_param", ("value1", "value2")), + DeviceParameter("param2", range(3), 1), + ), + ), +) +def test_init(params: Sequence[DeviceParameter], qtbot) -> None: + """Test the constructor.""" + device_type = DeviceTypeInfo("my_class", "My Device", params) + + with patch.object( + DeviceParametersWidget, "load_saved_parameter_values" + ) as load_params_mock: + widget = DeviceParametersWidget(device_type) + assert widget.device_type is device_type + assert list(widget._combos.keys()) == [p.name for p in params] + load_params_mock.assert_called_once_with() + + for param in params: + combo = widget._combos[param.name] + items = [combo.itemText(i) for i in range(combo.count())] + assert items == list(map(str, param.possible_values)) + assert ( + combo.currentData() == param.default_value + if param.default_value + else param.possible_values[0] + ) + + +def test_set_parameter_value(widget: DeviceParametersWidget) -> None: + """Test the set_parameter_value() method.""" + combo = widget._combos["my_param"] + assert combo.currentText() == "0" + + widget.set_parameter_value("my_param", 1) + assert combo.currentText() == "1" + + widget.set_parameter_value("my_param", 5) # invalid + assert combo.currentText() == "1" + + +@patch("finesse.gui.hardware_set.device_view.settings") +def test_load_saved_parameter_values( + settings_mock: Mock, widget: DeviceParametersWidget, qtbot +) -> None: + """Test the load_saved_parameter_values() method.""" + settings_mock.value.return_value = {"my_param": 1} + with patch.object(widget, "set_parameter_value") as set_param_mock: + widget.load_saved_parameter_values() + set_param_mock.assert_called_once_with("my_param", 1) + + +@patch("finesse.gui.hardware_set.device_view.settings") +def test_load_saved_parameter_values_none_saved( + settings_mock: Mock, widget: DeviceParametersWidget, qtbot +) -> None: + """Test the load_saved_parameter_values() method if there are no values saved.""" + settings_mock.value.return_value = None + with patch.object(widget, "set_parameter_value") as set_param_mock: + widget.load_saved_parameter_values() + set_param_mock.assert_not_called() + + +@patch("finesse.gui.hardware_set.device_view.settings") +@patch("finesse.gui.hardware_set.device_view.logging.warn") +def test_load_saved_parameter_values_error( + warn_mock: Mock, settings_mock: Mock, widget: DeviceParametersWidget, qtbot +) -> None: + """Test the load_saved_parameter_values() method ignores errors.""" + settings_mock.value.return_value = {"my_param": 1} + with patch.object(widget, "set_parameter_value") as set_param_mock: + set_param_mock.side_effect = KeyError + widget.load_saved_parameter_values() + set_param_mock.assert_called_once_with("my_param", 1) + warn_mock.assert_called_once() + + +def test_current_parameter_values(widget: DeviceParametersWidget, qtbot) -> None: + """Test the current_parameter_values property.""" + assert widget.current_parameter_values == {"my_param": 0} diff --git a/tests/gui/hardware_set/test_device_type_control.py b/tests/gui/hardware_set/test_device_type_control.py new file mode 100644 index 00000000..7ce9102c --- /dev/null +++ b/tests/gui/hardware_set/test_device_type_control.py @@ -0,0 +1,263 @@ +"""Test the DeviceTypeControl class.""" +from collections.abc import Sequence +from unittest.mock import MagicMock, Mock, PropertyMock, call, patch + +import pytest + +from finesse.device_info import DeviceInstanceRef, DeviceParameter, DeviceTypeInfo +from finesse.gui.hardware_set.device_view import ( + DeviceParametersWidget, + DeviceTypeControl, +) + +DEVICE_TYPES = [ + DeviceTypeInfo("my_class1", "Device 1", []), + DeviceTypeInfo("my_class2", "Device 2", []), +] + + +@pytest.fixture +def widget(qtbot) -> DeviceTypeControl: + """Create a DeviceTypeControl fixture.""" + return DeviceTypeControl( + "Device type", DeviceInstanceRef("base_type"), DEVICE_TYPES, None + ) + + +@pytest.mark.parametrize( + "connected_device,previous_device,expected_device", + ( + (connected, previous, connected if connected else default) + for previous, default in ( + (None, DEVICE_TYPES[0]), + (DEVICE_TYPES[1], DEVICE_TYPES[1]), + ) + for connected in (None, DEVICE_TYPES[0]) + ), +) +@patch( + "finesse.gui.hardware_set.device_view." + "DeviceParametersWidget.load_saved_parameter_values" +) +@patch( + "finesse.gui.hardware_set.device_view." + "DeviceTypeControl._update_open_btn_enabled_state" +) +@patch("finesse.gui.hardware_set.device_view.settings") +def test_init( + settings_mock: Mock, + update_btn_mock: Mock, + load_saved_mock: Mock, + connected_device: DeviceTypeInfo | None, + previous_device: DeviceTypeInfo | None, + expected_device: DeviceTypeInfo, + subscribe_mock: MagicMock, + qtbot, +) -> None: + """Test the constructor.""" + instance = DeviceInstanceRef("base_type") + + settings_mock.value.return_value = ( + previous_device.class_name if previous_device else None + ) + widget = DeviceTypeControl( + "Base type", + instance, + DEVICE_TYPES, + connected_device_type=connected_device.class_name if connected_device else None, + ) + assert widget._device_instance is instance + items = [ + widget._device_combo.itemText(i) for i in range(widget._device_combo.count()) + ] + assert items == [t.description for t in DEVICE_TYPES] + assert [w.device_type for w in widget._device_widgets] == DEVICE_TYPES + + assert widget._device_combo.currentText() == expected_device.description + + if connected_device and connected_device.class_name == expected_device.class_name: + assert widget._open_close_btn.text() == "Close" + else: + assert widget._open_close_btn.text() == "Open" + + # This should be called at least once. As it will also be called when the selected + # device type changes, it may be called more than once. + update_btn_mock.assert_called_once_with() + + subscribe_mock.assert_has_calls( + [ + call(widget._on_device_opened, f"device.opening.{instance.topic}"), + call(widget._set_device_closed, f"device.closed.{instance.topic}"), + call(widget._show_error_message, f"device.error.{instance.topic}"), + ] + ) + + assert len(widget.findChildren(DeviceParametersWidget)) == 1 + + +def test_init_no_device_types(qtbot) -> None: + """Test that the constructor raises an exception when no device types specified.""" + with pytest.raises(RuntimeError): + DeviceTypeControl("Device type", DeviceInstanceRef("base_type"), [], None) + + +@pytest.mark.parametrize( + "params,expected_enabled", + ( + ((DeviceParameter("param_not_poss", ()),), False), + ( + ( + DeviceParameter( + "param_poss", + range(2), + ), + ), + True, + ), + ( + ( + DeviceParameter("param_not_poss", ()), + DeviceParameter( + "param_poss", + range(2), + ), + ), + False, + ), + ( + ( + DeviceParameter( + "param_poss1", + range(2), + ), + DeviceParameter( + "param_poss2", + range(2), + ), + ), + True, + ), + ), +) +@patch( + "finesse.gui.hardware_set.device_view.DeviceTypeControl.current_device_type_widget", + new_callable=PropertyMock, +) +def test_update_open_btn_enabled_state( + widget_mock: Mock, + params: Sequence[DeviceParameter], + expected_enabled: bool, + widget: DeviceTypeControl, + qtbot, +) -> None: + """Test the _update_open_btn_enabled_state() method. + + The open/close button should be disabled if there are no possible values for at + least one parameter and enabled otherwise. + """ + widget_mock.device_type.parameters = params + with patch.object(widget, "_open_close_btn") as btn_mock: + widget._update_open_btn_enabled_state() + btn_mock.setEnabled(expected_enabled) + + +def test_change_device_type(widget: DeviceTypeControl, qtbot) -> None: + """Test that changing the selected device type causes the GUI to update.""" + with patch.object(widget, "_update_open_btn_enabled_state") as update_btn_mock: + assert widget.layout().itemAt(1).widget() is widget._device_widgets[0] + widget._device_combo.setCurrentIndex(1) + assert widget.layout().itemAt(1).widget() is widget._device_widgets[1] + assert widget._device_widgets[0].isHidden() + assert not widget._device_widgets[1].isHidden() + update_btn_mock.assert_called_once_with() + + +@pytest.mark.parametrize("enable", (True, False)) +@patch( + "finesse.gui.hardware_set.device_view.DeviceTypeControl.current_device_type_widget", + new_callable=PropertyMock, +) +def test_set_combos_enabled( + widget_mock: Mock, enable: bool, widget: DeviceTypeControl, qtbot +) -> None: + """Test the _set_combos_enabled() method.""" + device_widget = MagicMock() + widget_mock.return_value = device_widget + with patch.object(widget, "_device_combo") as combo_mock: + widget._set_combos_enabled(enable) + combo_mock.setEnabled.assert_called_once_with(enable) + device_widget.setEnabled.assert_called_once_with(enable) + + +def test_select_device(widget: DeviceTypeControl, qtbot) -> None: + """Test the _select_device() method.""" + with patch.object( + widget._device_widgets[1], "load_saved_parameter_values" + ) as load_params_mock: + assert widget._device_combo.currentIndex() == 0 + widget._select_device(DEVICE_TYPES[1].class_name) + assert widget._device_combo.currentIndex() == 1 + load_params_mock.assert_called_once_with() + + +@patch("finesse.gui.hardware_set.device_view.logging.warn") +def test_select_device_unknown_device( + warn_mock: Mock, widget: DeviceTypeControl, qtbot +) -> None: + """Test the _select_device() method with an unknown device.""" + assert widget._device_combo.currentIndex() == 0 + widget._select_device("made_up_class") + assert widget._device_combo.currentIndex() == 0 + warn_mock.assert_called_once() + + +@patch("finesse.gui.hardware_set.device_view.open_device") +def test_open_device(open_device_mock: Mock, widget: DeviceTypeControl, qtbot) -> None: + """Test the _open_device() method.""" + widget._open_device() + open_device_mock.assert_called_once_with( + DEVICE_TYPES[0].class_name, + widget._device_instance, + widget._device_widgets[0].current_parameter_values, + ) + + +@patch("finesse.gui.hardware_set.device_view.close_device") +def test_close_device( + close_device_mock: Mock, widget: DeviceTypeControl, qtbot +) -> None: + """Test the _close_device() method.""" + widget._close_device() + close_device_mock.assert_called_once_with(widget._device_instance) + + +def test_on_device_opened(widget: DeviceTypeControl, qtbot) -> None: + """Test the _on_device_opened() method.""" + with patch.object(widget, "_set_device_opened") as open_mock: + widget._on_device_opened(DeviceInstanceRef("base_type"), "some_class", {}) + open_mock.assert_called_once_with("some_class") + + +@patch("finesse.gui.hardware_set.device_view.show_error_message") +def test_show_error_message( + error_message_mock: Mock, widget: DeviceTypeControl, qtbot +) -> None: + """Test the _show_error_message() method.""" + widget._show_error_message(DeviceInstanceRef("base_type"), RuntimeError("boo")) + error_message_mock.assert_called_once() + + +def test_open_close_btn(widget: DeviceTypeControl, qtbot) -> None: + """Test the open/close button works.""" + with patch.object(widget, "_open_device") as open_mock: + with patch.object(widget, "_close_device") as close_mock: + assert widget._open_close_btn.text() == "Open" + widget._open_close_btn.click() + open_mock.assert_called_once_with() + close_mock.assert_not_called() + + open_mock.reset_mock() + widget._open_close_btn.setText("Close") + widget._open_close_btn.click() + open_mock.assert_not_called() + close_mock.assert_called_once_with() From e03fc80cc5b2d87fc8ac69ba0df22cb434a0122b Mon Sep 17 00:00:00 2001 From: Alex Dewar Date: Tue, 14 Nov 2023 13:44:51 +0000 Subject: [PATCH 17/18] Add tests for ManageDevicesDialog code Closes #398. --- tests/gui/hardware_set/test_hardware_sets_view.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/gui/hardware_set/test_hardware_sets_view.py b/tests/gui/hardware_set/test_hardware_sets_view.py index 094e0759..aa371b85 100644 --- a/tests/gui/hardware_set/test_hardware_sets_view.py +++ b/tests/gui/hardware_set/test_hardware_sets_view.py @@ -238,3 +238,18 @@ def test_on_device_closed_not_found(hw_sets: HardwareSetsControl, qtbot) -> None assert not hw_sets._connected_devices with does_not_raise(): hw_sets._on_device_closed(device.instance) + + +def test_show_manage_devices_dialog(hw_sets: HardwareSetsControl, qtbot) -> None: + """Test the _show_manage_devices_dialog() method.""" + # Check that the dialog is created if it doesn't exist + assert not hasattr(hw_sets, "_manage_devices_dialog") + hw_sets._show_manage_devices_dialog() + dialog = hw_sets._manage_devices_dialog + assert not dialog.isHidden() + + # If it already exists, check it is shown + dialog.hide() + hw_sets._show_manage_devices_dialog() + assert not dialog.isHidden() + assert hw_sets._manage_devices_dialog is dialog From 8387c0d39ed105d59db3b61bcfe30d361b103c02 Mon Sep 17 00:00:00 2001 From: Alex Dewar Date: Tue, 14 Nov 2023 15:04:26 +0000 Subject: [PATCH 18/18] Add missing pubsub fixtures This is needed so we don't get subtle bugs during testing. --- tests/gui/hardware_set/test_device_control.py | 2 +- tests/gui/hardware_set/test_device_type_control.py | 2 +- tests/gui/hardware_set/test_hardware_sets_view.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/gui/hardware_set/test_device_control.py b/tests/gui/hardware_set/test_device_control.py index b9b5c791..7b99c80c 100644 --- a/tests/gui/hardware_set/test_device_control.py +++ b/tests/gui/hardware_set/test_device_control.py @@ -19,7 +19,7 @@ @pytest.fixture -def widget(qtbot) -> DeviceControl: +def widget(sendmsg_mock: MagicMock, subscribe_mock: Mock, qtbot) -> DeviceControl: """Return a DeviceControl fixture.""" return DeviceControl(set(CONNECTED_DEVICES)) diff --git a/tests/gui/hardware_set/test_device_type_control.py b/tests/gui/hardware_set/test_device_type_control.py index 7ce9102c..b8fd6349 100644 --- a/tests/gui/hardware_set/test_device_type_control.py +++ b/tests/gui/hardware_set/test_device_type_control.py @@ -17,7 +17,7 @@ @pytest.fixture -def widget(qtbot) -> DeviceTypeControl: +def widget(subscribe_mock: MagicMock, qtbot) -> DeviceTypeControl: """Create a DeviceTypeControl fixture.""" return DeviceTypeControl( "Device type", DeviceInstanceRef("base_type"), DEVICE_TYPES, None diff --git a/tests/gui/hardware_set/test_hardware_sets_view.py b/tests/gui/hardware_set/test_hardware_sets_view.py index aa371b85..8119c84d 100644 --- a/tests/gui/hardware_set/test_hardware_sets_view.py +++ b/tests/gui/hardware_set/test_hardware_sets_view.py @@ -42,7 +42,7 @@ @pytest.fixture @patch("finesse.gui.hardware_set.hardware_sets_view.load_builtin_hardware_sets") def hw_sets( - load_hw_sets_mock: Mock, subscribe_mock: MagicMock, qtbot + load_hw_sets_mock: Mock, sendmsg_mock: MagicMock, subscribe_mock: MagicMock, qtbot ) -> HardwareSetsControl: """A fixture for the control.""" load_hw_sets_mock.return_value = HW_SETS