From 0a59d37e624e0d476259cfbf3b6dad0e1f71168f Mon Sep 17 00:00:00 2001 From: Erik Montnemery Date: Thu, 6 Oct 2022 20:01:54 +0200 Subject: [PATCH] Correct how unit used for statistics is determined (#79725) --- homeassistant/components/sensor/recorder.py | 35 ++++++++++++--------- tests/components/sensor/test_recorder.py | 24 +++++++------- 2 files changed, 33 insertions(+), 26 deletions(-) diff --git a/homeassistant/components/sensor/recorder.py b/homeassistant/components/sensor/recorder.py index 1a72444c758..beae06f78ff 100644 --- a/homeassistant/components/sensor/recorder.py +++ b/homeassistant/components/sensor/recorder.py @@ -149,13 +149,20 @@ def _normalize_states( state_unit = fstates[0][1].attributes.get(ATTR_UNIT_OF_MEASUREMENT) - if state_unit not in statistics.STATISTIC_UNIT_TO_UNIT_CONVERTER or ( - old_metadata - and old_metadata["unit_of_measurement"] - not in statistics.STATISTIC_UNIT_TO_UNIT_CONVERTER + statistics_unit: str | None + if not old_metadata: + # We've not seen this sensor before, the first valid state determines the unit + # used for statistics + statistics_unit = state_unit + else: + # We have seen this sensor before, use the unit from metadata + statistics_unit = old_metadata["unit_of_measurement"] + + if ( + not statistics_unit + or statistics_unit not in statistics.STATISTIC_UNIT_TO_UNIT_CONVERTER ): - # We're either not normalizing this device class or this entity is not stored - # in a unit which can be converted, return the states as they are + # The unit used by this sensor doesn't support unit conversion all_units = _get_units(fstates) if len(all_units) > 1: @@ -182,13 +189,9 @@ def _normalize_states( state_unit = fstates[0][1].attributes.get(ATTR_UNIT_OF_MEASUREMENT) return state_unit, state_unit, fstates - converter = statistics.STATISTIC_UNIT_TO_UNIT_CONVERTER[state_unit] + converter = statistics.STATISTIC_UNIT_TO_UNIT_CONVERTER[statistics_unit] valid_fstates: list[tuple[float, State]] = [] - statistics_unit: str | None = None - if old_metadata: - statistics_unit = old_metadata["unit_of_measurement"] - for fstate, state in fstates: state_unit = state.attributes.get(ATTR_UNIT_OF_MEASUREMENT) # Exclude states with unsupported unit from statistics @@ -198,14 +201,18 @@ def _normalize_states( if entity_id not in hass.data[WARN_UNSUPPORTED_UNIT]: hass.data[WARN_UNSUPPORTED_UNIT].add(entity_id) _LOGGER.warning( - "%s has unit %s which can't be converted to %s", + "The unit of %s (%s) can not be converted to the unit of previously " + "compiled statistics (%s). Generation of long term statistics " + "will be suppressed unless the unit changes back to %s or a " + "compatible unit. " + "Go to %s to fix this", entity_id, state_unit, statistics_unit, + statistics_unit, + LINK_DEV_STATISTICS, ) continue - if statistics_unit is None: - statistics_unit = state_unit valid_fstates.append( ( diff --git a/tests/components/sensor/test_recorder.py b/tests/components/sensor/test_recorder.py index 8d9e34d005f..0a72dcf6fcd 100644 --- a/tests/components/sensor/test_recorder.py +++ b/tests/components/sensor/test_recorder.py @@ -1900,12 +1900,13 @@ def test_list_statistic_ids_unsupported(hass_recorder, caplog, _attributes): @pytest.mark.parametrize( - "device_class, state_unit, display_unit, statistics_unit, unit_class, mean, min, max", + "device_class, state_unit, state_unit2, unit_class, mean, min, max", [ - (None, None, None, None, None, 13.050847, -10, 30), - (None, "%", "%", "%", None, 13.050847, -10, 30), - ("battery", "%", "%", "%", None, 13.050847, -10, 30), - ("battery", None, None, None, None, 13.050847, -10, 30), + (None, None, "cats", None, 13.050847, -10, 30), + (None, "%", "cats", None, 13.050847, -10, 30), + ("battery", "%", "cats", None, 13.050847, -10, 30), + ("battery", None, "cats", None, 13.050847, -10, 30), + (None, "kW", "Wh", "power", 13.050847, -10, 30), ], ) def test_compile_hourly_statistics_changing_units_1( @@ -1913,8 +1914,7 @@ def test_compile_hourly_statistics_changing_units_1( caplog, device_class, state_unit, - display_unit, - statistics_unit, + state_unit2, unit_class, mean, min, @@ -1931,7 +1931,7 @@ def test_compile_hourly_statistics_changing_units_1( "unit_of_measurement": state_unit, } four, states = record_states(hass, zero, "sensor.test1", attributes) - attributes["unit_of_measurement"] = "cats" + attributes["unit_of_measurement"] = state_unit2 four, _states = record_states( hass, zero + timedelta(minutes=5), "sensor.test1", attributes ) @@ -1954,7 +1954,7 @@ def test_compile_hourly_statistics_changing_units_1( "has_sum": False, "name": None, "source": "recorder", - "statistics_unit_of_measurement": statistics_unit, + "statistics_unit_of_measurement": state_unit, "unit_class": unit_class, }, ] @@ -1978,8 +1978,8 @@ def test_compile_hourly_statistics_changing_units_1( do_adhoc_statistics(hass, start=zero + timedelta(minutes=10)) wait_recording_done(hass) assert ( - "The unit of sensor.test1 (cats) can not be converted to the unit of " - f"previously compiled statistics ({display_unit})" in caplog.text + f"The unit of sensor.test1 ({state_unit2}) can not be converted to the unit of " + f"previously compiled statistics ({state_unit})" in caplog.text ) statistic_ids = list_statistic_ids(hass) assert statistic_ids == [ @@ -1989,7 +1989,7 @@ def test_compile_hourly_statistics_changing_units_1( "has_sum": False, "name": None, "source": "recorder", - "statistics_unit_of_measurement": statistics_unit, + "statistics_unit_of_measurement": state_unit, "unit_class": unit_class, }, ]