From d9a7c4a4839ba1468e018ef96861b03c3512a274 Mon Sep 17 00:00:00 2001 From: Erik Montnemery Date: Thu, 5 May 2022 20:04:00 +0200 Subject: [PATCH] Only lookup unknown Google Cast models once (#71348) Co-authored-by: Paulus Schoutsen --- homeassistant/components/cast/__init__.py | 4 +- homeassistant/components/cast/discovery.py | 2 +- homeassistant/components/cast/helpers.py | 47 +++++++- homeassistant/components/cast/manifest.json | 2 +- homeassistant/components/cast/media_player.py | 6 +- requirements_all.txt | 2 +- requirements_test_all.txt | 2 +- tests/components/cast/conftest.py | 11 ++ tests/components/cast/test_media_player.py | 109 +++++++++++++++++- 9 files changed, 166 insertions(+), 19 deletions(-) diff --git a/homeassistant/components/cast/__init__.py b/homeassistant/components/cast/__init__.py index d0b7cfc4158..63f0693b01a 100644 --- a/homeassistant/components/cast/__init__.py +++ b/homeassistant/components/cast/__init__.py @@ -58,7 +58,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: """Set up Cast from a config entry.""" await home_assistant_cast.async_setup_ha_cast(hass, entry) hass.config_entries.async_setup_platforms(entry, PLATFORMS) - hass.data[DOMAIN] = {} + hass.data[DOMAIN] = {"cast_platform": {}, "unknown_models": {}} await async_process_integration_platforms(hass, DOMAIN, _register_cast_platform) return True @@ -107,7 +107,7 @@ async def _register_cast_platform( or not hasattr(platform, "async_play_media") ): raise HomeAssistantError(f"Invalid cast platform {platform}") - hass.data[DOMAIN][integration_domain] = platform + hass.data[DOMAIN]["cast_platform"][integration_domain] = platform async def async_remove_entry(hass: HomeAssistant, entry: ConfigEntry) -> None: diff --git a/homeassistant/components/cast/discovery.py b/homeassistant/components/cast/discovery.py index bc0f1435f8b..485d2888a41 100644 --- a/homeassistant/components/cast/discovery.py +++ b/homeassistant/components/cast/discovery.py @@ -34,7 +34,7 @@ def discover_chromecast( _LOGGER.error("Discovered chromecast without uuid %s", info) return - info = info.fill_out_missing_chromecast_info() + info = info.fill_out_missing_chromecast_info(hass) _LOGGER.debug("Discovered new or updated chromecast %s", info) dispatcher_send(hass, SIGNAL_CAST_DISCOVERED, info) diff --git a/homeassistant/components/cast/helpers.py b/homeassistant/components/cast/helpers.py index 5c6f0fee62a..dd98a2bc051 100644 --- a/homeassistant/components/cast/helpers.py +++ b/homeassistant/components/cast/helpers.py @@ -15,8 +15,11 @@ from pychromecast import dial from pychromecast.const import CAST_TYPE_GROUP from pychromecast.models import CastInfo +from homeassistant.core import HomeAssistant from homeassistant.helpers import aiohttp_client +from .const import DOMAIN + _LOGGER = logging.getLogger(__name__) _PLS_SECTION_PLAYLIST = "playlist" @@ -47,18 +50,50 @@ class ChromecastInfo: """Return the UUID.""" return self.cast_info.uuid - def fill_out_missing_chromecast_info(self) -> ChromecastInfo: + def fill_out_missing_chromecast_info(self, hass: HomeAssistant) -> ChromecastInfo: """Return a new ChromecastInfo object with missing attributes filled in. Uses blocking HTTP / HTTPS. """ cast_info = self.cast_info if self.cast_info.cast_type is None or self.cast_info.manufacturer is None: - # Manufacturer and cast type is not available in mDNS data, get it over http - cast_info = dial.get_cast_type( - cast_info, - zconf=ChromeCastZeroconf.get_zeroconf(), - ) + unknown_models = hass.data[DOMAIN]["unknown_models"] + if self.cast_info.model_name not in unknown_models: + # Manufacturer and cast type is not available in mDNS data, get it over http + cast_info = dial.get_cast_type( + cast_info, + zconf=ChromeCastZeroconf.get_zeroconf(), + ) + unknown_models[self.cast_info.model_name] = ( + cast_info.cast_type, + cast_info.manufacturer, + ) + + report_issue = ( + "create a bug report at " + "https://github.com/home-assistant/core/issues?q=is%3Aopen+is%3Aissue" + "+label%3A%22integration%3A+cast%22" + ) + + _LOGGER.info( + "Fetched cast details for unknown model '%s' manufacturer: '%s', type: '%s'. Please %s", + cast_info.model_name, + cast_info.manufacturer, + cast_info.cast_type, + report_issue, + ) + else: + cast_type, manufacturer = unknown_models[self.cast_info.model_name] + cast_info = CastInfo( + cast_info.services, + cast_info.uuid, + cast_info.model_name, + cast_info.friendly_name, + cast_info.host, + cast_info.port, + cast_type, + manufacturer, + ) if not self.is_audio_group or self.is_dynamic_group is not None: # We have all information, no need to check HTTP API. diff --git a/homeassistant/components/cast/manifest.json b/homeassistant/components/cast/manifest.json index 389b837f200..7144a3872f5 100644 --- a/homeassistant/components/cast/manifest.json +++ b/homeassistant/components/cast/manifest.json @@ -3,7 +3,7 @@ "name": "Google Cast", "config_flow": true, "documentation": "https://www.home-assistant.io/integrations/cast", - "requirements": ["pychromecast==12.0.0"], + "requirements": ["pychromecast==12.1.0"], "after_dependencies": [ "cloud", "http", diff --git a/homeassistant/components/cast/media_player.py b/homeassistant/components/cast/media_player.py index e63fedd3598..b64c3372c15 100644 --- a/homeassistant/components/cast/media_player.py +++ b/homeassistant/components/cast/media_player.py @@ -535,7 +535,7 @@ class CastMediaPlayerEntity(CastDevice, MediaPlayerEntity): """Generate root node.""" children = [] # Add media browsers - for platform in self.hass.data[CAST_DOMAIN].values(): + for platform in self.hass.data[CAST_DOMAIN]["cast_platform"].values(): children.extend( await platform.async_get_media_browser_root_object( self.hass, self._chromecast.cast_type @@ -587,7 +587,7 @@ class CastMediaPlayerEntity(CastDevice, MediaPlayerEntity): if media_content_id is None: return await self._async_root_payload(content_filter) - for platform in self.hass.data[CAST_DOMAIN].values(): + for platform in self.hass.data[CAST_DOMAIN]["cast_platform"].values(): browse_media = await platform.async_browse_media( self.hass, media_content_type, @@ -646,7 +646,7 @@ class CastMediaPlayerEntity(CastDevice, MediaPlayerEntity): return # Try the cast platforms - for platform in self.hass.data[CAST_DOMAIN].values(): + for platform in self.hass.data[CAST_DOMAIN]["cast_platform"].values(): result = await platform.async_play_media( self.hass, self.entity_id, self._chromecast, media_type, media_id ) diff --git a/requirements_all.txt b/requirements_all.txt index eb0b504bdaa..777cd79af74 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -1402,7 +1402,7 @@ pycfdns==1.2.2 pychannels==1.0.0 # homeassistant.components.cast -pychromecast==12.0.0 +pychromecast==12.1.0 # homeassistant.components.pocketcasts pycketcasts==1.0.0 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index 01f4a2d339a..205ac163595 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -941,7 +941,7 @@ pybotvac==0.0.23 pycfdns==1.2.2 # homeassistant.components.cast -pychromecast==12.0.0 +pychromecast==12.1.0 # homeassistant.components.climacell pyclimacell==0.18.2 diff --git a/tests/components/cast/conftest.py b/tests/components/cast/conftest.py index 3b96f378906..52152b4a718 100644 --- a/tests/components/cast/conftest.py +++ b/tests/components/cast/conftest.py @@ -14,6 +14,13 @@ def get_multizone_status_mock(): return mock +@pytest.fixture() +def get_cast_type_mock(): + """Mock pychromecast dial.""" + mock = MagicMock(spec_set=pychromecast.dial.get_cast_type) + return mock + + @pytest.fixture() def castbrowser_mock(): """Mock pychromecast CastBrowser.""" @@ -43,6 +50,7 @@ def cast_mock( mz_mock, quick_play_mock, castbrowser_mock, + get_cast_type_mock, get_chromecast_mock, get_multizone_status_mock, ): @@ -52,6 +60,9 @@ def cast_mock( with patch( "homeassistant.components.cast.discovery.pychromecast.discovery.CastBrowser", castbrowser_mock, + ), patch( + "homeassistant.components.cast.helpers.dial.get_cast_type", + get_cast_type_mock, ), patch( "homeassistant.components.cast.helpers.dial.get_multizone_status", get_multizone_status_mock, diff --git a/tests/components/cast/test_media_player.py b/tests/components/cast/test_media_player.py index 88cf281c8a5..e4df84f6443 100644 --- a/tests/components/cast/test_media_player.py +++ b/tests/components/cast/test_media_player.py @@ -64,6 +64,8 @@ FAKE_MDNS_SERVICE = pychromecast.discovery.ServiceInfo( pychromecast.const.SERVICE_TYPE_MDNS, "the-service" ) +UNDEFINED = object() + def get_fake_chromecast(info: ChromecastInfo): """Generate a Fake Chromecast object with the specified arguments.""" @@ -74,7 +76,14 @@ def get_fake_chromecast(info: ChromecastInfo): def get_fake_chromecast_info( - host="192.168.178.42", port=8009, service=None, uuid: UUID | None = FakeUUID + *, + host="192.168.178.42", + port=8009, + service=None, + uuid: UUID | None = FakeUUID, + cast_type=UNDEFINED, + manufacturer=UNDEFINED, + model_name=UNDEFINED, ): """Generate a Fake ChromecastInfo with the specified arguments.""" @@ -82,16 +91,22 @@ def get_fake_chromecast_info( service = pychromecast.discovery.ServiceInfo( pychromecast.const.SERVICE_TYPE_HOST, (host, port) ) + if cast_type is UNDEFINED: + cast_type = CAST_TYPE_GROUP if port != 8009 else CAST_TYPE_CHROMECAST + if manufacturer is UNDEFINED: + manufacturer = "Nabu Casa" + if model_name is UNDEFINED: + model_name = "Chromecast" return ChromecastInfo( cast_info=pychromecast.models.CastInfo( services={service}, uuid=uuid, - model_name="Chromecast", + model_name=model_name, friendly_name="Speaker", host=host, port=port, - cast_type=CAST_TYPE_GROUP if port != 8009 else CAST_TYPE_CHROMECAST, - manufacturer="Nabu Casa", + cast_type=cast_type, + manufacturer=manufacturer, ) ) @@ -342,6 +357,92 @@ async def test_internal_discovery_callback_fill_out_group( get_multizone_status_mock.assert_called_once() +async def test_internal_discovery_callback_fill_out_cast_type_manufacturer( + hass, get_cast_type_mock, caplog +): + """Test internal discovery automatically filling out information.""" + discover_cast, _, _ = await async_setup_cast_internal_discovery(hass) + info = get_fake_chromecast_info( + host="host1", + port=8009, + service=FAKE_MDNS_SERVICE, + cast_type=None, + manufacturer=None, + ) + info2 = get_fake_chromecast_info( + host="host1", + port=8009, + service=FAKE_MDNS_SERVICE, + cast_type=None, + manufacturer=None, + model_name="Model 101", + ) + zconf = get_fake_zconf(host="host1", port=8009) + full_info = attr.evolve( + info, + cast_info=pychromecast.discovery.CastInfo( + services=info.cast_info.services, + uuid=FakeUUID, + model_name="Chromecast", + friendly_name="Speaker", + host=info.cast_info.host, + port=info.cast_info.port, + cast_type="audio", + manufacturer="TrollTech", + ), + is_dynamic_group=None, + ) + full_info2 = attr.evolve( + info2, + cast_info=pychromecast.discovery.CastInfo( + services=info.cast_info.services, + uuid=FakeUUID, + model_name="Model 101", + friendly_name="Speaker", + host=info.cast_info.host, + port=info.cast_info.port, + cast_type="cast", + manufacturer="Cyberdyne Systems", + ), + is_dynamic_group=None, + ) + + get_cast_type_mock.assert_not_called() + get_cast_type_mock.return_value = full_info.cast_info + + with patch( + "homeassistant.components.cast.discovery.ChromeCastZeroconf.get_zeroconf", + return_value=zconf, + ): + signal = MagicMock() + + async_dispatcher_connect(hass, "cast_discovered", signal) + discover_cast(FAKE_MDNS_SERVICE, info) + await hass.async_block_till_done() + + # when called with incomplete info, it should use HTTP to get missing + get_cast_type_mock.assert_called_once() + assert get_cast_type_mock.call_count == 1 + discover = signal.mock_calls[0][1][0] + assert discover == full_info + assert "Fetched cast details for unknown model 'Chromecast'" in caplog.text + + # Call again, the model name should be fetched from cache + discover_cast(FAKE_MDNS_SERVICE, info) + await hass.async_block_till_done() + assert get_cast_type_mock.call_count == 1 # No additional calls + discover = signal.mock_calls[1][1][0] + assert discover == full_info + + # Call for another model, need to call HTTP again + get_cast_type_mock.return_value = full_info2.cast_info + discover_cast(FAKE_MDNS_SERVICE, info2) + await hass.async_block_till_done() + assert get_cast_type_mock.call_count == 2 + discover = signal.mock_calls[2][1][0] + assert discover == full_info2 + + async def test_stop_discovery_called_on_stop(hass, castbrowser_mock): """Test pychromecast.stop_discovery called on shutdown.""" # start_discovery should be called with empty config