plant - check history for min_brightness (#9534)

* Added option to create a group for the plant and all of it's sensors so that they appear together in the UI

* fixed warnings from the hound

* added check for min_brightness over several days

* fixed hound complaints

* 1) added missing dependency on recorder
2) using group.Group instead of hass.states.async_set as requested by @pvizeli

* fixed pylint error in docstring

* changed the way the groups are created

* fixed requirements issue

* Changed the way the groups are implemented. This is proposal number 4...

* Data read from recorder only on startup.

We now only store one data point per day. If a recorder is configured, this data is initialized from the database. If not the list is empty on startup.

* added missing documentation

* fixed typo in comment

* removed group fature

* added group dependency since it's still needed

* fixed bug: now "None" is no longer added to the DailyHistory

* now also outputting unit of measurement if defined by the sensors

* removed iconss

* fixed line length

* Implemented changes requested in code reviews.

These changes affect the interface to the UI:
* renamed attribute for units of measurement to "unit_of_measurement_dict"
* renamed attribute for maximum in brightness history to "max_brightness"

* only loading the history if a brightness sensor was configured

* fixed testcase

* fixed stupid bug in check of brightness history

Also added test for this bug

* added missing docstring

* Fixed sporadic failure in test case.

Sometimes the component was created before the data was stored in the history. This lead to an empty history being read.

* removed unused import statement in testcase

* reverted change to test case

* Changed startup behavior of the component.

No failed tests after 20 local test runs.

* added missing docstring

* fixed tests

* added hass.start() to Setup
* fixed call parameters in constructor
* added time.sleep

* removed sleep

* fixed typo in variable name

* disabled loading from database as it's not stable at the moment and nobody knows why :(

* fixed flake8

* now using pytest.mark.skipif to skip test

homeassistant/components/plant.py

