From 4729b19dc6a90ca96bd67fe65fc1b01ca65a7df2 Mon Sep 17 00:00:00 2001
From: Robert Resch <robert@resch.dev>
Date: Tue, 5 Nov 2024 14:44:37 +0100
Subject: [PATCH] Skip adding providers if the camera has native WebRTC
 (#129808)

* Skip adding providers if the camera has native WebRTC

* Update homeassistant/components/camera/__init__.py

Co-authored-by: Martin Hjelmare <marhje52@gmail.com>

* Implement suggestion

* Add tests

* Shorten test name

* Fix test

---------

Co-authored-by: Martin Hjelmare <marhje52@gmail.com>
---
 homeassistant/components/camera/__init__.py | 40 ++++++++------
 tests/components/camera/common.py           | 50 +++++++++++++++++
 tests/components/camera/conftest.py         | 49 ++++++++++++++---
 tests/components/camera/test_init.py        | 20 ++++++-
 tests/components/camera/test_webrtc.py      | 60 ++-------------------
 5 files changed, 136 insertions(+), 83 deletions(-)

diff --git a/homeassistant/components/camera/__init__.py b/homeassistant/components/camera/__init__.py
index 47d8b9dfbd0..b600eae02c7 100644
--- a/homeassistant/components/camera/__init__.py
+++ b/homeassistant/components/camera/__init__.py
@@ -484,9 +484,13 @@ class Camera(Entity, cached_properties=CACHED_PROPERTIES_WITH_ATTR_):
         self._create_stream_lock: asyncio.Lock | None = None
         self._webrtc_provider: CameraWebRTCProvider | None = None
         self._legacy_webrtc_provider: CameraWebRTCLegacyProvider | None = None
-        self._webrtc_sync_offer = (
+        self._supports_native_sync_webrtc = (
             type(self).async_handle_web_rtc_offer != Camera.async_handle_web_rtc_offer
         )
+        self._supports_native_async_webrtc = (
+            type(self).async_handle_async_webrtc_offer
+            != Camera.async_handle_async_webrtc_offer
+        )
 
     @cached_property
     def entity_picture(self) -> str:
@@ -623,7 +627,7 @@ class Camera(Entity, cached_properties=CACHED_PROPERTIES_WITH_ATTR_):
 
         Integrations can override with a native WebRTC implementation.
         """
-        if self._webrtc_sync_offer:
+        if self._supports_native_sync_webrtc:
             try:
                 answer = await self.async_handle_web_rtc_offer(offer_sdp)
             except ValueError as ex:
@@ -788,18 +792,25 @@ class Camera(Entity, cached_properties=CACHED_PROPERTIES_WITH_ATTR_):
         providers or inputs to the state attributes change.
         """
         old_provider = self._webrtc_provider
-        new_provider = await self._async_get_supported_webrtc_provider(
-            async_get_supported_provider
-        )
-
         old_legacy_provider = self._legacy_webrtc_provider
+        new_provider = None
         new_legacy_provider = None
-        if new_provider is None:
-            # Only add the legacy provider if the new provider is not available
-            new_legacy_provider = await self._async_get_supported_webrtc_provider(
-                async_get_supported_legacy_provider
+
+        # Skip all providers if the camera has a native WebRTC implementation
+        if not (
+            self._supports_native_sync_webrtc or self._supports_native_async_webrtc
+        ):
+            # Camera doesn't have a native WebRTC implementation
+            new_provider = await self._async_get_supported_webrtc_provider(
+                async_get_supported_provider
             )
 
+            if new_provider is None:
+                # Only add the legacy provider if the new provider is not available
+                new_legacy_provider = await self._async_get_supported_webrtc_provider(
+                    async_get_supported_legacy_provider
+                )
+
         if old_provider != new_provider or old_legacy_provider != new_legacy_provider:
             self._webrtc_provider = new_provider
             self._legacy_webrtc_provider = new_legacy_provider
@@ -827,7 +838,7 @@ class Camera(Entity, cached_properties=CACHED_PROPERTIES_WITH_ATTR_):
         """Return the WebRTC client configuration and extend it with the registered ice servers."""
         config = self._async_get_webrtc_client_configuration()
 
-        if not self._webrtc_sync_offer:
+        if not self._supports_native_sync_webrtc:
             # Until 2024.11, the frontend was not resolving any ice servers
             # The async approach was added 2024.11 and new integrations need to use it
             ice_servers = [
@@ -867,12 +878,7 @@ class Camera(Entity, cached_properties=CACHED_PROPERTIES_WITH_ATTR_):
         """Return the camera capabilities."""
         frontend_stream_types = set()
         if CameraEntityFeature.STREAM in self.supported_features_compat:
-            if (
-                type(self).async_handle_web_rtc_offer
-                != Camera.async_handle_web_rtc_offer
-                or type(self).async_handle_async_webrtc_offer
-                != Camera.async_handle_async_webrtc_offer
-            ):
+            if self._supports_native_sync_webrtc or self._supports_native_async_webrtc:
                 # The camera has a native WebRTC implementation
                 frontend_stream_types.add(StreamType.WEB_RTC)
             else:
diff --git a/tests/components/camera/common.py b/tests/components/camera/common.py
index f7dcf46db01..569756c2640 100644
--- a/tests/components/camera/common.py
+++ b/tests/components/camera/common.py
@@ -6,6 +6,16 @@ components. Instead call the service directly.
 
 from unittest.mock import Mock
 
+from webrtc_models import RTCIceCandidate
+
+from homeassistant.components.camera import (
+    Camera,
+    CameraWebRTCProvider,
+    WebRTCAnswer,
+    WebRTCSendMessage,
+)
+from homeassistant.core import callback
+
 EMPTY_8_6_JPEG = b"empty_8_6"
 WEBRTC_ANSWER = "a=sendonly"
 STREAM_SOURCE = "rtsp://127.0.0.1/stream"
@@ -23,3 +33,43 @@ def mock_turbo_jpeg(
     mocked_turbo_jpeg.scale_with_quality.return_value = EMPTY_8_6_JPEG
     mocked_turbo_jpeg.encode.return_value = EMPTY_8_6_JPEG
     return mocked_turbo_jpeg
+
+
+class SomeTestProvider(CameraWebRTCProvider):
+    """Test provider."""
+
+    def __init__(self) -> None:
+        """Initialize the provider."""
+        self._is_supported = True
+
+    @property
+    def domain(self) -> str:
+        """Return the integration domain of the provider."""
+        return "some_test"
+
+    @callback
+    def async_is_supported(self, stream_source: str) -> bool:
+        """Determine if the provider supports the stream source."""
+        return self._is_supported
+
+    async def async_handle_async_webrtc_offer(
+        self,
+        camera: Camera,
+        offer_sdp: str,
+        session_id: str,
+        send_message: WebRTCSendMessage,
+    ) -> None:
+        """Handle the WebRTC offer and return the answer via the provided callback.
+
+        Return value determines if the offer was handled successfully.
+        """
+        send_message(WebRTCAnswer(answer="answer"))
+
+    async def async_on_webrtc_candidate(
+        self, session_id: str, candidate: RTCIceCandidate
+    ) -> None:
+        """Handle the WebRTC candidate."""
+
+    @callback
+    def async_close_session(self, session_id: str) -> None:
+        """Close the session."""
diff --git a/tests/components/camera/conftest.py b/tests/components/camera/conftest.py
index a88cd898e33..d6343959d41 100644
--- a/tests/components/camera/conftest.py
+++ b/tests/components/camera/conftest.py
@@ -4,6 +4,7 @@ from collections.abc import AsyncGenerator, Generator
 from unittest.mock import AsyncMock, Mock, PropertyMock, patch
 
 import pytest
+from webrtc_models import RTCIceCandidate
 
 from homeassistant.components import camera
 from homeassistant.components.camera.const import StreamType
@@ -14,7 +15,7 @@ from homeassistant.core import HomeAssistant
 from homeassistant.helpers.device_registry import DeviceInfo
 from homeassistant.setup import async_setup_component
 
-from .common import STREAM_SOURCE, WEBRTC_ANSWER
+from .common import STREAM_SOURCE, WEBRTC_ANSWER, SomeTestProvider
 
 from tests.common import (
     MockConfigEntry,
@@ -155,16 +156,15 @@ def mock_stream_source_fixture() -> Generator[AsyncMock]:
 
 
 @pytest.fixture
-async def mock_camera_webrtc_native_sync_offer(hass: HomeAssistant) -> None:
-    """Initialize a test camera with native sync WebRTC support."""
+async def mock_test_webrtc_cameras(hass: HomeAssistant) -> None:
+    """Initialize a test WebRTC cameras."""
 
     # Cannot use the fixture mock_camera_web_rtc as it's mocking Camera.async_handle_web_rtc_offer
     # and native support is checked by verify the function "async_handle_web_rtc_offer" was
     # overwritten(implemented) or not
-    class MockCamera(camera.Camera):
-        """Mock Camera Entity."""
+    class BaseCamera(camera.Camera):
+        """Base Camera."""
 
-        _attr_name = "Test"
         _attr_supported_features: camera.CameraEntityFeature = (
             camera.CameraEntityFeature.STREAM
         )
@@ -173,9 +173,30 @@ async def mock_camera_webrtc_native_sync_offer(hass: HomeAssistant) -> None:
         async def stream_source(self) -> str | None:
             return STREAM_SOURCE
 
+    class SyncCamera(BaseCamera):
+        """Mock Camera with native sync WebRTC support."""
+
+        _attr_name = "Sync"
+
         async def async_handle_web_rtc_offer(self, offer_sdp: str) -> str | None:
             return WEBRTC_ANSWER
 
+    class AsyncCamera(BaseCamera):
+        """Mock Camera with native async WebRTC support."""
+
+        _attr_name = "Async"
+
+        async def async_handle_async_webrtc_offer(
+            self, offer_sdp: str, session_id: str, send_message: WebRTCSendMessage
+        ) -> None:
+            send_message(WebRTCAnswer(WEBRTC_ANSWER))
+
+        async def async_on_webrtc_candidate(
+            self, session_id: str, candidate: RTCIceCandidate
+        ) -> None:
+            """Handle a WebRTC candidate."""
+            # Do nothing
+
     domain = "test"
 
     entry = MockConfigEntry(domain=domain)
@@ -208,10 +229,24 @@ async def mock_camera_webrtc_native_sync_offer(hass: HomeAssistant) -> None:
         ),
     )
     setup_test_component_platform(
-        hass, camera.DOMAIN, [MockCamera()], from_config_entry=True
+        hass, camera.DOMAIN, [SyncCamera(), AsyncCamera()], from_config_entry=True
     )
     mock_platform(hass, f"{domain}.config_flow", Mock())
 
     with mock_config_flow(domain, ConfigFlow):
         assert await hass.config_entries.async_setup(entry.entry_id)
         await hass.async_block_till_done()
+
+
+@pytest.fixture
+async def register_test_provider(
+    hass: HomeAssistant,
+) -> AsyncGenerator[SomeTestProvider]:
+    """Add WebRTC test provider."""
+    await async_setup_component(hass, "camera", {})
+
+    provider = SomeTestProvider()
+    unsub = camera.async_register_webrtc_provider(hass, provider)
+    await hass.async_block_till_done()
+    yield provider
+    unsub()
diff --git a/tests/components/camera/test_init.py b/tests/components/camera/test_init.py
index 0a173065564..621ac8b7fb3 100644
--- a/tests/components/camera/test_init.py
+++ b/tests/components/camera/test_init.py
@@ -979,7 +979,7 @@ async def test_camera_capabilities_hls(
     )
 
 
-@pytest.mark.usefixtures("mock_camera_webrtc_native_sync_offer")
+@pytest.mark.usefixtures("mock_test_webrtc_cameras")
 async def test_camera_capabilities_webrtc(
     hass: HomeAssistant,
     hass_ws_client: WebSocketGenerator,
@@ -987,5 +987,21 @@ async def test_camera_capabilities_webrtc(
     """Test WebRTC camera capabilities."""
 
     await _test_capabilities(
-        hass, hass_ws_client, "camera.test", {StreamType.WEB_RTC}, {StreamType.WEB_RTC}
+        hass, hass_ws_client, "camera.sync", {StreamType.WEB_RTC}, {StreamType.WEB_RTC}
     )
+
+
+@pytest.mark.parametrize(
+    ("entity_id", "expect_native_async_webrtc"),
+    [("camera.sync", False), ("camera.async", True)],
+)
+@pytest.mark.usefixtures("mock_test_webrtc_cameras", "register_test_provider")
+async def test_webrtc_provider_not_added_for_native_webrtc(
+    hass: HomeAssistant, entity_id: str, expect_native_async_webrtc: bool
+) -> None:
+    """Test that a WebRTC provider is not added to a camera when the camera has native WebRTC support."""
+    camera_obj = get_camera_from_entity_id(hass, entity_id)
+    assert camera_obj
+    assert camera_obj._webrtc_provider is None
+    assert camera_obj._supports_native_sync_webrtc is not expect_native_async_webrtc
+    assert camera_obj._supports_native_async_webrtc is expect_native_async_webrtc
diff --git a/tests/components/camera/test_webrtc.py b/tests/components/camera/test_webrtc.py
index 2970a41408c..f726eb29673 100644
--- a/tests/components/camera/test_webrtc.py
+++ b/tests/components/camera/test_webrtc.py
@@ -34,7 +34,7 @@ from homeassistant.exceptions import HomeAssistantError
 from homeassistant.helpers import issue_registry as ir
 from homeassistant.setup import async_setup_component
 
-from .common import STREAM_SOURCE, WEBRTC_ANSWER
+from .common import STREAM_SOURCE, WEBRTC_ANSWER, SomeTestProvider
 
 from tests.common import (
     MockConfigEntry,
@@ -51,46 +51,6 @@ HLS_STREAM_SOURCE = "http://127.0.0.1/example.m3u"
 TEST_INTEGRATION_DOMAIN = "test"
 
 
-class SomeTestProvider(CameraWebRTCProvider):
-    """Test provider."""
-
-    def __init__(self) -> None:
-        """Initialize the provider."""
-        self._is_supported = True
-
-    @property
-    def domain(self) -> str:
-        """Return the integration domain of the provider."""
-        return "some_test"
-
-    @callback
-    def async_is_supported(self, stream_source: str) -> bool:
-        """Determine if the provider supports the stream source."""
-        return self._is_supported
-
-    async def async_handle_async_webrtc_offer(
-        self,
-        camera: Camera,
-        offer_sdp: str,
-        session_id: str,
-        send_message: WebRTCSendMessage,
-    ) -> None:
-        """Handle the WebRTC offer and return the answer via the provided callback.
-
-        Return value determines if the offer was handled successfully.
-        """
-        send_message(WebRTCAnswer(answer="answer"))
-
-    async def async_on_webrtc_candidate(
-        self, session_id: str, candidate: RTCIceCandidate
-    ) -> None:
-        """Handle the WebRTC candidate."""
-
-    @callback
-    def async_close_session(self, session_id: str) -> None:
-        """Close the session."""
-
-
 class Go2RTCProvider(SomeTestProvider):
     """go2rtc provider."""
 
@@ -179,20 +139,6 @@ async def init_test_integration(
     return test_camera
 
 
-@pytest.fixture
-async def register_test_provider(
-    hass: HomeAssistant,
-) -> AsyncGenerator[SomeTestProvider]:
-    """Add WebRTC test provider."""
-    await async_setup_component(hass, "camera", {})
-
-    provider = SomeTestProvider()
-    unsub = async_register_webrtc_provider(hass, provider)
-    await hass.async_block_till_done()
-    yield provider
-    unsub()
-
-
 @pytest.mark.usefixtures("mock_camera", "mock_stream", "mock_stream_source")
 async def test_async_register_webrtc_provider(
     hass: HomeAssistant,
@@ -393,7 +339,7 @@ async def test_ws_get_client_config(
     }
 
 
-@pytest.mark.usefixtures("mock_camera_webrtc_native_sync_offer")
+@pytest.mark.usefixtures("mock_test_webrtc_cameras")
 async def test_ws_get_client_config_sync_offer(
     hass: HomeAssistant, hass_ws_client: WebSocketGenerator
 ) -> None:
@@ -403,7 +349,7 @@ async def test_ws_get_client_config_sync_offer(
 
     client = await hass_ws_client(hass)
     await client.send_json_auto_id(
-        {"type": "camera/webrtc/get_client_config", "entity_id": "camera.test"}
+        {"type": "camera/webrtc/get_client_config", "entity_id": "camera.sync"}
     )
     msg = await client.receive_json()
 
-- 
GitLab