From daf9d28565080dd083261fcfc0b397de1d7de75f Mon Sep 17 00:00:00 2001 From: ehendrix23 Date: Wed, 17 Oct 2018 10:42:39 -0600 Subject: [PATCH] Fix mold_indicator errors at startup (#17346) * Initial changes to resolve issue 16733 Added logic to ensure that if the state is unknown during startup that the error about being unable to parse the value is not logged. Further, also ensured that if an attribute is set to None it does not try to convert the None value to Fahrenheit as that will cause an error. * Cleaned up and added few comments Cleaned up some lines based on flake8 and pylint. Added some comment lines on the items added. * Changed to async and using async_added_to_hass Changed sensor to use async. Registering state tracking for sensors and initial setup is now done upon the home assistant start event. * Updated test and small fix Updated test to handle unavailable state of sensor and return of None for attributes when data is unavailable. Ensured that atributes are set to None when state is unavailable due to incorrect data. * Fixed some flake8 issues in test Fixed some flake8 issues in test_moldindicator.py. * Updates based on review Updates based on review from MartinHjelmare * Added sensor entity_id to logger errors Added sensor entity_id to logger error messages Update test to use constant STATE_UNKNOWN instead of fixed string. --- .../components/sensor/mold_indicator.py | 185 +++++++++++++----- tests/components/sensor/test_moldindicator.py | 123 +++++++++++- 2 files changed, 249 insertions(+), 59 deletions(-) diff --git a/homeassistant/components/sensor/mold_indicator.py b/homeassistant/components/sensor/mold_indicator.py index e5794ab1314..2a250f0e63d 100644 --- a/homeassistant/components/sensor/mold_indicator.py +++ b/homeassistant/components/sensor/mold_indicator.py @@ -9,12 +9,15 @@ import math import voluptuous as vol -from homeassistant.components.sensor import PLATFORM_SCHEMA from homeassistant import util -from homeassistant.helpers.entity import Entity -from homeassistant.helpers.event import track_state_change +from homeassistant.components.sensor import PLATFORM_SCHEMA +from homeassistant.core import callback from homeassistant.const import ( - ATTR_UNIT_OF_MEASUREMENT, TEMP_CELSIUS, TEMP_FAHRENHEIT, CONF_NAME) + ATTR_UNIT_OF_MEASUREMENT, EVENT_HOMEASSISTANT_START, STATE_UNKNOWN, + TEMP_CELSIUS, TEMP_FAHRENHEIT, CONF_NAME) +from homeassistant.helpers.entity import Entity +from homeassistant.helpers.event import async_track_state_change + import homeassistant.helpers.config_validation as cv _LOGGER = logging.getLogger(__name__) @@ -41,7 +44,8 @@ PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({ }) -def setup_platform(hass, config, add_entities, discovery_info=None): +async def async_setup_platform(hass, config, async_add_entities, + discovery_info=None): """Set up MoldIndicator sensor.""" name = config.get(CONF_NAME, DEFAULT_NAME) indoor_temp_sensor = config.get(CONF_INDOOR_TEMP) @@ -49,16 +53,16 @@ def setup_platform(hass, config, add_entities, discovery_info=None): indoor_humidity_sensor = config.get(CONF_INDOOR_HUMIDITY) calib_factor = config.get(CONF_CALIBRATION_FACTOR) - add_entities([MoldIndicator( - hass, name, indoor_temp_sensor, outdoor_temp_sensor, - indoor_humidity_sensor, calib_factor)], True) + async_add_entities([MoldIndicator( + name, hass.config.units.is_metric, indoor_temp_sensor, + outdoor_temp_sensor, indoor_humidity_sensor, calib_factor)], False) class MoldIndicator(Entity): """Represents a MoldIndication sensor.""" - def __init__(self, hass, name, indoor_temp_sensor, outdoor_temp_sensor, - indoor_humidity_sensor, calib_factor): + def __init__(self, name, is_metric, indoor_temp_sensor, + outdoor_temp_sensor, indoor_humidity_sensor, calib_factor): """Initialize the sensor.""" self._state = None self._name = name @@ -66,7 +70,11 @@ class MoldIndicator(Entity): self._indoor_humidity_sensor = indoor_humidity_sensor self._outdoor_temp_sensor = outdoor_temp_sensor self._calib_factor = calib_factor - self._is_metric = hass.config.units.is_metric + self._is_metric = is_metric + self._available = False + self._entities = set([self._indoor_temp_sensor, + self._indoor_humidity_sensor, + self._outdoor_temp_sensor]) self._dewpoint = None self._indoor_temp = None @@ -74,34 +82,85 @@ class MoldIndicator(Entity): self._indoor_hum = None self._crit_temp = None - track_state_change(hass, indoor_temp_sensor, self._sensor_changed) - track_state_change(hass, outdoor_temp_sensor, self._sensor_changed) - track_state_change(hass, indoor_humidity_sensor, self._sensor_changed) + async def async_added_to_hass(self): + """Register callbacks.""" + @callback + def mold_indicator_sensors_state_listener(entity, old_state, + new_state): + """Handle for state changes for dependent sensors.""" + _LOGGER.debug("Sensor state change for %s that had old state %s " + "and new state %s", entity, old_state, new_state) - # Read initial state - indoor_temp = hass.states.get(indoor_temp_sensor) - outdoor_temp = hass.states.get(outdoor_temp_sensor) - indoor_hum = hass.states.get(indoor_humidity_sensor) + if self._update_sensor(entity, old_state, new_state): + self.async_schedule_update_ha_state(True) - if indoor_temp: - self._indoor_temp = MoldIndicator._update_temp_sensor(indoor_temp) + @callback + def mold_indicator_startup(event): + """Add listeners and get 1st state.""" + _LOGGER.debug("Startup for %s", self.entity_id) - if outdoor_temp: - self._outdoor_temp = MoldIndicator._update_temp_sensor( - outdoor_temp) + async_track_state_change(self.hass, self._entities, + mold_indicator_sensors_state_listener) - if indoor_hum: - self._indoor_hum = MoldIndicator._update_hum_sensor(indoor_hum) + # Read initial state + indoor_temp = self.hass.states.get(self._indoor_temp_sensor) + outdoor_temp = self.hass.states.get(self._outdoor_temp_sensor) + indoor_hum = self.hass.states.get(self._indoor_humidity_sensor) + + schedule_update = self._update_sensor(self._indoor_temp_sensor, + None, indoor_temp) + + schedule_update = False if not self._update_sensor( + self._outdoor_temp_sensor, None, outdoor_temp) else\ + schedule_update + + schedule_update = False if not self._update_sensor( + self._indoor_humidity_sensor, None, indoor_hum) else\ + schedule_update + + if schedule_update: + self.async_schedule_update_ha_state(True) + + self.hass.bus.async_listen_once( + EVENT_HOMEASSISTANT_START, mold_indicator_startup) + + def _update_sensor(self, entity, old_state, new_state): + """Update information based on new sensor states.""" + _LOGGER.debug("Sensor update for %s", entity) + if new_state is None: + return False + + # If old_state is not set and new state is unknown then it means + # that the sensor just started up + if old_state is None and new_state.state == STATE_UNKNOWN: + return False + + if entity == self._indoor_temp_sensor: + self._indoor_temp = MoldIndicator._update_temp_sensor(new_state) + elif entity == self._outdoor_temp_sensor: + self._outdoor_temp = MoldIndicator._update_temp_sensor(new_state) + elif entity == self._indoor_humidity_sensor: + self._indoor_hum = MoldIndicator._update_hum_sensor(new_state) + + return True @staticmethod def _update_temp_sensor(state): """Parse temperature sensor value.""" + _LOGGER.debug("Updating temp sensor with value %s", state.state) + + # Return an error if the sensor change its state to Unknown. + if state.state == STATE_UNKNOWN: + _LOGGER.error("Unable to parse temperature sensor %s with state:" + " %s", state.entity_id, state.state) + return None + unit = state.attributes.get(ATTR_UNIT_OF_MEASUREMENT) temp = util.convert(state.state, float) if temp is None: - _LOGGER.error('Unable to parse sensor temperature: %s', - state.state) + _LOGGER.error("Unable to parse temperature sensor %s with state:" + " %s", state.entity_id, state.state) return None # convert to celsius if necessary @@ -109,56 +168,62 @@ class MoldIndicator(Entity): return util.temperature.fahrenheit_to_celsius(temp) if unit == TEMP_CELSIUS: return temp - _LOGGER.error("Temp sensor has unsupported unit: %s (allowed: %s, " - "%s)", unit, TEMP_CELSIUS, TEMP_FAHRENHEIT) + _LOGGER.error("Temp sensor %s has unsupported unit: %s (allowed: %s, " + "%s)", state.entity_id, unit, TEMP_CELSIUS, + TEMP_FAHRENHEIT) return None @staticmethod def _update_hum_sensor(state): """Parse humidity sensor value.""" + _LOGGER.debug("Updating humidity sensor with value %s", state.state) + + # Return an error if the sensor change its state to Unknown. + if state.state == STATE_UNKNOWN: + _LOGGER.error('Unable to parse humidity sensor %s, state: %s', + state.entity_id, state.state) + return None + unit = state.attributes.get(ATTR_UNIT_OF_MEASUREMENT) hum = util.convert(state.state, float) if hum is None: - _LOGGER.error('Unable to parse sensor humidity: %s', - state.state) + _LOGGER.error("Unable to parse humidity sensor %s, state: %s", + state.entity_id, state.state) return None if unit != '%': - _LOGGER.error("Humidity sensor has unsupported unit: %s %s", - unit, " (allowed: %)") + _LOGGER.error("Humidity sensor %s has unsupported unit: %s %s", + state.entity_id, unit, " (allowed: %)") + return None if hum > 100 or hum < 0: - _LOGGER.error("Humidity sensor out of range: %s %s", hum, - " (allowed: 0-100%)") + _LOGGER.error("Humidity sensor %s is out of range: %s %s", + state.entity_id, hum, "(allowed: 0-100%)") + return None return hum - def update(self): + async def async_update(self): """Calculate latest state.""" + _LOGGER.debug("Update state for %s", self.entity_id) # check all sensors if None in (self._indoor_temp, self._indoor_hum, self._outdoor_temp): + self._available = False + self._dewpoint = None + self._crit_temp = None return # re-calculate dewpoint and mold indicator self._calc_dewpoint() self._calc_moldindicator() - - def _sensor_changed(self, entity_id, old_state, new_state): - """Handle sensor state changes.""" - if new_state is None: - return - - if entity_id == self._indoor_temp_sensor: - self._indoor_temp = MoldIndicator._update_temp_sensor(new_state) - elif entity_id == self._outdoor_temp_sensor: - self._outdoor_temp = MoldIndicator._update_temp_sensor(new_state) - elif entity_id == self._indoor_humidity_sensor: - self._indoor_hum = MoldIndicator._update_hum_sensor(new_state) - - self.update() - self.schedule_update_ha_state() + if self._state is None: + self._available = False + self._dewpoint = None + self._crit_temp = None + else: + self._available = True def _calc_dewpoint(self): """Calculate the dewpoint for the indoor air.""" @@ -183,6 +248,8 @@ class MoldIndicator(Entity): " calibration-factor: %s", self._dewpoint, self._calib_factor) self._state = None + self._available = False + self._crit_temp = None return # first calculate the approximate temperature at the calibration point @@ -232,6 +299,11 @@ class MoldIndicator(Entity): """Return the state of the entity.""" return self._state + @property + def available(self): + """Return the availability of this sensor.""" + return self._available + @property def device_state_attributes(self): """Return the state attributes.""" @@ -240,9 +312,16 @@ class MoldIndicator(Entity): ATTR_DEWPOINT: self._dewpoint, ATTR_CRITICAL_TEMP: self._crit_temp, } + + dewpoint = util.temperature.celsius_to_fahrenheit(self._dewpoint) \ + if self._dewpoint is not None else None + + crit_temp = util.temperature.celsius_to_fahrenheit(self._crit_temp) \ + if self._crit_temp is not None else None + return { ATTR_DEWPOINT: - util.temperature.celsius_to_fahrenheit(self._dewpoint), + dewpoint, ATTR_CRITICAL_TEMP: - util.temperature.celsius_to_fahrenheit(self._crit_temp), + crit_temp, } diff --git a/tests/components/sensor/test_moldindicator.py b/tests/components/sensor/test_moldindicator.py index 4f1b40bf9ef..7b2480f1298 100644 --- a/tests/components/sensor/test_moldindicator.py +++ b/tests/components/sensor/test_moldindicator.py @@ -5,8 +5,8 @@ from homeassistant.setup import setup_component import homeassistant.components.sensor as sensor from homeassistant.components.sensor.mold_indicator import (ATTR_DEWPOINT, ATTR_CRITICAL_TEMP) -from homeassistant.const import (ATTR_UNIT_OF_MEASUREMENT, - TEMP_CELSIUS) +from homeassistant.const import ( + ATTR_UNIT_OF_MEASUREMENT, STATE_UNKNOWN, TEMP_CELSIUS) from tests.common import get_test_home_assistant @@ -44,7 +44,7 @@ class TestSensorMoldIndicator(unittest.TestCase): assert moldind assert '%' == moldind.attributes.get('unit_of_measurement') - def test_invalidhum(self): + def test_invalidcalib(self): """Test invalid sensor values.""" self.hass.states.set('test.indoortemp', '10', {ATTR_UNIT_OF_MEASUREMENT: TEMP_CELSIUS}) @@ -53,6 +53,32 @@ class TestSensorMoldIndicator(unittest.TestCase): self.hass.states.set('test.indoorhumidity', '0', {ATTR_UNIT_OF_MEASUREMENT: '%'}) + self.assertTrue(setup_component(self.hass, sensor.DOMAIN, { + 'sensor': { + 'platform': 'mold_indicator', + 'indoor_temp_sensor': 'test.indoortemp', + 'outdoor_temp_sensor': 'test.outdoortemp', + 'indoor_humidity_sensor': 'test.indoorhumidity', + 'calibration_factor': 0 + } + })) + self.hass.start() + self.hass.block_till_done() + moldind = self.hass.states.get('sensor.mold_indicator') + assert moldind + assert moldind.state == 'unavailable' + assert moldind.attributes.get(ATTR_DEWPOINT) is None + assert moldind.attributes.get(ATTR_CRITICAL_TEMP) is None + + def test_invalidhum(self): + """Test invalid sensor values.""" + self.hass.states.set('test.indoortemp', '10', + {ATTR_UNIT_OF_MEASUREMENT: TEMP_CELSIUS}) + self.hass.states.set('test.outdoortemp', '10', + {ATTR_UNIT_OF_MEASUREMENT: TEMP_CELSIUS}) + self.hass.states.set('test.indoorhumidity', '-1', + {ATTR_UNIT_OF_MEASUREMENT: '%'}) + self.assertTrue(setup_component(self.hass, sensor.DOMAIN, { 'sensor': { 'platform': 'mold_indicator', @@ -62,9 +88,32 @@ class TestSensorMoldIndicator(unittest.TestCase): 'calibration_factor': 2.0 } })) + + self.hass.start() + self.hass.block_till_done() moldind = self.hass.states.get('sensor.mold_indicator') assert moldind - assert moldind.state == '0' + assert moldind.state == 'unavailable' + assert moldind.attributes.get(ATTR_DEWPOINT) is None + assert moldind.attributes.get(ATTR_CRITICAL_TEMP) is None + + self.hass.states.set('test.indoorhumidity', 'A', + {ATTR_UNIT_OF_MEASUREMENT: '%'}) + self.hass.block_till_done() + moldind = self.hass.states.get('sensor.mold_indicator') + assert moldind + assert moldind.state == 'unavailable' + assert moldind.attributes.get(ATTR_DEWPOINT) is None + assert moldind.attributes.get(ATTR_CRITICAL_TEMP) is None + + self.hass.states.set('test.indoorhumidity', '10', + {ATTR_UNIT_OF_MEASUREMENT: TEMP_CELSIUS}) + self.hass.block_till_done() + moldind = self.hass.states.get('sensor.mold_indicator') + assert moldind + assert moldind.state == 'unavailable' + assert moldind.attributes.get(ATTR_DEWPOINT) is None + assert moldind.attributes.get(ATTR_CRITICAL_TEMP) is None def test_calculation(self): """Test the mold indicator internal calculations.""" @@ -77,7 +126,8 @@ class TestSensorMoldIndicator(unittest.TestCase): 'calibration_factor': 2.0 } })) - + self.hass.start() + self.hass.block_till_done() moldind = self.hass.states.get('sensor.mold_indicator') assert moldind @@ -98,6 +148,66 @@ class TestSensorMoldIndicator(unittest.TestCase): assert state assert state == '68' + def test_unknown_sensor(self): + """Test the sensor_changed function.""" + self.assertTrue(setup_component(self.hass, sensor.DOMAIN, { + 'sensor': { + 'platform': 'mold_indicator', + 'indoor_temp_sensor': 'test.indoortemp', + 'outdoor_temp_sensor': 'test.outdoortemp', + 'indoor_humidity_sensor': 'test.indoorhumidity', + 'calibration_factor': 2.0 + } + })) + self.hass.start() + + self.hass.states.set('test.indoortemp', STATE_UNKNOWN, + {ATTR_UNIT_OF_MEASUREMENT: TEMP_CELSIUS}) + self.hass.block_till_done() + moldind = self.hass.states.get('sensor.mold_indicator') + assert moldind + assert moldind.state == 'unavailable' + assert moldind.attributes.get(ATTR_DEWPOINT) is None + assert moldind.attributes.get(ATTR_CRITICAL_TEMP) is None + + self.hass.states.set('test.indoortemp', '30', + {ATTR_UNIT_OF_MEASUREMENT: TEMP_CELSIUS}) + self.hass.states.set('test.outdoortemp', STATE_UNKNOWN, + {ATTR_UNIT_OF_MEASUREMENT: TEMP_CELSIUS}) + self.hass.block_till_done() + moldind = self.hass.states.get('sensor.mold_indicator') + assert moldind + assert moldind.state == 'unavailable' + assert moldind.attributes.get(ATTR_DEWPOINT) is None + assert moldind.attributes.get(ATTR_CRITICAL_TEMP) is None + + self.hass.states.set('test.outdoortemp', '25', + {ATTR_UNIT_OF_MEASUREMENT: TEMP_CELSIUS}) + self.hass.states.set('test.indoorhumidity', STATE_UNKNOWN, + {ATTR_UNIT_OF_MEASUREMENT: '%'}) + self.hass.block_till_done() + moldind = self.hass.states.get('sensor.mold_indicator') + assert moldind + assert moldind.state == 'unavailable' + assert moldind.attributes.get(ATTR_DEWPOINT) is None + assert moldind.attributes.get(ATTR_CRITICAL_TEMP) is None + + self.hass.states.set('test.indoorhumidity', '20', + {ATTR_UNIT_OF_MEASUREMENT: '%'}) + self.hass.block_till_done() + moldind = self.hass.states.get('sensor.mold_indicator') + assert moldind + assert moldind.state == '23' + + dewpoint = moldind.attributes.get(ATTR_DEWPOINT) + assert dewpoint + assert dewpoint > 4.58 + assert dewpoint < 4.59 + + esttemp = moldind.attributes.get(ATTR_CRITICAL_TEMP) + assert esttemp + assert esttemp == 27.5 + def test_sensor_changed(self): """Test the sensor_changed function.""" self.assertTrue(setup_component(self.hass, sensor.DOMAIN, { @@ -109,6 +219,7 @@ class TestSensorMoldIndicator(unittest.TestCase): 'calibration_factor': 2.0 } })) + self.hass.start() self.hass.states.set('test.indoortemp', '30', {ATTR_UNIT_OF_MEASUREMENT: TEMP_CELSIUS}) @@ -121,6 +232,6 @@ class TestSensorMoldIndicator(unittest.TestCase): assert self.hass.states.get('sensor.mold_indicator').state == '57' self.hass.states.set('test.indoorhumidity', '20', - {ATTR_UNIT_OF_MEASUREMENT: TEMP_CELSIUS}) + {ATTR_UNIT_OF_MEASUREMENT: '%'}) self.hass.block_till_done() assert self.hass.states.get('sensor.mold_indicator').state == '23'