@@ -1,23 +1,24 @@
-"""
+"""Component to monitor plants.
-Component to monitor plants.
 
 For more details about this component, please refer to the documentation at
 https://home-assistant.io/components/plant/
 """
 import logging
 import asyncio
-
+from datetime import datetime, timedelta
+from collections import deque
 import voluptuous as vol
 
 from homeassistant.const import (
     STATE_OK, STATE_PROBLEM, STATE_UNKNOWN, TEMP_CELSIUS, ATTR_TEMPERATURE,
-    CONF_SENSORS, ATTR_UNIT_OF_MEASUREMENT, ATTR_ICON)
+    CONF_SENSORS, ATTR_UNIT_OF_MEASUREMENT)
 from homeassistant.components import group
 import homeassistant.helpers.config_validation as cv
 from homeassistant.helpers.entity import Entity
 from homeassistant.helpers.entity_component import EntityComponent
 from homeassistant.core import callback
 from homeassistant.helpers.event import async_track_state_change
+from homeassistant.components.recorder.util import session_scope, execute
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -30,7 +31,13 @@ READING_CONDUCTIVITY = 'conductivity'
 READING_BRIGHTNESS = 'brightness'
 
 ATTR_PROBLEM = 'problem'
+ATTR_SENSORS = 'sensors'
 PROBLEM_NONE = 'none'
+ATTR_MAX_BRIGHTNESS_HISTORY = 'max_brightness'
+
+# we're not returning only one value, we're returning a dict here. So we need
+# to have a separate literal for it to avoid confusion.
+ATTR_DICT_OF_UNITS_OF_MEASUREMENT = 'unit_of_measurement_dict'
 
 CONF_MIN_BATTERY_LEVEL = 'min_' + READING_BATTERY
 CONF_MIN_TEMPERATURE = 'min_' + READING_TEMPERATURE
@@ -41,6 +48,7 @@ CONF_MIN_CONDUCTIVITY = 'min_' + READING_CONDUCTIVITY
 CONF_MAX_CONDUCTIVITY = 'max_' + READING_CONDUCTIVITY
 CONF_MIN_BRIGHTNESS = 'min_' + READING_BRIGHTNESS
 CONF_MAX_BRIGHTNESS = 'max_' + READING_BRIGHTNESS
+CONF_CHECK_DAYS = 'check_days'
 
 CONF_SENSOR_BATTERY_LEVEL = READING_BATTERY
 CONF_SENSOR_MOISTURE = READING_MOISTURE
@@ -67,6 +75,7 @@ PLANT_SCHEMA = vol.Schema({
     vol.Optional(CONF_MAX_CONDUCTIVITY): cv.positive_int,
     vol.Optional(CONF_MIN_BRIGHTNESS): cv.positive_int,
     vol.Optional(CONF_MAX_BRIGHTNESS): cv.positive_int,
+    vol.Optional(CONF_CHECK_DAYS): cv.positive_int,
 })
 
 DOMAIN = 'plant'
@@ -82,6 +91,11 @@ CONFIG_SCHEMA = vol.Schema({
 }, extra=vol.ALLOW_EXTRA)
 
 
+# Flag for enabling/disabling the loading of the history from the database.
+# This feature is turned off right now as it's tests are not 100% stable.
+ENABLE_LOAD_HISTORY = False
+
+
 @asyncio.coroutine
 def async_setup(hass, config):
     """Set up the Plant component."""
@@ -98,7 +112,6 @@ def async_setup(hass, config):
         entities.append(entity)
 
     yield from component.async_add_entities(entities)
-
     return True
 
 
@@ -113,31 +126,26 @@ class Plant(Entity):
         READING_BATTERY: {
             ATTR_UNIT_OF_MEASUREMENT:  '%',
             'min': CONF_MIN_BATTERY_LEVEL,
-            'icon': 'mdi:battery-outline'
         },
         READING_TEMPERATURE: {
             ATTR_UNIT_OF_MEASUREMENT: TEMP_CELSIUS,
             'min': CONF_MIN_TEMPERATURE,
             'max': CONF_MAX_TEMPERATURE,
-            'icon': 'mdi:thermometer'
         },
         READING_MOISTURE: {
             ATTR_UNIT_OF_MEASUREMENT: '%',
             'min': CONF_MIN_MOISTURE,
             'max': CONF_MAX_MOISTURE,
-            'icon': 'mdi:water'
         },
         READING_CONDUCTIVITY: {
             ATTR_UNIT_OF_MEASUREMENT: 'µS/cm',
             'min': CONF_MIN_CONDUCTIVITY,
             'max': CONF_MAX_CONDUCTIVITY,
-            'icon': 'mdi:emoticon-poop'
         },
         READING_BRIGHTNESS: {
             ATTR_UNIT_OF_MEASUREMENT: 'lux',
             'min': CONF_MIN_BRIGHTNESS,
             'max': CONF_MAX_BRIGHTNESS,
-            'icon': 'mdi:white-balance-sunny'
         }
     }
 
@@ -145,8 +153,11 @@ class Plant(Entity):
         """Initialize the Plant component."""
         self._config = config
         self._sensormap = dict()
+        self._readingmap = dict()
+        self._unit_of_measurement = dict()
         for reading, entity_id in config['sensors'].items():
             self._sensormap[entity_id] = reading
+            self._readingmap[reading] = entity_id
         self._state = STATE_UNKNOWN
         self._name = name
         self._battery = None
@@ -154,9 +165,13 @@ class Plant(Entity):
         self._conductivity = None
         self._temperature = None
         self._brightness = None
-        self._icon = 'mdi:help-circle'
         self._problems = PROBLEM_NONE
 
+        self._conf_check_days = 3  # default check interval: 3 days
+        if CONF_CHECK_DAYS in self._config:
+            self._conf_check_days = self._config[CONF_CHECK_DAYS]
+        self._brightness_history = DailyHistory(self._conf_check_days)
+
     @callback
     def state_changed(self, entity_id, _, new_state):
         """Update the sensor status.
@@ -180,9 +195,14 @@ class Plant(Entity):
             self._conductivity = int(float(value))
         elif reading == READING_BRIGHTNESS:
             self._brightness = int(float(value))
+            self._brightness_history.add_measurement(self._brightness,
+                                                     new_state.last_updated)
         else:
             raise _LOGGER.error("Unknown reading from sensor %s: %s",
                                 entity_id, value)
+        if ATTR_UNIT_OF_MEASUREMENT in new_state.attributes:
+            self._unit_of_measurement[reading] = \
+                new_state.attributes.get(ATTR_UNIT_OF_MEASUREMENT)
         self._update_state()
 
     def _update_state(self):
@@ -192,28 +212,80 @@ class Plant(Entity):
             params = self.READINGS[sensor_name]
             value = getattr(self, '_{}'.format(sensor_name))
             if value is not None:
