Fix BOM weather '-' value

homeassistant/components/sensor/bom.py

@@ -19,8 +19,8 @@
 
 from homeassistant.components.sensor import PLATFORM_SCHEMA
 from homeassistant.const import (
-    CONF_MONITORED_CONDITIONS, TEMP_CELSIUS, STATE_UNKNOWN, CONF_NAME,
-    ATTR_ATTRIBUTION, CONF_LATITUDE, CONF_LONGITUDE)
+    CONF_MONITORED_CONDITIONS, TEMP_CELSIUS, CONF_NAME, ATTR_ATTRIBUTION,
+    CONF_LATITUDE, CONF_LONGITUDE)
 from homeassistant.helpers.entity import Entity
 from homeassistant.util import Throttle
 import homeassistant.helpers.config_validation as cv
@@ -145,21 +145,18 @@ def name(self):
     @property
     def state(self):
         """Return the state of the sensor."""
-        if self.bom_data.data and self._condition in self.bom_data.data:
-            return self.bom_data.data[self._condition]
-
-        return STATE_UNKNOWN
+        return self.bom_data.get_reading(self._condition)
 
     @property
     def device_state_attributes(self):
         """Return the state attributes of the device."""
         attr = {}
         attr['Sensor Id'] = self._condition
-        attr['Zone Id'] = self.bom_data.data['history_product']
-        attr['Station Id'] = self.bom_data.data['wmo']
-        attr['Station Name'] = self.bom_data.data['name']
+        attr['Zone Id'] = self.bom_data.latest_data['history_product']
+        attr['Station Id'] = self.bom_data.latest_data['wmo']
+        attr['Station Name'] = self.bom_data.latest_data['name']
         attr['Last Update'] = datetime.datetime.strptime(str(
-            self.bom_data.data['local_date_time_full']), '%Y%m%d%H%M%S')
+            self.bom_data.latest_data['local_date_time_full']), '%Y%m%d%H%M%S')
         attr[ATTR_ATTRIBUTION] = CONF_ATTRIBUTION
         return attr
 
@@ -180,22 +177,43 @@ def __init__(self, hass, station_id):
         """Initialize the data object."""
         self._hass = hass
         self._zone_id, self._wmo_id = station_id.split('.')
-        self.data = None
+        self._data = None
 
     def _build_url(self):
         url = _RESOURCE.format(self._zone_id, self._zone_id, self._wmo_id)
         _LOGGER.info("BOM URL %s", url)
         return url
 
+    @property
+    def latest_data(self):
+        """Return the latest data object."""
+        if self._data:
+            return self._data[0]
+        return None
+
+    def get_reading(self, condition):
+        """Return the value for the given condition.
+
+        BOM weather publishes condition readings for weather (and a few other
+        conditions) at intervals throughout the day. To avoid a `-` value in
+        the frontend for these conditions, we traverse the historical data
+        for the latest value that is not `-`.
+
+        Iterators are used in this method to avoid iterating needlessly
+        iterating through the entire BOM provided dataset
+        """
+        condition_readings = (entry[condition] for entry in self._data)
+        return next((x for x in condition_readings if x != '-'), None)
+
     @Throttle(MIN_TIME_BETWEEN_UPDATES)
     def update(self):
         """Get the latest data from BOM."""
         try:
             result = requests.get(self._build_url(), timeout=10).json()
-            self.data = result['observations']['data'][0]
+            self._data = result['observations']['data']
         except ValueError as err:
             _LOGGER.error("Check BOM %s", err.args)
-            self.data = None
+            self._data = None
             raise
 
 

homeassistant/components/weather/bom.py

@@ -48,7 +48,7 @@ class BOMWeather(WeatherEntity):
     def __init__(self, bom_data, stationname=None):
         """Initialise the platform with a data instance and station name."""
         self.bom_data = bom_data
-        self.stationname = stationname or self.bom_data.data.get('name')
+        self.stationname = stationname or self.bom_data.latest_data.get('name')
 
     def update(self):
         """Update current conditions."""
@@ -62,14 +62,14 @@ def name(self):
     @property
     def condition(self):
         """Return the current condition."""
-        return self.bom_data.data.get('weather')
+        return self.bom_data.get_reading('weather')
 
     # Now implement the WeatherEntity interface
 
     @property
     def temperature(self):
         """Return the platform temperature."""
-        return self.bom_data.data.get('air_temp')
+        return self.bom_data.get_reading('air_temp')
 
     @property
     def temperature_unit(self):
@@ -79,17 +79,17 @@ def temperature_unit(self):
     @property
     def pressure(self):
         """Return the mean sea-level pressure."""
-        return self.bom_data.data.get('press_msl')
+        return self.bom_data.get_reading('press_msl')
 
     @property
     def humidity(self):
         """Return the relative humidity."""
-        return self.bom_data.data.get('rel_hum')
+        return self.bom_data.get_reading('rel_hum')
 
     @property
     def wind_speed(self):
         """Return the wind speed."""
