diff --git a/homeassistant/components/sensor/__init__.py b/homeassistant/components/sensor/__init__.py index 45d8c8b3c06..05fec64608f 100644 --- a/homeassistant/components/sensor/__init__.py +++ b/homeassistant/components/sensor/__init__.py @@ -219,6 +219,7 @@ class SensorEntity(Entity, cached_properties=CACHED_PROPERTIES_WITH_ATTR_): _last_reset_reported = False _sensor_option_display_precision: int | None = None _sensor_option_unit_of_measurement: str | None | UndefinedType = UNDEFINED + _invalid_suggested_unit_of_measurement_reported = False @callback def add_to_platform_start( @@ -376,6 +377,34 @@ class SensorEntity(Entity, cached_properties=CACHED_PROPERTIES_WITH_ATTR_): return None + def _is_valid_suggested_unit(self, suggested_unit_of_measurement: str) -> bool: + """Validate the suggested unit. + + Validate that a unit converter exists for the sensor's device class and that the + unit converter supports both the native and the suggested units of measurement. + """ + # Make sure we can convert the units + if ( + (unit_converter := UNIT_CONVERTERS.get(self.device_class)) is None + or self.native_unit_of_measurement not in unit_converter.VALID_UNITS + or suggested_unit_of_measurement not in unit_converter.VALID_UNITS + ): + if not self._invalid_suggested_unit_of_measurement_reported: + self._invalid_suggested_unit_of_measurement_reported = True + report_issue = self._suggest_report_issue() + # This should raise in Home Assistant Core 2024.5 + _LOGGER.warning( + ( + "%s sets an invalid suggested_unit_of_measurement. Please %s. " + "This warning will become an error in Home Assistant Core 2024.5" + ), + type(self), + report_issue, + ) + return False + + return True + def _get_initial_suggested_unit(self) -> str | UndefinedType: """Return the initial unit.""" # Unit suggested by the integration @@ -390,6 +419,10 @@ class SensorEntity(Entity, cached_properties=CACHED_PROPERTIES_WITH_ATTR_): if suggested_unit_of_measurement is None: return UNDEFINED + # Make sure we can convert the units + if not self._is_valid_suggested_unit(suggested_unit_of_measurement): + return UNDEFINED + return suggested_unit_of_measurement def get_initial_entity_options(self) -> er.EntityOptionsType | None: @@ -486,16 +519,17 @@ class SensorEntity(Entity, cached_properties=CACHED_PROPERTIES_WITH_ATTR_): if self._sensor_option_unit_of_measurement is not UNDEFINED: return self._sensor_option_unit_of_measurement + native_unit_of_measurement = self.native_unit_of_measurement + # Second priority, for non registered entities: unit suggested by integration if not self.registry_entry and ( suggested_unit_of_measurement := self.suggested_unit_of_measurement ): - return suggested_unit_of_measurement + if self._is_valid_suggested_unit(suggested_unit_of_measurement): + return suggested_unit_of_measurement # Third priority: Legacy temperature conversion, which applies # to both registered and non registered entities - native_unit_of_measurement = self.native_unit_of_measurement - if ( native_unit_of_measurement in TEMPERATURE_UNITS and self.device_class is SensorDeviceClass.TEMPERATURE diff --git a/tests/components/ecovacs/snapshots/test_sensor.ambr b/tests/components/ecovacs/snapshots/test_sensor.ambr index ab0de50ea09..5b072b6c232 100644 --- a/tests/components/ecovacs/snapshots/test_sensor.ambr +++ b/tests/components/ecovacs/snapshots/test_sensor.ambr @@ -326,9 +326,6 @@ 'id': , 'name': None, 'options': dict({ - 'sensor.private': dict({ - 'suggested_unit_of_measurement': , - }), }), 'original_device_class': None, 'original_icon': None, @@ -338,7 +335,7 @@ 'supported_features': 0, 'translation_key': 'stats_time', 'unique_id': 'E1234567890000000001_stats_time', - 'unit_of_measurement': , + 'unit_of_measurement': , }) # --- # name: test_sensors[yna5x1-entity_ids0][sensor.ozmo_950_time_cleaned:state] @@ -468,9 +465,6 @@ 'id': , 'name': None, 'options': dict({ - 'sensor.private': dict({ - 'suggested_unit_of_measurement': , - }), }), 'original_device_class': None, 'original_icon': None, @@ -480,7 +474,7 @@ 'supported_features': 0, 'translation_key': 'total_stats_time', 'unique_id': 'E1234567890000000001_total_stats_time', - 'unit_of_measurement': , + 'unit_of_measurement': , }) # --- # name: test_sensors[yna5x1-entity_ids0][sensor.ozmo_950_total_time_cleaned:state] diff --git a/tests/components/sensor/test_init.py b/tests/components/sensor/test_init.py index 3172759520d..98b3f2423cc 100644 --- a/tests/components/sensor/test_init.py +++ b/tests/components/sensor/test_init.py @@ -4,6 +4,7 @@ from __future__ import annotations from collections.abc import Generator from datetime import UTC, date, datetime from decimal import Decimal +import logging from types import ModuleType from typing import Any @@ -29,6 +30,7 @@ from homeassistant.const import ( PERCENTAGE, STATE_UNKNOWN, EntityCategory, + UnitOfDataRate, UnitOfEnergy, UnitOfLength, UnitOfMass, @@ -2604,3 +2606,120 @@ def test_deprecated_constants_sensor_device_class( import_and_test_deprecated_constant_enum( caplog, sensor, enum, "DEVICE_CLASS_", "2025.1" ) + + +@pytest.mark.parametrize( + ("device_class", "native_unit"), + [ + (SensorDeviceClass.TEMPERATURE, UnitOfTemperature.CELSIUS), + (SensorDeviceClass.DATA_RATE, UnitOfDataRate.KILOBITS_PER_SECOND), + ], +) +async def test_suggested_unit_guard_invalid_unit( + hass: HomeAssistant, + caplog: pytest.LogCaptureFixture, + device_class: SensorDeviceClass, + native_unit: str, +) -> None: + """Test suggested_unit_of_measurement guard. + + An invalid suggested unit creates a log entry and the suggested unit will be ignored. + """ + entity_registry = er.async_get(hass) + platform = getattr(hass.components, "test.sensor") + platform.init(empty=True) + + state_value = 10 + invalid_suggested_unit = "invalid_unit" + + entity = platform.ENTITIES["0"] = platform.MockSensor( + name="Invalid", + device_class=device_class, + native_unit_of_measurement=native_unit, + suggested_unit_of_measurement=invalid_suggested_unit, + native_value=str(state_value), + unique_id="invalid", + ) + assert await async_setup_component(hass, "sensor", {"sensor": {"platform": "test"}}) + await hass.async_block_till_done() + + # Unit of measurement should be native one + state = hass.states.get(entity.entity_id) + assert int(state.state) == state_value + assert state.attributes[ATTR_UNIT_OF_MEASUREMENT] == native_unit + + # Assert the suggested unit is ignored and not stored in the entity registry + entry = entity_registry.async_get(entity.entity_id) + assert entry.unit_of_measurement == native_unit + assert entry.options == {} + assert ( + "homeassistant.components.sensor", + logging.WARNING, + ( + " sets an" + " invalid suggested_unit_of_measurement. Please report it to the author" + " of the 'test' custom integration. This warning will become an error in" + " Home Assistant Core 2024.5" + ), + ) in caplog.record_tuples + + +@pytest.mark.parametrize( + ("device_class", "native_unit", "native_value", "suggested_unit", "expect_value"), + [ + ( + SensorDeviceClass.TEMPERATURE, + UnitOfTemperature.CELSIUS, + 10, + UnitOfTemperature.KELVIN, + 283, + ), + ( + SensorDeviceClass.DATA_RATE, + UnitOfDataRate.KILOBITS_PER_SECOND, + 10, + UnitOfDataRate.BITS_PER_SECOND, + 10000, + ), + ], +) +async def test_suggested_unit_guard_valid_unit( + hass: HomeAssistant, + device_class: SensorDeviceClass, + native_unit: str, + native_value: int, + suggested_unit: str, + expect_value: float | int, +) -> None: + """Test suggested_unit_of_measurement guard. + + Suggested unit is valid and therefore should be used for unit conversion and stored + in the entity registry. + """ + entity_registry = er.async_get(hass) + platform = getattr(hass.components, "test.sensor") + platform.init(empty=True) + + entity = platform.ENTITIES["0"] = platform.MockSensor( + name="Valid", + device_class=device_class, + native_unit_of_measurement=native_unit, + native_value=str(native_value), + suggested_unit_of_measurement=suggested_unit, + unique_id="valid", + ) + + assert await async_setup_component(hass, "sensor", {"sensor": {"platform": "test"}}) + await hass.async_block_till_done() + + # Unit of measurement should set to the suggested unit of measurement + state = hass.states.get(entity.entity_id) + assert float(state.state) == expect_value + assert state.attributes[ATTR_UNIT_OF_MEASUREMENT] == suggested_unit + + # Assert the suggested unit of measurement is stored in the registry + entry = entity_registry.async_get(entity.entity_id) + assert entry.unit_of_measurement == suggested_unit + assert entry.options == { + "sensor.private": {"suggested_unit_of_measurement": suggested_unit}, + } diff --git a/tests/components/withings/snapshots/test_sensor.ambr b/tests/components/withings/snapshots/test_sensor.ambr index 29b3dafb910..8e3866a7561 100644 --- a/tests/components/withings/snapshots/test_sensor.ambr +++ b/tests/components/withings/snapshots/test_sensor.ambr @@ -71,9 +71,6 @@ 'id': , 'name': None, 'options': dict({ - 'sensor.private': dict({ - 'suggested_unit_of_measurement': , - }), }), 'original_device_class': , 'original_icon': None, @@ -83,7 +80,7 @@ 'supported_features': 0, 'translation_key': 'activity_active_duration_today', 'unique_id': 'withings_12345_activity_active_duration_today', - 'unit_of_measurement': , + 'unit_of_measurement': , }) # --- # name: test_all_entities[sensor.henk_active_time_today-state] @@ -1176,9 +1173,6 @@ 'id': , 'name': None, 'options': dict({ - 'sensor.private': dict({ - 'suggested_unit_of_measurement': , - }), }), 'original_device_class': , 'original_icon': None, @@ -1188,7 +1182,7 @@ 'supported_features': 0, 'translation_key': 'activity_intense_duration_today', 'unique_id': 'withings_12345_activity_intense_duration_today', - 'unit_of_measurement': , + 'unit_of_measurement': , }) # --- # name: test_all_entities[sensor.henk_intense_activity_today-state] @@ -1274,9 +1268,6 @@ 'id': , 'name': None, 'options': dict({ - 'sensor.private': dict({ - 'suggested_unit_of_measurement': , - }), }), 'original_device_class': , 'original_icon': None, @@ -1286,7 +1277,7 @@ 'supported_features': 0, 'translation_key': 'workout_duration', 'unique_id': 'withings_12345_workout_duration', - 'unit_of_measurement': , + 'unit_of_measurement': , }) # --- # name: test_all_entities[sensor.henk_last_workout_duration-state] @@ -1750,9 +1741,6 @@ 'id': , 'name': None, 'options': dict({ - 'sensor.private': dict({ - 'suggested_unit_of_measurement': , - }), }), 'original_device_class': , 'original_icon': None, @@ -1762,7 +1750,7 @@ 'supported_features': 0, 'translation_key': 'activity_moderate_duration_today', 'unique_id': 'withings_12345_activity_moderate_duration_today', - 'unit_of_measurement': , + 'unit_of_measurement': , }) # --- # name: test_all_entities[sensor.henk_moderate_activity_today-state] @@ -1851,9 +1839,6 @@ 'id': , 'name': None, 'options': dict({ - 'sensor.private': dict({ - 'suggested_unit_of_measurement': , - }), }), 'original_device_class': , 'original_icon': None, @@ -1863,7 +1848,7 @@ 'supported_features': 0, 'translation_key': 'workout_pause_duration', 'unique_id': 'withings_12345_workout_pause_duration', - 'unit_of_measurement': , + 'unit_of_measurement': , }) # --- # name: test_all_entities[sensor.henk_pause_during_last_workout-state] @@ -2045,9 +2030,6 @@ 'id': , 'name': None, 'options': dict({ - 'sensor.private': dict({ - 'suggested_unit_of_measurement': , - }), }), 'original_device_class': , 'original_icon': None, @@ -2057,7 +2039,7 @@ 'supported_features': 0, 'translation_key': 'sleep_goal', 'unique_id': 'withings_12345_sleep_goal', - 'unit_of_measurement': , + 'unit_of_measurement': , }) # --- # name: test_all_entities[sensor.henk_sleep_goal-state] @@ -2235,9 +2217,6 @@ 'id': , 'name': None, 'options': dict({ - 'sensor.private': dict({ - 'suggested_unit_of_measurement': , - }), }), 'original_device_class': , 'original_icon': None, @@ -2247,7 +2226,7 @@ 'supported_features': 0, 'translation_key': 'activity_soft_duration_today', 'unique_id': 'withings_12345_activity_soft_duration_today', - 'unit_of_measurement': , + 'unit_of_measurement': , }) # --- # name: test_all_entities[sensor.henk_soft_activity_today-state]