-                if 'min' in params and params['min'] in self._config:
+                if sensor_name == READING_BRIGHTNESS:
-                    min_value = self._config[params['min']]
+                    result.append(self._check_min(
-                    if value < min_value:
+                        sensor_name, self._brightness_history.max, params))
-                        result.append('{} low'.format(sensor_name))
+                else:
-                        self._icon = params['icon']
+                    result.append(self._check_min(sensor_name, value, params))
+                result.append(self._check_max(sensor_name, value, params))
 
-                if 'max' in params and params['max'] in self._config:
+        result = [r for r in result if r is not None]
-                    max_value = self._config[params['max']]
-                    if value > max_value:
-                        result.append('{} high'.format(sensor_name))
-                        self._icon = params['icon']
 
         if result:
             self._state = STATE_PROBLEM
-            self._problems = ','.join(result)
+            self._problems = ', '.join(result)
         else:
             self._state = STATE_OK
-            self._icon = 'mdi:thumb-up'
             self._problems = PROBLEM_NONE
         _LOGGER.debug("New data processed")
         self.async_schedule_update_ha_state()
 
+    def _check_min(self, sensor_name, value, params):
+        """If configured, check the value against the defined minimum value."""
+        if 'min' in params and params['min'] in self._config:
+            min_value = self._config[params['min']]
+            if value < min_value:
+                return '{} low'.format(sensor_name)
+
+    def _check_max(self, sensor_name, value, params):
+        """If configured, check the value against the defined maximum value."""
+        if 'max' in params and params['max'] in self._config:
+            max_value = self._config[params['max']]
+            if value > max_value:
+                return '{} high'.format(sensor_name)
+        return None
+
+    @asyncio.coroutine
+    def async_added_to_hass(self):
+        """After being added to hass, load from history."""
+        if ENABLE_LOAD_HISTORY and 'recorder' in self.hass.config.components:
+            # only use the database if it's configured
+            self.hass.async_add_job(self._load_history_from_db)
+
+    @asyncio.coroutine
+    def _load_history_from_db(self):
+        """Load the history of the brightness values from the database.
+
+        This only needs to be done once during startup.
+        """
+        from homeassistant.components.recorder.models import States
+        start_date = datetime.now() - timedelta(days=self._conf_check_days)
+        entity_id = self._readingmap.get(READING_BRIGHTNESS)
+        if entity_id is None:
+            _LOGGER.debug("not reading the history from the database as "
+                          "there is no brightness sensor configured.")
+            return
+
+        _LOGGER.debug("initializing values for %s from the database",
+                      self._name)
+        with session_scope(hass=self.hass) as session:
+            query = session.query(States).filter(
+                (States.entity_id == entity_id.lower()) and
+                (States.last_updated > start_date)
+            ).order_by(States.last_updated.asc())
+            states = execute(query)
+
+            for state in states:
+                # filter out all None, NaN and "unknown" states
+                # only keep real values
+                try:
+                    self._brightness_history.add_measurement(
+                        int(state.state), state.last_updated)
+                except ValueError:
+                    pass
+        _LOGGER.debug("initializing from database completed")
+        self.async_schedule_update_ha_state()
+
     @property
     def should_poll(self):
         """No polling needed."""
