From 3f241e3e7d3caa1584b5f21d64b5b031602ed2cb Mon Sep 17 00:00:00 2001 From: epenet <6771947+epenet@users.noreply.github.com> Date: Mon, 9 Jan 2023 11:10:10 +0000 Subject: [PATCH 1/3] Simplify sensor state validation --- homeassistant/components/sensor/__init__.py | 58 ++++++++++----------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/homeassistant/components/sensor/__init__.py b/homeassistant/components/sensor/__init__.py index 4a5ff3de89306..30cb6a48cda54 100644 --- a/homeassistant/components/sensor/__init__.py +++ b/homeassistant/components/sensor/__init__.py @@ -401,8 +401,33 @@ def state(self) -> Any: value = self.native_value device_class = self.device_class + # Sensors with device classes indicating a non-numeric value + # should not have a state class or unit of measurement + if device_class in { + SensorDeviceClass.DATE, + SensorDeviceClass.ENUM, + SensorDeviceClass.TIMESTAMP, + }: + if self.state_class: + raise ValueError( + f"Sensor {self.entity_id} has a state class and thus indicating " + "it has a numeric value; however, it has the non-numeric " + f"device class: {device_class}" + ) + + if unit_of_measurement: + raise ValueError( + f"Sensor {self.entity_id} has a unit of measurement and thus " + "indicating it has a numeric value; however, it has the " + f"non-numeric device class: {device_class}" + ) + + # Check below only apply if there is a value + if value is None: + return None + # Received a datetime - if value is not None and device_class == SensorDeviceClass.TIMESTAMP: + if device_class == SensorDeviceClass.TIMESTAMP: try: # We cast the value, to avoid using isinstance, but satisfy # typechecking. The errors are guarded in this try. @@ -424,7 +449,7 @@ def state(self) -> Any: ) from err # Received a date value - if value is not None and device_class == SensorDeviceClass.DATE: + if device_class == SensorDeviceClass.DATE: try: # We cast the value, to avoid using isinstance, but satisfy # typechecking. The errors are guarded in this try. @@ -436,31 +461,8 @@ def state(self) -> Any: f"but provides state {value}:{type(value)} resulting in '{err}'" ) from err - # Sensors with device classes indicating a non-numeric value - # should not have a state class or unit of measurement - if device_class in { - SensorDeviceClass.DATE, - SensorDeviceClass.ENUM, - SensorDeviceClass.TIMESTAMP, - }: - if self.state_class: - raise ValueError( - f"Sensor {self.entity_id} has a state class and thus indicating " - "it has a numeric value; however, it has the non-numeric " - f"device class: {device_class}" - ) - - if unit_of_measurement: - raise ValueError( - f"Sensor {self.entity_id} has a unit of measurement and thus " - "indicating it has a numeric value; however, it has the " - f"non-numeric device class: {device_class}" - ) - # Enum checks - if value is not None and ( - device_class == SensorDeviceClass.ENUM or self.options is not None - ): + if device_class == SensorDeviceClass.ENUM or self.options is not None: if device_class != SensorDeviceClass.ENUM: reason = "is missing the enum device class" if device_class is not None: @@ -476,8 +478,7 @@ def state(self) -> Any: ) if ( - value is not None - and native_unit_of_measurement != unit_of_measurement + native_unit_of_measurement != unit_of_measurement and device_class in UNIT_CONVERTERS ): assert unit_of_measurement @@ -514,7 +515,6 @@ def state(self) -> Any: # Validate unit of measurement used for sensors with a device class if ( not self._invalid_unit_of_measurement_reported - and value is not None and device_class and (units := DEVICE_CLASS_UNITS.get(device_class)) is not None and native_unit_of_measurement not in units From 6945b0c2cd0061a54455cce233cabb848bc2eb9a Mon Sep 17 00:00:00 2001 From: epenet <6771947+epenet@users.noreply.github.com> Date: Mon, 9 Jan 2023 11:26:47 +0000 Subject: [PATCH 2/3] Adjust tests --- tests/components/sensor/test_init.py | 50 +++++++++++++++++++--------- 1 file changed, 35 insertions(+), 15 deletions(-) diff --git a/tests/components/sensor/test_init.py b/tests/components/sensor/test_init.py index 7a33f6a90bd65..f917820012688 100644 --- a/tests/components/sensor/test_init.py +++ b/tests/components/sensor/test_init.py @@ -230,25 +230,25 @@ async def test_reject_timezoneless_datetime_str( RESTORE_DATA = { - "str": {"native_unit_of_measurement": "°F", "native_value": "abc123"}, + "str": {"native_unit_of_measurement": None, "native_value": "abc123"}, "int": {"native_unit_of_measurement": "°F", "native_value": 123}, "float": {"native_unit_of_measurement": "°F", "native_value": 123.0}, "date": { - "native_unit_of_measurement": "°F", + "native_unit_of_measurement": None, "native_value": { "__type": "", "isoformat": date(2020, 2, 8).isoformat(), }, }, "datetime": { - "native_unit_of_measurement": "°F", + "native_unit_of_measurement": None, "native_value": { "__type": "", "isoformat": datetime(2020, 2, 8, 15, tzinfo=timezone.utc).isoformat(), }, }, "Decimal": { - "native_unit_of_measurement": "°F", + "native_unit_of_measurement": "kWh", "native_value": { "__type": "", "decimal_str": "123.4", @@ -266,19 +266,38 @@ async def test_reject_timezoneless_datetime_str( # None | str | int | float | date | datetime | Decimal: @pytest.mark.parametrize( - "native_value, native_value_type, expected_extra_data, device_class", + "native_value, native_value_type, expected_extra_data, device_class, uom", [ - ("abc123", str, RESTORE_DATA["str"], None), - (123, int, RESTORE_DATA["int"], SensorDeviceClass.TEMPERATURE), - (123.0, float, RESTORE_DATA["float"], SensorDeviceClass.TEMPERATURE), - (date(2020, 2, 8), dict, RESTORE_DATA["date"], SensorDeviceClass.DATE), + ("abc123", str, RESTORE_DATA["str"], None, None), + ( + 123, + int, + RESTORE_DATA["int"], + SensorDeviceClass.TEMPERATURE, + UnitOfTemperature.FAHRENHEIT, + ), + ( + 123.0, + float, + RESTORE_DATA["float"], + SensorDeviceClass.TEMPERATURE, + UnitOfTemperature.FAHRENHEIT, + ), + (date(2020, 2, 8), dict, RESTORE_DATA["date"], SensorDeviceClass.DATE, None), ( datetime(2020, 2, 8, 15, tzinfo=timezone.utc), dict, RESTORE_DATA["datetime"], SensorDeviceClass.TIMESTAMP, + None, + ), + ( + Decimal("123.4"), + dict, + RESTORE_DATA["Decimal"], + SensorDeviceClass.ENERGY, + UnitOfEnergy.KILO_WATT_HOUR, ), - (Decimal("123.4"), dict, RESTORE_DATA["Decimal"], SensorDeviceClass.ENERGY), ], ) async def test_restore_sensor_save_state( @@ -289,6 +308,7 @@ async def test_restore_sensor_save_state( native_value_type, expected_extra_data, device_class, + uom, ): """Test RestoreSensor.""" platform = getattr(hass.components, "test.sensor") @@ -296,7 +316,7 @@ async def test_restore_sensor_save_state( platform.ENTITIES["0"] = platform.MockRestoreSensor( name="Test", native_value=native_value, - native_unit_of_measurement=TEMP_FAHRENHEIT, + native_unit_of_measurement=uom, device_class=device_class, ) @@ -318,23 +338,23 @@ async def test_restore_sensor_save_state( @pytest.mark.parametrize( "native_value, native_value_type, extra_data, device_class, uom", [ - ("abc123", str, RESTORE_DATA["str"], None, "°F"), + ("abc123", str, RESTORE_DATA["str"], None, None), (123, int, RESTORE_DATA["int"], SensorDeviceClass.TEMPERATURE, "°F"), (123.0, float, RESTORE_DATA["float"], SensorDeviceClass.TEMPERATURE, "°F"), - (date(2020, 2, 8), date, RESTORE_DATA["date"], SensorDeviceClass.DATE, "°F"), + (date(2020, 2, 8), date, RESTORE_DATA["date"], SensorDeviceClass.DATE, None), ( datetime(2020, 2, 8, 15, tzinfo=timezone.utc), datetime, RESTORE_DATA["datetime"], SensorDeviceClass.TIMESTAMP, - "°F", + None, ), ( Decimal("123.4"), Decimal, RESTORE_DATA["Decimal"], SensorDeviceClass.ENERGY, - "°F", + "kWh", ), (None, type(None), None, None, None), (None, type(None), {}, None, None), From f3b5d34d4650ddd1e659159767621d70f3b4b135 Mon Sep 17 00:00:00 2001 From: epenet <6771947+epenet@users.noreply.github.com> Date: Mon, 9 Jan 2023 12:40:16 +0000 Subject: [PATCH 3/3] grammar --- homeassistant/components/sensor/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/sensor/__init__.py b/homeassistant/components/sensor/__init__.py index 30cb6a48cda54..28b3b835e6c01 100644 --- a/homeassistant/components/sensor/__init__.py +++ b/homeassistant/components/sensor/__init__.py @@ -422,7 +422,7 @@ def state(self) -> Any: f"non-numeric device class: {device_class}" ) - # Check below only apply if there is a value + # Checks below only apply if there is a value if value is None: return None