From 00c2fdaadbb6c28292c405c396391b72e32be5b8 Mon Sep 17 00:00:00 2001 From: wakonig_k Date: Wed, 9 Sep 2026 16:00:45 +0200 Subject: [PATCH] fix(signal combobox): update devices from main thread --- .../signal_combobox/signal_combobox.py | 12 ++- tests/unit_tests/test_device_signal_input.py | 75 ++++++++++++++++++- 2 files changed, 85 insertions(+), 2 deletions(-) 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 b4f4667b..e02e378a 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 @@ -77,6 +77,7 @@ class SignalComboBox(BECWidget, QComboBox): device_signal_changed = Signal(str) signal_reset = Signal() + _device_update_requested = Signal() def __init__( self, @@ -137,8 +138,10 @@ class SignalComboBox(BECWidget, QComboBox): if self.config.autocomplete: self.autocomplete = True + self._device_update_register = None + self._device_update_requested.connect(self.update_signals_from_filters) self._device_update_register = self.bec_dispatcher.client.callbacks.register( - EventType.DEVICE_UPDATE, self.update_signals_from_filters + EventType.DEVICE_UPDATE, self._on_device_update ) self.currentTextChanged.connect(self.on_text_changed) @@ -196,6 +199,13 @@ class SignalComboBox(BECWidget, QComboBox): self.setCurrentText("") self.update_signals_from_filters() + def _on_device_update(self, action: str, content: dict) -> None: + """Deliver BEC callbacks to the widget thread before accessing Qt models.""" + if self._device_update_register is None or getattr(self, "_destroyed", False): + return + if action in ["add", "remove", "reload"]: + self._device_update_requested.emit() + @SafeSlot() @SafeSlot(str, dict) def update_signals_from_filters(self, action: str | None = None, content: dict | None = None): diff --git a/tests/unit_tests/test_device_signal_input.py b/tests/unit_tests/test_device_signal_input.py index b431d9ba..ce90bc80 100644 --- a/tests/unit_tests/test_device_signal_input.py +++ b/tests/unit_tests/test_device_signal_input.py @@ -1,7 +1,11 @@ +from concurrent.futures import ThreadPoolExecutor +from threading import get_ident from unittest import mock import pytest +from bec_lib.callback_handler import EventType from bec_lib.device import Signal +from qtpy.QtCore import Qt from bec_widgets.utils.ophyd_kind_util import Kind from bec_widgets.widgets.control.device_input.device_combobox.device_combobox import ( @@ -56,6 +60,59 @@ def test_signal_combobox_init(device_signal_combobox): assert device_signal_combobox.completer().model() == device_signal_combobox.model() +@pytest.mark.parametrize("action", ["add", "remove", "reload"]) +@pytest.mark.parametrize("class_filter", [False, True]) +def test_signal_combobox_device_callback_updates_on_gui_thread( + qtbot, mocked_client, action, class_filter +): + mocked_client.device_manager.get_bec_signals = mock.MagicMock( + return_value=[("samx", "readback", {"obj_name": "samx_readback"})] + ) + widget = create_widget( + qtbot=qtbot, + widget=SignalComboBox, + client=mocked_client, + device="samx", + autocomplete=True, + signal_class_filter=["AsyncSignal"] if class_filter else None, + ) + expected_signals = widget.signals.copy() + widget.signals = [] + gui_thread = get_ident() + rebuild_threads = [] + model_threads = [] + original_replace = widget._replace_signal_items + + def record_rebuild(*args, **kwargs): + rebuild_threads.append(get_ident()) + # Record a regression without actually mutating Qt models from a worker. + assert get_ident() == gui_thread + original_replace(*args, **kwargs) + + def record_model_change(*_args): + model_threads.append(get_ident()) + + widget.model().rowsInserted.connect(record_model_change, Qt.ConnectionType.DirectConnection) + widget._completer_model.modelReset.connect( + record_model_change, Qt.ConnectionType.DirectConnection + ) + with mock.patch.object(widget, "_replace_signal_items", side_effect=record_rebuild): + with ThreadPoolExecutor(max_workers=1) as executor: + executor.submit( + widget.bec_dispatcher.client.callbacks.run, + EventType.DEVICE_UPDATE, + action=action, + content={}, + ).result(timeout=5) + assert rebuild_threads == [] + qtbot.waitUntil(lambda: bool(rebuild_threads)) + + assert rebuild_threads == [gui_thread] + assert model_threads and all(thread == gui_thread for thread in model_threads) + assert widget.signals == expected_signals + assert widget._completer_model.stringList() == signal_names(expected_signals) + + def test_signal_combobox_config_defaults_are_independent_lists(): config_a = SignalComboBoxConfig(widget_class="SignalComboBox") config_b = SignalComboBoxConfig(widget_class="SignalComboBox") @@ -233,7 +290,7 @@ def test_signal_combobox_cleanup_blocks_in_flight_device_update(qtbot, mocked_cl callback_id = widget._device_update_register def trigger_in_flight_update(_): - widget.update_signals_from_filters("reload", {}) + widget._on_device_update("reload", {}) with ( mock.patch.object( @@ -247,11 +304,27 @@ def test_signal_combobox_cleanup_blocks_in_flight_device_update(qtbot, mocked_cl set_signal_groups.assert_not_called() +def test_signal_combobox_cleanup_blocks_queued_device_update(qtbot, mocked_client): + widget = create_widget(qtbot=qtbot, widget=SignalComboBox, client=mocked_client, device="samx") + callbacks = widget.bec_dispatcher.client.callbacks + callback_id = widget._device_update_register + with mock.patch.object(widget, "_set_signal_groups") as set_signal_groups: + with ThreadPoolExecutor(max_workers=1) as executor: + executor.submit(callbacks.run, EventType.DEVICE_UPDATE, "reload", {}).result(timeout=5) + widget.cleanup() + qtbot.wait(10) + callbacks.run(EventType.DEVICE_UPDATE, "reload", {}) + + assert callback_id not in callbacks.callbacks + set_signal_groups.assert_not_called() + + def test_signal_combobox_device_update_ignores_update_action(qtbot, mocked_client): widget = create_widget(qtbot=qtbot, widget=SignalComboBox, client=mocked_client) with mock.patch.object(widget, "_set_signal_groups") as set_signal_groups: widget.update_signals_from_filters("update", {}) + widget.bec_dispatcher.client.callbacks.run(EventType.DEVICE_UPDATE, "update", {}) set_signal_groups.assert_not_called()