From 8a79e7a9554921f2bd780e39f05b14f2447fa47e Mon Sep 17 00:00:00 2001 From: Tim Camise Date: Mon, 18 Sep 2023 11:54:30 -0700 Subject: [PATCH 1/6] Fix ble notifications --- .../docs/changelog.rst | 2 +- .../sdk_wireless_camera_control/noxfile.py | 2 +- .../open_gopro/demos/log_battery.py | 73 +++++++++---------- .../open_gopro/gopro_wireless.py | 11 ++- .../open_gopro/models/response.py | 42 +++++++---- .../pyproject.toml | 2 +- .../tests/conftest.py | 2 + .../tests/unit/test_responses.py | 6 +- .../tests/unit/test_wireless_gopro.py | 4 +- 9 files changed, 84 insertions(+), 60 deletions(-) diff --git a/demos/python/sdk_wireless_camera_control/docs/changelog.rst b/demos/python/sdk_wireless_camera_control/docs/changelog.rst index 3c61987e..3929587f 100644 --- a/demos/python/sdk_wireless_camera_control/docs/changelog.rst +++ b/demos/python/sdk_wireless_camera_control/docs/changelog.rst @@ -10,7 +10,7 @@ The format is based on `Keep a Changelog ` and this project adheres to `Semantic Versioning `_. 0.14.0 (September-13-2022) -------------------------- +-------------------------- * NOTE! This is a major update and includes massive API breaking changes. * Move to asyncio-based framework * Add HERO 12 support diff --git a/demos/python/sdk_wireless_camera_control/noxfile.py b/demos/python/sdk_wireless_camera_control/noxfile.py index 73b1e430..0b8bc436 100644 --- a/demos/python/sdk_wireless_camera_control/noxfile.py +++ b/demos/python/sdk_wireless_camera_control/noxfile.py @@ -78,6 +78,6 @@ def docs(session) -> None: "autodoc-pydantic", "darglint", ) - session.run("sphinx-build", "docs", "docs/build") + session.run("sphinx-build", "-W", "docs", "docs/build") # Clean up for Jekyll consumption session.run("rm", "-rf", "docs/build/.doctrees", "/docs/build/_sources", "/docs/build/_static/fonts", external=True) diff --git a/demos/python/sdk_wireless_camera_control/open_gopro/demos/log_battery.py b/demos/python/sdk_wireless_camera_control/open_gopro/demos/log_battery.py index 78cb29fa..cf3117d9 100644 --- a/demos/python/sdk_wireless_camera_control/open_gopro/demos/log_battery.py +++ b/demos/python/sdk_wireless_camera_control/open_gopro/demos/log_battery.py @@ -17,7 +17,7 @@ from open_gopro import WirelessGoPro, types from open_gopro.constants import StatusId from open_gopro.logger import set_stream_logging_level, setup_logging -from open_gopro.util import add_cli_args_and_parse +from open_gopro.util import add_cli_args_and_parse, ainput console = Console() @@ -84,56 +84,55 @@ async def process_battery_notifications(update: types.UpdateType, value: int) -> async def main(args: argparse.Namespace) -> None: logger = setup_logging(__name__, args.log) - global SAMPLE_INDEX gopro: Optional[WirelessGoPro] = None try: async with WirelessGoPro(args.identifier, enable_wifi=False) as gopro: set_stream_logging_level(logging.ERROR) - if args.poll: - with console.status("[bold green]Polling the battery until it dies..."): - while True: - SAMPLES.append( - Sample( - index=SAMPLE_INDEX, - percentage=(await gopro.ble_status.int_batt_per.get_value()).data, - bars=(await gopro.ble_status.batt_level.get_value()).data, + async def log_battery() -> None: + global SAMPLE_INDEX + if args.poll: + with console.status("[bold green]Polling the battery until it dies..."): + while True: + SAMPLES.append( + Sample( + index=SAMPLE_INDEX, + percentage=(await gopro.ble_status.int_batt_per.get_value()).data, + bars=(await gopro.ble_status.batt_level.get_value()).data, + ) ) - ) - console.print(str(SAMPLES[-1])) - SAMPLE_INDEX += 1 - await asyncio.sleep(args.poll) - # Otherwise set up notifications - else: - global last_bars - global last_percentage - - console.print("Configuring battery notifications...") - # Enable notifications of the relevant battery statuses. Also store initial values. - last_bars = ( - await gopro.ble_status.batt_level.register_value_update(process_battery_notifications) - ).data - last_percentage = ( - await gopro.ble_status.int_batt_per.register_value_update(process_battery_notifications) - ).data - # Append initial sample - SAMPLES.append(Sample(index=SAMPLE_INDEX, percentage=last_percentage, bars=last_bars)) - console.print(str(SAMPLES[-1])) - - # Start a thread to handle asynchronous battery level notifications - console.print("[bold green]Receiving battery notifications until it dies...") - while True: - await asyncio.sleep(1) + console.print(str(SAMPLES[-1])) + SAMPLE_INDEX += 1 + await asyncio.sleep(args.poll) + else: # Not polling. Set up notifications + global last_bars + global last_percentage + + console.print("Configuring battery notifications...") + # Enable notifications of the relevant battery statuses. Also store initial values. + last_bars = ( + await gopro.ble_status.batt_level.register_value_update(process_battery_notifications) + ).data + last_percentage = ( + await gopro.ble_status.int_batt_per.register_value_update(process_battery_notifications) + ).data + # Append initial sample + SAMPLES.append(Sample(index=SAMPLE_INDEX, percentage=last_percentage, bars=last_bars)) + console.print(str(SAMPLES[-1])) + console.print("[bold green]Receiving battery notifications until it dies...") + + asyncio.create_task(log_battery()) + await ainput("[purple]Press enter to exit.", console.print) + console.print("Exiting...") except KeyboardInterrupt: logger.warning("Received keyboard interrupt. Shutting down...") - if len(SAMPLES) > 0: + if SAMPLES: csv_location = Path(args.log.parent) / "battery_results.csv" dump_results_as_csv(csv_location) if gopro: await gopro.close() - console.print("Exiting...") def parse_arguments() -> argparse.Namespace: 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 2ec35932..a10c8dfc 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 @@ -27,7 +27,7 @@ ) from open_gopro.ble import BleakWrapperController, BleUUID from open_gopro.communicator_interface import GoProWirelessInterface, MessageRules -from open_gopro.constants import ActionId, GoProUUIDs, QueryCmdId, SettingId, StatusId +from open_gopro.constants import ActionId, GoProUUIDs, StatusId from open_gopro.gopro_base import GoProBase, GoProMessageInterface, MessageMethodType from open_gopro.logger import Logger from open_gopro.models.response import BleRespBuilder, GoProResp @@ -538,12 +538,17 @@ async def _update_internal_state(self, update: types.UpdateType, value: int) -> logger.trace("Control setting encoded started") # type: ignore self._encoding_started.set() + # TODO this needs unit testing async def _route_response(self, response: GoProResp) -> None: """After parsing response, route it to any stakeholders (such as registered listeners) Args: response (GoProResp): parsed response """ + # Flatten data if possible + if response._is_query and not response._is_push: + response.data = list(response.data.values())[0] + # Check if this is the awaited synchronous response (id matches). Note! these have to come in order. response_claimed = False if await self._sync_resp_wait_q.peek_front() == response.identifier: @@ -554,10 +559,10 @@ async def _route_response(self, response: GoProResp) -> None: # If this wasn't the awaited synchronous response... if not response_claimed: logger.info(Logger.build_log_rx_str(response, asynchronous=True)) - if isinstance(response.identifier, QueryCmdId): + if response._is_push: for update_id, value in response.data.items(): await self._notify_listeners(update_id, value) - elif isinstance(response.identifier, (StatusId, SettingId, ActionId)): + elif isinstance(response.identifier, ActionId): await self._notify_listeners(response.identifier, response.data) def _notification_handler(self, handle: int, data: bytearray) -> None: diff --git a/demos/python/sdk_wireless_camera_control/open_gopro/models/response.py b/demos/python/sdk_wireless_camera_control/open_gopro/models/response.py index df15512e..748a1a91 100644 --- a/demos/python/sdk_wireless_camera_control/open_gopro/models/response.py +++ b/demos/python/sdk_wireless_camera_control/open_gopro/models/response.py @@ -143,6 +143,28 @@ def ok(self) -> bool: """ return self.status in [ErrorCode.SUCCESS, ErrorCode.UNKNOWN] + @property + def _is_push(self) -> bool: + """Was this response an asynchronous push? + + Returns: + bool: True if yes, False otherwise + """ + return self.identifier in [ + QueryCmdId.STATUS_VAL_PUSH, + QueryCmdId.SETTING_VAL_PUSH, + QueryCmdId.SETTING_CAPABILITY_PUSH, + ] + + @property + def _is_query(self) -> bool: + """Is this response to a settings / status query? + + Returns: + bool: True if yes, False otherwise + """ + return isinstance(self.identifier, QueryCmdId) + class RespBuilder(ABC, Generic[T]): """Common Response Builder Interface""" @@ -437,12 +459,6 @@ def build(self) -> GoProResp: # Query (setting get value, status get value, etc.) if query_type: - is_multiple = self._identifier in [ - QueryCmdId.GET_CAPABILITIES_VAL, - QueryCmdId.REG_CAPABILITIES_UPDATE, - QueryCmdId.SETTING_CAPABILITY_PUSH, - ] - camera_state: types.CameraState = defaultdict(list) self._status = ErrorCode(buf[0]) buf = buf[1:] @@ -467,7 +483,11 @@ def build(self) -> GoProResp: camera_state[param_id] = param_val continue # These can be more than 1 value so use a list - if is_multiple: + if self._identifier in [ + QueryCmdId.GET_CAPABILITIES_VAL, + QueryCmdId.REG_CAPABILITIES_UPDATE, + QueryCmdId.SETTING_CAPABILITY_PUSH, + ]: # Parse using parser from global map and append camera_state[param_id].append(parser.parse(param_val)) else: @@ -479,12 +499,8 @@ def build(self) -> GoProResp: # isn't functionally critical logger.warning(f"{param_id} does not contain a value {param_val}") camera_state[param_id] = param_val - # Flatten if not multiple - if is_multiple: - self._identifier = list(camera_state.keys())[0] - parsed = list(camera_state.values())[0] - else: - parsed = camera_state + parsed = camera_state + else: # Commands, Protobuf, and direct Reads if is_cmd := isinstance(self._identifier, CmdId): # All (non-protobuf) commands have a status diff --git a/demos/python/sdk_wireless_camera_control/pyproject.toml b/demos/python/sdk_wireless_camera_control/pyproject.toml index 9c682ce8..ef4462f0 100644 --- a/demos/python/sdk_wireless_camera_control/pyproject.toml +++ b/demos/python/sdk_wireless_camera_control/pyproject.toml @@ -183,7 +183,7 @@ log_file_level = "DEBUG" log_file_format = "%(threadName)13s: %(name)40s:%(lineno)5d %(asctime)s.%(msecs)03d %(levelname)-8s | %(message)s" log_file_date_format = "%H:%M:%S" filterwarnings = "ignore::DeprecationWarning" -# timeout = 10 +timeout = 10 addopts = [ "-s", "--capture=tee-sys", diff --git a/demos/python/sdk_wireless_camera_control/tests/conftest.py b/demos/python/sdk_wireless_camera_control/tests/conftest.py index 75618167..10738c98 100644 --- a/demos/python/sdk_wireless_camera_control/tests/conftest.py +++ b/demos/python/sdk_wireless_camera_control/tests/conftest.py @@ -40,6 +40,7 @@ from open_gopro.communicator_interface import GoProBle, GoProWifi from open_gopro.constants import CmdId, ErrorCode, GoProUUIDs, StatusId from open_gopro.exceptions import ConnectFailed, FailedToFindDevice +from open_gopro.gopro_base import GoProBase from open_gopro.logger import set_logging_level, setup_logging from open_gopro.models.response import GoProResp from open_gopro.wifi import SsidState, WifiClient, WifiController @@ -444,6 +445,7 @@ def close(self) -> None: @pytest.fixture(params=versions) async def mock_wireless_gopro_basic(request): test_client = MockWirelessGoPro(request.param) + GoProBase.HTTP_GET_RETRIES = 1 yield test_client test_client.close() diff --git a/demos/python/sdk_wireless_camera_control/tests/unit/test_responses.py b/demos/python/sdk_wireless_camera_control/tests/unit/test_responses.py index f76b5259..d9920de8 100644 --- a/demos/python/sdk_wireless_camera_control/tests/unit/test_responses.py +++ b/demos/python/sdk_wireless_camera_control/tests/unit/test_responses.py @@ -35,8 +35,10 @@ def test_push_response_no_parameter_values(): assert builder.is_finished_accumulating r = builder.build() assert r.ok - assert r.identifier == SettingId.RESOLUTION - assert r.data == [] + assert r.identifier == QueryCmdId.SETTING_CAPABILITY_PUSH + assert r.data[SettingId.RESOLUTION] == [] + assert r.data[SettingId.FPS] == [] + assert r.data[SettingId.VIDEO_FOV] == [] test_read_receive = bytearray([0x64, 0x62, 0x32, 0x2D, 0x73, 0x58, 0x56, 0x2D, 0x66, 0x62, 0x38]) 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 5b04e2e2..b56269fc 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 @@ -131,7 +131,7 @@ async def receive_encoding_status(id: types.UpdateType, value: bool): event.set() mock_wireless_gopro_basic.register_update(receive_encoding_status, StatusId.ENCODING) - not_encoding = bytearray([0x05, 0x13, 0x00, StatusId.ENCODING.value, 0x01, 0x00]) + not_encoding = bytearray([0x05, 0x93, 0x00, StatusId.ENCODING.value, 0x01, 0x00]) mock_wireless_gopro_basic._notification_handler(0xFF, not_encoding) await event.wait() @@ -153,7 +153,7 @@ async def receive_encoding_status(id: types.UpdateType, value: bool): event.set() mock_wireless_gopro_basic.register_update(receive_encoding_status, StatusId.ENCODING) - not_encoding = bytearray([0x05, 0x13, 0x00, StatusId.ENCODING.value, 0x01, 0x00]) + not_encoding = bytearray([0x05, 0x93, 0x00, StatusId.ENCODING.value, 0x01, 0x00]) mock_wireless_gopro_basic._notification_handler(0xFF, not_encoding) await event.wait() From c4b17fb6ae05fb711bac2a0c6b966f39aeb215f5 Mon Sep 17 00:00:00 2001 From: Tim Camise Date: Mon, 18 Sep 2023 12:14:02 -0700 Subject: [PATCH 2/6] Fix nox build --- .github/workflows/python_sdk_test.yml | 11 ++++++++--- .../sdk_wireless_camera_control/docs/changelog.rst | 4 ++++ demos/python/sdk_wireless_camera_control/noxfile.py | 3 ++- .../python/sdk_wireless_camera_control/pyproject.toml | 2 +- 4 files changed, 15 insertions(+), 5 deletions(-) diff --git a/.github/workflows/python_sdk_test.yml b/.github/workflows/python_sdk_test.yml index d462f59f..3d1903cb 100644 --- a/.github/workflows/python_sdk_test.yml +++ b/.github/workflows/python_sdk_test.yml @@ -17,7 +17,7 @@ jobs: strategy: matrix: os: [windows-latest, macos-latest, ubuntu-latest] - python-version: ["3.9", "3.10", "3.11"] + python-version: ['3.9', '3.10', '3.11'] include: - os: ubuntu-latest path: ~/.cache/pip @@ -41,8 +41,8 @@ jobs: key: ${{ runner.os }}-${{ matrix.python-version }}-pip-${{ hashFiles('demos/python/sdk_wireless_camera_control/poetry.lock') }} restore-keys: ${{ runner.os }}-pip- - - name: Install Bluez on Ubuntu - run: sudo apt-get update && sudo apt install -y bluez + - name: Install prereqs on Ubuntu + run: sudo apt-get update && sudo apt install -y bluez graphviz if: ${{ matrix.os == 'ubuntu-latest'}} - name: Install dependencies @@ -57,6 +57,11 @@ jobs: working-directory: ./demos/python/sdk_wireless_camera_control/ run: nox -p ${{ matrix.python-version }} + # Only test docs on ubuntu since we need graphviz + - name: Test docs build + if: ${{ matrix.os == 'ubuntu-latest'}} + run: nox -s docs + - name: Archive test report on failure uses: actions/upload-artifact@v2 if: failure() diff --git a/demos/python/sdk_wireless_camera_control/docs/changelog.rst b/demos/python/sdk_wireless_camera_control/docs/changelog.rst index 3929587f..d34046d0 100644 --- a/demos/python/sdk_wireless_camera_control/docs/changelog.rst +++ b/demos/python/sdk_wireless_camera_control/docs/changelog.rst @@ -9,6 +9,10 @@ All notable changes to this project will be documented in this file. The format is based on `Keep a Changelog `_, and this project adheres to `Semantic Versioning `_. +Unreleased +---------- +* Fix BLE notifications not being routed correctly + 0.14.0 (September-13-2022) -------------------------- * NOTE! This is a major update and includes massive API breaking changes. diff --git a/demos/python/sdk_wireless_camera_control/noxfile.py b/demos/python/sdk_wireless_camera_control/noxfile.py index 0b8bc436..4c9bb5e1 100644 --- a/demos/python/sdk_wireless_camera_control/noxfile.py +++ b/demos/python/sdk_wireless_camera_control/noxfile.py @@ -6,7 +6,8 @@ import nox from nox_poetry import session -nox.options.sessions = "format", "lint", "tests", "docstrings", "docs" +# Don't run docs by default since it needs graphviz. +nox.options.sessions = "format", "lint", "tests", "docstrings" SUPPORTED_VERSIONS = [ "3.9", diff --git a/demos/python/sdk_wireless_camera_control/pyproject.toml b/demos/python/sdk_wireless_camera_control/pyproject.toml index ef4462f0..dfc1b184 100644 --- a/demos/python/sdk_wireless_camera_control/pyproject.toml +++ b/demos/python/sdk_wireless_camera_control/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "open_gopro" -version = "0.14.0" +version = "0.14.1" description = "Open GoPro API and Examples" authors = ["Tim Camise "] readme = "README.md" From 59488dbb696ebb2afa96b6ad62d75d98df338416 Mon Sep 17 00:00:00 2001 From: Tim Camise Date: Mon, 18 Sep 2023 12:19:04 -0700 Subject: [PATCH 3/6] Fix github actions python sdk test --- .github/workflows/python_sdk_test.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/python_sdk_test.yml b/.github/workflows/python_sdk_test.yml index 3d1903cb..6daebe42 100644 --- a/.github/workflows/python_sdk_test.yml +++ b/.github/workflows/python_sdk_test.yml @@ -57,9 +57,9 @@ jobs: working-directory: ./demos/python/sdk_wireless_camera_control/ run: nox -p ${{ matrix.python-version }} - # Only test docs on ubuntu since we need graphviz + # Only test docs with latest Python on ubuntu since we need graphviz - name: Test docs build - if: ${{ matrix.os == 'ubuntu-latest'}} + if: ${{ matrix.os == 'ubuntu-latest'}} && ${{ matrix.python-version == '3.11' }} run: nox -s docs - name: Archive test report on failure From 6e7fa324bd10a8ec8780dd78a7efc769435c8698 Mon Sep 17 00:00:00 2001 From: Tim Camise Date: Mon, 18 Sep 2023 12:21:33 -0700 Subject: [PATCH 4/6] Set working directory --- .github/workflows/python_sdk_test.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/python_sdk_test.yml b/.github/workflows/python_sdk_test.yml index 6daebe42..fc54eadf 100644 --- a/.github/workflows/python_sdk_test.yml +++ b/.github/workflows/python_sdk_test.yml @@ -59,7 +59,8 @@ jobs: # Only test docs with latest Python on ubuntu since we need graphviz - name: Test docs build - if: ${{ matrix.os == 'ubuntu-latest'}} && ${{ matrix.python-version == '3.11' }} + if: ${{ matrix.os == 'ubuntu-latest'}} + working-directory: ./demos/python/sdk_wireless_camera_control/ run: nox -s docs - name: Archive test report on failure From 548db03cf40354261666ea740b24908f8da7c8a0 Mon Sep 17 00:00:00 2001 From: Tim Camise Date: Mon, 18 Sep 2023 12:24:42 -0700 Subject: [PATCH 5/6] Only run docs on 3.11 --- .github/workflows/python_sdk_test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/python_sdk_test.yml b/.github/workflows/python_sdk_test.yml index fc54eadf..758095d4 100644 --- a/.github/workflows/python_sdk_test.yml +++ b/.github/workflows/python_sdk_test.yml @@ -59,7 +59,7 @@ jobs: # Only test docs with latest Python on ubuntu since we need graphviz - name: Test docs build - if: ${{ matrix.os == 'ubuntu-latest'}} + if: ${{ matrix.os == 'ubuntu-latest'}} && ${{ matrix.python-version == '3.11' }} working-directory: ./demos/python/sdk_wireless_camera_control/ run: nox -s docs From 37803d5920ff54ff1ce4d8719afa96bdaf795a9d Mon Sep 17 00:00:00 2001 From: Tim Camise Date: Mon, 18 Sep 2023 12:27:18 -0700 Subject: [PATCH 6/6] try again --- .github/workflows/python_sdk_test.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/python_sdk_test.yml b/.github/workflows/python_sdk_test.yml index 758095d4..f81e4d2b 100644 --- a/.github/workflows/python_sdk_test.yml +++ b/.github/workflows/python_sdk_test.yml @@ -53,13 +53,13 @@ jobs: pip install nox-poetry==1.0.3 pip install poetry - - name: Perform checks (format, types, lint, docstrings, unit tests, docs, safety) + - name: Perform checks (format, types, lint, docstrings, unit tests) working-directory: ./demos/python/sdk_wireless_camera_control/ run: nox -p ${{ matrix.python-version }} # Only test docs with latest Python on ubuntu since we need graphviz - name: Test docs build - if: ${{ matrix.os == 'ubuntu-latest'}} && ${{ matrix.python-version == '3.11' }} + if: matrix.os == 'ubuntu-latest' && matrix.python-version == '3.11' working-directory: ./demos/python/sdk_wireless_camera_control/ run: nox -s docs