Refactor Forecast.io (#2217)

* Refactor Forecast.io

* Some more refactoring and code review workoff

* Dict switch refactor

* CamelCase for data lookup

* Fixing unit_of_measure update

* Better default return for unit_of_measurement

* Test fix

homeassistant/components/sensor/forecast.py

@@ -6,8 +6,12 @@ https://home-assistant.io/components/sensor.forecast/
 """
 import logging
 from datetime import timedelta
+from requests.exceptions import ConnectionError as ConnectError, \
+    HTTPError, Timeout
 
+from homeassistant.components.sensor import DOMAIN
 from homeassistant.const import CONF_API_KEY, TEMP_CELSIUS
+from homeassistant.helpers import validate_config
 from homeassistant.helpers.entity import Entity
 from homeassistant.util import Throttle
 
@@ -48,21 +52,12 @@ MIN_TIME_BETWEEN_UPDATES = timedelta(seconds=120)
 
 def setup_platform(hass, config, add_devices, discovery_info=None):
     """Setup the Forecast.io sensor."""
-    import forecastio
+    # Validate the configuration
-
     if None in (hass.config.latitude, hass.config.longitude):
         _LOGGER.error("Latitude or longitude not set in Home Assistant config")
         return False
-
+    elif not validate_config({DOMAIN: config},
-    try:
+                             {DOMAIN: [CONF_API_KEY]}, _LOGGER):
-        forecast = forecastio.load_forecast(config.get(CONF_API_KEY, None),
-                                            hass.config.latitude,
-                                            hass.config.longitude)
-        forecast.currently()
-    except ValueError:
-        _LOGGER.error(
-            "Connection error "
-            "Please check your settings for Forecast.io.")
         return False
 
     if 'units' in config:
@@ -72,43 +67,41 @@ def setup_platform(hass, config, add_devices, discovery_info=None):
     else:
         units = 'us'
 
-    data = ForeCastData(config.get(CONF_API_KEY, None),
+    # Create a data fetcher to support all of the configured sensors. Then make
-                        hass.config.latitude,
+    # the first call to init the data and confirm we can connect.
-                        hass.config.longitude,
+    try:
-                        units)
+        forecast_data = ForeCastData(
+            config.get(CONF_API_KEY, None), hass.config.latitude,
+            hass.config.longitude, units)
+        forecast_data.update_currently()
+    except ValueError as error:
+        _LOGGER.error(error)
+        return False
 
-    dev = []
+    # Initialize and add all of the sensors.
+    sensors = []
     for variable in config['monitored_conditions']:
-        if variable not in SENSOR_TYPES:
+        if variable in SENSOR_TYPES:
-            _LOGGER.error('Sensor type: "%s" does not exist', variable)
+            sensors.append(ForeCastSensor(forecast_data, variable))
         else:
-            dev.append(ForeCastSensor(data, variable))
+            _LOGGER.error('Sensor type: "%s" does not exist', variable)
 
-    add_devices(dev)
+    add_devices(sensors)
 
 
 # pylint: disable=too-few-public-methods
 class ForeCastSensor(Entity):
     """Implementation of a Forecast.io sensor."""
 
-    def __init__(self, weather_data, sensor_type):
+    def __init__(self, forecast_data, sensor_type):
         """Initialize the sensor."""
         self.client_name = 'Weather'
         self._name = SENSOR_TYPES[sensor_type][0]
-        self.forecast_client = weather_data
+        self.forecast_data = forecast_data
         self.type = sensor_type
         self._state = None
-        self._unit_system = self.forecast_client.unit_system
+        self._unit_of_measurement = None
-        if self._unit_system == 'si':
+
-            self._unit_of_measurement = SENSOR_TYPES[self.type][1]
-        elif self._unit_system == 'us':
-            self._unit_of_measurement = SENSOR_TYPES[self.type][2]
-        elif self._unit_system == 'ca':
-            self._unit_of_measurement = SENSOR_TYPES[self.type][3]
-        elif self._unit_system == 'uk':
-            self._unit_of_measurement = SENSOR_TYPES[self.type][4]
-        elif self._unit_system == 'uk2':
-            self._unit_of_measurement = SENSOR_TYPES[self.type][5]
         self.update()
 
     @property
@@ -129,75 +122,72 @@ class ForeCastSensor(Entity):
     @property
     def unit_system(self):
         """Return the unit system of this entity."""
-        return self._unit_system
+        return self.forecast_data.unit_system
+
+    def update_unit_of_measurement(self):
+        """Update units based on unit system."""
+        unit_index = {
+            'si': 1,
+            'us': 2,
+            'ca': 3,
+            'uk': 4,
+            'uk2': 5
+        }.get(self.unit_system, 1)
+        self._unit_of_measurement = SENSOR_TYPES[self.type][unit_index]
 
-    # pylint: disable=too-many-branches,too-many-statements
     def update(self):
         """Get the latest data from Forecast.io and updates the states."""
-        import forecastio
+        # Call the API for new forecast data. Each sensor will re-trigger this
+        # same exact call, but thats fine. We cache results for a short period
+        # of time to prevent hitting API limits. Note that forecast.io will
+        # charge users for too many calls in 1 day, so take care when updating.
+        self.forecast_data.update()
+        self.update_unit_of_measurement()
 
-        self.forecast_client.update()
+        if self.type == 'minutely_summary':
+            self.forecast_data.update_minutely()
+            minutely = self.forecast_data.data_minutely
+            self._state = getattr(minutely, 'summary', '')
+        elif self.type == 'hourly_summary':
+            self.forecast_data.update_hourly()
+            hourly = self.forecast_data.data_hourly
+            self._state = getattr(hourly, 'summary', '')
+        elif self.type == 'daily_summary':
+            self.forecast_data.update_daily()
+            daily = self.forecast_data.data_daily
+            self._state = getattr(daily, 'summary', '')
+        else:
+            self.forecast_data.update_currently()
+            currently = self.forecast_data.data_currently
+            self._state = self.get_currently_state(currently)
 
-        try:
+    def get_currently_state(self, data):
-            if self.type == 'minutely_summary':
+        """
-                self.forecast_client.update_minutely()
+        Helper function that returns a new state based on the type.
-                self._state = self.forecast_client.data_minutely.summary
-                return
 
-            elif self.type == 'hourly_summary':
+        If the sensor type is unknown, the current state is returned.
-                self.forecast_client.update_hourly()
+        """
-                self._state = self.forecast_client.data_hourly.summary
+        lookup_type = convert_to_camel(self.type)
-                return
+        state = getattr(data, lookup_type, 0)
 
-            elif self.type == 'daily_summary':
+        # Some state data needs to be rounded to whole values or converted to
-                self.forecast_client.update_daily()
+        # percentages
-                self._state = self.forecast_client.data_daily.summary
+        if self.type in ['precip_probability', 'cloud_cover', 'humidity']:
-                return
+            return round(state * 100, 1)
+        elif (self.type in ['dew_point', 'temperature', 'apparent_temperature',
+                            'pressure', 'ozone']):
+            return round(state, 1)
+        return state
 
-        except forecastio.utils.PropertyUnavailable:
-            return
 
-        self.forecast_client.update_currently()
+def convert_to_camel(data):
-        data = self.forecast_client.data_currently
+    """
+    Convert snake case (foo_bar_bat) to camel case (fooBarBat).
 
-        try:
+    This is not pythonic, but needed for certain situations
-            if self.type == 'summary':
+    """
-                self._state = data.summary
+    components = data.split('_')
-            elif self.type == 'icon':
+    return components[0] + "".join(x.title() for x in components[1:])
-                self._state = data.icon
-            elif self.type == 'nearest_storm_distance':
-                self._state = data.nearestStormDistance
-            elif self.type == 'nearest_storm_bearing':
-                self._state = data.nearestStormBearing
-            elif self.type == 'precip_intensity':
-                self._state = data.precipIntensity
-            elif self.type == 'precip_type':
-                self._state = data.precipType
-            elif self.type == 'precip_probability':
-                self._state = round(data.precipProbability * 100, 1)
-            elif self.type == 'dew_point':
-                self._state = round(data.dewPoint, 1)
-            elif self.type == 'temperature':
-                self._state = round(data.temperature, 1)
-            elif self.type == 'apparent_temperature':
-                self._state = round(data.apparentTemperature, 1)
-            elif self.type == 'wind_speed':
-                self._state = data.windSpeed
-            elif self.type == 'wind_bearing':
-                self._state = data.windBearing
-            elif self.type == 'cloud_cover':
-                self._state = round(data.cloudCover * 100, 1)
-            elif self.type == 'humidity':
-                self._state = round(data.humidity * 100, 1)
-            elif self.type == 'pressure':
-                self._state = round(data.pressure, 1)
-            elif self.type == 'visibility':
-                self._state = data.visibility
-            elif self.type == 'ozone':
-                self._state = round(data.ozone, 1)
-
-        except forecastio.utils.PropertyUnavailable:
-            pass
 
 
 class ForeCastData(object):
@@ -226,10 +216,13 @@ class ForeCastData(object):
         """Get the latest data from Forecast.io."""
         import forecastio
 
-        self.data = forecastio.load_forecast(self._api_key,
+        try:
-                                             self.latitude,
+            self.data = forecastio.load_forecast(self._api_key,
-                                             self.longitude,
+                                                 self.latitude,
-                                             units=self.units)
+                                                 self.longitude,
+                                                 units=self.units)
+        except (ConnectError, HTTPError, Timeout, ValueError) as error:
+            raise ValueError("Unable to init Forecast.io. - %s", error)
         self.unit_system = self.data.json['flags']['units']
 
     @Throttle(MIN_TIME_BETWEEN_UPDATES)

tests/components/test_forecast.py

@@ -7,7 +7,6 @@ from unittest.mock import MagicMock, patch
 
 import forecastio
 import httpretty
-import pytest
 from requests.exceptions import HTTPError
 
 from homeassistant.components.sensor import forecast
@@ -46,8 +45,8 @@ class TestForecastSetup(unittest.TestCase):
         msg = '400 Client Error: Bad Request for url: {}'.format(url)
         mock_get_forecast.side_effect = HTTPError(msg,)
 
-        with pytest.raises(HTTPError):
+        response = forecast.setup_platform(self.hass, self.config, MagicMock())
-            forecast.setup_platform(self.hass, self.config, MagicMock())
+        self.assertFalse(response)
 
     @httpretty.activate
     @patch('forecastio.api.get_forecast', wraps=forecastio.api.get_forecast)
@@ -74,4 +73,4 @@ class TestForecastSetup(unittest.TestCase):
 
         forecast.setup_platform(self.hass, self.config, MagicMock())
         self.assertTrue(mock_get_forecast.called)
-        self.assertEqual(mock_get_forecast.call_count, 2)
+        self.assertEqual(mock_get_forecast.call_count, 1)