From ef2adb61b6b46b3a718f80a3bfb64277df93fa47 Mon Sep 17 00:00:00 2001 From: Tim Camise Date: Wed, 10 Jul 2024 13:04:43 -0700 Subject: [PATCH] Fix Python SDK Get All Query Values (#567) * Route all data values if a get all --- .../docs/changelog.rst | 1 + .../open_gopro/gopro_base.py | 4 -- .../open_gopro/gopro_wireless.py | 9 ++-- .../open_gopro/util.py | 8 +++ .../tests/unit/test_wireless_gopro.py | 54 ++++++++++++++++++- 5 files changed, 66 insertions(+), 10 deletions(-) diff --git a/demos/python/sdk_wireless_camera_control/docs/changelog.rst b/demos/python/sdk_wireless_camera_control/docs/changelog.rst index f703b586..c71b6de2 100644 --- a/demos/python/sdk_wireless_camera_control/docs/changelog.rst +++ b/demos/python/sdk_wireless_camera_control/docs/changelog.rst @@ -14,6 +14,7 @@ Unreleased * Add Setting 125 * Don't default to hardcoded parameters for set livestream mode +* Fix routing for Get All Setting / Status commands 0.16.1 (April-23-2024) ---------------------- diff --git a/demos/python/sdk_wireless_camera_control/open_gopro/gopro_base.py b/demos/python/sdk_wireless_camera_control/open_gopro/gopro_base.py index 3ac17141..4e814f4e 100644 --- a/demos/python/sdk_wireless_camera_control/open_gopro/gopro_base.py +++ b/demos/python/sdk_wireless_camera_control/open_gopro/gopro_base.py @@ -394,7 +394,6 @@ async def _get_json( url = self._base_url + message.build_url(**kwargs) logger.debug(f"Sending: {url}") logger.info(Logger.build_log_tx_str(pretty_print(message._as_dict(**kwargs)))) - response: GoProResp | None = None for retry in range(1, GoProBase.HTTP_GET_RETRIES + 1): try: http_response = requests.get(url, timeout=timeout, **self._build_http_request_args(message)) @@ -414,7 +413,6 @@ async def _get_json( else: raise GpException.ResponseTimeout(GoProBase.HTTP_GET_RETRIES) - assert response is not None logger.info(Logger.build_log_rx_str(pretty_print(response._as_dict()))) return response @@ -441,7 +439,6 @@ async def _put_json( url = self._base_url + message.build_url(**kwargs) body = message.build_body(**kwargs) logger.debug(f"Sending: {url} with body: {json.dumps(body, indent=4)}") - response: GoProResp | None = None for retry in range(1, GoProBase.HTTP_GET_RETRIES + 1): try: http_response = requests.put(url, timeout=timeout, json=body, **self._build_http_request_args(message)) @@ -461,5 +458,4 @@ async def _put_json( else: raise GpException.ResponseTimeout(GoProBase.HTTP_GET_RETRIES) - assert response is not None return response diff --git a/demos/python/sdk_wireless_camera_control/open_gopro/gopro_wireless.py b/demos/python/sdk_wireless_camera_control/open_gopro/gopro_wireless.py index 7467a81a..315f9d08 100644 --- a/demos/python/sdk_wireless_camera_control/open_gopro/gopro_wireless.py +++ b/demos/python/sdk_wireless_camera_control/open_gopro/gopro_wireless.py @@ -155,7 +155,7 @@ def __init__( # Responses that we are waiting for. self._sync_resp_wait_q: SnapshotQueue[types.ResponseType] = SnapshotQueue() # Synchronous response that has been parsed and are ready for their sender to receive as the response. - self._sync_resp_ready_q: SnapshotQueue[types.ResponseType] = SnapshotQueue() + self._sync_resp_ready_q: SnapshotQueue[GoProResp] = SnapshotQueue() self._listeners: dict[types.UpdateType, set[types.UpdateCb]] = defaultdict(set) @@ -531,7 +531,6 @@ async def _enforce_message_rules( self, wrapped: Callable, message: Message, rules: MessageRules = MessageRules(), **kwargs: Any ) -> GoProResp: # Acquire ready lock unless we are initializing or this is a Set Shutter Off command - response: GoProResp if self._should_maintain_state and self.is_open and not rules.is_fastpass(**kwargs): logger.trace(f"{wrapped.__name__} acquiring lock") # type: ignore await self._ready_lock.acquire() @@ -641,7 +640,9 @@ async def _route_response(self, response: GoProResp) -> None: response (GoProResp): parsed response to route """ original_response = deepcopy(response) - if response._is_query and not response._is_push: + # We only support queries for either one ID or all ID's. If this is an individual query, extract the value + # for cleaner response data + if response._is_query and not response._is_push and len(response.data) == 1: response.data = list(response.data.values())[0] # Check if this is the awaited synchronous response (id matches). Note! these have to come in order. @@ -722,7 +723,7 @@ async def _send_ble_message( # Wait to be notified that response was received try: - response: GoProResp = await asyncio.wait_for(self._sync_resp_ready_q.get(), WirelessGoPro.WRITE_TIMEOUT) + response = await asyncio.wait_for(self._sync_resp_ready_q.get(), WirelessGoPro.WRITE_TIMEOUT) except queue.Empty as e: logger.error(f"Response timeout of {WirelessGoPro.WRITE_TIMEOUT} seconds!") raise GpException.ResponseTimeout(WirelessGoPro.WRITE_TIMEOUT) from e diff --git a/demos/python/sdk_wireless_camera_control/open_gopro/util.py b/demos/python/sdk_wireless_camera_control/open_gopro/util.py index 1d16bb16..059a4f10 100644 --- a/demos/python/sdk_wireless_camera_control/open_gopro/util.py +++ b/demos/python/sdk_wireless_camera_control/open_gopro/util.py @@ -231,6 +231,14 @@ def __init__(self, maxsize: int = 0) -> None: self._lock = asyncio.Lock() super().__init__(maxsize) + async def get(self) -> T: + """Wrapper for passing generic type through to subclass + + Returns: + T: type of this Snapshot queue + """ + return await super().get() + async def peek_front(self) -> T | None: """Get the first element without dequeueing it diff --git a/demos/python/sdk_wireless_camera_control/tests/unit/test_wireless_gopro.py b/demos/python/sdk_wireless_camera_control/tests/unit/test_wireless_gopro.py index 81f303b8..36230d83 100644 --- a/demos/python/sdk_wireless_camera_control/tests/unit/test_wireless_gopro.py +++ b/demos/python/sdk_wireless_camera_control/tests/unit/test_wireless_gopro.py @@ -14,10 +14,10 @@ import requests_mock from open_gopro.communicator_interface import HttpMessage -from open_gopro.constants import SettingId, StatusId +from open_gopro.constants import ErrorCode, QueryCmdId, SettingId, StatusId from open_gopro.exceptions import GoProNotOpened, ResponseTimeout from open_gopro.gopro_wireless import Params, WirelessGoPro, types -from open_gopro.models.response import GlobalParsers +from open_gopro.models.response import GlobalParsers, GoProResp from tests import mock_good_response @@ -145,6 +145,56 @@ async def receive_encoding_status(id: types.UpdateType, value: bool): await asyncio.wait_for(event.wait(), 1) +@pytest.mark.asyncio +async def test_route_all_data(mock_wireless_gopro_basic: WirelessGoPro): + mock_wireless_gopro_basic._loop = asyncio.get_running_loop() + + # GIVEN + mock_data = {"one": 1, "two": 2} + mock_response = GoProResp( + protocol=GoProResp.Protocol.BLE, + status=ErrorCode.SUCCESS, + data=mock_data, + identifier=QueryCmdId.GET_SETTING_VAL, + ) + + # WHEN + # Make it appear to be the synchronous response + await mock_wireless_gopro_basic._sync_resp_wait_q.put(mock_response) + # Route the mock response + await mock_wireless_gopro_basic._route_response(mock_response) + # Get the routed response + routed_response = await mock_wireless_gopro_basic._sync_resp_ready_q.get() + + # THEN + assert routed_response.data == mock_data + + +@pytest.mark.asyncio +async def test_route_individual_data(mock_wireless_gopro_basic: WirelessGoPro): + mock_wireless_gopro_basic._loop = asyncio.get_running_loop() + + # GIVEN + mock_data = {"one": 1} + mock_response = GoProResp( + protocol=GoProResp.Protocol.BLE, + status=ErrorCode.SUCCESS, + data=mock_data, + identifier=QueryCmdId.GET_SETTING_VAL, + ) + + # WHEN + # Make it appear to be the synchronous response + await mock_wireless_gopro_basic._sync_resp_wait_q.put(mock_response) + # Route the mock response + await mock_wireless_gopro_basic._route_response(mock_response) + # Get the routed response + routed_response = await mock_wireless_gopro_basic._sync_resp_ready_q.get() + + # THEN + assert routed_response.data == 1 + + @pytest.mark.asyncio async def test_get_update_unregister_all(mock_wireless_gopro_basic: WirelessGoPro): event = asyncio.Event()