diff --git a/homeassistant/components/hue/bridge.py b/homeassistant/components/hue/bridge.py index a153ed7a096..2c164e5769a 100644 --- a/homeassistant/components/hue/bridge.py +++ b/homeassistant/components/hue/bridge.py @@ -1,6 +1,8 @@ """Code to handle a Hue bridge.""" import asyncio +from functools import partial +from aiohttp import client_exceptions import aiohue import async_timeout import slugify as unicode_slug @@ -21,6 +23,8 @@ ATTR_SCENE_NAME = "scene_name" SCENE_SCHEMA = vol.Schema( {vol.Required(ATTR_GROUP_NAME): cv.string, vol.Required(ATTR_SCENE_NAME): cv.string} ) +# How long should we sleep if the hub is busy +HUB_BUSY_SLEEP = 0.01 class HueBridge: @@ -101,11 +105,33 @@ class HueBridge: self.authorized = True return True - async def async_request_call(self, coro): - """Process request batched.""" + async def async_request_call(self, task): + """Limit parallel requests to Hue hub. + The Hue hub can only handle a certain amount of parallel requests, total. + Although we limit our parallel requests, we still will run into issues because + other products are hitting up Hue. + + ClientOSError means hub closed the socket on us. + ContentResponseError means hub raised an error. + Since we don't make bad requests, this is on them. + """ async with self.parallel_updates_semaphore: - return await coro + for tries in range(4): + try: + return await task() + except ( + client_exceptions.ClientOSError, + client_exceptions.ClientResponseError, + ) as err: + if tries == 3 or ( + # We only retry if it's a server error. So raise on all 4XX errors. + isinstance(err, client_exceptions.ClientResponseError) + and err.status < 500 + ): + raise + + await asyncio.sleep(HUB_BUSY_SLEEP * tries) async def async_reset(self): """Reset this bridge to default state. @@ -167,8 +193,8 @@ class HueBridge: # If we can't find it, fetch latest info. if not updated and (group is None or scene is None): - await self.api.groups.update() - await self.api.scenes.update() + await self.async_request_call(self.api.groups.update) + await self.async_request_call(self.api.scenes.update) await self.hue_activate_scene(call, updated=True) return @@ -180,7 +206,7 @@ class HueBridge: LOGGER.warning("Unable to find scene %s", scene_name) return - await group.set_action(scene=scene.id) + await self.async_request_call(partial(group.set_action, scene=scene.id)) async def handle_unauthorized_error(self): """Create a new config flow when the authorization is no longer valid.""" @@ -210,7 +236,7 @@ async def authenticate_bridge(hass: core.HomeAssistant, bridge: aiohue.Bridge): except (aiohue.LinkButtonNotPressed, aiohue.Unauthorized): raise AuthenticationRequired - except (asyncio.TimeoutError, aiohue.RequestError): + except (asyncio.TimeoutError, client_exceptions.ClientOSError): raise CannotConnect except aiohue.AiohueException: LOGGER.exception("Unknown Hue linking error occurred") diff --git a/homeassistant/components/hue/const.py b/homeassistant/components/hue/const.py index e48cd4a8583..d8b33c60048 100644 --- a/homeassistant/components/hue/const.py +++ b/homeassistant/components/hue/const.py @@ -3,7 +3,6 @@ import logging LOGGER = logging.getLogger(__package__) DOMAIN = "hue" -API_NUPNP = "https://www.meethue.com/api/nupnp" # How long to wait to actually do the refresh after requesting it. # We wait some time so if we control multiple lights, we batch requests. diff --git a/homeassistant/components/hue/light.py b/homeassistant/components/hue/light.py index 2e27bf65c98..14ed2483cc6 100644 --- a/homeassistant/components/hue/light.py +++ b/homeassistant/components/hue/light.py @@ -153,7 +153,7 @@ async def async_safe_fetch(bridge, fetch_method): """Safely fetch data.""" try: with async_timeout.timeout(4): - return await bridge.async_request_call(fetch_method()) + return await bridge.async_request_call(fetch_method) except aiohue.Unauthorized: await bridge.handle_unauthorized_error() raise UpdateFailed @@ -376,9 +376,13 @@ class HueLight(Light): command["effect"] = "none" if self.is_group: - await self.bridge.async_request_call(self.light.set_action(**command)) + await self.bridge.async_request_call( + partial(self.light.set_action, **command) + ) else: - await self.bridge.async_request_call(self.light.set_state(**command)) + await self.bridge.async_request_call( + partial(self.light.set_state, **command) + ) await self.coordinator.async_request_refresh() @@ -401,9 +405,13 @@ class HueLight(Light): command["alert"] = "none" if self.is_group: - await self.bridge.async_request_call(self.light.set_action(**command)) + await self.bridge.async_request_call( + partial(self.light.set_action, **command) + ) else: - await self.bridge.async_request_call(self.light.set_state(**command)) + await self.bridge.async_request_call( + partial(self.light.set_state, **command) + ) await self.coordinator.async_request_refresh() diff --git a/homeassistant/components/hue/sensor_base.py b/homeassistant/components/hue/sensor_base.py index 1e518c05ee5..0bc7cd53536 100644 --- a/homeassistant/components/hue/sensor_base.py +++ b/homeassistant/components/hue/sensor_base.py @@ -55,7 +55,7 @@ class SensorManager: try: with async_timeout.timeout(4): return await self.bridge.async_request_call( - self.bridge.api.sensors.update() + self.bridge.api.sensors.update ) except Unauthorized: await self.bridge.handle_unauthorized_error() diff --git a/tests/components/hue/test_config_flow.py b/tests/components/hue/test_config_flow.py index 75cba40dafb..6929d6272df 100644 --- a/tests/components/hue/test_config_flow.py +++ b/tests/components/hue/test_config_flow.py @@ -2,7 +2,9 @@ import asyncio from unittest.mock import Mock +from aiohttp import client_exceptions import aiohue +from aiohue.discovery import URL_NUPNP from asynctest import CoroutineMock, patch import pytest import voluptuous as vol @@ -84,7 +86,7 @@ async def test_flow_works(hass): async def test_flow_no_discovered_bridges(hass, aioclient_mock): """Test config flow discovers no bridges.""" - aioclient_mock.get(const.API_NUPNP, json=[]) + aioclient_mock.get(URL_NUPNP, json=[]) result = await hass.config_entries.flow.async_init( const.DOMAIN, context={"source": "user"} @@ -95,9 +97,7 @@ async def test_flow_no_discovered_bridges(hass, aioclient_mock): async def test_flow_all_discovered_bridges_exist(hass, aioclient_mock): """Test config flow discovers only already configured bridges.""" - aioclient_mock.get( - const.API_NUPNP, json=[{"internalipaddress": "1.2.3.4", "id": "bla"}] - ) + aioclient_mock.get(URL_NUPNP, json=[{"internalipaddress": "1.2.3.4", "id": "bla"}]) MockConfigEntry( domain="hue", unique_id="bla", data={"host": "1.2.3.4"} ).add_to_hass(hass) @@ -111,9 +111,7 @@ async def test_flow_all_discovered_bridges_exist(hass, aioclient_mock): async def test_flow_one_bridge_discovered(hass, aioclient_mock): """Test config flow discovers one bridge.""" - aioclient_mock.get( - const.API_NUPNP, json=[{"internalipaddress": "1.2.3.4", "id": "bla"}] - ) + aioclient_mock.get(URL_NUPNP, json=[{"internalipaddress": "1.2.3.4", "id": "bla"}]) result = await hass.config_entries.flow.async_init( const.DOMAIN, context={"source": "user"} @@ -130,7 +128,7 @@ async def test_flow_two_bridges_discovered(hass, aioclient_mock): ).add_to_hass(hass) aioclient_mock.get( - const.API_NUPNP, + URL_NUPNP, json=[ {"internalipaddress": "1.2.3.4", "id": "bla"}, {"internalipaddress": "5.6.7.8", "id": "beer"}, @@ -153,7 +151,7 @@ async def test_flow_two_bridges_discovered(hass, aioclient_mock): async def test_flow_two_bridges_discovered_one_new(hass, aioclient_mock): """Test config flow discovers two bridges.""" aioclient_mock.get( - const.API_NUPNP, + URL_NUPNP, json=[ {"internalipaddress": "1.2.3.4", "id": "bla"}, {"internalipaddress": "5.6.7.8", "id": "beer"}, @@ -259,7 +257,7 @@ async def test_flow_link_button_not_pressed(hass): async def test_flow_link_unknown_host(hass): """Test config flow .""" mock_bridge = get_mock_bridge( - mock_create_user=CoroutineMock(side_effect=aiohue.RequestError), + mock_create_user=CoroutineMock(side_effect=client_exceptions.ClientOSError), ) with patch( "homeassistant.components.hue.config_flow.discover_nupnp", diff --git a/tests/components/hue/test_light.py b/tests/components/hue/test_light.py index d57c15bfa36..de50d23b947 100644 --- a/tests/components/hue/test_light.py +++ b/tests/components/hue/test_light.py @@ -206,8 +206,8 @@ def mock_bridge(hass): return bridge.mock_group_responses.popleft() return None - async def async_request_call(coro): - await coro + async def async_request_call(task): + await task() bridge.async_request_call = async_request_call bridge.api.config.apiversion = "9.9.9" diff --git a/tests/components/hue/test_sensor_base.py b/tests/components/hue/test_sensor_base.py index 78255116831..ca83da725fa 100644 --- a/tests/components/hue/test_sensor_base.py +++ b/tests/components/hue/test_sensor_base.py @@ -279,8 +279,8 @@ def create_mock_bridge(hass): return bridge.mock_sensor_responses.popleft() return None - async def async_request_call(coro): - await coro + async def async_request_call(task): + await task() bridge.async_request_call = async_request_call bridge.api.config.apiversion = "9.9.9"