From 38d42de2c089376ed0babd612f221a9e877920d5 Mon Sep 17 00:00:00 2001 From: Erik Montnemery Date: Sat, 4 Sep 2021 10:47:42 +0200 Subject: [PATCH] Handle negative numbers in sensor long term statistics (#55708) * Handle negative numbers in sensor long term statistics * Use negative states in tests --- homeassistant/components/sensor/recorder.py | 44 +++++++------- tests/components/sensor/test_recorder.py | 64 ++++++++++----------- 2 files changed, 52 insertions(+), 56 deletions(-) diff --git a/homeassistant/components/sensor/recorder.py b/homeassistant/components/sensor/recorder.py index 0054b01abd2..8bf251ffb18 100644 --- a/homeassistant/components/sensor/recorder.py +++ b/homeassistant/components/sensor/recorder.py @@ -129,13 +129,6 @@ def _get_entities(hass: HomeAssistant) -> list[tuple[str, str, str | None]]: return entity_ids -# Faster than try/except -# From https://stackoverflow.com/a/23639915 -def _is_number(s: str) -> bool: # pylint: disable=invalid-name - """Return True if string is a number.""" - return s.replace(".", "", 1).isdigit() - - def _time_weighted_average( fstates: list[tuple[float, State]], start: datetime.datetime, end: datetime.datetime ) -> float: @@ -190,9 +183,13 @@ def _normalize_states( if device_class not in UNIT_CONVERSIONS: # We're not normalizing this device class, return the state as they are - fstates = [ - (float(el.state), el) for el in entity_history if _is_number(el.state) - ] + fstates = [] + for state in entity_history: + try: + fstates.append((float(state.state), state)) + except ValueError: + continue + if fstates: all_units = _get_units(fstates) if len(all_units) > 1: @@ -220,23 +217,22 @@ def _normalize_states( fstates = [] for state in entity_history: - # Exclude non numerical states from statistics - if not _is_number(state.state): - continue + try: + fstate = float(state.state) + unit = state.attributes.get(ATTR_UNIT_OF_MEASUREMENT) + # Exclude unsupported units from statistics + if unit not in UNIT_CONVERSIONS[device_class]: + if WARN_UNSUPPORTED_UNIT not in hass.data: + hass.data[WARN_UNSUPPORTED_UNIT] = set() + if entity_id not in hass.data[WARN_UNSUPPORTED_UNIT]: + hass.data[WARN_UNSUPPORTED_UNIT].add(entity_id) + _LOGGER.warning("%s has unknown unit %s", entity_id, unit) + continue - fstate = float(state.state) - unit = state.attributes.get(ATTR_UNIT_OF_MEASUREMENT) - # Exclude unsupported units from statistics - if unit not in UNIT_CONVERSIONS[device_class]: - if WARN_UNSUPPORTED_UNIT not in hass.data: - hass.data[WARN_UNSUPPORTED_UNIT] = set() - if entity_id not in hass.data[WARN_UNSUPPORTED_UNIT]: - hass.data[WARN_UNSUPPORTED_UNIT].add(entity_id) - _LOGGER.warning("%s has unknown unit %s", entity_id, unit) + fstates.append((UNIT_CONVERSIONS[device_class][unit](fstate), state)) + except ValueError: continue - fstates.append((UNIT_CONVERSIONS[device_class][unit](fstate), state)) - return DEVICE_CLASS_UNITS[device_class], fstates diff --git a/tests/components/sensor/test_recorder.py b/tests/components/sensor/test_recorder.py index 115473c23de..aeeab317eb1 100644 --- a/tests/components/sensor/test_recorder.py +++ b/tests/components/sensor/test_recorder.py @@ -50,18 +50,18 @@ GAS_SENSOR_ATTRIBUTES = { @pytest.mark.parametrize( "device_class,unit,native_unit,mean,min,max", [ - (None, "%", "%", 16.440677, 10, 30), - ("battery", "%", "%", 16.440677, 10, 30), - ("battery", None, None, 16.440677, 10, 30), - ("humidity", "%", "%", 16.440677, 10, 30), - ("humidity", None, None, 16.440677, 10, 30), - ("pressure", "Pa", "Pa", 16.440677, 10, 30), - ("pressure", "hPa", "Pa", 1644.0677, 1000, 3000), - ("pressure", "mbar", "Pa", 1644.0677, 1000, 3000), - ("pressure", "inHg", "Pa", 55674.53, 33863.89, 101591.67), - ("pressure", "psi", "Pa", 113354.48, 68947.57, 206842.71), - ("temperature", "°C", "°C", 16.440677, 10, 30), - ("temperature", "°F", "°C", -8.644068, -12.22222, -1.111111), + (None, "%", "%", 13.050847, -10, 30), + ("battery", "%", "%", 13.050847, -10, 30), + ("battery", None, None, 13.050847, -10, 30), + ("humidity", "%", "%", 13.050847, -10, 30), + ("humidity", None, None, 13.050847, -10, 30), + ("pressure", "Pa", "Pa", 13.050847, -10, 30), + ("pressure", "hPa", "Pa", 1305.0847, -1000, 3000), + ("pressure", "mbar", "Pa", 1305.0847, -1000, 3000), + ("pressure", "inHg", "Pa", 44195.25, -33863.89, 101591.67), + ("pressure", "psi", "Pa", 89982.42, -68947.57, 206842.71), + ("temperature", "°C", "°C", 13.050847, -10, 30), + ("temperature", "°F", "°C", -10.52731, -23.33333, -1.111111), ], ) def test_compile_hourly_statistics( @@ -155,8 +155,8 @@ def test_compile_hourly_statistics_unsupported(hass_recorder, caplog, attributes { "statistic_id": "sensor.test1", "start": process_timestamp_to_utc_isoformat(zero), - "mean": approx(16.440677966101696), - "min": approx(10.0), + "mean": approx(13.050847), + "min": approx(-10.0), "max": approx(30.0), "last_reset": None, "state": None, @@ -167,8 +167,8 @@ def test_compile_hourly_statistics_unsupported(hass_recorder, caplog, attributes { "statistic_id": "sensor.test6", "start": process_timestamp_to_utc_isoformat(zero), - "mean": approx(16.440677966101696), - "min": approx(10.0), + "mean": approx(13.050847), + "min": approx(-10.0), "max": approx(30.0), "last_reset": None, "state": None, @@ -179,8 +179,8 @@ def test_compile_hourly_statistics_unsupported(hass_recorder, caplog, attributes { "statistic_id": "sensor.test7", "start": process_timestamp_to_utc_isoformat(zero), - "mean": approx(16.440677966101696), - "min": approx(10.0), + "mean": approx(13.050847), + "min": approx(-10.0), "max": approx(30.0), "last_reset": None, "state": None, @@ -988,10 +988,10 @@ def test_list_statistic_ids_unsupported(hass_recorder, caplog, _attributes): @pytest.mark.parametrize( "device_class,unit,native_unit,mean,min,max", [ - (None, None, None, 16.440677, 10, 30), - (None, "%", "%", 16.440677, 10, 30), - ("battery", "%", "%", 16.440677, 10, 30), - ("battery", None, None, 16.440677, 10, 30), + (None, None, None, 13.050847, -10, 30), + (None, "%", "%", 13.050847, -10, 30), + ("battery", "%", "%", 13.050847, -10, 30), + ("battery", None, None, 13.050847, -10, 30), ], ) def test_compile_hourly_statistics_changing_units_1( @@ -1074,10 +1074,10 @@ def test_compile_hourly_statistics_changing_units_1( @pytest.mark.parametrize( "device_class,unit,native_unit,mean,min,max", [ - (None, None, None, 16.440677, 10, 30), - (None, "%", "%", 16.440677, 10, 30), - ("battery", "%", "%", 16.440677, 10, 30), - ("battery", None, None, 16.440677, 10, 30), + (None, None, None, 13.050847, -10, 30), + (None, "%", "%", 13.050847, -10, 30), + ("battery", "%", "%", 13.050847, -10, 30), + ("battery", None, None, 13.050847, -10, 30), ], ) def test_compile_hourly_statistics_changing_units_2( @@ -1119,10 +1119,10 @@ def test_compile_hourly_statistics_changing_units_2( @pytest.mark.parametrize( "device_class,unit,native_unit,mean,min,max", [ - (None, None, None, 16.440677, 10, 30), - (None, "%", "%", 16.440677, 10, 30), - ("battery", "%", "%", 16.440677, 10, 30), - ("battery", None, None, 16.440677, 10, 30), + (None, None, None, 13.050847, -10, 30), + (None, "%", "%", 13.050847, -10, 30), + ("battery", "%", "%", 13.050847, -10, 30), + ("battery", None, None, 13.050847, -10, 30), ], ) def test_compile_hourly_statistics_changing_units_3( @@ -1203,7 +1203,7 @@ def test_compile_hourly_statistics_changing_units_3( @pytest.mark.parametrize( "device_class,unit,native_unit,mean,min,max", [ - (None, None, None, 16.440677, 10, 30), + (None, None, None, 13.050847, -10, 30), ], ) def test_compile_hourly_statistics_changing_statistics( @@ -1309,7 +1309,7 @@ def record_states(hass, zero, entity_id, attributes): states = {entity_id: []} with patch("homeassistant.components.recorder.dt_util.utcnow", return_value=one): - states[entity_id].append(set_state(entity_id, "10", attributes=attributes)) + states[entity_id].append(set_state(entity_id, "-10", attributes=attributes)) with patch("homeassistant.components.recorder.dt_util.utcnow", return_value=two): states[entity_id].append(set_state(entity_id, "15", attributes=attributes))