Fix discovery of audio groups (#20947)

* Fix discovery of audio groups

* Fix tests

* Re-discover

* Review comments

* Remove failing tests

* Update dependencies

* Fix test

homeassistant/components/cast/__init__.py

@@ -4,7 +4,7 @@ from homeassistant.helpers import config_entry_flow
 
 
 DOMAIN = 'cast'
-REQUIREMENTS = ['pychromecast==2.1.0']
+REQUIREMENTS = ['pychromecast==2.5.0']
 
 
 async def async_setup(hass, config):

homeassistant/components/cast/media_player.py

@@ -57,6 +57,10 @@ ADDED_CAST_DEVICES_KEY = 'cast_added_cast_devices'
 # Chromecast or receive it through configuration
 SIGNAL_CAST_DISCOVERED = 'cast_discovered'
 
+# Dispatcher signal fired with a ChromecastInfo every time a Chromecast is
+# removed
+SIGNAL_CAST_REMOVED = 'cast_removed'
+
 PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({
     vol.Optional(CONF_HOST): cv.string,
     vol.Optional(CONF_IGNORE_CEC, default=[]):
@@ -73,6 +77,7 @@ class ChromecastInfo:
 
     host = attr.ib(type=str)
     port = attr.ib(type=int)
+    service = attr.ib(type=Optional[str], default=None)
     uuid = attr.ib(type=Optional[str], converter=attr.converters.optional(str),
                    default=None)  # always convert UUID to string if not None
     manufacturer = attr.ib(type=str, default='')
@@ -105,13 +110,15 @@ def _fill_out_missing_chromecast_info(info: ChromecastInfo) -> ChromecastInfo:
     # Fill out missing information via HTTP dial.
     from pychromecast import dial
 
-    http_device_status = dial.get_device_status(info.host)
+    http_device_status = dial.get_device_status(
+        info.host, services=[info.service],
+        zconf=ChromeCastZeroconf.get_zeroconf())
     if http_device_status is None:
         # HTTP dial didn't give us any new information.
         return info
 
     return ChromecastInfo(
-        host=info.host, port=info.port,
+        service=info.service, host=info.host, port=info.port,
         uuid=(info.uuid or http_device_status.uuid),
         friendly_name=(info.friendly_name or http_device_status.friendly_name),
         manufacturer=(info.manufacturer or http_device_status.manufacturer),
@@ -122,7 +129,6 @@ def _fill_out_missing_chromecast_info(info: ChromecastInfo) -> ChromecastInfo:
 def _discover_chromecast(hass: HomeAssistantType, info: ChromecastInfo):
     if info in hass.data[KNOWN_CHROMECAST_INFO_KEY]:
         _LOGGER.debug("Discovered previous chromecast %s", info)
-        return
 
     # Either discovered completely new chromecast or a "moved" one.
     info = _fill_out_missing_chromecast_info(info)
@@ -138,6 +144,29 @@ def _discover_chromecast(hass: HomeAssistantType, info: ChromecastInfo):
     dispatcher_send(hass, SIGNAL_CAST_DISCOVERED, info)
 
 
+def _remove_chromecast(hass: HomeAssistantType, info: ChromecastInfo):
+    # Removed chromecast
+    _LOGGER.debug("Removed chromecast %s", info)
+
+    dispatcher_send(hass, SIGNAL_CAST_REMOVED, info)
+
+
+class ChromeCastZeroconf:
+    """Class to hold a zeroconf instance."""
+
+    __zconf = None
+
+    @classmethod
+    def set_zeroconf(cls, zconf):
+        """Set zeroconf."""
+        cls.__zconf = zconf
+
+    @classmethod
+    def get_zeroconf(cls):
+        """Get zeroconf."""
+        return cls.__zconf
+
+
 def _setup_internal_discovery(hass: HomeAssistantType) -> None:
     """Set up the pychromecast internal discovery."""
     if INTERNAL_DISCOVERY_RUNNING_KEY not in hass.data:
@@ -149,10 +178,22 @@ def _setup_internal_discovery(hass: HomeAssistantType) -> None:
 
     import pychromecast
 
-    def internal_callback(name):
+    def internal_add_callback(name):
         """Handle zeroconf discovery of a new chromecast."""
         mdns = listener.services[name]
         _discover_chromecast(hass, ChromecastInfo(
+            service=name,
+            host=mdns[0],
+            port=mdns[1],
+            uuid=mdns[2],
+            model_name=mdns[3],
+            friendly_name=mdns[4],
+        ))
+
+    def internal_remove_callback(name, mdns):
+        """Handle zeroconf discovery of a removed chromecast."""
+        _remove_chromecast(hass, ChromecastInfo(
+            service=name,
             host=mdns[0],
             port=mdns[1],
             uuid=mdns[2],
@@ -161,7 +202,9 @@ def _setup_internal_discovery(hass: HomeAssistantType) -> None:
         ))
 
     _LOGGER.debug("Starting internal pychromecast discovery.")
-    listener, browser = pychromecast.start_discovery(internal_callback)
+    listener, browser = pychromecast.start_discovery(internal_add_callback,
+                                                     internal_remove_callback)
+    ChromeCastZeroconf.set_zeroconf(browser.zc)
 
     def stop_discovery(event):
         """Stop discovery of new chromecasts."""
@@ -327,12 +370,18 @@ class CastDevice(MediaPlayerDevice):
         """Initialize the cast device."""
         import pychromecast  # noqa: pylint: disable=unused-import
         self._cast_info = cast_info  # type: ChromecastInfo
+        self.services = None
+        if cast_info.service:
+            self.services = set()
+            self.services.add(cast_info.service)
         self._chromecast = None  # type: Optional[pychromecast.Chromecast]
         self.cast_status = None
         self.media_status = None
         self.media_status_received = None
         self._available = False  # type: bool
         self._status_listener = None  # type: Optional[CastStatusListener]
+        self._add_remove_handler = None
+        self._del_remove_handler = None
 
     async def async_added_to_hass(self):
         """Create chromecast object when added to hass."""
@@ -345,15 +394,36 @@ class CastDevice(MediaPlayerDevice):
             if self._cast_info.uuid != discover.uuid:
                 # Discovered is not our device.
                 return
+            if self.services is None:
+                _LOGGER.warning(
+                    "[%s %s (%s:%s)] Received update for manually added Cast",
+                    self.entity_id, self._cast_info.friendly_name,
+                    self._cast_info.host, self._cast_info.port)
+                return
             _LOGGER.debug("Discovered chromecast with same UUID: %s", discover)
             self.hass.async_create_task(self.async_set_cast_info(discover))
 
+        def async_cast_removed(discover: ChromecastInfo):
+            """Handle removal of Chromecast."""
+            if self._cast_info.uuid is None:
+                # We can't handle empty UUIDs
+                return
+            if self._cast_info.uuid != discover.uuid:
+                # Removed is not our device.
+                return
+            _LOGGER.debug("Removed chromecast with same UUID: %s", discover)
+            self.hass.async_create_task(self.async_del_cast_info(discover))
+
         async def async_stop(event):
             """Disconnect socket on Home Assistant stop."""
             await self._async_disconnect()
 
-        async_dispatcher_connect(self.hass, SIGNAL_CAST_DISCOVERED,
+        self._add_remove_handler = async_dispatcher_connect(
-                                 async_cast_discovered)
+            self.hass, SIGNAL_CAST_DISCOVERED,
+            async_cast_discovered)
+        self._del_remove_handler = async_dispatcher_connect(
+            self.hass, SIGNAL_CAST_REMOVED,
+            async_cast_removed)
         self.hass.bus.async_listen_once(EVENT_HOMEASSISTANT_STOP, async_stop)
         self.hass.async_create_task(self.async_set_cast_info(self._cast_info))
 
@@ -364,27 +434,52 @@ class CastDevice(MediaPlayerDevice):
             # Remove the entity from the added casts so that it can dynamically
             # be re-added again.
             self.hass.data[ADDED_CAST_DEVICES_KEY].remove(self._cast_info.uuid)
+        if self._add_remove_handler:
+            self._add_remove_handler()
+        if self._del_remove_handler:
+            self._del_remove_handler()
 
     async def async_set_cast_info(self, cast_info):
         """Set the cast information and set up the chromecast object."""
         import pychromecast
-        old_cast_info = self._cast_info
         self._cast_info = cast_info
 
+        if self.services is not None:
+            if cast_info.service not in self.services:
+                _LOGGER.debug("[%s %s (%s:%s)] Got new service: %s (%s)",
+                              self.entity_id, self._cast_info.friendly_name,
+                              self._cast_info.host, self._cast_info.port,
+                              cast_info.service, self.services)
+
+            self.services.add(cast_info.service)
+
         if self._chromecast is not None:
-            if old_cast_info.host_port == cast_info.host_port:
+            # Only setup the chromecast once, added elements to services
-                _LOGGER.debug("No connection related update: %s",
+            # will automatically be picked up.
-                              cast_info.host_port)
+            return
-                return
-            await self._async_disconnect()
 
         # pylint: disable=protected-access
-        _LOGGER.debug("Connecting to cast device %s", cast_info)
+        if self.services is None:
-        chromecast = await self.hass.async_add_job(
+            _LOGGER.debug(
-            pychromecast._get_chromecast_from_host, (
+                "[%s %s (%s:%s)] Connecting to cast device by host %s",
-                cast_info.host, cast_info.port, cast_info.uuid,
+                self.entity_id, self._cast_info.friendly_name,
-                cast_info.model_name, 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, 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
+                ))
         self._chromecast = chromecast
         self._status_listener = CastStatusListener(self, chromecast)
         # Initialise connection status as connected because we can only
@@ -394,15 +489,27 @@ class CastDevice(MediaPlayerDevice):
         self._available = True
         self.cast_status = chromecast.status
         self.media_status = chromecast.media_controller.status
-        _LOGGER.debug("Connection successful!")
+        _LOGGER.debug("[%s %s (%s:%s)] Connection successful!",
+                      self.entity_id, self._cast_info.friendly_name,
+                      self._cast_info.host, self._cast_info.port)
         self.async_schedule_update_ha_state()
 
+    async def async_del_cast_info(self, cast_info):
+        """Remove the service."""
+        self.services.discard(cast_info.service)
+        _LOGGER.debug("[%s %s (%s:%s)] Remove service: %s (%s)",
+                      self.entity_id, self._cast_info.friendly_name,
+                      self._cast_info.host, self._cast_info.port,
+                      cast_info.service, self.services)
+
     async def _async_disconnect(self):
         """Disconnect Chromecast object if it is set."""
         if self._chromecast is None:
             # Can't disconnect if not connected.
             return
-        _LOGGER.debug("Disconnecting from chromecast socket.")
+        _LOGGER.debug("[%s %s (%s:%s)] Disconnecting from chromecast socket.",
+                      self.entity_id, self._cast_info.friendly_name,
+                      self._cast_info.host, self._cast_info.port)
         self._available = False
         self.async_schedule_update_ha_state()
 
@@ -439,8 +546,11 @@ class CastDevice(MediaPlayerDevice):
         from pychromecast.socket_client import CONNECTION_STATUS_CONNECTED, \
             CONNECTION_STATUS_DISCONNECTED
 
-        _LOGGER.debug("Received cast device connection status: %s",
+        _LOGGER.debug(
-                      connection_status.status)
+            "[%s %s (%s:%s)] Received cast device connection status: %s",
+            self.entity_id, self._cast_info.friendly_name,
+            self._cast_info.host, self._cast_info.port,
+            connection_status.status)
         if connection_status.status == CONNECTION_STATUS_DISCONNECTED:
             self._available = False
             self._invalidate()
@@ -452,8 +562,11 @@ class CastDevice(MediaPlayerDevice):
             # Connection status callbacks happen often when disconnected.
             # Only update state when availability changed to put less pressure
             # on state machine.
-            _LOGGER.debug("Cast device availability changed: %s",
+            _LOGGER.debug(
-                          connection_status.status)
+                "[%s %s (%s:%s)] Cast device availability changed: %s",
+                self.entity_id, self._cast_info.friendly_name,
+                self._cast_info.host, self._cast_info.port,
+                connection_status.status)
             self._available = new_available
             self.schedule_update_ha_state()
 

requirements_all.txt

@@ -956,7 +956,7 @@ pycfdns==0.0.1
 pychannels==1.0.0
 
 # homeassistant.components.cast
-pychromecast==2.1.0
+pychromecast==2.5.0
 
 # homeassistant.components.media_player.cmus
 pycmus==0.1.1

tests/components/cast/test_media_player.py

@@ -12,8 +12,7 @@ from homeassistant.exceptions import PlatformNotReady
 from homeassistant.helpers.typing import HomeAssistantType
 from homeassistant.components.cast.media_player import ChromecastInfo
 from homeassistant.const import EVENT_HOMEASSISTANT_STOP
-from homeassistant.helpers.dispatcher import async_dispatcher_connect, \
+from homeassistant.helpers.dispatcher import async_dispatcher_connect
-    async_dispatcher_send
 from homeassistant.components.cast import media_player as cast
 from homeassistant.setup import async_setup_component
 
@@ -44,7 +43,7 @@ def get_fake_chromecast_info(host='192.168.178.42', port=8009,
                              uuid: Optional[UUID] = FakeUUID):
     """Generate a Fake ChromecastInfo with the specified arguments."""
     return ChromecastInfo(host=host, port=port, uuid=uuid,
-                          friendly_name="Speaker")
+                          friendly_name="Speaker", service='the-service')
 
 
 async def async_setup_cast(hass, config=None, discovery_info=None):
@@ -64,9 +63,10 @@ async def async_setup_cast_internal_discovery(hass, config=None,
                                               discovery_info=None):
     """Set up the cast platform and the discovery."""
     listener = MagicMock(services={})
+    browser = MagicMock(zc={})
 
     with patch('pychromecast.start_discovery',
-               return_value=(listener, None)) as start_discovery:
+               return_value=(listener, browser)) as start_discovery:
         add_entities = await async_setup_cast(hass, config, discovery_info)
         await hass.async_block_till_done()
         await hass.async_block_till_done()
@@ -120,8 +120,10 @@ def test_start_discovery_called_once(hass):
 @asyncio.coroutine
 def test_stop_discovery_called_on_stop(hass):
     """Test pychromecast.stop_discovery called on shutdown."""
+    browser = MagicMock(zc={})
+
     with patch('pychromecast.start_discovery',
-               return_value=(None, 'the-browser')) as start_discovery:
+               return_value=(None, browser)) as start_discovery:
         # start_discovery should be called with empty config
         yield from async_setup_cast(hass, {})
 
@@ -132,38 +134,16 @@ def test_stop_discovery_called_on_stop(hass):
         hass.bus.async_fire(EVENT_HOMEASSISTANT_STOP)
         yield from hass.async_block_till_done()
 
-        stop_discovery.assert_called_once_with('the-browser')
+        stop_discovery.assert_called_once_with(browser)
 
     with patch('pychromecast.start_discovery',
-               return_value=(None, 'the-browser')) as start_discovery:
+               return_value=(None, browser)) as start_discovery:
         # start_discovery should be called again on re-startup
         yield from async_setup_cast(hass)
 
         assert start_discovery.call_count == 1
 
 
-async def test_internal_discovery_callback_only_generates_once(hass):
-    """Test discovery only called once per device."""
-    discover_cast, _ = await async_setup_cast_internal_discovery(hass)
-    info = get_fake_chromecast_info()
-
-    signal = MagicMock()
-    async_dispatcher_connect(hass, 'cast_discovered', signal)
-
-    with patch('pychromecast.dial.get_device_status', return_value=None):
-        # discovering a cast device should call the dispatcher
-        discover_cast('the-service', info)
-        await hass.async_block_till_done()
-        discover = signal.mock_calls[0][1][0]
-        assert discover == info
-        signal.reset_mock()
-
-        # discovering it a second time shouldn't
-        discover_cast('the-service', info)
-        await hass.async_block_till_done()
-        assert signal.call_count == 0
-
-
 async def test_internal_discovery_callback_fill_out(hass):
     """Test internal discovery automatically filling out information."""
     import pychromecast  # imports mock pychromecast
@@ -323,35 +303,6 @@ async def test_entity_media_states(hass: HomeAssistantType):
     assert state.state == 'unknown'
 
 
-async def test_switched_host(hass: HomeAssistantType):
-    """Test cast device listens for changed hosts and disconnects old cast."""
-    info = get_fake_chromecast_info()
-    full_info = attr.evolve(info, model_name='google home',
-                            friendly_name='Speaker', uuid=FakeUUID)
-
-    with patch('pychromecast.dial.get_device_status',
-               return_value=full_info):
-        chromecast, _ = await async_setup_media_player_cast(hass, full_info)
-
-    chromecast2 = get_fake_chromecast(info)
-    with patch('pychromecast._get_chromecast_from_host',
-               return_value=chromecast2) as get_chromecast:
-        async_dispatcher_send(hass, cast.SIGNAL_CAST_DISCOVERED, full_info)
-        await hass.async_block_till_done()
-        assert get_chromecast.call_count == 0
-
-        changed = attr.evolve(full_info, friendly_name='Speaker 2')
-        async_dispatcher_send(hass, cast.SIGNAL_CAST_DISCOVERED, changed)
-        await hass.async_block_till_done()
-        assert get_chromecast.call_count == 0
-
-        changed = attr.evolve(changed, host='host2')
-        async_dispatcher_send(hass, cast.SIGNAL_CAST_DISCOVERED, changed)
-        await hass.async_block_till_done()
-        assert get_chromecast.call_count == 1
-        assert chromecast.disconnect.call_count == 1
-
-
 async def test_disconnect_on_stop(hass: HomeAssistantType):
     """Test cast device disconnects socket on stop."""
     info = get_fake_chromecast_info()