From 564ad7e22a04273a6c9bee6d096b72b6ef2bc11a Mon Sep 17 00:00:00 2001
From: Jason Hu <awarecan@users.noreply.github.com>
Date: Sun, 23 Sep 2018 14:35:07 -0700
Subject: [PATCH] Rework chromecast fix (#16804)

* Revert changes of #16471, and fix the platform setup issue

* Fix unit test

* Fix

* Fix comment

* Fix test

* Address review comment

* Address review comment
---
 homeassistant/components/media_player/cast.py | 20 ++++++++++---------
 tests/common.py                               | 10 ++++++----
 tests/components/media_player/test_cast.py    | 20 +++++++++++++++++++
 3 files changed, 37 insertions(+), 13 deletions(-)

diff --git a/homeassistant/components/media_player/cast.py b/homeassistant/components/media_player/cast.py
index a05653cd115..67d8ea0b419 100644
--- a/homeassistant/components/media_player/cast.py
+++ b/homeassistant/components/media_player/cast.py
@@ -61,10 +61,6 @@ PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({
         vol.All(cv.ensure_list, [cv.string]),
 })
 
-CONNECTION_RETRY = 3
-CONNECTION_RETRY_WAIT = 2
-CONNECTION_TIMEOUT = 10
-
 
 @attr.s(slots=True, frozen=True)
 class ChromecastInfo:
@@ -216,9 +212,12 @@ async def async_setup_entry(hass, config_entry, async_add_entities):
     if not isinstance(config, list):
         config = [config]
 
-    await asyncio.wait([
+    # no pending task
+    done, _ = await asyncio.wait([
         _async_setup_platform(hass, cfg, async_add_entities, None)
         for cfg in config])
+    if any([task.exception() for task in done]):
+        raise PlatformNotReady
 
 
 async def _async_setup_platform(hass: HomeAssistantType, config: ConfigType,
@@ -250,8 +249,8 @@ async def _async_setup_platform(hass: HomeAssistantType, config: ConfigType,
         if cast_device is not None:
             async_add_entities([cast_device])
 
-    async_dispatcher_connect(hass, SIGNAL_CAST_DISCOVERED,
-                             async_cast_discovered)
+    remove_handler = async_dispatcher_connect(
+        hass, SIGNAL_CAST_DISCOVERED, async_cast_discovered)
     # Re-play the callback for all past chromecasts, store the objects in
     # a list to avoid concurrent modification resulting in exception.
     for chromecast in list(hass.data[KNOWN_CHROMECAST_INFO_KEY]):
@@ -265,8 +264,11 @@ async def _async_setup_platform(hass: HomeAssistantType, config: ConfigType,
         info = await hass.async_add_job(_fill_out_missing_chromecast_info,
                                         info)
         if info.friendly_name is None:
-            # HTTP dial failed, so we won't be able to connect.
+            _LOGGER.debug("Cannot retrieve detail information for chromecast"
+                          " %s, the device may not be online", info)
+            remove_handler()
             raise PlatformNotReady
+
         hass.async_add_job(_discover_chromecast, hass, info)
 
 
@@ -379,7 +381,7 @@ class CastDevice(MediaPlayerDevice):
             pychromecast._get_chromecast_from_host, (
                 cast_info.host, cast_info.port, cast_info.uuid,
                 cast_info.model_name, cast_info.friendly_name
-            ), CONNECTION_RETRY, CONNECTION_RETRY_WAIT, CONNECTION_TIMEOUT)
+            ))
         self._chromecast = chromecast
         self._status_listener = CastStatusListener(self, chromecast)
         # Initialise connection status as connected because we can only
diff --git a/tests/common.py b/tests/common.py
index 287f1e24ee5..0cb15d683b5 100644
--- a/tests/common.py
+++ b/tests/common.py
@@ -609,16 +609,18 @@ def patch_yaml_files(files_dict, endswith=True):
     return patch.object(yaml, 'open', mock_open_f, create=True)
 
 
-def mock_coro(return_value=None):
-    """Return a coro that returns a value."""
-    return mock_coro_func(return_value)()
+def mock_coro(return_value=None, exception=None):
+    """Return a coro that returns a value or raise an exception."""
+    return mock_coro_func(return_value, exception)()
 
 
-def mock_coro_func(return_value=None):
+def mock_coro_func(return_value=None, exception=None):
     """Return a method to create a coro function that returns a value."""
     @asyncio.coroutine
     def coro(*args, **kwargs):
         """Fake coroutine."""
+        if exception:
+            raise exception
         return return_value
 
     return coro
diff --git a/tests/components/media_player/test_cast.py b/tests/components/media_player/test_cast.py
index 7345fd0c158..8fd1ae18841 100644
--- a/tests/components/media_player/test_cast.py
+++ b/tests/components/media_player/test_cast.py
@@ -415,3 +415,23 @@ async def test_entry_setup_list_config(hass: HomeAssistantType):
     assert len(mock_setup.mock_calls) == 2
     assert mock_setup.mock_calls[0][1][1] == {'host': 'bla'}
     assert mock_setup.mock_calls[1][1][1] == {'host': 'blu'}
+
+
+async def test_entry_setup_platform_not_ready(hass: HomeAssistantType):
+    """Test failed setting up entry will raise PlatformNotReady."""
+    await async_setup_component(hass, 'cast', {
+        'cast': {
+            'media_player': {
+                'host': 'bla'
+            }
+        }
+    })
+
+    with patch(
+        'homeassistant.components.media_player.cast._async_setup_platform',
+            return_value=mock_coro(exception=Exception)) as mock_setup:
+        with pytest.raises(PlatformNotReady):
+            await cast.async_setup_entry(hass, MockConfigEntry(), None)
+
+    assert len(mock_setup.mock_calls) == 1
+    assert mock_setup.mock_calls[0][1][1] == {'host': 'bla'}
-- 
GitLab