Add native WebRTC streaming support for Abode Cam 2#166662
Add native WebRTC streaming support for Abode Cam 2#166662goabodedevops wants to merge 4 commits intohome-assistant:devfrom
Conversation
| Hey there @shred86, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
There was a problem hiding this comment.
Pull request overview
Adds native WebRTC signaling to the Abode camera entity (targeting Abode Cam 2 / other KVS-capable cameras) while keeping the existing HLS playback URL path as a fallback, and expands the Abode camera test suite to cover the new streaming/signaling and resilience behavior.
Changes:
- Implement KVS signaling fetch + caching, WebRTC offer/candidate handling, and WebSocket message listening/cleanup in the Abode camera entity.
- Add snapshot-first image refresh logic with safer exception handling and “keep last good snapshot” behavior.
- Add extensive tests for snapshot/timeline behavior, HLS stream source requests, WebRTC signaling flows, cache behavior, and failure paths.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
homeassistant/components/abode/camera.py | Implements snapshot-based stills, HLS stream source via Abode playback URL, and native WebRTC signaling/session management via Abode KVS endpoints. |
tests/components/abode/test_camera.py | Adds comprehensive test coverage for the new snapshot, HLS, and native WebRTC signaling behaviors (including error handling and caching). |
| try: | ||
| message_data = json.loads(msg.data) | ||
| except (TypeError, json.JSONDecodeError): | ||
| return None | ||
| | ||
| message_type = message_data.get("messageType") | ||
| encoded_payload = message_data.get("messagePayload") | ||
| if not isinstance(message_type, str) or not isinstance(encoded_payload, str): | ||
| return None |
There was a problem hiding this comment.
Guard decoded websocket JSON with an isinstance(..., dict) check before calling .get() to avoid crashes on non-object messages.
There was a problem hiding this comment.
Addressed in baa880e: added a guard to ensure decoded signaling JSON is a dict before accessing fields.
| return RTCIceCandidateInit( | ||
| candidate, | ||
| sdp_mid=sdp_mid if isinstance(sdp_mid, str) else None, | ||
| sdp_m_line_index=( | ||
| sdp_m_line_index if isinstance(sdp_m_line_index, int) else None | ||
| ), |
There was a problem hiding this comment.
Coerce sdpMLineIndex values that arrive as numeric strings into an int so remote ICE candidates keep their m-line index instead of being dropped to None.
| return RTCIceCandidateInit( | |
| candidate, | |
| sdp_mid=sdp_mid if isinstance(sdp_mid, str) else None, | |
| sdp_m_line_index=( | |
| sdp_m_line_index if isinstance(sdp_m_line_index, int) else None | |
| ), | |
| sdp_m_line_index_int: int | None | |
| if isinstance(sdp_m_line_index, int): | |
| sdp_m_line_index_int = sdp_m_line_index | |
| elif isinstance(sdp_m_line_index, str): | |
| try: | |
| sdp_m_line_index_int = int(sdp_m_line_index) | |
| except (TypeError, ValueError): | |
| sdp_m_line_index_int = None | |
| else: | |
| sdp_m_line_index_int = None | |
| return RTCIceCandidateInit( | |
| candidate, | |
| sdp_mid=sdp_mid if isinstance(sdp_mid, str) else None, | |
| sdp_m_line_index=sdp_m_line_index_int, |
There was a problem hiding this comment.
Addressed in baa880e: fallback ICE parsing now coerces numeric-string sdpMLineIndex values to int, with tests updated to cover it.
| def _async_get_webrtc_client_configuration(self) -> WebRTCClientConfiguration: | ||
| """Return WebRTC client configuration for Abode cameras.""" | ||
| return WebRTCClientConfiguration( | ||
| configuration=RTCConfiguration(ice_servers=self._webrtc_ice_servers.copy()) | ||
| ) |
There was a problem hiding this comment.
The method is named with a leading underscore (_async_get_webrtc_client_configuration), which makes it a private helper and is unlikely to be discovered by the Camera/WebRTC machinery. If this is intended to provide the entity’s WebRTC client configuration to Home Assistant (and the frontend), it should be exposed with the expected public name (e.g., async_get_webrtc_client_configuration) and signature so it’s actually used.
There was a problem hiding this comment.
No change needed here: this is intentionally overriding Camera._async_get_webrtc_client_configuration(). The public Camera.async_get_webrtc_client_configuration() calls this override and then appends global ICE servers.
| LOGGER.warning("Failed to get camera stream URL: %s", err) | ||
| return None | ||
| | ||
| playback_url = response.json().get("playbackUrl") |
There was a problem hiding this comment.
response.json() is assumed to return a dict; if it returns a non-dict (or JSON decoding fails), this will raise (AttributeError/ValueError) and break stream source retrieval. Consider validating the decoded JSON is a dict (similar to _get_kvs_signaling_info) and handling JSON decode errors so this method reliably returns None on malformed responses.
| playback_url = response.json().get("playbackUrl") | |
| try: | |
| playback_data = response.json() | |
| except (ValueError, json.JSONDecodeError) as err: | |
| LOGGER.warning("Failed to decode camera stream response JSON: %s", err) | |
| return None | |
| if not isinstance(playback_data, dict): | |
| LOGGER.warning( | |
| "Failed to get camera stream URL: unexpected response format: %s", | |
| type(playback_data), | |
| ) | |
| return None | |
| playback_url = playback_data.get("playbackUrl") |
There was a problem hiding this comment.
Addressed in 98ae3ab: stream-source parsing now handles JSON decode errors and verifies the decoded payload is a dict before reading playbackUrl.
| def _refresh_snapshot_image(self) -> bool: | ||
| """Attempt to refresh image using Abode snapshot endpoint.""" | ||
| try: | ||
| if not self._device.snapshot(): | ||
| LOGGER.debug("Camera snapshot request did not return image data") | ||
| return False | ||
| except AbodeException as err: | ||
| LOGGER.warning("Failed to refresh camera snapshot image: %s", err) | ||
| return False | ||
| | ||
| snapshot_data_url = self._device.snapshot_data_url(get_snapshot=False) | ||
| _, separator, encoded_snapshot = snapshot_data_url.partition(",") | ||
| if not separator: | ||
| LOGGER.warning( | ||
| "Failed to decode camera snapshot: unexpected snapshot format" | ||
| ) | ||
| return False | ||
| | ||
| try: | ||
| self._snapshot_image = base64.b64decode(encoded_snapshot) | ||
| except (binascii.Error, ValueError) as err: | ||
| LOGGER.warning("Failed to decode camera snapshot: %s", err) | ||
| return False |
There was a problem hiding this comment.
Two concrete robustness gaps here: (1) snapshot_data_url(...) can raise Abode API/auth exceptions and currently isn’t caught, which can bubble out of refresh_image() despite the new resilience goals; wrap it in the same AbodeException handling as snapshot(). (2) base64.b64decode(...) is called without validate=True, which can silently accept/ignore non-base64 characters; using validate=True makes malformed snapshots fail fast and consistently hit the error path you already test.
There was a problem hiding this comment.
Addressed in 98ae3ab: snapshot_data_url is now wrapped in AbodeException handling, and snapshot base64 decoding uses validate=True for strict validation.
| async def _async_listen_webrtc_messages( | ||
| self, | ||
| session_id: str, | ||
| ws: ClientWebSocketResponse, | ||
| send_message: WebRTCSendMessage, | ||
| ) -> None: | ||
| """Listen for KVS signaling messages for a session.""" | ||
| try: | ||
| while True: | ||
| msg = await ws.receive() | ||
| if msg.type in (WSMsgType.CLOSE, WSMsgType.CLOSED, WSMsgType.CLOSING): | ||
| break |
There was a problem hiding this comment.
The listener loop waits indefinitely on ws.receive() with no timeout/heartbeat. If the websocket stays open but becomes silent (or if the frontend never calls close_webrtc_session), the task and session entry can linger indefinitely. Consider adding an idle timeout (and emitting a WebRTCError / closing the session) or using aiohttp heartbeat/ping configuration to ensure sessions self-clean.
There was a problem hiding this comment.
Kept as-is for this PR. Sessions are currently cleaned up via close_webrtc_session and websocket close/error paths. Adding an idle-timeout/heartbeat policy is a good enhancement, but it is a behavioral change that is better handled in a focused follow-up.
| with pytest.raises(HomeAssistantError, match="does not support WebRTC"): | ||
| await camera.async_handle_async_webrtc_offer( | ||
| "offer-sdp", "session-1", list.append | ||
| ) |
There was a problem hiding this comment.
These tests pass list.append as the send_message callback. list.append is an unbound method that will raise if it ever gets called (missing self), which makes the tests fragile if control flow changes or if a background listener emits an error event earlier than expected. Prefer passing a safe callable (e.g., events.append after creating events: list[Any] = [], or lambda *_: None) in all offer tests.
There was a problem hiding this comment.
Addressed in 98ae3ab: replaced unbound list.append callbacks with safe no-op lambdas in the offer-failure tests.
Breaking change
None.
Proposed change
This PR adds native WebRTC streaming support for Abode Cam 2 (AC2) and other KVS-capable Abode cameras in Home Assistant.
What is included:
/integrations/v1/camera/{camera_id}/kvs/stream)./integrations/v1/media/getPlaybackUrl) for HLS fallback.This contribution is submitted from the official Abode Systems account.
Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests:
Local validation
PATH=".venv/bin:$PATH" .venv/bin/python -m script.hassfest --action validate --integration-path homeassistant/components/abode.venv/bin/ruff check homeassistant/components/abode/camera.py tests/components/abode/test_camera.py.venv/bin/mypy homeassistant/components/abode.venv/bin/pytest tests/components/abode/test_camera.py --cov=homeassistant.components.abode.camera --cov-report=term-missing -q.venv/bin/pytest tests/components/abode -q --maxfail=1