@@ -237,11 +309,59 @@ class Plant(Entity):
         sensor in the attributes of the device.
         """
         attrib = {
-            ATTR_ICON: self._icon,
             ATTR_PROBLEM: self._problems,
+            ATTR_SENSORS: self._readingmap,
+            ATTR_DICT_OF_UNITS_OF_MEASUREMENT: self._unit_of_measurement,
         }
 
         for reading in self._sensormap.values():
             attrib[reading] = getattr(self, '_{}'.format(reading))
 
+        if self._brightness_history.max is not None:
+            attrib[ATTR_MAX_BRIGHTNESS_HISTORY] = self._brightness_history.max
+
         return attrib
+
+
+class DailyHistory(object):
+    """Stores one measurement per day for a maximum number of days.
+
+    At the moment only the maximum value per day is kept.
+    """
+
+    def __init__(self, max_length):
+        """Create new DailyHistory with a maximum length of the history."""
+        self.max_length = max_length
+        self._days = None
+        self._max_dict = dict()
+        self.max = None
+
+    def add_measurement(self, value, timestamp=datetime.now()):
+        """Add a new measurement for a certain day."""
+        day = timestamp.date()
+        if value is None:
+            return
+        if self._days is None:
+            self._days = deque()
+            self._add_day(day, value)
+        else:
+            current_day = self._days[-1]
+            if day == current_day:
+                self._max_dict[day] = max(value, self._max_dict[day])
+            elif day > current_day:
+                self._add_day(day, value)
+            else:
+                _LOGGER.warning('received old measurement, not storing it!')
+
+        self.max = max(self._max_dict.values())
+
+    def _add_day(self, day, value):
+        """Add a new day to the history.
+
+        Deletes the oldest day, if the queue becomes too long.
+        """
+        if len(self._days) == self.max_length:
+            oldest = self._days.popleft()
+            del self._max_dict[oldest]
+        self._days.append(day)
+        self._max_dict[day] = value

tests/components/test_plant.py

@@ -1,7 +1,16 @@
 """Unit tests for platform/plant.py."""
 import asyncio
+import unittest
+import pytest
+from datetime import datetime, timedelta
 
+from homeassistant.const import (ATTR_UNIT_OF_MEASUREMENT, STATE_UNKNOWN,
+                                 STATE_PROBLEM, STATE_OK)
+from homeassistant.components import recorder
 import homeassistant.components.plant as plant
+from homeassistant.setup import setup_component
+
+from tests.common import get_test_home_assistant, init_recorder_component
 
 
 GOOD_DATA = {
@@ -12,19 +21,23 @@ GOOD_DATA = {
     'brightness': 987,
 }
 
+BRIGHTNESS_ENTITY = 'sensor.mqtt_plant_brightness'
+MOISTURE_ENTITY = 'sensor.mqtt_plant_moisture'
+
 GOOD_CONFIG = {
     'sensors': {
-        'moisture': 'sensor.mqtt_plant_moisture',
+        'moisture': MOISTURE_ENTITY,
         'battery': 'sensor.mqtt_plant_battery',
         'temperature': 'sensor.mqtt_plant_temperature',
         'conductivity': 'sensor.mqtt_plant_conductivity',
-        'brightness': 'sensor.mqtt_plant_brightness',
+        'brightness': BRIGHTNESS_ENTITY,
     },
     'min_moisture': 20,
     'max_moisture': 60,
     'min_battery': 17,
     'min_conductivity': 500,
     'min_temperature': 15,
+    'min_brightness': 500,
 }
 
 
@@ -34,11 +47,23 @@ class _MockState(object):
         self.state = state
 
 
-@asyncio.coroutine
+class TestPlant(unittest.TestCase):
-def test_valid_data(hass):
+    """Tests for component "plant"."""
+
+    def setUp(self):
+        """Create test instance of home assistant."""
+        self.hass = get_test_home_assistant()
+        self.hass.start()
+
+    def tearDown(self):
+        """Stop everything that was started."""
+        self.hass.stop()
+
+    @asyncio.coroutine
+    def test_valid_data(self):
         """Test processing valid data."""
         sensor = plant.Plant('my plant', GOOD_CONFIG)
-    sensor.hass = hass
+        sensor.hass = self.hass
         for reading, value in GOOD_DATA.items():
             sensor.state_changed(
                 GOOD_CONFIG['sensors'][reading], None,
@@ -50,14 +75,124 @@ def test_valid_data(hass):
             # the JSON format than in hass
             assert attrib[reading] == value
 
-
+    @asyncio.coroutine
-@asyncio.coroutine
+    def test_low_battery(self):
-def test_low_battery(hass):
         """Test processing with low battery data and limit set."""
-    sensor = plant.Plant(hass, GOOD_CONFIG)
+        sensor = plant.Plant('other plant', GOOD_CONFIG)
-    sensor.hass = hass
+        sensor.hass = self.hass
         assert sensor.state_attributes['problem'] == 'none'
         sensor.state_changed('sensor.mqtt_plant_battery',
                              _MockState(45), _MockState(10))
         assert sensor.state == 'problem'
         assert sensor.state_attributes['problem'] == 'battery low'
+
+    def test_update_states(self):
+        """Test updating the state of a sensor.
+
+        Make sure that plant processes this correctly.
+        """
+        plant_name = 'some_plant'
+        assert setup_component(self.hass, plant.DOMAIN, {
+            plant.DOMAIN: {
+                plant_name: GOOD_CONFIG
+            }
+        })
+        self.hass.states.set(MOISTURE_ENTITY, 5,
+                             {ATTR_UNIT_OF_MEASUREMENT: 'us/cm'})
+        self.hass.block_till_done()
+        state = self.hass.states.get('plant.'+plant_name)
+        self.assertEquals(STATE_PROBLEM, state.state)
+        self.assertEquals(5, state.attributes[plant.READING_MOISTURE])
+
+    @pytest.mark.skipif(plant.ENABLE_LOAD_HISTORY is False,
+                        reason="tests for loading from DB are instable, thus"
+                               "this feature is turned of until tests become"
+                               "stable")
+    def test_load_from_db(self):
+        """Test bootstrapping the brightness history from the database.
+
+        This test can should only be executed if the loading of the history
+        is enabled via plant.ENABLE_LOAD_HISTORY.
+        """
+        init_recorder_component(self.hass)
+        plant_name = 'wise_plant'
+        for value in [20, 30, 10]:
+
+            self.hass.states.set(BRIGHTNESS_ENTITY, value,
+                                 {ATTR_UNIT_OF_MEASUREMENT: 'Lux'})
+            self.hass.block_till_done()
+        # wait for the recorder to really store the data
+        self.hass.data[recorder.DATA_INSTANCE].block_till_done()
+
+        assert setup_component(self.hass, plant.DOMAIN, {
+            plant.DOMAIN: {
+                plant_name: GOOD_CONFIG
+            }
+        })
+        self.hass.block_till_done()
+
+        state = self.hass.states.get('plant.'+plant_name)
+        self.assertEquals(STATE_UNKNOWN, state.state)
+        max_brightness = state.attributes.get(
+            plant.ATTR_MAX_BRIGHTNESS_HISTORY)
+        self.assertEquals(30, max_brightness)
+
+    def test_brightness_history(self):
+        """Test the min_brightness check."""
+        plant_name = 'some_plant'
+        assert setup_component(self.hass, plant.DOMAIN, {
+            plant.DOMAIN: {
+                plant_name: GOOD_CONFIG
+            }
+        })
+        self.hass.states.set(BRIGHTNESS_ENTITY, 100,
+                             {ATTR_UNIT_OF_MEASUREMENT: 'lux'})
+        self.hass.block_till_done()
+        state = self.hass.states.get('plant.'+plant_name)
+        self.assertEquals(STATE_PROBLEM, state.state)
+
+        self.hass.states.set(BRIGHTNESS_ENTITY, 600,
+                             {ATTR_UNIT_OF_MEASUREMENT: 'lux'})
+        self.hass.block_till_done()
+        state = self.hass.states.get('plant.'+plant_name)
+        self.assertEquals(STATE_OK, state.state)
+
+        self.hass.states.set(BRIGHTNESS_ENTITY, 100,
+                             {ATTR_UNIT_OF_MEASUREMENT: 'lux'})
+        self.hass.block_till_done()
+        state = self.hass.states.get('plant.'+plant_name)
+        self.assertEquals(STATE_OK, state.state)
+
+
+class TestDailyHistory(unittest.TestCase):
+    """Test the DailyHistory helper class."""
+
+    def test_no_data(self):
+        """Test with empty history."""
+        dh = plant.DailyHistory(3)
+        self.assertIsNone(dh.max)
+
+    def test_one_day(self):
+        """Test storing data for the same day."""
+        dh = plant.DailyHistory(3)
+        values = [-2, 10, 0, 5, 20]
+        for i in range(len(values)):
+            dh.add_measurement(values[i])
+            max_value = max(values[0:i+1])
+            self.assertEqual(1, len(dh._days))
+            self.assertEqual(dh.max, max_value)
+
+    def test_multiple_days(self):
+        """Test storing data for different days."""
+        dh = plant.DailyHistory(3)
+        today = datetime.now()
+        today_minus_1 = today - timedelta(days=1)
+        today_minus_2 = today_minus_1 - timedelta(days=1)
+        today_minus_3 = today_minus_2 - timedelta(days=1)
+        days = [today_minus_3, today_minus_2, today_minus_1, today]
+        values = [10, 1, 7, 3]
+        max_values = [10, 10, 10, 7]
+
+        for i in range(len(days)):
+            dh.add_measurement(values[i], days[i])
+            self.assertEquals(max_values[i], dh.max)