From 8877f37da01a13ef6d12a48f9bd8339b5a4c2403 Mon Sep 17 00:00:00 2001 From: Erik Montnemery Date: Tue, 24 Aug 2021 17:02:34 +0200 Subject: [PATCH] Fix statistics for sensors setting last_reset (#55136) * Re-add state_class total to sensor * Make energy cost sensor enforce state_class total_increasing * Drop state_class total * Only report energy sensor issues once --- homeassistant/components/energy/sensor.py | 26 ++++- homeassistant/components/recorder/models.py | 2 + .../components/recorder/statistics.py | 2 + homeassistant/components/sensor/recorder.py | 27 ++++- tests/components/energy/test_sensor.py | 98 ++++++++++++++++--- tests/components/history/test_init.py | 1 + tests/components/recorder/test_statistics.py | 3 + tests/components/sensor/test_recorder.py | 48 ++++++--- 8 files changed, 177 insertions(+), 30 deletions(-) diff --git a/homeassistant/components/energy/sensor.py b/homeassistant/components/energy/sensor.py index 497c762add9..5d14d50cfe2 100644 --- a/homeassistant/components/energy/sensor.py +++ b/homeassistant/components/energy/sensor.py @@ -6,6 +6,7 @@ import logging from typing import Any, Final, Literal, TypeVar, cast from homeassistant.components.sensor import ( + ATTR_STATE_CLASS, DEVICE_CLASS_MONETARY, STATE_CLASS_TOTAL_INCREASING, SensorEntity, @@ -188,6 +189,9 @@ class EnergyCostSensor(SensorEntity): utility. """ + _wrong_state_class_reported = False + _wrong_unit_reported = False + def __init__( self, adapter: SourceAdapter, @@ -223,6 +227,18 @@ class EnergyCostSensor(SensorEntity): if energy_state is None: return + if ( + state_class := energy_state.attributes.get(ATTR_STATE_CLASS) + ) != STATE_CLASS_TOTAL_INCREASING: + if not self._wrong_state_class_reported: + self._wrong_state_class_reported = True + _LOGGER.warning( + "Found unexpected state_class %s for %s", + state_class, + energy_state.entity_id, + ) + return + try: energy = float(energy_state.state) except ValueError: @@ -272,9 +288,13 @@ class EnergyCostSensor(SensorEntity): energy_unit = None if energy_unit is None: - _LOGGER.warning( - "Found unexpected unit %s for %s", energy_unit, energy_state.entity_id - ) + if not self._wrong_unit_reported: + self._wrong_unit_reported = True + _LOGGER.warning( + "Found unexpected unit %s for %s", + energy_state.attributes.get(ATTR_UNIT_OF_MEASUREMENT), + energy_state.entity_id, + ) return if energy < float(self._last_energy_sensor_state): diff --git a/homeassistant/components/recorder/models.py b/homeassistant/components/recorder/models.py index 6c532e92292..017c65cd75f 100644 --- a/homeassistant/components/recorder/models.py +++ b/homeassistant/components/recorder/models.py @@ -226,6 +226,7 @@ class StatisticData(TypedDict, total=False): mean: float min: float max: float + last_reset: datetime | None state: float sum: float @@ -249,6 +250,7 @@ class Statistics(Base): # type: ignore mean = Column(DOUBLE_TYPE) min = Column(DOUBLE_TYPE) max = Column(DOUBLE_TYPE) + last_reset = Column(DATETIME_TYPE) state = Column(DOUBLE_TYPE) sum = Column(DOUBLE_TYPE) diff --git a/homeassistant/components/recorder/statistics.py b/homeassistant/components/recorder/statistics.py index f8a9e3a6c89..34112fcc059 100644 --- a/homeassistant/components/recorder/statistics.py +++ b/homeassistant/components/recorder/statistics.py @@ -44,6 +44,7 @@ QUERY_STATISTICS = [ Statistics.mean, Statistics.min, Statistics.max, + Statistics.last_reset, Statistics.state, Statistics.sum, ] @@ -382,6 +383,7 @@ def _sorted_statistics_to_dict( "mean": convert(db_state.mean, units), "min": convert(db_state.min, units), "max": convert(db_state.max, units), + "last_reset": _process_timestamp_to_utc_isoformat(db_state.last_reset), "state": convert(db_state.state, units), "sum": convert(db_state.sum, units), } diff --git a/homeassistant/components/sensor/recorder.py b/homeassistant/components/sensor/recorder.py index 8722fef0e99..b5c00f17141 100644 --- a/homeassistant/components/sensor/recorder.py +++ b/homeassistant/components/sensor/recorder.py @@ -42,6 +42,7 @@ from homeassistant.const import ( VOLUME_CUBIC_METERS, ) from homeassistant.core import HomeAssistant, State +import homeassistant.util.dt as dt_util import homeassistant.util.pressure as pressure_util import homeassistant.util.temperature as temperature_util import homeassistant.util.volume as volume_util @@ -273,11 +274,13 @@ def compile_statistics( stat["mean"] = _time_weighted_average(fstates, start, end) if "sum" in wanted_statistics: + last_reset = old_last_reset = None new_state = old_state = None _sum = 0 last_stats = statistics.get_last_statistics(hass, 1, entity_id) if entity_id in last_stats: # We have compiled history for this sensor before, use that as a starting point + last_reset = old_last_reset = last_stats[entity_id][0]["last_reset"] new_state = old_state = last_stats[entity_id][0]["state"] _sum = last_stats[entity_id][0]["sum"] @@ -291,7 +294,13 @@ def compile_statistics( continue reset = False - if old_state is None: + if ( + state_class != STATE_CLASS_TOTAL_INCREASING + and (last_reset := state.attributes.get("last_reset")) + != old_last_reset + ): + reset = True + elif old_state is None and last_reset is None: reset = True _LOGGER.info( "Compiling initial sum statistics for %s, zero point set to %s", @@ -315,14 +324,24 @@ def compile_statistics( _sum += new_state - old_state # ..and update the starting point new_state = fstate - # Force a new cycle to start at 0 - if old_state is not None: + old_last_reset = last_reset + # Force a new cycle for STATE_CLASS_TOTAL_INCREASING to start at 0 + if ( + state_class == STATE_CLASS_TOTAL_INCREASING + and old_state is not None + ): old_state = 0.0 else: old_state = new_state else: new_state = fstate + # Deprecated, will be removed in Home Assistant 2021.11 + if last_reset is None and state_class == STATE_CLASS_MEASUREMENT: + # No valid updates + result.pop(entity_id) + continue + if new_state is None or old_state is None: # No valid updates result.pop(entity_id) @@ -330,6 +349,8 @@ def compile_statistics( # Update the sum with the last state _sum += new_state - old_state + if last_reset is not None: + stat["last_reset"] = dt_util.parse_datetime(last_reset) stat["sum"] = _sum stat["state"] = new_state diff --git a/tests/components/energy/test_sensor.py b/tests/components/energy/test_sensor.py index ea183ec52f4..7cb2640d3d2 100644 --- a/tests/components/energy/test_sensor.py +++ b/tests/components/energy/test_sensor.py @@ -17,6 +17,7 @@ from homeassistant.const import ( DEVICE_CLASS_MONETARY, ENERGY_KILO_WATT_HOUR, ENERGY_WATT_HOUR, + STATE_UNKNOWN, VOLUME_CUBIC_METERS, ) from homeassistant.setup import async_setup_component @@ -93,6 +94,11 @@ async def test_cost_sensor_price_entity( def _compile_statistics(_): return compile_statistics(hass, now, now + timedelta(seconds=1)) + energy_attributes = { + ATTR_UNIT_OF_MEASUREMENT: ENERGY_KILO_WATT_HOUR, + ATTR_STATE_CLASS: STATE_CLASS_TOTAL_INCREASING, + } + await async_init_recorder_component(hass) energy_data = data.EnergyManager.default_preferences() energy_data["energy_sources"].append( @@ -136,7 +142,7 @@ async def test_cost_sensor_price_entity( hass.states.async_set( usage_sensor_entity_id, initial_energy, - {ATTR_UNIT_OF_MEASUREMENT: ENERGY_KILO_WATT_HOUR}, + energy_attributes, ) hass.states.async_set("sensor.energy_price", "1") @@ -155,9 +161,7 @@ async def test_cost_sensor_price_entity( hass.states.async_set( usage_sensor_entity_id, "0", - { - ATTR_UNIT_OF_MEASUREMENT: ENERGY_KILO_WATT_HOUR, - }, + energy_attributes, ) await hass.async_block_till_done() @@ -176,7 +180,7 @@ async def test_cost_sensor_price_entity( hass.states.async_set( usage_sensor_entity_id, "10", - {ATTR_UNIT_OF_MEASUREMENT: ENERGY_KILO_WATT_HOUR}, + energy_attributes, ) await hass.async_block_till_done() state = hass.states.get(cost_sensor_entity_id) @@ -200,7 +204,7 @@ async def test_cost_sensor_price_entity( hass.states.async_set( usage_sensor_entity_id, "14.5", - {ATTR_UNIT_OF_MEASUREMENT: ENERGY_KILO_WATT_HOUR}, + energy_attributes, ) await hass.async_block_till_done() state = hass.states.get(cost_sensor_entity_id) @@ -216,7 +220,7 @@ async def test_cost_sensor_price_entity( hass.states.async_set( usage_sensor_entity_id, "4", - {ATTR_UNIT_OF_MEASUREMENT: ENERGY_KILO_WATT_HOUR}, + energy_attributes, ) await hass.async_block_till_done() state = hass.states.get(cost_sensor_entity_id) @@ -226,7 +230,7 @@ async def test_cost_sensor_price_entity( hass.states.async_set( usage_sensor_entity_id, "10", - {ATTR_UNIT_OF_MEASUREMENT: ENERGY_KILO_WATT_HOUR}, + energy_attributes, ) await hass.async_block_till_done() state = hass.states.get(cost_sensor_entity_id) @@ -241,6 +245,10 @@ async def test_cost_sensor_price_entity( async def test_cost_sensor_handle_wh(hass, hass_storage) -> None: """Test energy cost price from sensor entity.""" + energy_attributes = { + ATTR_UNIT_OF_MEASUREMENT: ENERGY_WATT_HOUR, + ATTR_STATE_CLASS: STATE_CLASS_TOTAL_INCREASING, + } energy_data = data.EnergyManager.default_preferences() energy_data["energy_sources"].append( { @@ -269,7 +277,7 @@ async def test_cost_sensor_handle_wh(hass, hass_storage) -> None: hass.states.async_set( "sensor.energy_consumption", 10000, - {"unit_of_measurement": ENERGY_WATT_HOUR}, + energy_attributes, ) with patch("homeassistant.util.dt.utcnow", return_value=now): @@ -282,7 +290,7 @@ async def test_cost_sensor_handle_wh(hass, hass_storage) -> None: hass.states.async_set( "sensor.energy_consumption", 20000, - {"unit_of_measurement": ENERGY_WATT_HOUR}, + energy_attributes, ) await hass.async_block_till_done() @@ -292,6 +300,10 @@ async def test_cost_sensor_handle_wh(hass, hass_storage) -> None: async def test_cost_sensor_handle_gas(hass, hass_storage) -> None: """Test gas cost price from sensor entity.""" + energy_attributes = { + ATTR_UNIT_OF_MEASUREMENT: VOLUME_CUBIC_METERS, + ATTR_STATE_CLASS: STATE_CLASS_TOTAL_INCREASING, + } energy_data = data.EnergyManager.default_preferences() energy_data["energy_sources"].append( { @@ -314,7 +326,7 @@ async def test_cost_sensor_handle_gas(hass, hass_storage) -> None: hass.states.async_set( "sensor.gas_consumption", 100, - {"unit_of_measurement": VOLUME_CUBIC_METERS}, + energy_attributes, ) with patch("homeassistant.util.dt.utcnow", return_value=now): @@ -327,9 +339,71 @@ async def test_cost_sensor_handle_gas(hass, hass_storage) -> None: hass.states.async_set( "sensor.gas_consumption", 200, - {"unit_of_measurement": VOLUME_CUBIC_METERS}, + energy_attributes, ) await hass.async_block_till_done() state = hass.states.get("sensor.gas_consumption_cost") assert state.state == "50.0" + + +@pytest.mark.parametrize("state_class", [None]) +async def test_cost_sensor_wrong_state_class( + hass, hass_storage, caplog, state_class +) -> None: + """Test energy sensor rejects wrong state_class.""" + energy_attributes = { + ATTR_UNIT_OF_MEASUREMENT: ENERGY_KILO_WATT_HOUR, + ATTR_STATE_CLASS: state_class, + } + energy_data = data.EnergyManager.default_preferences() + energy_data["energy_sources"].append( + { + "type": "grid", + "flow_from": [ + { + "stat_energy_from": "sensor.energy_consumption", + "entity_energy_from": "sensor.energy_consumption", + "stat_cost": None, + "entity_energy_price": None, + "number_energy_price": 0.5, + } + ], + "flow_to": [], + "cost_adjustment_day": 0, + } + ) + + hass_storage[data.STORAGE_KEY] = { + "version": 1, + "data": energy_data, + } + + now = dt_util.utcnow() + + hass.states.async_set( + "sensor.energy_consumption", + 10000, + energy_attributes, + ) + + with patch("homeassistant.util.dt.utcnow", return_value=now): + await setup_integration(hass) + + state = hass.states.get("sensor.energy_consumption_cost") + assert state.state == STATE_UNKNOWN + assert ( + f"Found unexpected state_class {state_class} for sensor.energy_consumption" + in caplog.text + ) + + # Energy use bumped to 10 kWh + hass.states.async_set( + "sensor.energy_consumption", + 20000, + energy_attributes, + ) + await hass.async_block_till_done() + + state = hass.states.get("sensor.energy_consumption_cost") + assert state.state == STATE_UNKNOWN diff --git a/tests/components/history/test_init.py b/tests/components/history/test_init.py index 8de44843626..7909d8f0239 100644 --- a/tests/components/history/test_init.py +++ b/tests/components/history/test_init.py @@ -911,6 +911,7 @@ async def test_statistics_during_period( "mean": approx(value), "min": approx(value), "max": approx(value), + "last_reset": None, "state": None, "sum": None, } diff --git a/tests/components/recorder/test_statistics.py b/tests/components/recorder/test_statistics.py index 995ad537ab4..318d82422d7 100644 --- a/tests/components/recorder/test_statistics.py +++ b/tests/components/recorder/test_statistics.py @@ -44,6 +44,7 @@ def test_compile_hourly_statistics(hass_recorder): "mean": approx(14.915254237288135), "min": approx(10.0), "max": approx(20.0), + "last_reset": None, "state": None, "sum": None, } @@ -53,6 +54,7 @@ def test_compile_hourly_statistics(hass_recorder): "mean": approx(20.0), "min": approx(20.0), "max": approx(20.0), + "last_reset": None, "state": None, "sum": None, } @@ -125,6 +127,7 @@ def test_rename_entity(hass_recorder): "mean": approx(14.915254237288135), "min": approx(10.0), "max": approx(20.0), + "last_reset": None, "state": None, "sum": None, } diff --git a/tests/components/sensor/test_recorder.py b/tests/components/sensor/test_recorder.py index 3a2572f8141..660c63de599 100644 --- a/tests/components/sensor/test_recorder.py +++ b/tests/components/sensor/test_recorder.py @@ -95,6 +95,7 @@ def test_compile_hourly_statistics( "mean": approx(mean), "min": approx(min), "max": approx(max), + "last_reset": None, "state": None, "sum": None, } @@ -144,6 +145,7 @@ def test_compile_hourly_statistics_unsupported(hass_recorder, caplog, attributes "mean": approx(16.440677966101696), "min": approx(10.0), "max": approx(30.0), + "last_reset": None, "state": None, "sum": None, } @@ -152,6 +154,7 @@ def test_compile_hourly_statistics_unsupported(hass_recorder, caplog, attributes assert "Error while processing event StatisticsTask" not in caplog.text +@pytest.mark.parametrize("state_class", ["measurement"]) @pytest.mark.parametrize( "device_class,unit,native_unit,factor", [ @@ -164,7 +167,7 @@ def test_compile_hourly_statistics_unsupported(hass_recorder, caplog, attributes ], ) def test_compile_hourly_sum_statistics_amount( - hass_recorder, caplog, device_class, unit, native_unit, factor + hass_recorder, caplog, state_class, device_class, unit, native_unit, factor ): """Test compiling hourly statistics.""" zero = dt_util.utcnow() @@ -173,7 +176,7 @@ def test_compile_hourly_sum_statistics_amount( setup_component(hass, "sensor", {}) attributes = { "device_class": device_class, - "state_class": "measurement", + "state_class": state_class, "unit_of_measurement": unit, "last_reset": None, } @@ -206,6 +209,7 @@ def test_compile_hourly_sum_statistics_amount( "max": None, "mean": None, "min": None, + "last_reset": process_timestamp_to_utc_isoformat(zero), "state": approx(factor * seq[2]), "sum": approx(factor * 10.0), }, @@ -215,8 +219,9 @@ def test_compile_hourly_sum_statistics_amount( "max": None, "mean": None, "min": None, + "last_reset": process_timestamp_to_utc_isoformat(four), "state": approx(factor * seq[5]), - "sum": approx(factor * 30.0), + "sum": approx(factor * 10.0), }, { "statistic_id": "sensor.test1", @@ -224,8 +229,9 @@ def test_compile_hourly_sum_statistics_amount( "max": None, "mean": None, "min": None, + "last_reset": process_timestamp_to_utc_isoformat(four), "state": approx(factor * seq[8]), - "sum": approx(factor * 60.0), + "sum": approx(factor * 40.0), }, ] } @@ -283,6 +289,7 @@ def test_compile_hourly_sum_statistics_total_increasing( "max": None, "mean": None, "min": None, + "last_reset": None, "state": approx(factor * seq[2]), "sum": approx(factor * 10.0), }, @@ -292,6 +299,7 @@ def test_compile_hourly_sum_statistics_total_increasing( "max": None, "mean": None, "min": None, + "last_reset": None, "state": approx(factor * seq[5]), "sum": approx(factor * 50.0), }, @@ -301,6 +309,7 @@ def test_compile_hourly_sum_statistics_total_increasing( "max": None, "mean": None, "min": None, + "last_reset": None, "state": approx(factor * seq[8]), "sum": approx(factor * 80.0), }, @@ -367,6 +376,7 @@ def test_compile_hourly_energy_statistics_unsupported(hass_recorder, caplog): "max": None, "mean": None, "min": None, + "last_reset": process_timestamp_to_utc_isoformat(zero), "state": approx(20.0), "sum": approx(10.0), }, @@ -376,8 +386,9 @@ def test_compile_hourly_energy_statistics_unsupported(hass_recorder, caplog): "max": None, "mean": None, "min": None, + "last_reset": process_timestamp_to_utc_isoformat(four), "state": approx(40.0), - "sum": approx(30.0), + "sum": approx(10.0), }, { "statistic_id": "sensor.test1", @@ -385,8 +396,9 @@ def test_compile_hourly_energy_statistics_unsupported(hass_recorder, caplog): "max": None, "mean": None, "min": None, + "last_reset": process_timestamp_to_utc_isoformat(four), "state": approx(70.0), - "sum": approx(60.0), + "sum": approx(40.0), }, ] } @@ -447,6 +459,7 @@ def test_compile_hourly_energy_statistics_multiple(hass_recorder, caplog): "max": None, "mean": None, "min": None, + "last_reset": process_timestamp_to_utc_isoformat(zero), "state": approx(20.0), "sum": approx(10.0), }, @@ -456,8 +469,9 @@ def test_compile_hourly_energy_statistics_multiple(hass_recorder, caplog): "max": None, "mean": None, "min": None, + "last_reset": process_timestamp_to_utc_isoformat(four), "state": approx(40.0), - "sum": approx(30.0), + "sum": approx(10.0), }, { "statistic_id": "sensor.test1", @@ -465,8 +479,9 @@ def test_compile_hourly_energy_statistics_multiple(hass_recorder, caplog): "max": None, "mean": None, "min": None, + "last_reset": process_timestamp_to_utc_isoformat(four), "state": approx(70.0), - "sum": approx(60.0), + "sum": approx(40.0), }, ], "sensor.test2": [ @@ -476,6 +491,7 @@ def test_compile_hourly_energy_statistics_multiple(hass_recorder, caplog): "max": None, "mean": None, "min": None, + "last_reset": process_timestamp_to_utc_isoformat(zero), "state": approx(130.0), "sum": approx(20.0), }, @@ -485,8 +501,9 @@ def test_compile_hourly_energy_statistics_multiple(hass_recorder, caplog): "max": None, "mean": None, "min": None, + "last_reset": process_timestamp_to_utc_isoformat(four), "state": approx(45.0), - "sum": approx(-65.0), + "sum": approx(-95.0), }, { "statistic_id": "sensor.test2", @@ -494,8 +511,9 @@ def test_compile_hourly_energy_statistics_multiple(hass_recorder, caplog): "max": None, "mean": None, "min": None, + "last_reset": process_timestamp_to_utc_isoformat(four), "state": approx(75.0), - "sum": approx(-35.0), + "sum": approx(-65.0), }, ], "sensor.test3": [ @@ -505,6 +523,7 @@ def test_compile_hourly_energy_statistics_multiple(hass_recorder, caplog): "max": None, "mean": None, "min": None, + "last_reset": process_timestamp_to_utc_isoformat(zero), "state": approx(5.0 / 1000), "sum": approx(5.0 / 1000), }, @@ -514,8 +533,9 @@ def test_compile_hourly_energy_statistics_multiple(hass_recorder, caplog): "max": None, "mean": None, "min": None, + "last_reset": process_timestamp_to_utc_isoformat(four), "state": approx(50.0 / 1000), - "sum": approx(50.0 / 1000), + "sum": approx(30.0 / 1000), }, { "statistic_id": "sensor.test3", @@ -523,8 +543,9 @@ def test_compile_hourly_energy_statistics_multiple(hass_recorder, caplog): "max": None, "mean": None, "min": None, + "last_reset": process_timestamp_to_utc_isoformat(four), "state": approx(90.0 / 1000), - "sum": approx(90.0 / 1000), + "sum": approx(70.0 / 1000), }, ], } @@ -575,6 +596,7 @@ def test_compile_hourly_statistics_unchanged( "mean": approx(value), "min": approx(value), "max": approx(value), + "last_reset": None, "state": None, "sum": None, } @@ -606,6 +628,7 @@ def test_compile_hourly_statistics_partially_unavailable(hass_recorder, caplog): "mean": approx(21.1864406779661), "min": approx(10.0), "max": approx(25.0), + "last_reset": None, "state": None, "sum": None, } @@ -662,6 +685,7 @@ def test_compile_hourly_statistics_unavailable( "mean": approx(value), "min": approx(value), "max": approx(value), + "last_reset": None, "state": None, "sum": None, }