Do not use pychromecast.Chromecast for Cast Groups (#8786)

* Do not use pychromecast.Chromecast for Cast Groups

pychromecast.Chromecast creates Chromecast instance with friendly_name and cast_type of the device and not of a group.
Which leads to collisions

* Update cast.py

* using hass.data

* Fixed and extended tests

* Line length in tests

* Lint in tests

homeassistant/components/media_player/cast.py

@@ -33,7 +33,7 @@ SUPPORT_CAST = SUPPORT_PAUSE | SUPPORT_VOLUME_SET | SUPPORT_VOLUME_MUTE | \
     SUPPORT_TURN_ON | SUPPORT_TURN_OFF | SUPPORT_PREVIOUS_TRACK | \
     SUPPORT_NEXT_TRACK | SUPPORT_PLAY_MEDIA | SUPPORT_STOP | SUPPORT_PLAY
 
-KNOWN_HOSTS = []
+KNOWN_HOSTS_KEY = 'cast_known_hosts'
 
 PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({
     vol.Optional(CONF_HOST): cv.string,
@@ -49,22 +49,29 @@ def setup_platform(hass, config, add_devices, discovery_info=None):
     # Import CEC IGNORE attributes
     pychromecast.IGNORE_CEC += config.get(CONF_IGNORE_CEC, [])
 
-    hosts = []
+    known_hosts = hass.data.get(KNOWN_HOSTS_KEY)
+    if known_hosts is None:
+        known_hosts = hass.data[KNOWN_HOSTS_KEY] = []
 
     if discovery_info:
         host = (discovery_info.get('host'), discovery_info.get('port'))
 
-        if host in KNOWN_HOSTS:
+        if host in known_hosts:
             return
 
         hosts = [host]
 
     elif CONF_HOST in config:
-        hosts = [(config.get(CONF_HOST), DEFAULT_PORT)]
+        host = (config.get(CONF_HOST), DEFAULT_PORT)
+
+        if host in known_hosts:
+            return
+
+        hosts = [host]
 
     else:
         hosts = [tuple(dev[:2]) for dev in pychromecast.discover_chromecasts()
-                 if tuple(dev[:2]) not in KNOWN_HOSTS]
+                 if tuple(dev[:2]) not in known_hosts]
 
     casts = []
 
@@ -73,19 +80,24 @@ def setup_platform(hass, config, add_devices, discovery_info=None):
     all_chromecasts = pychromecast.get_chromecasts()
 
     for host in hosts:
+        (_, port) = host
         found = [device for device in all_chromecasts
                  if (device.host, device.port) == host]
         if found:
             try:
                 casts.append(CastDevice(found[0]))
-                KNOWN_HOSTS.append(host)
+                known_hosts.append(host)
             except pychromecast.ChromecastConnectionError:
                 pass
-        else:
+
+        # do not add groups using pychromecast.Chromecast as it leads to names
+        # collision since pychromecast.Chromecast will get device name instead
+        # of group name
+        elif port == DEFAULT_PORT:
             try:
                 # add the device anyway, get_chromecasts couldn't find it
                 casts.append(CastDevice(pychromecast.Chromecast(*host)))
-                KNOWN_HOSTS.append(host)
+                known_hosts.append(host)
             except pychromecast.ChromecastConnectionError:
                 pass
 

tests/components/media_player/test_cast.py

@@ -6,6 +6,7 @@ from unittest.mock import patch, MagicMock
 import pytest
 
 from homeassistant.components.media_player import cast
+from tests.common import get_test_home_assistant
 
 
 @pytest.fixture(autouse=True)
@@ -29,6 +30,14 @@ class FakeChromeCast(object):
 class TestCastMediaPlayer(unittest.TestCase):
     """Test the media_player module."""
 
+    def setUp(self):
+        """Setup things to be run when tests are started."""
+        self.hass = get_test_home_assistant()
+
+    def tearDown(self):
+        """Stop everything that was started."""
+        self.hass.stop()
+
     @patch('homeassistant.components.media_player.cast.CastDevice')
     @patch('pychromecast.get_chromecasts')
     def test_filter_duplicates(self, mock_get_chromecasts, mock_device):
@@ -38,7 +47,7 @@ class TestCastMediaPlayer(unittest.TestCase):
         ]
 
         # Test chromecasts as if they were hardcoded in configuration.yaml
-        cast.setup_platform(None, {
+        cast.setup_platform(self.hass, {
             'host': 'some_host'
         }, lambda _: _)
 
@@ -48,8 +57,44 @@ class TestCastMediaPlayer(unittest.TestCase):
         assert not mock_device.called
 
         # Test chromecasts as if they were automatically discovered
-        cast.setup_platform(None, {}, lambda _: _, {
+        cast.setup_platform(self.hass, {}, lambda _: _, {
             'host': 'some_host',
             'port': cast.DEFAULT_PORT,
         })
         assert not mock_device.called
+
+    @patch('homeassistant.components.media_player.cast.CastDevice')
+    @patch('pychromecast.get_chromecasts')
+    @patch('pychromecast.Chromecast')
+    def test_fallback_cast(self, mock_chromecast, mock_get_chromecasts,
+                           mock_device):
+        """Test falling back to creating Chromecast when not discovered."""
+        mock_get_chromecasts.return_value = [
+            FakeChromeCast('some_host', cast.DEFAULT_PORT)
+        ]
+
+        # Test chromecasts as if they were hardcoded in configuration.yaml
+        cast.setup_platform(self.hass, {
+            'host': 'some_other_host'
+        }, lambda _: _)
+
+        assert mock_chromecast.called
+        assert mock_device.called
+
+    @patch('homeassistant.components.media_player.cast.CastDevice')
+    @patch('pychromecast.get_chromecasts')
+    @patch('pychromecast.Chromecast')
+    def test_fallback_cast_group(self, mock_chromecast, mock_get_chromecasts,
+                                 mock_device):
+        """Test not creating Cast Group when not discovered."""
+        mock_get_chromecasts.return_value = [
+            FakeChromeCast('some_host', cast.DEFAULT_PORT)
+        ]
+
+        # Test chromecasts as if they were automatically discovered
+        cast.setup_platform(self.hass, {}, lambda _: _, {
+            'host': 'some_other_host',
+            'port': 43546,
+        })
+        assert not mock_chromecast.called
+        assert not mock_device.called