From f76a4b28064811b657246e1abbb972aeed69eb1d Mon Sep 17 00:00:00 2001 From: micw Date: Tue, 11 Apr 2017 13:52:12 +0200 Subject: [PATCH] Feature/min max improvements (#6786) * Fix #6783, remove a test that makes no sense anymore * Fix #6784 * Fix typo in docstring * Fix handling of known->unknown state, extended test, fix lint errors * Better handling of mismatch in unit of measurement. Set state to "unkown" and unit of measurement to "ERR" if unit of measurement differs between aggregatet states. Add entity_id to logged warning. * Make icon configurable * Fix typo * Fix lint * Fix lint * Fix lint * Add option to set entity_id on min_max sensor * Fix lint logging-not-lazy * Revert "Add option to set entity_id on min_max sensor" This reverts commit 4685f266477dfaba92cf8a256ecfe62c929c877b. * Revert "Make icon configurable" This reverts commit fe45aec82d9a07e730bebb4d47dc9618fb695fe6. * Fixes --- homeassistant/components/sensor/min_max.py | 71 +++++++++++++++++----- tests/components/sensor/test_min_max.py | 36 +++++++---- 2 files changed, 79 insertions(+), 28 deletions(-) diff --git a/homeassistant/components/sensor/min_max.py b/homeassistant/components/sensor/min_max.py index d612ca5cf26..33ffc769991 100644 --- a/homeassistant/components/sensor/min_max.py +++ b/homeassistant/components/sensor/min_max.py @@ -34,8 +34,6 @@ ATTR_TO_PROPERTY = [ CONF_ENTITY_IDS = 'entity_ids' CONF_ROUND_DIGITS = 'round_digits' -DEFAULT_NAME = 'Min/Max/Avg Sensor' - ICON = 'mdi:calculator' SENSOR_TYPES = { @@ -47,7 +45,7 @@ SENSOR_TYPES = { PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({ vol.Optional(CONF_TYPE, default=SENSOR_TYPES[ATTR_MAX_VALUE]): vol.All(cv.string, vol.In(SENSOR_TYPES.values())), - vol.Optional(CONF_NAME, default=DEFAULT_NAME): cv.string, + vol.Optional(CONF_NAME): cv.string, vol.Required(CONF_ENTITY_IDS): cv.entity_ids, vol.Optional(CONF_ROUND_DIGITS, default=2): vol.Coerce(int), }) @@ -67,6 +65,39 @@ def async_setup_platform(hass, config, async_add_devices, discovery_info=None): return True +def calc_min(sensor_values): + """Calculate min value, honoring unknown states.""" + val = STATE_UNKNOWN + for sval in sensor_values: + if sval != STATE_UNKNOWN: + if val == STATE_UNKNOWN or val > sval: + val = sval + return val + + +def calc_max(sensor_values): + """Calculate max value, honoring unknown states.""" + val = STATE_UNKNOWN + for sval in sensor_values: + if sval != STATE_UNKNOWN: + if val == STATE_UNKNOWN or val < sval: + val = sval + return val + + +def calc_mean(sensor_values, round_digits): + """Calculate mean value, honoring unknown states.""" + val = 0 + count = 0 + for sval in sensor_values: + if sval != STATE_UNKNOWN: + val += sval + count += 1 + if count == 0: + return STATE_UNKNOWN + return round(val/count, round_digits) + + class MinMaxSensor(Entity): """Representation of a min/max sensor.""" @@ -76,10 +107,15 @@ class MinMaxSensor(Entity): self._entity_ids = entity_ids self._sensor_type = sensor_type self._round_digits = round_digits - self._name = '{} {}'.format( - name, next(v for k, v in SENSOR_TYPES.items() - if self._sensor_type == v)) + + if name: + self._name = name + else: + self._name = '{} sensor'.format( + next(v for k, v in SENSOR_TYPES.items() + if self._sensor_type == v)).capitalize() self._unit_of_measurement = None + self._unit_of_measurement_mismatch = False self.min_value = self.max_value = self.mean = STATE_UNKNOWN self.count_sensors = len(self._entity_ids) self.states = {} @@ -89,6 +125,8 @@ class MinMaxSensor(Entity): def async_min_max_sensor_state_listener(entity, old_state, new_state): """Called when the sensor changes state.""" if new_state.state is None or new_state.state in STATE_UNKNOWN: + self.states[entity] = STATE_UNKNOWN + hass.async_add_job(self.async_update_ha_state, True) return if self._unit_of_measurement is None: @@ -97,8 +135,11 @@ class MinMaxSensor(Entity): if self._unit_of_measurement != new_state.attributes.get( ATTR_UNIT_OF_MEASUREMENT): - _LOGGER.warning("Units of measurement do not match") - return + _LOGGER.warning( + "Units of measurement do not match for entity %s", + self.entity_id) + self._unit_of_measurement_mismatch = True + try: self.states[entity] = float(new_state.state) except ValueError: @@ -118,12 +159,16 @@ class MinMaxSensor(Entity): @property def state(self): """Return the state of the sensor.""" + if self._unit_of_measurement_mismatch: + return STATE_UNKNOWN return getattr(self, next( k for k, v in SENSOR_TYPES.items() if self._sensor_type == v)) @property def unit_of_measurement(self): """Return the unit the value is expressed in.""" + if self._unit_of_measurement_mismatch: + return "ERR" return self._unit_of_measurement @property @@ -150,10 +195,6 @@ class MinMaxSensor(Entity): """Get the latest data and updates the states.""" sensor_values = [self.states[k] for k in self._entity_ids if k in self.states] - if len(sensor_values) == self.count_sensors: - self.min_value = min(sensor_values) - self.max_value = max(sensor_values) - self.mean = round(sum(sensor_values) / self.count_sensors, - self._round_digits) - else: - self.min_value = self.max_value = self.mean = STATE_UNKNOWN + self.min_value = calc_min(sensor_values) + self.max_value = calc_max(sensor_values) + self.mean = calc_mean(sensor_values, self._round_digits) diff --git a/tests/components/sensor/test_min_max.py b/tests/components/sensor/test_min_max.py index b610775b39b..a6d6a5adc68 100644 --- a/tests/components/sensor/test_min_max.py +++ b/tests/components/sensor/test_min_max.py @@ -30,7 +30,7 @@ class TestMinMaxSensor(unittest.TestCase): config = { 'sensor': { 'platform': 'min_max', - 'name': 'test', + 'name': 'test_min', 'type': 'min', 'entity_ids': [ 'sensor.test_1', @@ -59,7 +59,7 @@ class TestMinMaxSensor(unittest.TestCase): config = { 'sensor': { 'platform': 'min_max', - 'name': 'test', + 'name': 'test_max', 'type': 'max', 'entity_ids': [ 'sensor.test_1', @@ -88,7 +88,7 @@ class TestMinMaxSensor(unittest.TestCase): config = { 'sensor': { 'platform': 'min_max', - 'name': 'test', + 'name': 'test_mean', 'type': 'mean', 'entity_ids': [ 'sensor.test_1', @@ -117,7 +117,7 @@ class TestMinMaxSensor(unittest.TestCase): config = { 'sensor': { 'platform': 'min_max', - 'name': 'test', + 'name': 'test_mean', 'type': 'mean', 'round_digits': 1, 'entity_ids': [ @@ -147,7 +147,7 @@ class TestMinMaxSensor(unittest.TestCase): config = { 'sensor': { 'platform': 'min_max', - 'name': 'test', + 'name': 'test_mean', 'type': 'mean', 'round_digits': 4, 'entity_ids': [ @@ -177,7 +177,7 @@ class TestMinMaxSensor(unittest.TestCase): config = { 'sensor': { 'platform': 'min_max', - 'name': 'test', + 'name': 'test_max', 'type': 'max', 'entity_ids': [ 'sensor.test_1', @@ -191,7 +191,7 @@ class TestMinMaxSensor(unittest.TestCase): entity_ids = config['sensor']['entity_ids'] - self.hass.states.set(entity_ids[0], self.values[0]) + self.hass.states.set(entity_ids[0], STATE_UNKNOWN) self.hass.block_till_done() state = self.hass.states.get('sensor.test_max') @@ -201,14 +201,20 @@ class TestMinMaxSensor(unittest.TestCase): self.hass.block_till_done() state = self.hass.states.get('sensor.test_max') - self.assertEqual(STATE_UNKNOWN, state.state) + self.assertNotEqual(STATE_UNKNOWN, state.state) - self.hass.states.set(entity_ids[2], self.values[2]) + self.hass.states.set(entity_ids[2], STATE_UNKNOWN) self.hass.block_till_done() state = self.hass.states.get('sensor.test_max') self.assertNotEqual(STATE_UNKNOWN, state.state) + self.hass.states.set(entity_ids[1], STATE_UNKNOWN) + self.hass.block_till_done() + + state = self.hass.states.get('sensor.test_max') + self.assertEqual(STATE_UNKNOWN, state.state) + def test_different_unit_of_measurement(self): """Test for different unit of measurement.""" config = { @@ -232,21 +238,25 @@ class TestMinMaxSensor(unittest.TestCase): {ATTR_UNIT_OF_MEASUREMENT: TEMP_CELSIUS}) self.hass.block_till_done() - state = self.hass.states.get('sensor.test_mean') + state = self.hass.states.get('sensor.test') - self.assertEqual(STATE_UNKNOWN, state.state) + self.assertEqual(str(float(self.values[0])), state.state) self.assertEqual('°C', state.attributes.get('unit_of_measurement')) self.hass.states.set(entity_ids[1], self.values[1], {ATTR_UNIT_OF_MEASUREMENT: TEMP_FAHRENHEIT}) self.hass.block_till_done() + state = self.hass.states.get('sensor.test') + self.assertEqual(STATE_UNKNOWN, state.state) - self.assertEqual('°C', state.attributes.get('unit_of_measurement')) + self.assertEqual('ERR', state.attributes.get('unit_of_measurement')) self.hass.states.set(entity_ids[2], self.values[2], {ATTR_UNIT_OF_MEASUREMENT: '%'}) self.hass.block_till_done() + state = self.hass.states.get('sensor.test') + self.assertEqual(STATE_UNKNOWN, state.state) - self.assertEqual('°C', state.attributes.get('unit_of_measurement')) + self.assertEqual('ERR', state.attributes.get('unit_of_measurement'))