-        return self.bom_data.data.get('wind_spd_kmh')
+        return self.bom_data.get_reading('wind_spd_kmh')
 
     @property
     def wind_bearing(self):
@@ -99,7 +99,7 @@ def wind_bearing(self):
                       'S', 'SSW', 'SW', 'WSW',
                       'W', 'WNW', 'NW', 'NNW']
         wind = {name: idx * 360 / 16 for idx, name in enumerate(directions)}
-        return wind.get(self.bom_data.data.get('wind_dir'))
+        return wind.get(self.bom_data.get_reading('wind_dir'))
 
     @property
     def attribution(self):

tests/components/sensor/test_bom.py

@@ -0,0 +1,97 @@
+"""The tests for the BOM Weather sensor platform."""
+import re
+import unittest
+import json
+import requests
+from unittest.mock import patch
+from urllib.parse import urlparse
+
+from homeassistant.setup import setup_component
+from homeassistant.components import sensor
+
+from tests.common import (
+    get_test_home_assistant, assert_setup_component, load_fixture)
+
+VALID_CONFIG = {
+    'platform': 'bom',
+    'station': 'IDN60901.94767',
+    'name': 'Fake',
+    'monitored_conditions': [
+        'apparent_t',
+        'press',
+        'weather'
+    ]
+}
+
+
+def mocked_requests(*args, **kwargs):
+    """Mock requests.get invocations."""
+    class MockResponse:
+        """Class to represent a mocked response."""
+
+        def __init__(self, json_data, status_code):
+            """Initialize the mock response class."""
+            self.json_data = json_data
+            self.status_code = status_code
+
+        def json(self):
+            """Return the json of the response."""
+            return self.json_data
+
+        @property
+        def content(self):
+            """Return the content of the response."""
+            return self.json()
+
+        def raise_for_status(self):
+            """Raise an HTTPError if status is not 200."""
+            if self.status_code != 200:
+                raise requests.HTTPError(self.status_code)
+
+    url = urlparse(args[0])
+    if re.match(r'^/fwo/[\w]+/[\w.]+\.json', url.path):
+        return MockResponse(json.loads(load_fixture('bom_weather.json')), 200)
+
+    raise NotImplementedError('Unknown route {}'.format(url.path))
+
+
+class TestBOMWeatherSensor(unittest.TestCase):
+    """Test the BOM Weather sensor."""
+
+    def setUp(self):
+        """Set up things to be run when tests are started."""
+        self.hass = get_test_home_assistant()
+        self.config = VALID_CONFIG
+
+    def tearDown(self):
+        """Stop everything that was started."""
+        self.hass.stop()
+
+    @patch('requests.get', side_effect=mocked_requests)
+    def test_setup(self, mock_get):
+        """Test the setup with custom settings."""
+        with assert_setup_component(1, sensor.DOMAIN):
+            self.assertTrue(setup_component(self.hass, sensor.DOMAIN, {
+                'sensor': VALID_CONFIG}))
+
+        fake_entities = [
+            'bom_fake_feels_like_c',
+            'bom_fake_pressure_mb',
+            'bom_fake_weather']
+
+        for entity_id in fake_entities:
+            state = self.hass.states.get('sensor.{}'.format(entity_id))
+            self.assertIsNotNone(state)
+
+    @patch('requests.get', side_effect=mocked_requests)
+    def test_sensor_values(self, mock_get):
+        """Test retrieval of sensor values."""
+        self.assertTrue(setup_component(
+            self.hass, sensor.DOMAIN, {'sensor': VALID_CONFIG}))
+
+        self.assertEqual('Fine', self.hass.states.get(
+            'sensor.bom_fake_weather').state)
+        self.assertEqual('1021.7', self.hass.states.get(
+            'sensor.bom_fake_pressure_mb').state)
+        self.assertEqual('25.0', self.hass.states.get(
+            'sensor.bom_fake_feels_like_c').state)

tests/fixtures/bom_weather.json

@@ -0,0 +1,42 @@
+{
+  "observations": {
+    "data": [
+      {
+        "wmo": 94767,
+        "name": "Fake",
+        "history_product": "IDN00000",
+        "local_date_time_full": "20180422130000",
+        "apparent_t": 25.0,
+        "press": 1021.7,
+        "weather": "-"
+      },
+      {
+        "wmo": 94767,
+        "name": "Fake",
+        "history_product": "IDN00000",
+        "local_date_time_full": "20180422130000",
+        "apparent_t": 22.0,
+        "press": 1019.7,
+        "weather": "-"
+      },
+      {
+        "wmo": 94767,
+        "name": "Fake",
+        "history_product": "IDN00000",
+        "local_date_time_full": "20180422130000",
+        "apparent_t": 20.0,
+        "press": 1011.7,
+        "weather": "Fine"
+      },
+      {
+        "wmo": 94767,
+        "name": "Fake",
+        "history_product": "IDN00000",
+        "local_date_time_full": "20180422130000",
+        "apparent_t": 18.0,
+        "press": 1010.0,
+        "weather": "-"
+      }
+    ]
+  }
+}