Create repair issue for legacy webrtc provider (#129334)

* Add repair issue

* Add tests

* Add option to not use builtin go2rtc provider

* Add test

* Add domain to new providers

* Add learn more url

* Update placeholder

* Promote the builtin provider

* Refactor provider storage

* Move check for legacy provider conflict to refresh

* Test provider registration race

* Add test for registering the same legacy provider twice

* Test test_get_not_supported_legacy_provider

* Remove blank line between bullets

* Call it built-in

Co-authored-by: Joost Lekkerkerker <joostlek@outlook.com>

* Revert "Add option to not use builtin go2rtc provider"

This reverts commit 4e31bad6c0c23d5a1c0935c985351808a46163d6.

* Revert "Add test"

This reverts commit ddf85fd4db2c78b15c1cdc716804b965f3a1f4e3.

* Update issue description

* async_close_session is optional

* Clean up after rebase

* Add required domain property to provider tests

---------

Co-authored-by: Joost Lekkerkerker <joostlek@outlook.com>

homeassistant/components/camera/strings.json

@@ -46,6 +46,10 @@
           }
         }
       }
+    },
+    "legacy_webrtc_provider": {
+      "title": "Detected use of legacy WebRTC provider registered by {legacy_integration}",
+      "description": "The {legacy_integration} integration has registered a legacy WebRTC provider. Home Assistant prefers using the built-in modern WebRTC provider registered by the {builtin_integration} integration.\n\nBenefits of the built-in integration are:\n\n- The camera stream is started faster.\n- More camera devices are supported.\n\nTo fix this issue, you can either keep using the built-in modern WebRTC provider and remove the {legacy_integration} integration or remove the {builtin_integration} integration to use the legacy provider, and then restart Home Assistant."
     }
   },
   "services": {

homeassistant/components/camera/webrtc.py

@@ -2,6 +2,7 @@
 
 from __future__ import annotations
 
+from abc import ABC, abstractmethod
 import asyncio
 from collections.abc import Awaitable, Callable, Iterable
 from dataclasses import asdict, dataclass, field
@@ -15,7 +16,7 @@ from webrtc_models import RTCConfiguration, RTCIceServer
 from homeassistant.components import websocket_api
 from homeassistant.core import HomeAssistant, callback
 from homeassistant.exceptions import HomeAssistantError
-from homeassistant.helpers import config_validation as cv
+from homeassistant.helpers import config_validation as cv, issue_registry as ir
 from homeassistant.util.hass_dict import HassKey
 from homeassistant.util.ulid import ulid
 
@@ -31,7 +32,7 @@ _LOGGER = logging.getLogger(__name__)
 DATA_WEBRTC_PROVIDERS: HassKey[set[CameraWebRTCProvider]] = HassKey(
     "camera_webrtc_providers"
 )
-DATA_WEBRTC_LEGACY_PROVIDERS: HassKey[set[CameraWebRTCLegacyProvider]] = HassKey(
+DATA_WEBRTC_LEGACY_PROVIDERS: HassKey[dict[str, CameraWebRTCLegacyProvider]] = HassKey(
     "camera_webrtc_legacy_providers"
 )
 DATA_ICE_SERVERS: HassKey[list[Callable[[], Iterable[RTCIceServer]]]] = HassKey(
@@ -113,13 +114,20 @@ class WebRTCClientConfiguration:
         return data
 
 
-class CameraWebRTCProvider(Protocol):
+class CameraWebRTCProvider(ABC):
     """WebRTC provider."""
 
+    @property
+    @abstractmethod
+    def domain(self) -> str:
+        """Return the integration domain of the provider."""
+
     @callback
+    @abstractmethod
     def async_is_supported(self, stream_source: str) -> bool:
         """Determine if the provider supports the stream source."""
 
+    @abstractmethod
     async def async_handle_async_webrtc_offer(
         self,
         camera: Camera,
@@ -129,6 +137,7 @@ class CameraWebRTCProvider(Protocol):
     ) -> None:
         """Handle the WebRTC offer and return the answer via the provided callback."""
 
+    @abstractmethod
     async def async_on_webrtc_candidate(self, session_id: str, candidate: str) -> None:
         """Handle the WebRTC candidate."""
 
@@ -150,10 +159,10 @@ class CameraWebRTCLegacyProvider(Protocol):
         """Handle the WebRTC offer and return an answer."""
 
 
-def _async_register_webrtc_provider[_T](
+@callback
+def async_register_webrtc_provider(
     hass: HomeAssistant,
-    key: HassKey[set[_T]],
+    provider: CameraWebRTCProvider,
-    provider: _T,
 ) -> Callable[[], None]:
     """Register a WebRTC provider.
 
@@ -162,7 +171,7 @@ def _async_register_webrtc_provider[_T](
     if DOMAIN not in hass.data:
         raise ValueError("Unexpected state, camera not loaded")
 
-    providers = hass.data.setdefault(key, set())
+    providers = hass.data.setdefault(DATA_WEBRTC_PROVIDERS, set())
 
     @callback
     def remove_provider() -> None:
@@ -177,20 +186,9 @@ def _async_register_webrtc_provider[_T](
     return remove_provider
 
 
-@callback
-def async_register_webrtc_provider(
-    hass: HomeAssistant,
-    provider: CameraWebRTCProvider,
-) -> Callable[[], None]:
-    """Register a WebRTC provider.
-
-    The first provider to satisfy the offer will be used.
-    """
-    return _async_register_webrtc_provider(hass, DATA_WEBRTC_PROVIDERS, provider)
-
-
 async def _async_refresh_providers(hass: HomeAssistant) -> None:
     """Check all cameras for any state changes for registered providers."""
+    _async_check_conflicting_legacy_provider(hass)
 
     component = hass.data[DATA_COMPONENT]
     await asyncio.gather(
@@ -334,11 +332,11 @@ def async_register_ws(hass: HomeAssistant) -> None:
     websocket_api.async_register_command(hass, ws_candidate)
 
 
-async def _async_get_supported_provider[
+async def async_get_supported_provider(
-    _T: CameraWebRTCLegacyProvider | CameraWebRTCProvider
+    hass: HomeAssistant, camera: Camera
-](hass: HomeAssistant, camera: Camera, key: HassKey[set[_T]]) -> _T | None:
+) -> CameraWebRTCProvider | None:
     """Return the first supported provider for the camera."""
-    providers = hass.data.get(key)
+    providers = hass.data.get(DATA_WEBRTC_PROVIDERS)
     if not providers or not (stream_source := await camera.stream_source()):
         return None
 
@@ -349,20 +347,19 @@ async def _async_get_supported_provider[
     return None
 
 
-async def async_get_supported_provider(
-    hass: HomeAssistant, camera: Camera
-) -> CameraWebRTCProvider | None:
-    """Return the first supported provider for the camera."""
-    return await _async_get_supported_provider(hass, camera, DATA_WEBRTC_PROVIDERS)
-
-
 async def async_get_supported_legacy_provider(
     hass: HomeAssistant, camera: Camera
 ) -> CameraWebRTCLegacyProvider | None:
     """Return the first supported provider for the camera."""
-    return await _async_get_supported_provider(
+    providers = hass.data.get(DATA_WEBRTC_LEGACY_PROVIDERS)
-        hass, camera, DATA_WEBRTC_LEGACY_PROVIDERS
+    if not providers or not (stream_source := await camera.stream_source()):
-    )
+        return None
+
+    for provider in providers.values():
+        if await provider.async_is_supported(stream_source):
+            return provider
+
+    return None
 
 
 @callback
@@ -425,7 +422,49 @@ def async_register_rtsp_to_web_rtc_provider(
 
     The first provider to satisfy the offer will be used.
     """
+    if DOMAIN not in hass.data:
+        raise ValueError("Unexpected state, camera not loaded")
+
+    legacy_providers = hass.data.setdefault(DATA_WEBRTC_LEGACY_PROVIDERS, {})
+
+    if domain in legacy_providers:
+        raise ValueError("Provider already registered")
+
     provider_instance = _CameraRtspToWebRTCProvider(provider)
-    return _async_register_webrtc_provider(
+
-        hass, DATA_WEBRTC_LEGACY_PROVIDERS, provider_instance
+    @callback
-    )
+    def remove_provider() -> None:
+        legacy_providers.pop(domain)
+        hass.async_create_task(_async_refresh_providers(hass))
+
+    legacy_providers[domain] = provider_instance
+    hass.async_create_task(_async_refresh_providers(hass))
+
+    return remove_provider
+
+
+@callback
+def _async_check_conflicting_legacy_provider(hass: HomeAssistant) -> None:
+    """Check if a legacy provider is registered together with the builtin provider."""
+    builtin_provider_domain = "go2rtc"
+    if (
+        (legacy_providers := hass.data.get(DATA_WEBRTC_LEGACY_PROVIDERS))
+        and (providers := hass.data.get(DATA_WEBRTC_PROVIDERS))
+        and any(provider.domain == builtin_provider_domain for provider in providers)
+    ):
+        for domain in legacy_providers:
+            ir.async_create_issue(
+                hass,
+                DOMAIN,
+                f"legacy_webrtc_provider_{domain}",
+                is_fixable=False,
+                is_persistent=False,
+                issue_domain=domain,
+                learn_more_url="https://www.home-assistant.io/integrations/go2rtc/",
+                severity=ir.IssueSeverity.WARNING,
+                translation_key="legacy_webrtc_provider",
+                translation_placeholders={
+                    "legacy_integration": domain,
+                    "builtin_integration": builtin_provider_domain,
+                },
+            )

homeassistant/components/go2rtc/__init__.py

@@ -172,6 +172,11 @@ class WebRTCProvider(CameraWebRTCProvider):
         self._rest_client = Go2RtcRestClient(self._session, url)
         self._sessions: dict[str, Go2RtcWsClient] = {}
 
+    @property
+    def domain(self) -> str:
+        """Return the integration domain of the provider."""
+        return DOMAIN
+
     @callback
     def async_is_supported(self, stream_source: str) -> bool:
         """Return if this provider is supports the Camera as source."""

tests/components/camera/test_init.py

@@ -938,6 +938,11 @@ async def _test_capabilities(
     class SomeTestProvider(CameraWebRTCProvider):
         """Test provider."""
 
+        @property
+        def domain(self) -> str:
+            """Return domain."""
+            return "test"
+
         @callback
         def async_is_supported(self, stream_source: str) -> bool:
             """Determine if the provider supports the stream source."""

tests/components/camera/test_webrtc.py

@@ -20,6 +20,7 @@ from homeassistant.components.camera import (
     WebRTCError,
     WebRTCMessage,
     WebRTCSendMessage,
+    async_get_supported_legacy_provider,
     async_register_ice_servers,
     async_register_rtsp_to_web_rtc_provider,
     async_register_webrtc_provider,
@@ -30,6 +31,7 @@ from homeassistant.config_entries import ConfigEntry, ConfigFlow
 from homeassistant.core import HomeAssistant, callback
 from homeassistant.core_config import async_process_ha_core_config
 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
@@ -49,13 +51,18 @@ HLS_STREAM_SOURCE = "http://127.0.0.1/example.m3u"
 TEST_INTEGRATION_DOMAIN = "test"
 
 
-class TestProvider(CameraWebRTCProvider):
+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."""
@@ -82,6 +89,15 @@ class TestProvider(CameraWebRTCProvider):
         """Close the session."""
 
 
+class Go2RTCProvider(SomeTestProvider):
+    """go2rtc provider."""
+
+    @property
+    def domain(self) -> str:
+        """Return the integration domain of the provider."""
+        return "go2rtc"
+
+
 class MockCamera(Camera):
     """Mock Camera Entity."""
 
@@ -162,11 +178,13 @@ async def init_test_integration(
 
 
 @pytest.fixture
-async def register_test_provider(hass: HomeAssistant) -> AsyncGenerator[TestProvider]:
+async def register_test_provider(
+    hass: HomeAssistant,
+) -> AsyncGenerator[SomeTestProvider]:
     """Add WebRTC test provider."""
     await async_setup_component(hass, "camera", {})
 
-    provider = TestProvider()
+    provider = SomeTestProvider()
     unsub = async_register_webrtc_provider(hass, provider)
     await hass.async_block_till_done()
     yield provider
@@ -183,7 +201,7 @@ async def test_async_register_webrtc_provider(
     camera = get_camera_from_entity_id(hass, "camera.demo_camera")
     assert camera.frontend_stream_type is StreamType.HLS
 
-    provider = TestProvider()
+    provider = SomeTestProvider()
     unregister = async_register_webrtc_provider(hass, provider)
     await hass.async_block_till_done()
 
@@ -211,7 +229,7 @@ async def test_async_register_webrtc_provider(
 @pytest.mark.usefixtures("mock_camera", "mock_stream", "mock_stream_source")
 async def test_async_register_webrtc_provider_twice(
     hass: HomeAssistant,
-    register_test_provider: TestProvider,
+    register_test_provider: SomeTestProvider,
 ) -> None:
     """Test registering a WebRTC provider twice should raise."""
     with pytest.raises(ValueError, match="Provider already registered"):
@@ -223,7 +241,7 @@ async def test_async_register_webrtc_provider_camera_not_loaded(
 ) -> None:
     """Test registering a WebRTC provider when camera is not loaded."""
     with pytest.raises(ValueError, match="Unexpected state, camera not loaded"):
-        async_register_webrtc_provider(hass, TestProvider())
+        async_register_webrtc_provider(hass, SomeTestProvider())
 
 
 @pytest.mark.usefixtures("mock_camera", "mock_stream", "mock_stream_source")
@@ -494,7 +512,7 @@ async def test_websocket_webrtc_offer(
 async def test_websocket_webrtc_offer_webrtc_provider(
     hass: HomeAssistant,
     hass_ws_client: WebSocketGenerator,
-    register_test_provider: TestProvider,
+    register_test_provider: SomeTestProvider,
     message: WebRTCMessage,
     expected_frontend_message: dict[str, Any],
 ) -> None:
@@ -997,7 +1015,7 @@ async def test_ws_webrtc_candidate_not_supported(
 async def test_ws_webrtc_candidate_webrtc_provider(
     hass: HomeAssistant,
     hass_ws_client: WebSocketGenerator,
-    register_test_provider: TestProvider,
+    register_test_provider: SomeTestProvider,
 ) -> None:
     """Test ws webrtc candidate command with WebRTC provider."""
     with patch.object(
@@ -1045,7 +1063,7 @@ async def test_ws_webrtc_candidate_invalid_entity(
 
 
 @pytest.mark.usefixtures("mock_camera_webrtc")
-async def test_ws_webrtc_canidate_missing_candidtae(
+async def test_ws_webrtc_canidate_missing_candidate(
     hass: HomeAssistant, hass_ws_client: WebSocketGenerator
 ) -> None:
     """Test ws WebRTC candidate command with missing required fields."""
@@ -1094,6 +1112,11 @@ async def test_webrtc_provider_optional_interface(hass: HomeAssistant) -> None:
     class OnlyRequiredInterfaceProvider(CameraWebRTCProvider):
         """Test provider."""
 
+        @property
+        def domain(self) -> str:
+            """Return the domain of the provider."""
+            return "test"
+
         @callback
         def async_is_supported(self, stream_source: str) -> bool:
             """Determine if the provider supports the stream source."""
@@ -1125,3 +1148,79 @@ async def test_webrtc_provider_optional_interface(hass: HomeAssistant) -> None:
     )
     await provider.async_on_webrtc_candidate("session_id", "candidate")
     provider.async_close_session("session_id")
+
+
+@pytest.mark.usefixtures("mock_camera")
+async def test_repair_issue_legacy_provider(
+    hass: HomeAssistant,
+    issue_registry: ir.IssueRegistry,
+) -> None:
+    """Test repair issue created for legacy provider."""
+    # Ensure no issue if no provider is registered
+    assert not issue_registry.async_get_issue(
+        "camera", "legacy_webrtc_provider_mock_domain"
+    )
+
+    # Register a legacy provider
+    legacy_provider = Mock(side_effect=provide_webrtc_answer)
+    unsub_legacy_provider = async_register_rtsp_to_web_rtc_provider(
+        hass, "mock_domain", legacy_provider
+    )
+    await hass.async_block_till_done()
+
+    # Ensure no issue if only legacy provider is registered
+    assert not issue_registry.async_get_issue(
+        "camera", "legacy_webrtc_provider_mock_domain"
+    )
+
+    provider = Go2RTCProvider()
+    unsub_go2rtc_provider = async_register_webrtc_provider(hass, provider)
+    await hass.async_block_till_done()
+
+    # Ensure issue when legacy and builtin provider are registered
+    issue = issue_registry.async_get_issue(
+        "camera", "legacy_webrtc_provider_mock_domain"
+    )
+    assert issue
+    assert issue.is_fixable is False
+    assert issue.is_persistent is False
+    assert issue.issue_domain == "mock_domain"
+    assert issue.learn_more_url == "https://www.home-assistant.io/integrations/go2rtc/"
+    assert issue.severity == ir.IssueSeverity.WARNING
+    assert issue.issue_id == "legacy_webrtc_provider_mock_domain"
+    assert issue.translation_key == "legacy_webrtc_provider"
+    assert issue.translation_placeholders == {
+        "legacy_integration": "mock_domain",
+        "builtin_integration": "go2rtc",
+    }
+
+    unsub_legacy_provider()
+    unsub_go2rtc_provider()
+
+
+@pytest.mark.usefixtures("mock_camera", "register_test_provider", "mock_rtsp_to_webrtc")
+async def test_no_repair_issue_without_new_provider(
+    hass: HomeAssistant,
+    issue_registry: ir.IssueRegistry,
+) -> None:
+    """Test repair issue not created if no go2rtc provider exists."""
+    assert not issue_registry.async_get_issue(
+        "camera", "legacy_webrtc_provider_mock_domain"
+    )
+
+
+@pytest.mark.usefixtures("mock_camera", "mock_rtsp_to_webrtc")
+async def test_registering_same_legacy_provider(
+    hass: HomeAssistant,
+) -> None:
+    """Test registering the same legacy provider twice."""
+    legacy_provider = Mock(side_effect=provide_webrtc_answer)
+    with pytest.raises(ValueError, match="Provider already registered"):
+        async_register_rtsp_to_web_rtc_provider(hass, "mock_domain", legacy_provider)
+
+
+@pytest.mark.usefixtures("mock_hls_stream_source", "mock_camera", "mock_rtsp_to_webrtc")
+async def test_get_not_supported_legacy_provider(hass: HomeAssistant) -> None:
+    """Test getting a not supported legacy provider."""
+    camera = get_camera_from_entity_id(hass, "camera.demo_camera")
+    assert await async_get_supported_legacy_provider(hass, camera) is None