From ec2c7ea932ca686d462f30c0ac09f4f04eca547b Mon Sep 17 00:00:00 2001 From: Erik Montnemery Date: Mon, 13 Apr 2020 00:02:28 +0200 Subject: [PATCH] Don't do http requests to determine Cast device details (#34082) * Don't do http requests to determine cast device details * Fix tests * Update homeassistant/components/cast/media_player.py Co-authored-by: Paulus Schoutsen --- homeassistant/components/cast/discovery.py | 1 - homeassistant/components/cast/helpers.py | 43 ------------ homeassistant/components/cast/media_player.py | 65 ++++++------------- tests/components/cast/test_media_player.py | 56 ++-------------- 4 files changed, 26 insertions(+), 139 deletions(-) diff --git a/homeassistant/components/cast/discovery.py b/homeassistant/components/cast/discovery.py index 6991c83959d..9f90b074c3d 100644 --- a/homeassistant/components/cast/discovery.py +++ b/homeassistant/components/cast/discovery.py @@ -25,7 +25,6 @@ def discover_chromecast(hass: HomeAssistant, info: ChromecastInfo): _LOGGER.debug("Discovered previous chromecast %s", info) # Either discovered completely new chromecast or a "moved" one. - info = info.fill_out_missing_chromecast_info() _LOGGER.debug("Discovered chromecast %s", info) if info.uuid is not None: diff --git a/homeassistant/components/cast/helpers.py b/homeassistant/components/cast/helpers.py index bd8ce68c5f1..5a99d30f087 100644 --- a/homeassistant/components/cast/helpers.py +++ b/homeassistant/components/cast/helpers.py @@ -2,7 +2,6 @@ from typing import Optional, Tuple import attr -from pychromecast import dial from pychromecast.const import CAST_MANUFACTURERS from .const import DEFAULT_PORT @@ -29,11 +28,6 @@ class ChromecastInfo: """Return if this is an audio group.""" return self.port != DEFAULT_PORT - @property - def is_information_complete(self) -> bool: - """Return if all information is filled out.""" - return all(attr.astuple(self)) - @property def host_port(self) -> Tuple[str, int]: """Return the host+port tuple.""" @@ -46,43 +40,6 @@ class ChromecastInfo: return None return CAST_MANUFACTURERS.get(self.model_name.lower(), "Google Inc.") - def fill_out_missing_chromecast_info(self) -> "ChromecastInfo": - """Return a new ChromecastInfo object with missing attributes filled in. - - Uses blocking HTTP. - """ - if self.is_information_complete: - # We have all information, no need to check HTTP API. Or this is an - # audio group, so checking via HTTP won't give us any new information. - return self - - if self.is_audio_group: - return ChromecastInfo( - service=self.service, - host=self.host, - port=self.port, - uuid=self.uuid, - friendly_name=self.friendly_name, - model_name=self.model_name, - ) - - # Fill out some missing information (friendly_name, uuid) via HTTP dial. - http_device_status = dial.get_device_status( - self.host, services=[self.service], zconf=ChromeCastZeroconf.get_zeroconf() - ) - if http_device_status is None: - # HTTP dial didn't give us any new information. - return self - - return ChromecastInfo( - service=self.service, - host=self.host, - port=self.port, - uuid=(self.uuid or http_device_status.uuid), - friendly_name=(self.friendly_name or http_device_status.friendly_name), - model_name=self.model_name, - ) - class ChromeCastZeroconf: """Class to hold a zeroconf instance.""" diff --git a/homeassistant/components/cast/media_player.py b/homeassistant/components/cast/media_player.py index 5fcbbaf1caa..dadd528c649 100644 --- a/homeassistant/components/cast/media_player.py +++ b/homeassistant/components/cast/media_player.py @@ -91,9 +91,8 @@ def _async_create_cast_device(hass: HomeAssistantType, info: ChromecastInfo): """ _LOGGER.debug("_async_create_cast_device: %s", info) if info.uuid is None: - # Found a cast without UUID, we don't store it because we won't be able - # to update it anyway. - return CastDevice(info) + _LOGGER.error("_async_create_cast_device uuid none: %s", info) + return None # Found a cast with UUID added_casts = hass.data[ADDED_CAST_DEVICES_KEY] @@ -156,7 +155,7 @@ async def _async_setup_platform( def async_cast_discovered(discover: ChromecastInfo) -> None: """Handle discovery of a new chromecast.""" if info is not None and info.host_port != discover.host_port: - # Not our requested cast device. + # Waiting for a specific cast device, this is not it. return cast_device = _async_create_cast_device(hass, discover) @@ -262,44 +261,24 @@ class CastDevice(MediaPlayerDevice): return # pylint: disable=protected-access - if self.services is None: - _LOGGER.debug( - "[%s %s (%s:%s)] Connecting to cast device by host %s", - self.entity_id, - self._cast_info.friendly_name, - self._cast_info.host, - self._cast_info.port, - cast_info, - ) - chromecast = await self.hass.async_add_job( - pychromecast._get_chromecast_from_host, - ( - cast_info.host, - cast_info.port, - cast_info.uuid, - cast_info.model_name, - cast_info.friendly_name, - ), - ) - else: - _LOGGER.debug( - "[%s %s (%s:%s)] Connecting to cast device by service %s", - self.entity_id, - self._cast_info.friendly_name, - self._cast_info.host, - self._cast_info.port, + _LOGGER.debug( + "[%s %s (%s:%s)] Connecting to cast device by service %s", + self.entity_id, + self._cast_info.friendly_name, + self._cast_info.host, + self._cast_info.port, + self.services, + ) + chromecast = await self.hass.async_add_executor_job( + pychromecast._get_chromecast_from_service, + ( self.services, - ) - chromecast = await self.hass.async_add_job( - pychromecast._get_chromecast_from_service, - ( - self.services, - ChromeCastZeroconf.get_zeroconf(), - cast_info.uuid, - cast_info.model_name, - cast_info.friendly_name, - ), - ) + ChromeCastZeroconf.get_zeroconf(), + cast_info.uuid, + cast_info.model_name, + cast_info.friendly_name, + ), + ) self._chromecast = chromecast if CAST_MULTIZONE_MANAGER_KEY not in self.hass.data: @@ -403,10 +382,6 @@ class CastDevice(MediaPlayerDevice): self._cast_info.port, connection_status.status, ) - info = self._cast_info - if info.friendly_name is None and not info.is_audio_group: - # We couldn't find friendly_name when the cast was added, retry - self._cast_info = info.fill_out_missing_chromecast_info() self._available = new_available self.schedule_update_ha_state() diff --git a/tests/components/cast/test_media_player.py b/tests/components/cast/test_media_player.py index b05be45e79f..f241b79f26e 100644 --- a/tests/components/cast/test_media_player.py +++ b/tests/components/cast/test_media_player.py @@ -11,7 +11,6 @@ from homeassistant.components.cast import media_player as cast from homeassistant.components.cast.media_player import ChromecastInfo from homeassistant.const import EVENT_HOMEASSISTANT_STOP from homeassistant.exceptions import PlatformNotReady -from homeassistant.helpers.dispatcher import async_dispatcher_connect from homeassistant.helpers.typing import HomeAssistantType from homeassistant.setup import async_setup_component @@ -33,8 +32,6 @@ def cast_mock(): "homeassistant.components.cast.media_player.pychromecast", pycast_mock ), patch( "homeassistant.components.cast.discovery.pychromecast", pycast_mock - ), patch( - "homeassistant.components.cast.helpers.dial", dial_mock ), patch( "homeassistant.components.cast.media_player.MultizoneManager", MagicMock() ): @@ -199,36 +196,11 @@ async def test_stop_discovery_called_on_stop(hass): assert start_discovery.call_count == 1 -async def test_internal_discovery_callback_fill_out(hass): - """Test internal discovery automatically filling out information.""" - import pychromecast # imports mock pychromecast - - pychromecast.ChromecastConnectionError = IOError - - discover_cast, _ = await async_setup_cast_internal_discovery(hass) - info = get_fake_chromecast_info(uuid=None) - full_info = attr.evolve(info, model_name="", friendly_name="Speaker", uuid=FakeUUID) - - with patch( - "homeassistant.components.cast.helpers.dial.get_device_status", - return_value=full_info, - ): - signal = MagicMock() - - async_dispatcher_connect(hass, "cast_discovered", signal) - discover_cast("the-service", info) - await hass.async_block_till_done() - - # when called with incomplete info, it should use HTTP to get missing - discover = signal.mock_calls[0][1][0] - assert discover == full_info - - async def test_create_cast_device_without_uuid(hass): - """Test create a cast device with no UUId should still create an entity.""" + """Test create a cast device with no UUId does not create an entity.""" info = get_fake_chromecast_info(uuid=None) cast_device = cast._async_create_cast_device(hass, info) - assert cast_device is not None + assert cast_device is None async def test_create_cast_device_with_uuid(hass): @@ -334,11 +306,7 @@ async def test_entity_media_states(hass: HomeAssistantType): info, model_name="google home", friendly_name="Speaker", uuid=FakeUUID ) - with patch( - "homeassistant.components.cast.helpers.dial.get_device_status", - return_value=full_info, - ): - chromecast, entity = await async_setup_media_player_cast(hass, info) + chromecast, entity = await async_setup_media_player_cast(hass, info) entity._available = True entity.schedule_update_ha_state() @@ -392,11 +360,7 @@ async def test_group_media_states(hass: HomeAssistantType): info, model_name="google home", friendly_name="Speaker", uuid=FakeUUID ) - with patch( - "homeassistant.components.cast.helpers.dial.get_device_status", - return_value=full_info, - ): - chromecast, entity = await async_setup_media_player_cast(hass, info) + chromecast, entity = await async_setup_media_player_cast(hass, info) entity._available = True entity.schedule_update_ha_state() @@ -442,11 +406,7 @@ async def test_group_media_control(hass: HomeAssistantType): info, model_name="google home", friendly_name="Speaker", uuid=FakeUUID ) - with patch( - "homeassistant.components.cast.helpers.dial.get_device_status", - return_value=full_info, - ): - chromecast, entity = await async_setup_media_player_cast(hass, info) + chromecast, entity = await async_setup_media_player_cast(hass, info) entity._available = True entity.async_write_ha_state() @@ -495,11 +455,7 @@ async def test_disconnect_on_stop(hass: HomeAssistantType): """Test cast device disconnects socket on stop.""" info = get_fake_chromecast_info() - with patch( - "homeassistant.components.cast.helpers.dial.get_device_status", - return_value=info, - ): - chromecast, _ = await async_setup_media_player_cast(hass, info) + chromecast, _ = await async_setup_media_player_cast(hass, info) hass.bus.async_fire(EVENT_HOMEASSISTANT_STOP) await hass.async_block_till_done()