From 2c275bd3fd5d4a9acffc7ab82411b01566164f2d Mon Sep 17 00:00:00 2001 From: Jorge Capona Date: Fri, 11 Nov 2022 17:24:37 -0300 Subject: [PATCH 1/8] Subscribe only to relevant PTDM events --- packages/common/pitop/common/ptdm.py | 58 +++++++++---------- .../miniscreen/pitop/miniscreen/miniscreen.py | 4 +- 2 files changed, 32 insertions(+), 30 deletions(-) diff --git a/packages/common/pitop/common/ptdm.py b/packages/common/pitop/common/ptdm.py index 5323a57fc..99bc5814f 100644 --- a/packages/common/pitop/common/ptdm.py +++ b/packages/common/pitop/common/ptdm.py @@ -414,7 +414,7 @@ class PTDMSubscribeClient: def __init__(self): self.__thread = Thread(target=self.__thread_method, daemon=True) - self._callback_funcs = None + self._callback_funcs = {} self._zmq_context = None self._zmq_socket = None @@ -429,7 +429,9 @@ def __exit__(self, exc_type, exc_value, exc_traceback): def __connect_to_socket(self): self._zmq_context = zmq.Context() self._zmq_socket = self._zmq_context.socket(zmq.SUB) - self._zmq_socket.setsockopt_string(zmq.SUBSCRIBE, "") + + for message_id in self._callback_funcs.keys(): + self._zmq_socket.setsockopt(zmq.SUBSCRIBE, str(message_id).encode()) try: self._zmq_socket.connect(self.URI) @@ -453,39 +455,37 @@ def __cleanup(self): self._zmq_context = None def __thread_method(self): - poller = zmq.Poller() - poller.register(self._zmq_socket, zmq.POLLIN) while self.__continue: - events = poller.poll(_TIMEOUT_MS) - - for _ in range(len(events)): - message_string = self._zmq_socket.recv_string() - message = Message.from_string(message_string) - - id = message.message_id() - if id in self._callback_funcs: - self.invoke_callback_func_if_exists( - self._callback_funcs[id], message.parameters - ) - - def invoke_callback_func_if_exists(self, func, params=list()): - if not callable(func): - return - - func_arg_no = len(signature(func).parameters) - if func_arg_no > 1: - logger.error( - "Invalid callback function - it should receive at most one argument." - ) - return "" + message_string = self._zmq_socket.recv_string() + message = Message.from_string(message_string) + + callback = self._callback_funcs.get(message.message_id()) + if callback: + self.invoke_callback_func_if_exists(callback, message.parameters) + def invoke_callback_func_if_exists(self, callback, params=list()): + func_arg_no = len(signature(callback).parameters) if params == list() or func_arg_no == 0: - func() + callback() else: - func(params) + callback(params) def initialise(self, callback_funcs): - self._callback_funcs = callback_funcs + for message_id, callback in callback_funcs.items(): + if not callable(callback): + logger.error( + f"Invalid callback function for message {message_id} - not callable. Skipping..." + ) + continue + + func_arg_no = len(signature(callback).parameters) + if func_arg_no > 1: + logger.error( + f"Invalid callback function for message {message_id} - it should receive at most one argument. Skipping..." + ) + continue + + self._callback_funcs.update({message_id: callback}) def start_listening(self): if not self.__connect_to_socket(): diff --git a/packages/miniscreen/pitop/miniscreen/miniscreen.py b/packages/miniscreen/pitop/miniscreen/miniscreen.py index 2609039dd..672643c6d 100644 --- a/packages/miniscreen/pitop/miniscreen/miniscreen.py +++ b/packages/miniscreen/pitop/miniscreen/miniscreen.py @@ -31,9 +31,11 @@ def __init__(self): def __setup_subscribe_client(self): def set_button_state(button, pressed): button.is_pressed = pressed - self.__ptdm_subscribe_client.invoke_callback_func_if_exists( + callback = ( button.when_pressed if button.is_pressed else button.when_released ) + if callable(callback): + callback() self.__ptdm_subscribe_client = PTDMSubscribeClient() self.__ptdm_subscribe_client.initialise( From 0c0d4585d0ea8f885fea8ca1d302cf5947ca473e Mon Sep 17 00:00:00 2001 From: Jorge Capona Date: Fri, 11 Nov 2022 19:24:18 -0300 Subject: [PATCH 2/8] Update tests --- tests/test_ptdm.py | 33 ++++++++------------------------- 1 file changed, 8 insertions(+), 25 deletions(-) diff --git a/tests/test_ptdm.py b/tests/test_ptdm.py index 2278f2177..3def10f86 100644 --- a/tests/test_ptdm.py +++ b/tests/test_ptdm.py @@ -8,30 +8,21 @@ class PTDMSubscribeClientTestCase(TestCase): def setUp(self): self.zmq_patch = patch("pitop.common.ptdm.zmq") self.zmq_mock = self.zmq_patch.start() - self.poller_mock = Mock() self.context_mock = Mock() self.socket_mock = Mock() self.socket_mock.recv_string.return_value = "" self.context_mock.socket.return_value = self.socket_mock self.zmq_mock.Context.return_value = self.context_mock - self.zmq_mock.Poller.return_value = self.poller_mock - self.poller_mock.poll.return_value = [] self.addCleanup(self.zmq_patch.stop) @skip def test_callback_called_when_message_is_published(self): from pitop.common.ptdm import Message, PTDMSubscribeClient - self.poller_mock.poll.side_effect = ( - lambda _: [1] if self.poller_mock.poll.call_count == 1 else [] - ) self.socket_mock.recv_string.return_value = f"{Message.PUB_LOW_BATTERY_WARNING}" - def callback(): - callback.counter += 1 - - callback.counter = 0 + callback = Mock() client = PTDMSubscribeClient() client.initialise( @@ -39,18 +30,14 @@ def callback(): Message.PUB_LOW_BATTERY_WARNING: callback, } ) + client.start_listening() - assert callback.counter == 0 - wait_until(lambda: callback.counter > 10, timeout=10) - assert callback.counter == 1 + wait_until(lambda: callback.call_count == 1, timeout=3) client.stop_listening() - def test_callback_not_called_if_it_has_wrong_signature(self): + def test_callback_not_included_if_has_wrong_signature(self): from pitop.common.ptdm import Message, PTDMSubscribeClient - self.poller_mock.poll.side_effect = ( - lambda _: [1] if self.poller_mock.poll.call_count == 1 else [] - ) self.socket_mock.recv_string.return_value = f"{Message.PUB_LOW_BATTERY_WARNING}" def callback(x, y): @@ -64,15 +51,14 @@ def callback(x, y): Message.PUB_LOW_BATTERY_WARNING: callback, } ) - client.start_listening() - assert callback.counter == 0 - wait_until(lambda: self.poller_mock.poll.call_count > 10, timeout=5) - assert callback.counter == 0 - client.stop_listening() + + assert client._callback_funcs.get(Message.PUB_LOW_BATTERY_WARNING) is None def test_subscribe_client_cleanup_closes_socket(self): from pitop.common.ptdm import Message, PTDMSubscribeClient + self.socket_mock.recv_string.return_value = f"{Message.PUB_LOW_BATTERY_WARNING}" + client = PTDMSubscribeClient() client.initialise( { @@ -94,15 +80,12 @@ class PTDMRequestClientTestCase(TestCase): def setUp(self): self.zmq_patch = patch("pitop.common.ptdm.zmq") self.zmq_mock = self.zmq_patch.start() - self.poller_mock = Mock() self.context_mock = Mock() self.socket_mock = Mock() self.socket_mock.recv_string.return_value = "" self.context_mock.socket.return_value = self.socket_mock self.zmq_mock.Context.return_value = self.context_mock - self.zmq_mock.Poller.return_value = self.poller_mock - self.poller_mock.poll.return_value = [] self.addCleanup(self.zmq_patch.stop) def test_uri(self): From dfd18d0ca9b550454e21a4fad3c429ab4754a993 Mon Sep 17 00:00:00 2001 From: Jorge Capona Date: Mon, 14 Nov 2022 14:55:26 -0300 Subject: [PATCH 3/8] Poll for events --- packages/common/pitop/common/ptdm.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/common/pitop/common/ptdm.py b/packages/common/pitop/common/ptdm.py index 99bc5814f..0af7cf1e1 100644 --- a/packages/common/pitop/common/ptdm.py +++ b/packages/common/pitop/common/ptdm.py @@ -455,13 +455,17 @@ def __cleanup(self): self._zmq_context = None def __thread_method(self): + poller = zmq.Poller() + poller.register(self._zmq_socket, zmq.POLLIN) while self.__continue: - message_string = self._zmq_socket.recv_string() - message = Message.from_string(message_string) - - callback = self._callback_funcs.get(message.message_id()) - if callback: - self.invoke_callback_func_if_exists(callback, message.parameters) + events = poller.poll(_TIMEOUT_MS) + for _ in range(len(events)): + message_string = self._zmq_socket.recv_string() + message = Message.from_string(message_string) + + callback = self._callback_funcs.get(message.message_id()) + if callback: + self.invoke_callback_func_if_exists(callback, message.parameters) def invoke_callback_func_if_exists(self, callback, params=list()): func_arg_no = len(signature(callback).parameters) From d7eb29c8529be2d0d4a7b40d5e4d5991ed20f6d5 Mon Sep 17 00:00:00 2001 From: Jorge Capona Date: Tue, 6 Dec 2022 17:55:47 -0300 Subject: [PATCH 4/8] Update function default value --- packages/common/pitop/common/ptdm.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/common/pitop/common/ptdm.py b/packages/common/pitop/common/ptdm.py index 0af7cf1e1..03b3107b2 100644 --- a/packages/common/pitop/common/ptdm.py +++ b/packages/common/pitop/common/ptdm.py @@ -467,9 +467,12 @@ def __thread_method(self): if callback: self.invoke_callback_func_if_exists(callback, message.parameters) - def invoke_callback_func_if_exists(self, callback, params=list()): + def invoke_callback_func_if_exists(self, callback, params=None): + if params is None: + params = list() + func_arg_no = len(signature(callback).parameters) - if params == list() or func_arg_no == 0: + if len(params) == 0 or func_arg_no == 0: callback() else: callback(params) From 53e38c8685896a371be39230c0913c2323555427 Mon Sep 17 00:00:00 2001 From: Jorge Capona Date: Tue, 6 Dec 2022 17:59:02 -0300 Subject: [PATCH 5/8] invoke_callback_if_exists -> invoke_callback --- packages/common/pitop/common/ptdm.py | 4 ++-- packages/display/pitop/display/display.py | 18 +++++------------- 2 files changed, 7 insertions(+), 15 deletions(-) diff --git a/packages/common/pitop/common/ptdm.py b/packages/common/pitop/common/ptdm.py index 03b3107b2..50bf4e463 100644 --- a/packages/common/pitop/common/ptdm.py +++ b/packages/common/pitop/common/ptdm.py @@ -465,9 +465,9 @@ def __thread_method(self): callback = self._callback_funcs.get(message.message_id()) if callback: - self.invoke_callback_func_if_exists(callback, message.parameters) + self.invoke_callback(callback, message.parameters) - def invoke_callback_func_if_exists(self, callback, params=None): + def invoke_callback(self, callback, params=None): if params is None: params = list() diff --git a/packages/display/pitop/display/display.py b/packages/display/pitop/display/display.py index 24dd8f296..791daa1f4 100644 --- a/packages/display/pitop/display/display.py +++ b/packages/display/pitop/display/display.py @@ -18,29 +18,21 @@ def __init__(self): def __setup_subscribe_client(self): def on_brightness_changed(parameters): - self.__ptdm_subscribe_client.invoke_callback_func_if_exists( + self.__ptdm_subscribe_client.invoke_callback( self.when_brightness_changed, parameters[0] ) def on_screen_blanked(): - self.__ptdm_subscribe_client.invoke_callback_func_if_exists( - self.when_screen_blanked - ) + self.__ptdm_subscribe_client.invoke_callback(self.when_screen_blanked) def on_screen_unblanked(): - self.__ptdm_subscribe_client.invoke_callback_func_if_exists( - self.when_screen_unblanked - ) + self.__ptdm_subscribe_client.invoke_callback(self.when_screen_unblanked) def on_lid_closed(): - self.__ptdm_subscribe_client.invoke_callback_func_if_exists( - self.when_lid_closed - ) + self.__ptdm_subscribe_client.invoke_callback(self.when_lid_closed) def on_lid_opened(): - self.__ptdm_subscribe_client.invoke_callback_func_if_exists( - self.when_lid_opened - ) + self.__ptdm_subscribe_client.invoke_callback(self.when_lid_opened) self.__ptdm_subscribe_client = PTDMSubscribeClient() self.__ptdm_subscribe_client.initialise( From bd81adb58474c557be88836f455a63a786b6f62b Mon Sep 17 00:00:00 2001 From: Jorge Capona Date: Tue, 6 Dec 2022 18:26:20 -0300 Subject: [PATCH 6/8] Unskip test --- tests/test_ptdm.py | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/tests/test_ptdm.py b/tests/test_ptdm.py index 3def10f86..d568823f2 100644 --- a/tests/test_ptdm.py +++ b/tests/test_ptdm.py @@ -1,4 +1,4 @@ -from unittest import TestCase, skip +from unittest import TestCase from unittest.mock import Mock, patch from tests.utils import wait_until @@ -8,31 +8,34 @@ class PTDMSubscribeClientTestCase(TestCase): def setUp(self): self.zmq_patch = patch("pitop.common.ptdm.zmq") self.zmq_mock = self.zmq_patch.start() + self.poller_mock = Mock() self.context_mock = Mock() self.socket_mock = Mock() self.socket_mock.recv_string.return_value = "" self.context_mock.socket.return_value = self.socket_mock self.zmq_mock.Context.return_value = self.context_mock + self.zmq_mock.Poller.return_value = self.poller_mock + self.poller_mock.poll.return_value = [] self.addCleanup(self.zmq_patch.stop) - @skip def test_callback_called_when_message_is_published(self): from pitop.common.ptdm import Message, PTDMSubscribeClient + self.poller_mock.poll.return_value = [1] self.socket_mock.recv_string.return_value = f"{Message.PUB_LOW_BATTERY_WARNING}" - callback = Mock() + def callback(): + callback.counter += 1 - client = PTDMSubscribeClient() - client.initialise( - { - Message.PUB_LOW_BATTERY_WARNING: callback, - } - ) + callback.counter = 0 + client = PTDMSubscribeClient() + client.initialise({Message.PUB_LOW_BATTERY_WARNING: callback}) client.start_listening() - wait_until(lambda: callback.call_count == 1, timeout=3) + + wait_until(lambda: callback.counter > 0, timeout=3) + client.stop_listening() def test_callback_not_included_if_has_wrong_signature(self): From 7e1b4f3710c3797a4ff6d12bbd105975a5547094 Mon Sep 17 00:00:00 2001 From: Jorge Capona Date: Tue, 6 Dec 2022 18:33:41 -0300 Subject: [PATCH 7/8] Test case where a callback with args is called --- tests/test_ptdm.py | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/tests/test_ptdm.py b/tests/test_ptdm.py index d568823f2..abc02dbec 100644 --- a/tests/test_ptdm.py +++ b/tests/test_ptdm.py @@ -19,22 +19,38 @@ def setUp(self): self.poller_mock.poll.return_value = [] self.addCleanup(self.zmq_patch.stop) - def test_callback_called_when_message_is_published(self): + def test_correct_callback_called_when_message_is_published(self): from pitop.common.ptdm import Message, PTDMSubscribeClient self.poller_mock.poll.return_value = [1] - self.socket_mock.recv_string.return_value = f"{Message.PUB_LOW_BATTERY_WARNING}" - def callback(): - callback.counter += 1 + def callback_without_args(): + callback_without_args.counter += 1 - callback.counter = 0 + callback_without_args.counter = 0 + + def callback_with_args(): + callback_with_args.counter += 1 + + callback_with_args.counter = 0 client = PTDMSubscribeClient() - client.initialise({Message.PUB_LOW_BATTERY_WARNING: callback}) + client.initialise( + { + Message.PUB_LOW_BATTERY_WARNING: callback_without_args, + Message.PUB_BRIGHTNESS_CHANGED: callback_with_args, + } + ) client.start_listening() - wait_until(lambda: callback.counter > 0, timeout=3) + self.socket_mock.recv_string.return_value = f"{Message.PUB_LOW_BATTERY_WARNING}" + wait_until(lambda: callback_without_args.counter > 0, timeout=3) + assert callback_with_args.counter == 0 + + self.socket_mock.recv_string.return_value = ( + f"{Message.PUB_BRIGHTNESS_CHANGED}|1" + ) + wait_until(lambda: callback_with_args.counter > 0, timeout=3) client.stop_listening() From 28739105f5b959fc8eeefef22eef94ce882c7616 Mon Sep 17 00:00:00 2001 From: Jorge Capona Date: Tue, 6 Dec 2022 20:23:15 -0300 Subject: [PATCH 8/8] Increase test timeout --- conftest.py | 10 +++++++++- tests/test_ptdm.py | 13 ++++++++++--- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/conftest.py b/conftest.py index 41b52042c..33640369e 100644 --- a/conftest.py +++ b/conftest.py @@ -27,12 +27,20 @@ ]: sys.modules[module] = Mock() + # use gpiozero fake pins environ["GPIOZERO_PIN_FACTORY"] = "mock" @pytest.fixture -def oled_mocks(): +def zmq_poller_mock(): + poller_mock = Mock() + poller_mock.poll.return_value = [] + sys.modules["zmq"].Poller.return_value = poller_mock + + +@pytest.fixture +def oled_mocks(zmq_poller_mock): SIZE = (128, 64) MODE = "1" SPI_BUS = 0 diff --git a/tests/test_ptdm.py b/tests/test_ptdm.py index abc02dbec..b5f3f8ddd 100644 --- a/tests/test_ptdm.py +++ b/tests/test_ptdm.py @@ -22,7 +22,7 @@ def setUp(self): def test_correct_callback_called_when_message_is_published(self): from pitop.common.ptdm import Message, PTDMSubscribeClient - self.poller_mock.poll.return_value = [1] + self.poller_mock.poll.return_value = [2] def callback_without_args(): callback_without_args.counter += 1 @@ -43,14 +43,19 @@ def callback_with_args(): ) client.start_listening() + assert callback_without_args.counter == 0 + assert callback_with_args.counter == 0 + + # Emit event that doesn't use an argument self.socket_mock.recv_string.return_value = f"{Message.PUB_LOW_BATTERY_WARNING}" - wait_until(lambda: callback_without_args.counter > 0, timeout=3) + wait_until(lambda: callback_without_args.counter > 0, timeout=5) assert callback_with_args.counter == 0 + # Emit event that uses an argument self.socket_mock.recv_string.return_value = ( f"{Message.PUB_BRIGHTNESS_CHANGED}|1" ) - wait_until(lambda: callback_with_args.counter > 0, timeout=3) + wait_until(lambda: callback_with_args.counter > 0, timeout=5) client.stop_listening() @@ -59,6 +64,7 @@ def test_callback_not_included_if_has_wrong_signature(self): self.socket_mock.recv_string.return_value = f"{Message.PUB_LOW_BATTERY_WARNING}" + # Callback should have only 1 argument def callback(x, y): callback.counter += 1 @@ -71,6 +77,7 @@ def callback(x, y): } ) + # Callback wasn't saved assert client._callback_funcs.get(Message.PUB_LOW_BATTERY_WARNING) is None def test_subscribe_client_cleanup_closes_socket(self):