Skip to content
Merged
90 changes: 49 additions & 41 deletions homeassistant/components/sensor/yr.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import asyncio
import logging

from datetime import timedelta
from random import randrange
from xml.parsers.expat import ExpatError

Expand All @@ -22,16 +21,17 @@
ATTR_ATTRIBUTION, CONF_NAME)
from homeassistant.helpers.aiohttp_client import async_get_clientsession
from homeassistant.helpers.entity import Entity
from homeassistant.helpers.event import (
async_track_point_in_utc_time, async_track_utc_time_change)
from homeassistant.helpers.event import (async_track_utc_time_change,
async_call_later)
from homeassistant.util import dt as dt_util

REQUIREMENTS = ['xmltodict==0.11.0']

_LOGGER = logging.getLogger(__name__)

CONF_ATTRIBUTION = "Weather forecast from yr.no, delivered by the Norwegian " \
"Meteorological Institute and the NRK."
CONF_ATTRIBUTION = "Weather forecast from met.no, delivered " \
"by the Norwegian Meteorological Institute."
# https://api.met.no/license_data.html

SENSOR_TYPES = {
'symbol': ['Symbol', None],
Expand Down Expand Up @@ -92,10 +92,8 @@ def async_setup_platform(hass, config, async_add_devices, discovery_info=None):

weather = YrData(hass, coordinates, forecast, dev)
# Update weather on the hour, spread seconds
async_track_utc_time_change(
hass, weather.async_update, minute=randrange(1, 10),
second=randrange(0, 59))
yield from weather.async_update()
async_track_utc_time_change(hass, weather.async_update, second=0)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not on the hour. This is called every minute.

@balloob balloob Feb 15, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Read the code, looks like just the comment is stale.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is very inefficient. Instead of calling update every minute, we should just keep track when we want to fetch data, then when data is fetched, we should call the update method.

I think that it should be something like this:

def async_setup_platform(…):
    async_call_later(hass, 5, weather.update_data)

class YrData(object):
    @asyncio.coroutine
    def update_data(self):
        try:
            yield from fetching_data()
            yield from updating_devices()
            async_call_later(hass, random(X), self.update_data)
        except …:
            async_call_later(hass, random(15), self.update_data)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, it seems like you're doing this already. So this line can be removed

@Danielhiversen Danielhiversen Feb 16, 2018

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want to call updating_devices() more often and more regular than fetching_data(), but every minute is probably overkill.

When we fetch the data we get the forecast for several hours:
Eg: 10:00, 11:00, 12:00, 13:00, 14:00,
So at 10:31 we want to show the 11:00 forecast as that is closest in time.

We still want to fetch new data each hour, to make sure we have the newest and best forecast.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see. In a perfect world we would set a timer for exactly that time to push an update.

yield from weather.download_new_data()


class YrSensor(Entity):
Expand Down Expand Up @@ -153,50 +151,60 @@ def __init__(self, hass, coordinates, forecast, devices):
self._url = 'https://aa015h6buqvih86i1.api.met.no/'\
'weatherapi/locationforecast/1.9/'
self._urlparams = coordinates
self._nextrun = None
self._forecast = forecast
self.devices = devices
self.data = {}
self.hass = hass
self._unsubscribe_download_new_data = None
self._update_time = [randrange(60), randrange(60)]

@asyncio.coroutine
def async_update(self, *_):
def download_new_data(self, *_):
"""Get the latest data from yr.no."""
import xmltodict

if self._unsubscribe_download_new_data:
self._unsubscribe_download_new_data()
self._unsubscribe_download_new_data = None

def try_again(err: str):
"""Retry in 15 minutes."""
_LOGGER.warning("Retrying in 15 minutes: %s", err)
self._nextrun = None
nxt = dt_util.utcnow() + timedelta(minutes=15)
if nxt.minute >= 15:
async_track_point_in_utc_time(self.hass, self.async_update,
nxt)

if self._nextrun is None or dt_util.utcnow() >= self._nextrun:
try:
websession = async_get_clientsession(self.hass)
with async_timeout.timeout(10, loop=self.hass.loop):
resp = yield from websession.get(
self._url, params=self._urlparams)
if resp.status != 200:
try_again('{} returned {}'.format(resp.url, resp.status))
return
text = yield from resp.text()

except (asyncio.TimeoutError, aiohttp.ClientError) as err:
try_again(err)
"""Retry in 15 to 20 minutes."""
minutes = 15 + randrange(6)
_LOGGER.error("Retrying in %i minutes: %s", minutes, err)
async_call_later(self.hass, minutes*60, self.download_new_data)
try:
websession = async_get_clientsession(self.hass)
with async_timeout.timeout(10, loop=self.hass.loop):
resp = yield from websession.get(
self._url, params=self._urlparams)
if resp.status != 200:
try_again('{} returned {}'.format(resp.url, resp.status))
return
text = yield from resp.text()

try:
self.data = xmltodict.parse(text)['weatherdata']
model = self.data['meta']['model']
if '@nextrun' not in model:
model = model[0]
self._nextrun = dt_util.parse_datetime(model['@nextrun'])
except (ExpatError, IndexError) as err:
try_again(err)
return
except (asyncio.TimeoutError, aiohttp.ClientError) as err:
try_again(err)
return

try:
self.data = xmltodict.parse(text)['weatherdata']
except (ExpatError, IndexError) as err:
try_again(err)
return

_unsub = async_track_utc_time_change(self.hass,
self.download_new_data,
minute=self._update_time[0],
second=self._update_time[1])
self._unsubscribe_download_new_data = _unsub

yield from self.async_update()

@asyncio.coroutine
def async_update(self, *_):
"""Find the current data from self.data."""
if not self.data:
return

now = dt_util.utcnow()
forecast_time = now + dt_util.dt.timedelta(hours=self._forecast)
Expand Down