From 0dfcaa4b708948af0a40ec7cf34d03ff1e96ffac Mon Sep 17 00:00:00 2001 From: Mathias Guijarro Date: Tue, 30 Apr 2024 13:20:48 +0200 Subject: [PATCH] fix: 'disconnect_slot' has to be symmetric with 'connect_slot' regarding QtThreadSafeCallback --- bec_widgets/utils/bec_dispatcher.py | 10 ++- tests/unit_tests/test_bec_dispatcher.py | 85 ++++++++++++++++++++++++- 2 files changed, 92 insertions(+), 3 deletions(-) diff --git a/bec_widgets/utils/bec_dispatcher.py b/bec_widgets/utils/bec_dispatcher.py index e20cf781..db188390 100644 --- a/bec_widgets/utils/bec_dispatcher.py +++ b/bec_widgets/utils/bec_dispatcher.py @@ -129,7 +129,15 @@ class BECDispatcher: self._slots[slot].update(set(topics_str)) def disconnect_slot(self, slot: Callable, topics: Union[str, list]): - self.client.connector.unregister(topics, cb=slot) + # find the right slot to disconnect from ; + # slot callbacks are wrapped in QtThreadSafeCallback objects, + # but the slot we receive here is the original callable + for connected_slot in self._slots: + if connected_slot.cb == slot: + break + else: + return + self.client.connector.unregister(topics, cb=connected_slot) topics_str, _ = self.client.connector._convert_endpointinfo(topics) self._slots[slot].difference_update(set(topics_str)) if not self._slots[slot]: diff --git a/tests/unit_tests/test_bec_dispatcher.py b/tests/unit_tests/test_bec_dispatcher.py index 2b34592c..dab01bf2 100644 --- a/tests/unit_tests/test_bec_dispatcher.py +++ b/tests/unit_tests/test_bec_dispatcher.py @@ -1,4 +1,5 @@ # pylint: disable = no-name-in-module,missing-class-docstring, missing-module-docstring +import threading import time from unittest import mock @@ -13,8 +14,9 @@ from bec_widgets.utils.bec_dispatcher import QtRedisConnector @pytest.fixture -def bec_dispatcher_w_connector(bec_dispatcher, topics_msg_list): +def bec_dispatcher_w_connector(bec_dispatcher, topics_msg_list, send_msg_event): def pubsub_msg_generator(): + send_msg_event.wait() for topic, msg in topics_msg_list: yield {"channel": topic.encode(), "pattern": None, "data": msg} while True: @@ -33,6 +35,11 @@ def bec_dispatcher_w_connector(bec_dispatcher, topics_msg_list): dummy_msg = MsgpackSerialization.dumps(ScanMessage(point_id=0, scan_id="0", data={})) +@pytest.fixture +def send_msg_event(): + return threading.Event() + + @pytest.mark.parametrize( "topics_msg_list", [ @@ -43,7 +50,7 @@ dummy_msg = MsgpackSerialization.dumps(ScanMessage(point_id=0, scan_id="0", data ) ], ) -def test_dispatcher_disconnect_all(bec_dispatcher_w_connector, qtbot): +def test_dispatcher_disconnect_all(bec_dispatcher_w_connector, qtbot, send_msg_event): bec_dispatcher = bec_dispatcher_w_connector cb1 = mock.Mock(spec=[]) cb2 = mock.Mock(spec=[]) @@ -53,7 +60,81 @@ def test_dispatcher_disconnect_all(bec_dispatcher_w_connector, qtbot): bec_dispatcher.connect_slot(cb2, "topic2") bec_dispatcher.connect_slot(cb2, "topic3") assert len(bec_dispatcher.client.connector._topics_cb) == 3 + send_msg_event.set() + qtbot.wait(10) + assert cb1.call_count == 2 + assert cb2.call_count == 2 bec_dispatcher.disconnect_all() assert len(bec_dispatcher.client.connector._topics_cb) == 0 + + +@pytest.mark.parametrize( + "topics_msg_list", + [ + ( + ("topic1", dummy_msg), + ("topic2", dummy_msg), + ) + ], +) +def test_dispatcher_disconnect_one(bec_dispatcher_w_connector, qtbot, send_msg_event): + # test for BEC issue #276 + bec_dispatcher = bec_dispatcher_w_connector + cb1 = mock.Mock(spec=[]) + cb2 = mock.Mock(spec=[]) + + bec_dispatcher.connect_slot(cb1, "topic1") + bec_dispatcher.connect_slot(cb2, "topic2") + assert len(bec_dispatcher.client.connector._topics_cb) == 2 + bec_dispatcher.disconnect_slot(cb1, "topic1") + assert len(bec_dispatcher.client.connector._topics_cb) == 1 + + send_msg_event.set() + qtbot.wait(10) + assert cb1.call_count == 0 + cb2.assert_called_once() + + +@pytest.mark.parametrize("topics_msg_list", [(("topic1", dummy_msg),)]) +def test_dispatcher_2_cb_same_topic(bec_dispatcher_w_connector, qtbot, send_msg_event): + # test for BEC issue #276 + bec_dispatcher = bec_dispatcher_w_connector + cb1 = mock.Mock(spec=[]) + cb2 = mock.Mock(spec=[]) + + bec_dispatcher.connect_slot(cb1, "topic1") + bec_dispatcher.connect_slot(cb2, "topic1") + assert len(bec_dispatcher.client.connector._topics_cb) == 1 + bec_dispatcher.disconnect_slot(cb1, "topic1") + + send_msg_event.set() + qtbot.wait(10) + assert cb1.call_count == 0 + cb2.assert_called_once() + + +@pytest.mark.parametrize( + "topics_msg_list", + [ + ( + ("topic1", dummy_msg), + ("topic2", dummy_msg), + ) + ], +) +def test_dispatcher_2_topic_same_cb(bec_dispatcher_w_connector, qtbot, send_msg_event): + # test for BEC issue #276 + bec_dispatcher = bec_dispatcher_w_connector + cb1 = mock.Mock(spec=[]) + + bec_dispatcher.connect_slot(cb1, "topic1") + bec_dispatcher.connect_slot(cb1, "topic2") + assert len(bec_dispatcher.client.connector._topics_cb) == 2 + bec_dispatcher.disconnect_slot(cb1, "topic1") + assert len(bec_dispatcher.client.connector._topics_cb) == 1 + + send_msg_event.set() + qtbot.wait(10) + cb1.assert_called_once()