Skip to content

Commit 470888d

Browse files
authored
Python SDK: Make Cohn feature backwards compatible (#757)
This PR updates the SDK to support legacy COHN protobuf flows by introducing a unified `ProtobufId` identifier, adding support checks via a `require_supported` decorator, and enhancing the response parser and routing to handle unsupported protobuf responses gracefully. - Introduce `ProtobufId` type and update all parsers/commands to use it - Add `require_supported` decorator and `is_supported` checks for features - Extend `BleRespBuilder` and `_route_response` to identify and route unsupported protobuf errors - Update tests and mocks to cover new unsupported-CoHN behavior - Bump third-party dependency versions and enable debug logging for response parsing <details> <summary>Show a summary per file</summary> | File | Description | | ------------------------------------------------------------------------------- | --------------------------------------------------------------- | | demos/python/sdk_wireless_camera_control/thirdPartyDependencies.csv | Bump `packaging` to 25.0 and `protobuf` to 6.31.0 | | demos/python/sdk_wireless_camera_control/tests/unit/test_wireless_gopro.py | Add unsupported protobuf operation test; update imports | | demos/python/sdk_wireless_camera_control/tests/unit/test_responses.py | Add and adjust tests for unsupported protobuf responses | | demos/python/sdk_wireless_camera_control/tests/unit/test_ble_commands.py | Import new `ErrorCode` and `FeatureId` for BLE command tests | | demos/python/sdk_wireless_camera_control/tests/mocks.py | Extend mocks for protobuf and remove legacy BLE send override | | demos/python/sdk_wireless_camera_control/open_gopro/util/logger.py | Enable DEBUG logging for `open_gopro.parsers.response` | | demos/python/sdk_wireless_camera_control/open_gopro/parsers/response.py | Add `validResponseProtobufIds` and handle missing action IDs | | demos/python/sdk_wireless_camera_control/open_gopro/models/types.py | Introduce `ProtobufId` and update `ResponseType` | | demos/python/sdk_wireless_camera_control/open_gopro/models/constants/constants.py | Remove deprecated `PROTOBUF_QUERY` enum | | demos/python/sdk_wireless_camera_control/open_gopro/gopro_wireless.py | Enhance `_route_response` to match unsupported protobuf errors | | demos/python/sdk_wireless_camera_control/open_gopro/features/base_feature.py | Implement `require_supported` decorator and `is_supported` abstract property | | demos/python/sdk_wireless_camera_control/open_gopro/features/cohn_feature.py | Add support tracking and guard methods with `require_supported` | | demos/python/sdk_wireless_camera_control/open_gopro/features/streaming/stream_feature.py | Add `is_supported` property | | demos/python/sdk_wireless_camera_control/open_gopro/features/access_point_feature.py | Add `is_supported` property | | demos/python/sdk_wireless_camera_control/open_gopro/api/builders.py | Update `BleProtoCommand` to use `ProtobufId` | | demos/python/sdk_wireless_camera_control/open_gopro/api/ble_commands.py | Update protobuf commands to use `ProtobufId` in `additional_matching_ids` | </details>
1 parent a78269b commit 470888d

23 files changed

+599
-312
lines changed

demos/python/sdk_wireless_camera_control/open_gopro/api/ble_commands.py

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@
4949
SettingId,
5050
StatusId,
5151
)
52-
from open_gopro.models.types import CameraState
52+
from open_gopro.models.types import CameraState, ProtobufId
5353
from open_gopro.parsers.bytes import (
5454
ConstructByteParserBuilder,
5555
DateTimeByteParserBuilder,
@@ -306,9 +306,7 @@ async def get_date_time_tz_dst(self) -> GoProResp[TzDstDateTime]:
306306
@ble_write_command(
307307
uuid=GoProUUID.CQ_COMMAND,
308308
cmd=CmdId.REBOOT,
309-
rules=MessageRules(
310-
fastpass_analyzer=lambda **_: True,
311-
),
309+
rules=MessageRules(fastpass_analyzer=lambda **_: True),
312310
)
313311
async def reboot(self) -> GoProResp[None]:
314312
"""Reboot the camera (approximating a battery pull)
@@ -430,7 +428,7 @@ async def set_turbo_mode(self, *, mode: constants.Toggle) -> GoProResp[None]:
430428
response_action_id=ActionId.GET_PRESET_STATUS_RSP,
431429
request_proto=proto.RequestGetPresetStatus,
432430
response_proto=proto.NotifyPresetStatus,
433-
additional_matching_ids={ActionId.PRESET_MODIFIED_NOTIFICATION},
431+
additional_matching_ids={ProtobufId(FeatureId.QUERY, ActionId.PRESET_MODIFIED_NOTIFICATION)},
434432
)
435433
async def get_preset_status(
436434
self,
@@ -559,7 +557,7 @@ async def get_ap_entries(
559557
response_action_id=ActionId.REQUEST_WIFI_CONNECT_RSP,
560558
request_proto=proto.RequestConnect,
561559
response_proto=proto.ResponseConnect,
562-
additional_matching_ids={ActionId.REQUEST_WIFI_CONNECT_RSP},
560+
additional_matching_ids={ProtobufId(FeatureId.NETWORK_MANAGEMENT, ActionId.REQUEST_WIFI_CONNECT_RSP)},
563561
)
564562
async def request_wifi_connect(self, *, ssid: str) -> GoProResp[proto.ResponseConnect]:
565563
"""Request the camera to connect to a WiFi network that is already provisioned.
@@ -580,7 +578,7 @@ async def request_wifi_connect(self, *, ssid: str) -> GoProResp[proto.ResponseCo
580578
response_action_id=ActionId.REQUEST_WIFI_CONNECT_NEW_RSP,
581579
request_proto=proto.RequestConnectNew,
582580
response_proto=proto.ResponseConnectNew,
583-
additional_matching_ids={ActionId.REQUEST_WIFI_CONNECT_NEW_RSP},
581+
additional_matching_ids={ProtobufId(FeatureId.NETWORK_MANAGEMENT, ActionId.REQUEST_WIFI_CONNECT_NEW_RSP)},
584582
)
585583
async def request_wifi_connect_new(self, *, ssid: str, password: str) -> GoProResp[proto.ResponseConnectNew]:
586584
"""Request the camera to connect to a WiFi network that is not already provisioned.
@@ -666,7 +664,7 @@ async def set_livestream_mode(
666664
response_action_id=ActionId.LIVESTREAM_STATUS_RSP,
667665
request_proto=proto.RequestGetLiveStreamStatus,
668666
response_proto=proto.NotifyLiveStreamStatus,
669-
additional_matching_ids={ActionId.LIVESTREAM_STATUS_NOTIF},
667+
additional_matching_ids={ProtobufId(FeatureId.QUERY, ActionId.LIVESTREAM_STATUS_NOTIF)},
670668
)
671669
async def register_livestream_status(
672670
self,
@@ -694,6 +692,7 @@ async def register_livestream_status(
694692
response_action_id=ActionId.RELEASE_NETWORK_RSP,
695693
request_proto=proto.RequestReleaseNetwork,
696694
response_proto=proto.ResponseGeneric,
695+
rules=MessageRules(fastpass_analyzer=lambda **_: True),
697696
)
698697
async def release_network(self) -> GoProResp[None]:
699698
"""Disconnect the camera Wifi network in STA mode so that it returns to AP mode.
@@ -819,5 +818,5 @@ class BleAsyncResponses:
819818
def add_parsers(cls) -> None:
820819
"""Add all of the defined asynchronous responses to the global parser map"""
821820
for response in cls.responses:
822-
GlobalParsers.add(response.action_id, response.parser)
821+
GlobalParsers.add(ProtobufId(response.feature_id, response.action_id), response.parser)
823822
GlobalParsers.add_feature_action_id_mapping(response.feature_id, response.action_id)

demos/python/sdk_wireless_camera_control/open_gopro/api/builders.py

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@
4444
SettingId,
4545
StatusId,
4646
)
47-
from open_gopro.models.types import CameraState, JsonDict, Protobuf
47+
from open_gopro.models.types import CameraState, JsonDict, Protobuf, ProtobufId
4848
from open_gopro.network.ble import BleUUID
4949
from open_gopro.parsers.bytes import (
5050
ConstructByteParserBuilder,
@@ -193,7 +193,7 @@ class BleProtoCommand(BleMessage):
193193
request_proto (type[Protobuf]): the action ID that will be in the response
194194
response_proto (type[Protobuf]): protobuf used to parse received bytestream
195195
parser (Parser | None): Optional response parser. Defaults to None.
196-
additional_matching_ids (set[ActionId | CmdId] | None): Other action ID's to share this parser. This is used, for
196+
additional_matching_ids (set[ProtobufId | CmdId] | None): Other action ID's to share this parser. This is used, for
197197
example, if a notification shares the same ID as the synchronous response. Defaults to None. Defaults to None.
198198
"""
199199

@@ -206,19 +206,19 @@ def __init__(
206206
request_proto: type[Protobuf],
207207
response_proto: type[Protobuf],
208208
parser: Parser | None,
209-
additional_matching_ids: set[ActionId | CmdId] | None = None,
209+
additional_matching_ids: set[ProtobufId | CmdId] | None = None,
210210
) -> None:
211211
p = parser or Parser()
212212
p.byte_json_adapter = ProtobufByteParser(response_proto)
213-
super().__init__(uuid=uuid, parser=p, identifier=response_action_id)
213+
super().__init__(uuid=uuid, parser=p, identifier=ProtobufId(feature_id, response_action_id))
214214
self.feature_id = feature_id
215215
self.action_id = action_id
216216
self.response_action_id = response_action_id
217217
self.request_proto = request_proto
218218
self.response_proto = response_proto
219-
self.additional_matching_ids: set[ActionId | CmdId] = additional_matching_ids or set()
219+
self.additional_matching_ids: set[ProtobufId | CmdId] = additional_matching_ids or set()
220220
assert self._parser
221-
for matching_id in [*self.additional_matching_ids, response_action_id]:
221+
for matching_id in [*self.additional_matching_ids, ProtobufId(feature_id, response_action_id)]:
222222
GlobalParsers.add(matching_id, self._parser)
223223
GlobalParsers.add_feature_action_id_mapping(self.feature_id, self.response_action_id)
224224

@@ -366,7 +366,8 @@ def ble_proto_command(
366366
request_proto: type[Protobuf],
367367
response_proto: type[Protobuf],
368368
parser: Parser | None = None,
369-
additional_matching_ids: set[ActionId | CmdId] | None = None,
369+
additional_matching_ids: set[ProtobufId | CmdId] | None = None,
370+
rules: MessageRules = MessageRules(),
370371
) -> Callable:
371372
"""Decorator to build a BLE Protobuf command and wrapper to execute it
372373
@@ -378,8 +379,9 @@ def ble_proto_command(
378379
request_proto (type[Protobuf]): the action ID that will be in the response
379380
response_proto (type[Protobuf]): protobuf used to parse received bytestream
380381
parser (Parser | None): Response parser to transform received Protobuf bytes. Defaults to None.
381-
additional_matching_ids (set[ActionId | CmdId] | None): Other action ID's to share this parser. This is used,
382+
additional_matching_ids (set[ProtobufId | CmdId] | None): Other action ID's to share this parser. This is used,
382383
for example, if a notification shares the same ID as the synchronous response. Defaults to None.
384+
rules (MessageRules): rules that describe sending / receiving the message. Defaults to MessageRules() (no rules).
383385
384386
Returns:
385387
Callable: Generated method to perform command
@@ -397,7 +399,7 @@ def ble_proto_command(
397399

398400
@wrapt.decorator
399401
async def wrapper(wrapped: Callable, instance: BleMessages, _: Any, kwargs: Any) -> GoProResp:
400-
return await instance._communicator._send_ble_message(message, **(await wrapped(**kwargs) or kwargs))
402+
return await instance._communicator._send_ble_message(message, rules, **(await wrapped(**kwargs) or kwargs))
401403

402404
return wrapper
403405

@@ -888,6 +890,7 @@ def build_url(self, **kwargs: Any) -> str:
888890
Returns:
889891
str: built URL
890892
"""
893+
assert not isinstance(self._identifier, ProtobufId) # needed to satisfy typing
891894
return self._endpoint.format(setting=int(self._identifier), option=int(kwargs["value"]))
892895

893896
async def set(self, value: T) -> GoProResp:

demos/python/sdk_wireless_camera_control/open_gopro/demos/gui/livestream.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,15 +42,19 @@ async def main(args: argparse.Namespace) -> None:
4242
)
4343
assert gopro.streaming.url
4444
console.print(
45-
f"[yellow]Livestream to {gopro.streaming.url} is now streaming and should be available for viewing at."
45+
f"[yellow]Livestream to {gopro.streaming.url} is now streaming and should be available for viewing."
4646
)
4747
await ainput("Press enter to stop livestreaming...\n")
4848

49+
console.print("[yellow]Stopping livestream...")
4950
await gopro.streaming.stop_active_stream()
5051

5152
# TODO merge this into access point feature
53+
console.print("[yellow]Disconnecting GoPro from network...")
5254
await gopro.ble_command.release_network()
5355

56+
console.print(" [yellow]Exiting...")
57+
5458

5559
def parse_arguments() -> argparse.Namespace:
5660
parser = argparse.ArgumentParser(

demos/python/sdk_wireless_camera_control/open_gopro/features/access_point_feature.py

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
from open_gopro.features.base_feature import BaseFeature
1515
from open_gopro.models import proto
1616
from open_gopro.models.constants import ActionId
17+
from open_gopro.models.constants.constants import FeatureId
1718
from open_gopro.models.proto import (
1819
EnumProvisioning,
1920
EnumResultGeneric,
@@ -25,6 +26,7 @@
2526
ResponseConnectNew,
2627
ResponseStartScanning,
2728
)
29+
from open_gopro.models.types import ProtobufId
2830

2931
logger = logging.getLogger(__name__)
3032

@@ -33,13 +35,10 @@ class AccessPointFeature(BaseFeature):
3335
"""Access Point (AP) feature abstraction"""
3436

3537
@property
36-
def is_ready(self) -> bool: # noqa: D102
37-
# It's ready upon initialization
38+
def is_supported(self) -> bool: # noqa: D102
39+
# All Open GoPro cameras support access point
3840
return True
3941

40-
async def wait_for_ready(self) -> None: # noqa: D102
41-
return
42-
4342
async def close(self) -> None: # noqa: D102
4443
return
4544

@@ -54,18 +53,20 @@ async def scan_wifi_networks(self, timeout: int = 60) -> Result[proto.ResponseGe
5453
Result[proto.ResponseGetApEntries, GoProError]: Discovered AP entries on success or error
5554
"""
5655
# Wait to receive scanning success
57-
logger.info("Scanning for Wifi networks")
56+
logger.info("Scanning for Wifi networks...")
5857

5958
async with GoproObserverDistinctInitial[ResponseStartScanning, NotifStartScanning](
6059
gopro=self._gopro,
61-
update=ActionId.NOTIF_START_SCAN,
60+
update=ProtobufId(FeatureId.NETWORK_MANAGEMENT, ActionId.NOTIF_START_SCAN),
6261
register_command=self._gopro.ble_command.scan_wifi_networks(),
6362
) as observable, asyncio.timeout(timeout):
6463
if observable.initial_response.result != EnumResultGeneric.RESULT_SUCCESS:
6564
return Result.from_failure(GoProError("Failed to start scanning."))
6665

66+
logger.info("Waiting for scanning to complete...")
6767
result = await observable.observe().first(lambda s: s.scanning_state == EnumScanning.SCANNING_SUCCESS)
6868
entries = await self._gopro.ble_command.get_ap_entries(scan_id=result.scan_id)
69+
logger.info(f"Scan complete. Found {len(entries.data.entries)} networks.")
6970
return Result.from_value(entries.data)
7071

7172
async def connect(
@@ -99,7 +100,7 @@ async def connect(
99100
NotifProvisioningState,
100101
](
101102
gopro=self._gopro,
102-
update=ActionId.NOTIF_PROVIS_STATE,
103+
update=ProtobufId(FeatureId.NETWORK_MANAGEMENT, ActionId.NOTIF_PROVIS_STATE),
103104
register_command=command,
104105
) as observable:
105106
if observable.initial_response.result != EnumResultGeneric.RESULT_SUCCESS:

demos/python/sdk_wireless_camera_control/open_gopro/features/base_feature.py

Lines changed: 61 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2,44 +2,86 @@
22
# This copyright was auto-generated on Mon May 12 23:03:50 UTC 2025
33

44
"""Functionality common to all features."""
5+
from __future__ import annotations
56

67
import asyncio
78
from abc import ABC, abstractmethod
9+
from typing import Any, Callable
10+
11+
import wrapt
812

9-
from open_gopro.api.api import WirelessApi
1013
from open_gopro.gopro_base import GoProBase
1114

1215

13-
class BaseFeature(ABC):
14-
"""Base Feature definition / interface
16+
@wrapt.decorator
17+
def require_supported(wrapped: Callable, instance: BaseFeature, args: Any, kwargs: Any) -> Any:
18+
"""Ensure the feature is supported before allowing access.
1519
1620
Args:
17-
gopro (GoProBase[WirelessApi]): camera to operate on
18-
loop (asyncio.AbstractEventLoop): asyncio loop to use for this feature
21+
wrapped (Callable): The wrapped function or property
22+
instance (BaseFeature): The class instance (or None if accessing a property getter)
23+
args (Any): Positional arguments
24+
kwargs (Any): Keyword arguments
25+
26+
Returns:
27+
Any: Result of the wrapped function
28+
29+
Raises:
30+
RuntimeError: If feature is not supported
1931
"""
32+
# Handle the case where instance might be None (property access)
33+
# In this case, the instance is actually the first argument in args
34+
actual_instance = instance if instance is not None else args[0]
2035

21-
def __init__(
22-
self,
23-
gopro: GoProBase[WirelessApi],
24-
loop: asyncio.AbstractEventLoop,
25-
) -> None:
26-
self._gopro = gopro
27-
self._loop = loop
36+
if not actual_instance.is_supported:
37+
raise RuntimeError(f"{actual_instance.__class__.__name__} feature is not supported on this camera")
38+
39+
if instance is None:
40+
# Property getter case - call it with the rest of args (skipping the first one which is the instance)
41+
if len(args) > 1:
42+
return wrapped(args[0], *args[1:], **kwargs)
43+
return wrapped(args[0])
44+
45+
# Normal method call
46+
return wrapped(*args, **kwargs)
47+
48+
49+
class BaseFeature(ABC):
50+
"""Base Feature definition / interface"""
51+
52+
def __init__(self) -> None:
53+
self._loop: asyncio.AbstractEventLoop
54+
self._gopro: GoProBase[Any]
2855

2956
@property
3057
@abstractmethod
31-
def is_ready(self) -> bool:
32-
"""Is the feature ready to use?
33-
34-
No other methods (besides wait_for_ready) should be used until the feature is ready
58+
def is_supported(self) -> bool:
59+
"""Is the feature supported on the current camera?
3560
3661
Returns:
3762
bool: True if ready, False otherwise
3863
"""
3964

40-
@abstractmethod
41-
async def wait_for_ready(self) -> None:
42-
"""Wait until the feature is ready to use"""
65+
async def open( # pylint: disable=unused-argument
66+
self,
67+
loop: asyncio.AbstractEventLoop,
68+
gopro: GoProBase[Any],
69+
*args: Any,
70+
**kwargs: Any,
71+
) -> None:
72+
"""Open the feature for use.
73+
74+
This should be called before using any other methods in the feature. It must be called for each
75+
GoProBase instance that is used with the feature.
76+
77+
Args:
78+
loop (asyncio.AbstractEventLoop): asyncio loop to use for this feature
79+
gopro (GoProBase[Any]): camera to operate on
80+
*args (Any): additional arguments for subclasses
81+
**kwargs (Any): additional keyword arguments for subclasses
82+
"""
83+
self._loop = loop
84+
self._gopro = gopro
4385

4486
@abstractmethod
4587
async def close(self) -> None:

0 commit comments

Comments
 (0)