From dc3129357b1c147a63aafe94f56df6f0d30cbf1a Mon Sep 17 00:00:00 2001 From: wyzula-jan Date: Sun, 18 Jan 2026 14:29:51 +0100 Subject: [PATCH] fix(signal_combo_box): get_signal_name added; remove duplicates from heatmap and scatter waveform settings; --- bec_widgets/cli/client.py | 9 ++ .../signal_combobox/signal_combobox.py | 20 +++- .../plots/heatmap/settings/heatmap_setting.py | 31 +---- .../settings/scatter_curve_setting.py | 12 +- tests/unit_tests/test_device_signal_input.py | 59 +++++++++- tests/unit_tests/test_scatter_waveform.py | 106 +++++++++++++++++- 6 files changed, 200 insertions(+), 37 deletions(-) diff --git a/bec_widgets/cli/client.py b/bec_widgets/cli/client.py index 7f82ef2e..8dcb29d3 100644 --- a/bec_widgets/cli/client.py +++ b/bec_widgets/cli/client.py @@ -5557,6 +5557,15 @@ class SignalComboBox(RPCBase): list[str]: List of device signals. """ + @rpc_call + def get_signal_name(self) -> "str": + """ + Get the signal name from the combobox. + + Returns: + str: The signal name. + """ + class SignalLabel(RPCBase): @property 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 134bc836..c895b9db 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 @@ -27,7 +27,7 @@ class SignalComboBox(DeviceSignalInputBase, QComboBox): arg_name: Argument name, can be used for the other widgets which has to call some other function in bec using correct argument names. """ - USER_ACCESS = ["set_signal", "set_device", "signals"] + USER_ACCESS = ["set_signal", "set_device", "signals", "get_signal_name"] ICON_NAME = "list_alt" PLUGIN = True @@ -148,6 +148,24 @@ class SignalComboBox(DeviceSignalInputBase, QComboBox): return True return False + def get_signal_name(self) -> str: + """ + Get the signal name from the combobox. + + Returns: + str: The signal name. + """ + signal_name = self.currentText() + index = self.findText(signal_name) + if index == -1: + return signal_name + + signal_info = self.itemData(index) + if signal_info: + signal_name = signal_info.get("obj_name", signal_name) + + return signal_name if signal_name else "" + @SafeSlot() def reset_selection(self): """Reset the selection of the combobox.""" diff --git a/bec_widgets/widgets/plots/heatmap/settings/heatmap_setting.py b/bec_widgets/widgets/plots/heatmap/settings/heatmap_setting.py index 84dfa610..c55d3406 100644 --- a/bec_widgets/widgets/plots/heatmap/settings/heatmap_setting.py +++ b/bec_widgets/widgets/plots/heatmap/settings/heatmap_setting.py @@ -1,7 +1,6 @@ from __future__ import annotations import os -from typing import TYPE_CHECKING from qtpy.QtWidgets import QFrame, QScrollArea, QVBoxLayout @@ -9,11 +8,6 @@ from bec_widgets.utils import UILoader from bec_widgets.utils.error_popups import SafeSlot from bec_widgets.utils.settings_dialog import SettingWidget -if TYPE_CHECKING: - from bec_widgets.widgets.control.device_input.signal_combobox.signal_combobox import ( - SignalComboBox, - ) - class HeatmapSettings(SettingWidget): def __init__(self, parent=None, target_widget=None, popup=False, *args, **kwargs): @@ -120,36 +114,17 @@ class HeatmapSettings(SettingWidget): getattr(self.target_widget._image_config, "enforce_interpolation", False) ) - def _get_signal_name(self, signal: SignalComboBox) -> str: - """ - Get the signal name from the signal combobox. - Args: - signal (SignalComboBox): The signal combobox to get the name from. - Returns: - str: The signal name. - """ - device_entry = signal.currentText() - index = signal.findText(device_entry) - if index == -1: - return device_entry - - device_entry_info = signal.itemData(index) - if device_entry_info: - device_entry = device_entry_info.get("obj_name", device_entry) - - return device_entry if device_entry else "" - @SafeSlot() def accept_changes(self): """ Apply all properties from the settings widget to the target widget. """ x_name = self.ui.x_name.currentText() - x_entry = self._get_signal_name(self.ui.x_entry) + x_entry = self.ui.x_entry.get_signal_name() y_name = self.ui.y_name.currentText() - y_entry = self._get_signal_name(self.ui.y_entry) + y_entry = self.ui.y_entry.get_signal_name() z_name = self.ui.z_name.currentText() - z_entry = self._get_signal_name(self.ui.z_entry) + z_entry = self.ui.z_entry.get_signal_name() validate_bec = self.ui.validate_bec.checked color_map = self.ui.color_map.colormap interpolation = self.ui.interpolation.currentText() diff --git a/bec_widgets/widgets/plots/scatter_waveform/settings/scatter_curve_setting.py b/bec_widgets/widgets/plots/scatter_waveform/settings/scatter_curve_setting.py index c0aa6c0d..475181c5 100644 --- a/bec_widgets/widgets/plots/scatter_waveform/settings/scatter_curve_setting.py +++ b/bec_widgets/widgets/plots/scatter_waveform/settings/scatter_curve_setting.py @@ -103,12 +103,12 @@ class ScatterCurveSettings(SettingWidget): """ Apply all properties from the settings widget to the target widget. """ - x_name = self.ui.x_name.text() - x_entry = self.ui.x_entry.text() - y_name = self.ui.y_name.text() - y_entry = self.ui.y_entry.text() - z_name = self.ui.z_name.text() - z_entry = self.ui.z_entry.text() + x_name = self.ui.x_name.currentText() + x_entry = self.ui.x_entry.get_signal_name() + y_name = self.ui.y_name.currentText() + y_entry = self.ui.y_entry.get_signal_name() + z_name = self.ui.z_name.currentText() + z_entry = self.ui.z_entry.get_signal_name() validate_bec = self.ui.validate_bec.checked color_map = self.ui.color_map.colormap diff --git a/tests/unit_tests/test_device_signal_input.py b/tests/unit_tests/test_device_signal_input.py index 65bffd23..ec2426fd 100644 --- a/tests/unit_tests/test_device_signal_input.py +++ b/tests/unit_tests/test_device_signal_input.py @@ -4,7 +4,6 @@ import pytest from bec_lib.device import Signal from qtpy.QtWidgets import QWidget -from bec_widgets.tests.utils import FakeDevice from bec_widgets.utils.ophyd_kind_util import Kind from bec_widgets.widgets.control.device_input.base_classes.device_input_base import BECDeviceFilter from bec_widgets.widgets.control.device_input.base_classes.device_signal_input_base import ( @@ -153,3 +152,61 @@ def test_device_signal_input_base_cleanup(qtbot, mocked_client): widget.deleteLater() mocked_client.callbacks.remove.assert_called_once_with(widget._device_update_register) + + +def test_signal_combobox_get_signal_name_with_item_data(qtbot, device_signal_combobox): + """Test get_signal_name returns obj_name from item data when available.""" + device_signal_combobox.include_normal_signals = True + device_signal_combobox.include_hinted_signals = True + device_signal_combobox.set_device("samx") + + # Select a signal that has item data with obj_name + device_signal_combobox.setCurrentText("samx (readback)") + + # get_signal_name should return the obj_name from item data + signal_name = device_signal_combobox.get_signal_name() + assert signal_name == "samx" + + +def test_signal_combobox_get_signal_name_without_item_data(qtbot, device_signal_combobox): + """Test get_signal_name returns currentText when no item data available.""" + # Add a custom item without item data + device_signal_combobox.addItem("custom_signal") + device_signal_combobox.setCurrentText("custom_signal") + + signal_name = device_signal_combobox.get_signal_name() + assert signal_name == "custom_signal" + + +def test_signal_combobox_get_signal_name_not_found(qtbot, device_signal_combobox): + """Test get_signal_name when text is not found in combobox (index == -1).""" + # Set editable to allow text that's not in items + device_signal_combobox.setEditable(True) + device_signal_combobox.setCurrentText("nonexistent_signal") + + signal_name = device_signal_combobox.get_signal_name() + assert signal_name == "nonexistent_signal" + + +def test_signal_combobox_get_signal_name_empty(qtbot, device_signal_combobox): + """Test get_signal_name when combobox is empty.""" + device_signal_combobox.clear() + device_signal_combobox.setEditable(True) + device_signal_combobox.setCurrentText("") + + signal_name = device_signal_combobox.get_signal_name() + assert signal_name == "" + + +def test_signal_combobox_get_signal_name_with_velocity(qtbot, device_signal_combobox): + """Test get_signal_name with velocity signal.""" + device_signal_combobox.include_normal_signals = True + device_signal_combobox.include_hinted_signals = True + device_signal_combobox.include_config_signals = True + device_signal_combobox.set_device("samx") + + # Select velocity signal + device_signal_combobox.setCurrentText("velocity") + + signal_name = device_signal_combobox.get_signal_name() + assert signal_name == "samx_velocity" diff --git a/tests/unit_tests/test_scatter_waveform.py b/tests/unit_tests/test_scatter_waveform.py index 95d950a6..9abb01b0 100644 --- a/tests/unit_tests/test_scatter_waveform.py +++ b/tests/unit_tests/test_scatter_waveform.py @@ -1,4 +1,4 @@ -import json +from unittest.mock import patch import numpy as np @@ -7,6 +7,9 @@ from bec_widgets.widgets.plots.scatter_waveform.scatter_curve import ( ScatterDeviceSignal, ) from bec_widgets.widgets.plots.scatter_waveform.scatter_waveform import ScatterWaveform +from bec_widgets.widgets.plots.scatter_waveform.settings.scatter_curve_setting import ( + ScatterCurveSettings, +) from tests.unit_tests.client_mocks import create_dummy_scan_item, mocked_client from .conftest import create_widget @@ -460,3 +463,104 @@ def test_device_properties_with_none_values(qtbot, mocked_client): # Entry None should not change anything swf.y_device_entry = None assert swf.y_device_entry # Should still have validated entry + + +################################################################################ +# ScatterCurveSettings Tests +################################################################################ + + +def test_scatter_curve_settings_accept_changes(qtbot, mocked_client): + """Test that accept_changes correctly extracts data from widgets and calls plot().""" + swf = create_widget(qtbot, ScatterWaveform, client=mocked_client) + + # Create the settings widget + settings = ScatterCurveSettings(parent=None, target_widget=swf, popup=True) + qtbot.addWidget(settings) + + # Set up the widgets with test values + settings.ui.x_name.set_device("samx") + settings.ui.y_name.set_device("samy") + settings.ui.z_name.set_device("bpm4i") + + # Mock the plot method to verify it gets called with correct arguments + with patch.object(swf, "plot") as mock_plot: + settings.accept_changes() + + # Verify plot was called + mock_plot.assert_called_once() + + # Get the call arguments + call_kwargs = mock_plot.call_args[1] + + # Verify device names were extracted correctly + assert call_kwargs["x_name"] == "samx" + assert call_kwargs["y_name"] == "samy" + assert call_kwargs["z_name"] == "bpm4i" + + +def test_scatter_curve_settings_accept_changes_with_entries(qtbot, mocked_client): + """Test that accept_changes correctly extracts signal entries from SignalComboBox.""" + swf = create_widget(qtbot, ScatterWaveform, client=mocked_client) + + # Create the settings widget + settings = ScatterCurveSettings(parent=None, target_widget=swf, popup=True) + qtbot.addWidget(settings) + + # Set devices first to populate signal comboboxes + settings.ui.x_name.set_device("samx") + settings.ui.y_name.set_device("samy") + settings.ui.z_name.set_device("bpm4i") + qtbot.wait(100) # Allow time for signals to populate + + # Mock the plot method + with patch.object(swf, "plot") as mock_plot: + settings.accept_changes() + + mock_plot.assert_called_once() + call_kwargs = mock_plot.call_args[1] + + # Verify entries are extracted (will use get_signal_name()) + assert "x_entry" in call_kwargs + assert "y_entry" in call_kwargs + assert "z_entry" in call_kwargs + + +def test_scatter_curve_settings_accept_changes_color_map(qtbot, mocked_client): + """Test that accept_changes correctly extracts color_map from widget.""" + + swf = create_widget(qtbot, ScatterWaveform, client=mocked_client) + + # Create the settings widget + settings = ScatterCurveSettings(parent=None, target_widget=swf, popup=True) + qtbot.addWidget(settings) + + # Set devices + settings.ui.x_name.set_device("samx") + settings.ui.y_name.set_device("samy") + settings.ui.z_name.set_device("bpm4i") + + # Get the current colormap + color_map = settings.ui.color_map.colormap + + with patch.object(swf, "plot") as mock_plot: + settings.accept_changes() + call_kwargs = mock_plot.call_args[1] + assert call_kwargs["color_map"] == color_map + + +def test_scatter_curve_settings_fetch_all_properties(qtbot, mocked_client): + """Test that fetch_all_properties correctly populates the settings from target widget.""" + swf = create_widget(qtbot, ScatterWaveform, client=mocked_client) + + # First set up the scatter waveform with some data + swf.plot(x_name="samx", y_name="samy", z_name="bpm4i") + + # Create the settings widget - it should fetch properties automatically + settings = ScatterCurveSettings(parent=None, target_widget=swf, popup=True) + qtbot.addWidget(settings) + + # Verify the settings widget has fetched the values + assert settings.ui.x_name.currentText() == "samx" + assert settings.ui.y_name.currentText() == "samy" + assert settings.ui.z_name.currentText() == "bpm4i"