From 119566f280598dbaf6d6d9ecd2d342c870e97036 Mon Sep 17 00:00:00 2001 From: Bas Nijholt Date: Mon, 3 Feb 2020 23:22:47 +0100 Subject: [PATCH] Keep track of the derivative for unit_time (#31397) * keep track of the derivative for unit_time In this way, you will get a better estimate of the derivate during the timescale that is relavant to the sensor. This solved a problem where sensors have a low output resolution. For example a temperature sensor that can only be integer numbers. It might report many values that are the same and then suddenly go up one value. Only in that moment (with the current implementation) the derivative will be finite. With my proposed implementation, this problem will not occur, because it takes the average derivative of the last `unit_time`. * only loop as much as needed * treat the special case of 1 entry * add option time_window * use cv.time_period * fix comment * set time_window=0 by default * rephrase comment * use timedelta for time_window * fix the "G" unit_prefix and add more prefixes https://en.wikipedia.org/wiki/Unit_prefix * add debugging lines * simplify logic * fix bug where the there was a division of unit_time instead of multiplication * simplify tests * add test_data_moving_average_for_discrete_sensor * fix test_dataSet6 * improve readability of the tests * better explain the test * remove debugging log lines --- homeassistant/components/derivative/sensor.py | 60 +++-- tests/components/derivative/test_sensor.py | 205 ++++++------------ 2 files changed, 113 insertions(+), 152 deletions(-) diff --git a/homeassistant/components/derivative/sensor.py b/homeassistant/components/derivative/sensor.py index 177d1258f3c..5e68b268685 100644 --- a/homeassistant/components/derivative/sensor.py +++ b/homeassistant/components/derivative/sensor.py @@ -27,9 +27,19 @@ CONF_ROUND_DIGITS = "round" CONF_UNIT_PREFIX = "unit_prefix" CONF_UNIT_TIME = "unit_time" CONF_UNIT = "unit" +CONF_TIME_WINDOW = "time_window" # SI Metric prefixes -UNIT_PREFIXES = {None: 1, "k": 10 ** 3, "G": 10 ** 6, "T": 10 ** 9} +UNIT_PREFIXES = { + None: 1, + "n": 1e-9, + "µ": 1e-6, + "m": 1e-3, + "k": 1e3, + "M": 1e6, + "G": 1e9, + "T": 1e12, +} # SI Time prefixes UNIT_TIME = {"s": 1, "min": 60, "h": 60 * 60, "d": 24 * 60 * 60} @@ -37,6 +47,7 @@ UNIT_TIME = {"s": 1, "min": 60, "h": 60 * 60, "d": 24 * 60 * 60} ICON = "mdi:chart-line" DEFAULT_ROUND = 3 +DEFAULT_TIME_WINDOW = 0 PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend( { @@ -46,6 +57,7 @@ PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend( vol.Optional(CONF_UNIT_PREFIX, default=None): vol.In(UNIT_PREFIXES), vol.Optional(CONF_UNIT_TIME, default="h"): vol.In(UNIT_TIME), vol.Optional(CONF_UNIT): cv.string, + vol.Optional(CONF_TIME_WINDOW, default=DEFAULT_TIME_WINDOW): cv.time_period, } ) @@ -53,12 +65,13 @@ PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend( async def async_setup_platform(hass, config, async_add_entities, discovery_info=None): """Set up the derivative sensor.""" derivative = DerivativeSensor( - config[CONF_SOURCE], - config.get(CONF_NAME), - config[CONF_ROUND_DIGITS], - config[CONF_UNIT_PREFIX], - config[CONF_UNIT_TIME], - config.get(CONF_UNIT), + source_entity=config[CONF_SOURCE], + name=config.get(CONF_NAME), + round_digits=config[CONF_ROUND_DIGITS], + unit_prefix=config[CONF_UNIT_PREFIX], + unit_time=config[CONF_UNIT_TIME], + unit_of_measurement=config.get(CONF_UNIT), + time_window=config[CONF_TIME_WINDOW], ) async_add_entities([derivative]) @@ -75,11 +88,13 @@ class DerivativeSensor(RestoreEntity): unit_prefix, unit_time, unit_of_measurement, + time_window, ): """Initialize the derivative sensor.""" self._sensor_source_id = source_entity self._round_digits = round_digits self._state = 0 + self._state_list = [] # List of tuples with (timestamp, sensor_value) self._name = name if name is not None else f"{source_entity} derivative" @@ -93,6 +108,7 @@ class DerivativeSensor(RestoreEntity): self._unit_prefix = UNIT_PREFIXES[unit_prefix] self._unit_time = UNIT_TIME[unit_time] + self._time_window = time_window.total_seconds() async def async_added_to_hass(self): """Handle entity which will be added.""" @@ -114,6 +130,19 @@ class DerivativeSensor(RestoreEntity): ): return + now = new_state.last_updated + # Filter out the tuples that are older than (and outside of the) `time_window` + self._state_list = [ + (timestamp, state) + for timestamp, state in self._state_list + if (now - timestamp).total_seconds() < self._time_window + ] + # It can happen that the list is now empty, in that case + # we use the old_state, because we cannot do anything better. + if len(self._state_list) == 0: + self._state_list.append((old_state.last_updated, old_state.state)) + self._state_list.append((new_state.last_updated, new_state.state)) + if self._unit_of_measurement is None: unit = new_state.attributes.get(ATTR_UNIT_OF_MEASUREMENT) self._unit_of_measurement = self._unit_template.format( @@ -122,13 +151,16 @@ class DerivativeSensor(RestoreEntity): try: # derivative of previous measures. - gradient = 0 - elapsed_time = ( - new_state.last_updated - old_state.last_updated - ).total_seconds() - gradient = Decimal(new_state.state) - Decimal(old_state.state) - derivative = gradient / ( - Decimal(elapsed_time) * (self._unit_prefix * self._unit_time) + last_time, last_value = self._state_list[-1] + first_time, first_value = self._state_list[0] + + elapsed_time = (last_time - first_time).total_seconds() + delta_value = Decimal(last_value) - Decimal(first_value) + derivative = ( + delta_value + / Decimal(elapsed_time) + / Decimal(self._unit_prefix) + * Decimal(self._unit_time) ) assert isinstance(derivative, Decimal) except ValueError as err: diff --git a/tests/components/derivative/test_sensor.py b/tests/components/derivative/test_sensor.py index 8893319ab36..05ce55223d0 100644 --- a/tests/components/derivative/test_sensor.py +++ b/tests/components/derivative/test_sensor.py @@ -38,26 +38,30 @@ async def test_state(hass): assert state.attributes.get("unit_of_measurement") == "kW" -async def test_dataSet1(hass): - """Test derivative sensor state.""" - config = { - "sensor": { - "platform": "derivative", - "name": "power", - "source": "sensor.energy", - "unit_time": "s", - "round": 2, - } +async def _setup_sensor(hass, config): + default_config = { + "platform": "derivative", + "name": "power", + "source": "sensor.energy", + "round": 2, } + config = {"sensor": dict(default_config, **config)} assert await async_setup_component(hass, "sensor", config) entity_id = config["sensor"]["source"] hass.states.async_set(entity_id, 0, {}) await hass.async_block_till_done() + return config, entity_id + + +async def setup_tests(hass, config, times, values, expected_state): + """Test derivative sensor state.""" + config, entity_id = await _setup_sensor(hass, config) + # Testing a energy sensor with non-monotonic intervals and values - for time, value in [(20, 10), (30, 30), (40, 5), (50, 0)]: + for time, value in zip(times, values): now = dt_util.utcnow() + timedelta(seconds=time) with patch("homeassistant.util.dt.utcnow", return_value=now): hass.states.async_set(entity_id, value, {}, force_update=True) @@ -66,163 +70,88 @@ async def test_dataSet1(hass): state = hass.states.get("sensor.power") assert state is not None - assert round(float(state.state), config["sensor"]["round"]) == -0.5 + assert round(float(state.state), config["sensor"]["round"]) == expected_state + + return state + + +async def test_dataSet1(hass): + """Test derivative sensor state.""" + await setup_tests( + hass, + {"unit_time": "s"}, + times=[20, 30, 40, 50], + values=[10, 30, 5, 0], + expected_state=-0.5, + ) async def test_dataSet2(hass): """Test derivative sensor state.""" - config = { - "sensor": { - "platform": "derivative", - "name": "power", - "source": "sensor.energy", - "unit_time": "s", - "round": 2, - } - } - - assert await async_setup_component(hass, "sensor", config) - - entity_id = config["sensor"]["source"] - hass.states.async_set(entity_id, 0, {}) - await hass.async_block_till_done() - - # Testing a energy sensor with non-monotonic intervals and values - for time, value in [(20, 5), (30, 0)]: - now = dt_util.utcnow() + timedelta(seconds=time) - with patch("homeassistant.util.dt.utcnow", return_value=now): - hass.states.async_set(entity_id, value, {}, force_update=True) - await hass.async_block_till_done() - - state = hass.states.get("sensor.power") - assert state is not None - - assert round(float(state.state), config["sensor"]["round"]) == -0.5 + await setup_tests( + hass, {"unit_time": "s"}, times=[20, 30], values=[5, 0], expected_state=-0.5 + ) async def test_dataSet3(hass): """Test derivative sensor state.""" - config = { - "sensor": { - "platform": "derivative", - "name": "power", - "source": "sensor.energy", - "unit_time": "s", - "round": 2, - } - } - - assert await async_setup_component(hass, "sensor", config) - - entity_id = config["sensor"]["source"] - hass.states.async_set(entity_id, 0, {}) - await hass.async_block_till_done() - - # Testing a energy sensor with non-monotonic intervals and values - for time, value in [(20, 5), (30, 10)]: - now = dt_util.utcnow() + timedelta(seconds=time) - with patch("homeassistant.util.dt.utcnow", return_value=now): - hass.states.async_set(entity_id, value, {}, force_update=True) - await hass.async_block_till_done() - - state = hass.states.get("sensor.power") - assert state is not None - - assert round(float(state.state), config["sensor"]["round"]) == 0.5 + state = await setup_tests( + hass, {"unit_time": "s"}, times=[20, 30], values=[5, 10], expected_state=0.5 + ) assert state.attributes.get("unit_of_measurement") == "/s" async def test_dataSet4(hass): """Test derivative sensor state.""" - config = { - "sensor": { - "platform": "derivative", - "name": "power", - "source": "sensor.energy", - "unit_time": "s", - "round": 2, - } - } - - assert await async_setup_component(hass, "sensor", config) - - entity_id = config["sensor"]["source"] - hass.states.async_set(entity_id, 0, {}) - await hass.async_block_till_done() - - # Testing a energy sensor with non-monotonic intervals and values - for time, value in [(20, 5), (30, 5)]: - now = dt_util.utcnow() + timedelta(seconds=time) - with patch("homeassistant.util.dt.utcnow", return_value=now): - hass.states.async_set(entity_id, value, {}, force_update=True) - await hass.async_block_till_done() - - state = hass.states.get("sensor.power") - assert state is not None - - assert round(float(state.state), config["sensor"]["round"]) == 0 + await setup_tests( + hass, {"unit_time": "s"}, times=[20, 30], values=[5, 5], expected_state=0 + ) async def test_dataSet5(hass): """Test derivative sensor state.""" - config = { - "sensor": { - "platform": "derivative", - "name": "power", - "source": "sensor.energy", - "unit_time": "s", - "round": 2, - } - } - - assert await async_setup_component(hass, "sensor", config) - - entity_id = config["sensor"]["source"] - hass.states.async_set(entity_id, 0, {}) - await hass.async_block_till_done() - - # Testing a energy sensor with non-monotonic intervals and values - for time, value in [(20, 10), (30, -10)]: - now = dt_util.utcnow() + timedelta(seconds=time) - with patch("homeassistant.util.dt.utcnow", return_value=now): - hass.states.async_set(entity_id, value, {}, force_update=True) - await hass.async_block_till_done() - - state = hass.states.get("sensor.power") - assert state is not None - - assert round(float(state.state), config["sensor"]["round"]) == -2 + await setup_tests( + hass, {"unit_time": "s"}, times=[20, 30], values=[10, -10], expected_state=-2 + ) async def test_dataSet6(hass): """Test derivative sensor state.""" - config = { - "sensor": { - "platform": "derivative", - "name": "power", - "source": "sensor.energy", - "round": 2, - } - } + await setup_tests(hass, {}, times=[0, 60], values=[0, 1 / 60], expected_state=1) - assert await async_setup_component(hass, "sensor", config) - entity_id = config["sensor"]["source"] - hass.states.async_set(entity_id, 0, {}) - await hass.async_block_till_done() +async def test_data_moving_average_for_discrete_sensor(hass): + """Test derivative sensor state.""" + # We simulate the following situation: + # The temperature rises 1 °C per minute for 1 hour long. + # There is a data point every second, however, the sensor returns + # the temperature rounded down to an integer value. + # We use a time window of 10 minutes and therefore we can expect + # (because the true derivative is 1 °C/min) an error of less than 10%. - # Testing a energy sensor with non-monotonic intervals and values - for time, value in [(20, 0), (30, 36000)]: + temperature_values = [] + for temperature in range(60): + temperature_values += [temperature] * 60 + time_window = 600 + + times = list(range(len(temperature_values))) + config, entity_id = await _setup_sensor( + hass, {"time_window": {"seconds": time_window}, "unit_time": "min", "round": 1} + ) # two minute window + + for time, value in zip(times, temperature_values): now = dt_util.utcnow() + timedelta(seconds=time) with patch("homeassistant.util.dt.utcnow", return_value=now): hass.states.async_set(entity_id, value, {}, force_update=True) await hass.async_block_till_done() - state = hass.states.get("sensor.power") - assert state is not None - - assert round(float(state.state), config["sensor"]["round"]) == 1 + if time_window < time < times[-1] - time_window: + state = hass.states.get("sensor.power") + derivative = round(float(state.state), config["sensor"]["round"]) + # Test that the error is never more than + # (time_window_in_minutes / true_derivative * 100) = 10% + assert abs(1 - derivative) <= 0.1 async def test_prefix(hass):