From 0eb387916fefc1367984e88ca05c07072c5b7ce9 Mon Sep 17 00:00:00 2001 From: Ties de Kock Date: Wed, 12 Jun 2019 00:26:04 +0200 Subject: [PATCH] Camera platform for buienradar imagery (#23358) * Add camera for buienradar radar * Use asyncio.Conditions instead of asyncio.Lock * Add test and fix python 3.5 compatibility * rename interval to delta for consistency with BOM integration * fix linting error introduced during rebase * Improved buienradar.camera documentation and tests * Incorporated one comment on a redundant/cargo cult function * Improved documentation * Increase test coverage by fixing one test by making it a coroutine (to make it actually run), adding another test case, and changing the flow in the implementation. * style changes after review, additional test case * Use python 3.5 style mypy type annotations in __init__ * Remove explicit passing of event loop * Adopt buienradar camera as codeowner * Update manifest.json * Update CODEOWNERS through hassfest Updated CODEOWNERS through hassfest (instead of manually), thanks to @balloob for the hint. --- CODEOWNERS | 1 + homeassistant/components/buienradar/camera.py | 178 +++++++++++++++ .../components/buienradar/manifest.json | 2 +- tests/components/buienradar/test_camera.py | 202 ++++++++++++++++++ 4 files changed, 382 insertions(+), 1 deletion(-) create mode 100644 homeassistant/components/buienradar/camera.py create mode 100644 tests/components/buienradar/test_camera.py diff --git a/CODEOWNERS b/CODEOWNERS index 0069ff34e96..4af6e742cbb 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -44,6 +44,7 @@ homeassistant/components/braviatv/* @robbiet480 homeassistant/components/broadlink/* @danielhiversen homeassistant/components/brunt/* @eavanvalkenburg homeassistant/components/bt_smarthub/* @jxwolstenholme +homeassistant/components/buienradar/* @ties homeassistant/components/cisco_ios/* @fbradyirl homeassistant/components/cisco_mobility_express/* @fbradyirl homeassistant/components/cisco_webex_teams/* @fbradyirl diff --git a/homeassistant/components/buienradar/camera.py b/homeassistant/components/buienradar/camera.py new file mode 100644 index 00000000000..b390a86d622 --- /dev/null +++ b/homeassistant/components/buienradar/camera.py @@ -0,0 +1,178 @@ +"""Provide animated GIF loops of Buienradar imagery.""" +import asyncio +import logging +from datetime import datetime, timedelta +from typing import Optional + +import aiohttp +import voluptuous as vol + +from homeassistant.components.camera import PLATFORM_SCHEMA, Camera +from homeassistant.const import CONF_NAME + +from homeassistant.helpers import config_validation as cv +from homeassistant.helpers.aiohttp_client import async_get_clientsession + +from homeassistant.util import dt as dt_util + + +CONF_DIMENSION = 'dimension' +CONF_DELTA = 'delta' + +RADAR_MAP_URL_TEMPLATE = ('https://api.buienradar.nl/image/1.0/' + 'RadarMapNL?w={w}&h={h}') + +_LOG = logging.getLogger(__name__) + +# Maximum range according to docs +DIM_RANGE = vol.All(vol.Coerce(int), vol.Range(min=120, max=700)) + +PLATFORM_SCHEMA = vol.All( + PLATFORM_SCHEMA.extend({ + vol.Optional(CONF_DIMENSION, default=512): DIM_RANGE, + vol.Optional(CONF_DELTA, default=600.0): vol.All(vol.Coerce(float), + vol.Range(min=0)), + vol.Optional(CONF_NAME, default="Buienradar loop"): cv.string, + })) + + +async def async_setup_platform(hass, config, async_add_entities, + discovery_info=None): + """Set up buienradar radar-loop camera component.""" + dimension = config[CONF_DIMENSION] + delta = config[CONF_DELTA] + name = config[CONF_NAME] + + async_add_entities([BuienradarCam(name, dimension, delta)]) + + +class BuienradarCam(Camera): + """ + A camera component producing animated buienradar radar-imagery GIFs. + + Rain radar imagery camera based on image URL taken from [0]. + + [0]: https://www.buienradar.nl/overbuienradar/gratis-weerdata + """ + + def __init__(self, name: str, dimension: int, delta: float): + """ + Initialize the component. + + This constructor must be run in the event loop. + """ + super().__init__() + + self._name = name + + # dimension (x and y) of returned radar image + self._dimension = dimension + + # time a cached image stays valid for + self._delta = delta + + # Condition that guards the loading indicator. + # + # Ensures that only one reader can cause an http request at the same + # time, and that all readers are notified after this request completes. + # + # invariant: this condition is private to and owned by this instance. + self._condition = asyncio.Condition() + + self._last_image = None # type: Optional[bytes] + # value of the last seen last modified header + self._last_modified = None # type: Optional[str] + # loading status + self._loading = False + # deadline for image refresh - self.delta after last successful load + self._deadline = None # type: Optional[datetime] + + @property + def name(self) -> str: + """Return the component name.""" + return self._name + + def __needs_refresh(self) -> bool: + if not (self._delta and self._deadline and self._last_image): + return True + + return dt_util.utcnow() > self._deadline + + async def __retrieve_radar_image(self) -> bool: + """Retrieve new radar image and return whether this succeeded.""" + session = async_get_clientsession(self.hass) + + url = RADAR_MAP_URL_TEMPLATE.format(w=self._dimension, + h=self._dimension) + + if self._last_modified: + headers = {'If-Modified-Since': self._last_modified} + else: + headers = {} + + try: + async with session.get(url, timeout=5, headers=headers) as res: + res.raise_for_status() + + if res.status == 304: + _LOG.debug("HTTP 304 - success") + return True + + last_modified = res.headers.get('Last-Modified', None) + if last_modified: + self._last_modified = last_modified + + self._last_image = await res.read() + _LOG.debug("HTTP 200 - Last-Modified: %s", last_modified) + + return True + except (asyncio.TimeoutError, aiohttp.ClientError) as err: + _LOG.error("Failed to fetch image, %s", type(err)) + return False + + async def async_camera_image(self) -> Optional[bytes]: + """ + Return a still image response from the camera. + + Uses ayncio conditions to make sure only one task enters the critical + section at the same time. Otherwise, two http requests would start + when two tabs with home assistant are open. + + The condition is entered in two sections because otherwise the lock + would be held while doing the http request. + + A boolean (_loading) is used to indicate the loading status instead of + _last_image since that is initialized to None. + + For reference: + * :func:`asyncio.Condition.wait` releases the lock and acquires it + again before continuing. + * :func:`asyncio.Condition.notify_all` requires the lock to be held. + """ + if not self.__needs_refresh(): + return self._last_image + + # get lock, check iff loading, await notification if loading + async with self._condition: + # can not be tested - mocked http response returns immediately + if self._loading: + _LOG.debug("already loading - waiting for notification") + await self._condition.wait() + return self._last_image + + # Set loading status **while holding lock**, makes other tasks wait + self._loading = True + + try: + now = dt_util.utcnow() + was_updated = await self.__retrieve_radar_image() + # was updated? Set new deadline relative to now before loading + if was_updated: + self._deadline = now + timedelta(seconds=self._delta) + + return self._last_image + finally: + # get lock, unset loading status, notify all waiting tasks + async with self._condition: + self._loading = False + self._condition.notify_all() diff --git a/homeassistant/components/buienradar/manifest.json b/homeassistant/components/buienradar/manifest.json index 98fc5fbdeac..1ed313348f7 100644 --- a/homeassistant/components/buienradar/manifest.json +++ b/homeassistant/components/buienradar/manifest.json @@ -6,5 +6,5 @@ "buienradar==0.91" ], "dependencies": [], - "codeowners": [] + "codeowners": ["@ties"] } diff --git a/tests/components/buienradar/test_camera.py b/tests/components/buienradar/test_camera.py new file mode 100644 index 00000000000..a05cc9413e5 --- /dev/null +++ b/tests/components/buienradar/test_camera.py @@ -0,0 +1,202 @@ +"""The tests for generic camera component.""" +import asyncio +from aiohttp.client_exceptions import ClientResponseError + +from homeassistant.util import dt as dt_util + +from homeassistant.setup import async_setup_component + +# An infinitesimally small time-delta. +EPSILON_DELTA = 0.0000000001 + + +def radar_map_url(dim: int = 512) -> str: + """Build map url, defaulting to 512 wide (as in component).""" + return ("https://api.buienradar.nl/" + "image/1.0/RadarMapNL?w={dim}&h={dim}").format(dim=dim) + + +async def test_fetching_url_and_caching(aioclient_mock, hass, hass_client): + """Test that it fetches the given url.""" + aioclient_mock.get(radar_map_url(), text='hello world') + + await async_setup_component(hass, 'camera', { + 'camera': { + 'name': 'config_test', + 'platform': 'buienradar', + }}) + + client = await hass_client() + + resp = await client.get('/api/camera_proxy/camera.config_test') + + assert resp.status == 200 + assert aioclient_mock.call_count == 1 + body = await resp.text() + assert body == 'hello world' + + # default delta is 600s -> should be the same when calling immediately + # afterwards. + + resp = await client.get('/api/camera_proxy/camera.config_test') + assert aioclient_mock.call_count == 1 + + +async def test_expire_delta(aioclient_mock, hass, hass_client): + """Test that the cache expires after delta.""" + aioclient_mock.get(radar_map_url(), text='hello world') + + await async_setup_component(hass, 'camera', { + 'camera': { + 'name': 'config_test', + 'platform': 'buienradar', + 'delta': EPSILON_DELTA, + }}) + + client = await hass_client() + + resp = await client.get('/api/camera_proxy/camera.config_test') + + assert resp.status == 200 + assert aioclient_mock.call_count == 1 + body = await resp.text() + assert body == 'hello world' + + await asyncio.sleep(EPSILON_DELTA) + # tiny delta has passed -> should immediately call again + resp = await client.get('/api/camera_proxy/camera.config_test') + assert aioclient_mock.call_count == 2 + + +async def test_only_one_fetch_at_a_time(aioclient_mock, hass, hass_client): + """Test that it fetches with only one request at the same time.""" + aioclient_mock.get(radar_map_url(), text='hello world') + + await async_setup_component(hass, 'camera', { + 'camera': { + 'name': 'config_test', + 'platform': 'buienradar', + }}) + + client = await hass_client() + + resp_1 = client.get('/api/camera_proxy/camera.config_test') + resp_2 = client.get('/api/camera_proxy/camera.config_test') + + resp = await resp_1 + resp_2 = await resp_2 + + assert (await resp.text()) == (await resp_2.text()) + + assert aioclient_mock.call_count == 1 + + +async def test_dimension(aioclient_mock, hass, hass_client): + """Test that it actually adheres to the dimension.""" + aioclient_mock.get(radar_map_url(700), text='hello world') + + await async_setup_component(hass, 'camera', { + 'camera': { + 'name': 'config_test', + 'platform': 'buienradar', + 'dimension': 700, + }}) + + client = await hass_client() + + await client.get('/api/camera_proxy/camera.config_test') + + assert aioclient_mock.call_count == 1 + + +async def test_failure_response_not_cached(aioclient_mock, hass, hass_client): + """Test that it does not cache a failure response.""" + aioclient_mock.get(radar_map_url(), text='hello world', status=401) + + await async_setup_component(hass, 'camera', { + 'camera': { + 'name': 'config_test', + 'platform': 'buienradar', + }}) + + client = await hass_client() + + await client.get('/api/camera_proxy/camera.config_test') + await client.get('/api/camera_proxy/camera.config_test') + + assert aioclient_mock.call_count == 2 + + +async def test_last_modified_updates(aioclient_mock, hass, hass_client): + """Test that it does respect HTTP not modified.""" + # Build Last-Modified header value + now = dt_util.utcnow() + last_modified = now.strftime("%a, %d %m %Y %H:%M:%S GMT") + + aioclient_mock.get(radar_map_url(), text='hello world', status=200, + headers={ + 'Last-Modified': last_modified, + }) + + await async_setup_component(hass, 'camera', { + 'camera': { + 'name': 'config_test', + 'platform': 'buienradar', + 'delta': EPSILON_DELTA, + }}) + + client = await hass_client() + + resp_1 = await client.get('/api/camera_proxy/camera.config_test') + # It is not possible to check if header was sent. + assert aioclient_mock.call_count == 1 + + await asyncio.sleep(EPSILON_DELTA) + + # Content has expired, change response to a 304 NOT MODIFIED, which has no + # text, i.e. old value should be kept + aioclient_mock.clear_requests() + # mock call count is now reset as well: + assert aioclient_mock.call_count == 0 + + aioclient_mock.get(radar_map_url(), text=None, status=304) + + resp_2 = await client.get('/api/camera_proxy/camera.config_test') + assert aioclient_mock.call_count == 1 + + assert (await resp_1.read()) == (await resp_2.read()) + + +async def test_retries_after_error(aioclient_mock, hass, hass_client): + """Test that it does retry after an error instead of caching.""" + await async_setup_component(hass, 'camera', { + 'camera': { + 'name': 'config_test', + 'platform': 'buienradar', + }}) + + client = await hass_client() + + aioclient_mock.get(radar_map_url(), text=None, status=500) + + # A 404 should not return data and throw: + try: + await client.get('/api/camera_proxy/camera.config_test') + except ClientResponseError: + pass + + assert aioclient_mock.call_count == 1 + + # Change the response to a 200 + aioclient_mock.clear_requests() + aioclient_mock.get(radar_map_url(), text="DEADBEEF") + + assert aioclient_mock.call_count == 0 + + # http error should not be cached, immediate retry. + resp_2 = await client.get('/api/camera_proxy/camera.config_test') + assert aioclient_mock.call_count == 1 + + # Binary text can not be added as body to `aioclient_mock.get(text=...)`, + # while `resp.read()` returns bytes, encode the value. + assert (await resp_2.read()) == b"DEADBEEF"