Skip to content

Add sensor unit of measurement validation for device classes #84366

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
frenck merged 2 commits into from
Dec 21, 2022
Merged

Add sensor unit of measurement validation for device classes #84366

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f431e12
Add sensor uom validation for device classes
frenck Dec 21, 2022
742cf8e
Fix doc string
frenck Dec 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 117 additions & 0 deletions homeassistant/components/sensor/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
from homeassistant.backports.enum import StrEnum
from homeassistant.config_entries import ConfigEntry
from homeassistant.const import ( # noqa: F401, pylint: disable=[hass-deprecated-import]
CONCENTRATION_MICROGRAMS_PER_CUBIC_METER,
CONCENTRATION_PARTS_PER_MILLION,
CONF_UNIT_OF_MEASUREMENT,
DEVICE_CLASS_AQI,
DEVICE_CLASS_BATTERY,
Expand Down Expand Up @@ -45,7 +47,30 @@
DEVICE_CLASS_TIMESTAMP,
DEVICE_CLASS_VOLATILE_ORGANIC_COMPOUNDS,
DEVICE_CLASS_VOLTAGE,
LIGHT_LUX,
PERCENTAGE,
POWER_VOLT_AMPERE_REACTIVE,
SIGNAL_STRENGTH_DECIBELS,
SIGNAL_STRENGTH_DECIBELS_MILLIWATT,
UnitOfApparentPower,
UnitOfDataRate,
UnitOfElectricCurrent,
UnitOfElectricPotential,
UnitOfEnergy,
UnitOfFrequency,
UnitOfInformation,
UnitOfIrradiance,
UnitOfLength,
UnitOfMass,
UnitOfPower,
UnitOfPrecipitationDepth,
UnitOfPressure,
UnitOfSoundPressure,
UnitOfSpeed,
UnitOfTemperature,
UnitOfTime,
UnitOfVolume,
UnitOfVolumetricFlux,
)
from homeassistant.core import HomeAssistant, callback
from homeassistant.helpers import entity_registry as er
Expand Down Expand Up @@ -453,6 +478,74 @@ class SensorStateClass(StrEnum):
SensorDeviceClass.WIND_SPEED: SpeedConverter,
}

DEVICE_CLASS_UNITS: dict[SensorDeviceClass, set[type[StrEnum] | str | None]] = {
Copy link
Copy Markdown
Contributor

@epenet epenet Dec 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to have the same validation for number entities?
And would it make sense to have this dict shared somewhere?

Copy link
Copy Markdown
Member Author

@frenck frenck Dec 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have no validations on numbers whatsoever at this point. But yes, we can add more validation there too.

I suggest doing that at a later point.

Essentially, those are not the same device classes... (we can enforce keeping them in sync though)

SensorDeviceClass.APPARENT_POWER: set(UnitOfApparentPower),
SensorDeviceClass.AQI: {None},
SensorDeviceClass.ATMOSPHERIC_PRESSURE: set(UnitOfPressure),
SensorDeviceClass.BATTERY: {PERCENTAGE},
SensorDeviceClass.CO: {CONCENTRATION_PARTS_PER_MILLION},
SensorDeviceClass.CO2: {CONCENTRATION_PARTS_PER_MILLION},
SensorDeviceClass.CURRENT: {UnitOfElectricCurrent.AMPERE},
SensorDeviceClass.DATA_RATE: set(UnitOfDataRate),
SensorDeviceClass.DATA_SIZE: set(UnitOfInformation),
SensorDeviceClass.DISTANCE: set(UnitOfLength),
SensorDeviceClass.DURATION: {
UnitOfTime.DAYS,
UnitOfTime.HOURS,
UnitOfTime.MINUTES,
UnitOfTime.SECONDS,
},
SensorDeviceClass.ENERGY: set(UnitOfEnergy),
SensorDeviceClass.FREQUENCY: set(UnitOfFrequency),
SensorDeviceClass.GAS: {
UnitOfVolume.CENTUM_CUBIC_FEET,
UnitOfVolume.CUBIC_FEET,
UnitOfVolume.CUBIC_METERS,
},
SensorDeviceClass.HUMIDITY: {PERCENTAGE},
SensorDeviceClass.ILLUMINANCE: {LIGHT_LUX, "lm"},
Copy link
Copy Markdown
Contributor

@epenet epenet Dec 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if lm should be kept here. Lumen is a unit of Luminous flux, not a unit of Illuminance

I have opened an architecture discussion about it in home-assistant/architecture#843

Copy link
Copy Markdown
Member Author

@frenck frenck Dec 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is not adding something new, the discussion is fine, but it doesn't have to block this.

SensorDeviceClass.IRRADIANCE: set(UnitOfIrradiance),
SensorDeviceClass.MOISTURE: {PERCENTAGE},
SensorDeviceClass.NITROGEN_DIOXIDE: {CONCENTRATION_MICROGRAMS_PER_CUBIC_METER},
SensorDeviceClass.NITROGEN_MONOXIDE: {CONCENTRATION_MICROGRAMS_PER_CUBIC_METER},
SensorDeviceClass.NITROUS_OXIDE: {CONCENTRATION_MICROGRAMS_PER_CUBIC_METER},
SensorDeviceClass.OZONE: {CONCENTRATION_MICROGRAMS_PER_CUBIC_METER},
SensorDeviceClass.PM1: {CONCENTRATION_MICROGRAMS_PER_CUBIC_METER},
SensorDeviceClass.PM10: {CONCENTRATION_MICROGRAMS_PER_CUBIC_METER},
SensorDeviceClass.PM25: {CONCENTRATION_MICROGRAMS_PER_CUBIC_METER},
SensorDeviceClass.POWER_FACTOR: {PERCENTAGE},
SensorDeviceClass.POWER: {UnitOfPower.WATT, UnitOfPower.KILO_WATT},
SensorDeviceClass.PRECIPITATION: set(UnitOfPrecipitationDepth),
SensorDeviceClass.PRECIPITATION_INTENSITY: set(UnitOfVolumetricFlux),
SensorDeviceClass.PRESSURE: set(UnitOfPressure),
SensorDeviceClass.REACTIVE_POWER: {POWER_VOLT_AMPERE_REACTIVE},
SensorDeviceClass.SIGNAL_STRENGTH: {
SIGNAL_STRENGTH_DECIBELS,
SIGNAL_STRENGTH_DECIBELS_MILLIWATT,
},
SensorDeviceClass.SOUND_PRESSURE: set(UnitOfSoundPressure),
SensorDeviceClass.SPEED: set(UnitOfSpeed).union(set(UnitOfVolumetricFlux)),
SensorDeviceClass.SULPHUR_DIOXIDE: {CONCENTRATION_MICROGRAMS_PER_CUBIC_METER},
SensorDeviceClass.TEMPERATURE: {
UnitOfTemperature.CELSIUS,
UnitOfTemperature.FAHRENHEIT,
},
SensorDeviceClass.VOLATILE_ORGANIC_COMPOUNDS: {
CONCENTRATION_MICROGRAMS_PER_CUBIC_METER
},
SensorDeviceClass.VOLTAGE: {UnitOfElectricPotential.VOLT},
SensorDeviceClass.VOLUME: set(UnitOfVolume),
SensorDeviceClass.WATER: {
UnitOfVolume.CENTUM_CUBIC_FEET,
UnitOfVolume.CUBIC_FEET,
UnitOfVolume.CUBIC_METERS,
UnitOfVolume.GALLONS,
UnitOfVolume.LITERS,
},
SensorDeviceClass.WEIGHT: set(UnitOfMass),
SensorDeviceClass.WIND_SPEED: set(UnitOfSpeed),
}

# mypy: disallow-any-generics


Expand Down Expand Up @@ -506,6 +599,7 @@ class SensorEntity(Entity):
_attr_unit_of_measurement: None = (
None # Subclasses of SensorEntity should not set this
)
_invalid_unit_of_measurement_reported = False
_last_reset_reported = False
_sensor_option_unit_of_measurement: str | None = None

Expand Down Expand Up @@ -862,6 +956,29 @@ def state(self) -> Any:
# Round to the wanted precision
value = round(value_f_new) if prec == 0 else round(value_f_new, prec)

# Validate unit of measurement used for sensors with a device class
if (
not self._invalid_unit_of_measurement_reported
and device_class
and (units := DEVICE_CLASS_UNITS.get(device_class)) is not None
and native_unit_of_measurement not in units
):
self._invalid_unit_of_measurement_reported = True
report_issue = self._suggest_report_issue()

# This should raise in Home Assistant Core 2023.6
_LOGGER.warning(
"Entity %s (%s) is using native unit of measurement '%s' which "
"is not a valid unit for the device class ('%s') it is using; "
"Please update your configuration if your entity is manually "
"configured, otherwise %s",
self.entity_id,
type(self),
native_unit_of_measurement,
device_class,
report_issue,
)

return value

def __repr__(self) -> str:
Expand Down
72 changes: 72 additions & 0 deletions tests/components/sensor/test_init.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1084,3 +1084,75 @@ async def test_non_numeric_device_class_with_unit_of_measurement(
"Sensor sensor.test has a unit of measurement and thus indicating it has "
f"a numeric value; however, it has the non-numeric device class: {device_class}"
) in caplog.text


@pytest.mark.parametrize(
"device_class",
(
SensorDeviceClass.APPARENT_POWER,
SensorDeviceClass.AQI,
SensorDeviceClass.ATMOSPHERIC_PRESSURE,
SensorDeviceClass.BATTERY,
SensorDeviceClass.CO,
SensorDeviceClass.CO2,
SensorDeviceClass.CURRENT,
SensorDeviceClass.DATA_RATE,
SensorDeviceClass.DATA_SIZE,
SensorDeviceClass.DISTANCE,
SensorDeviceClass.DURATION,
SensorDeviceClass.ENERGY,
SensorDeviceClass.FREQUENCY,
SensorDeviceClass.GAS,
SensorDeviceClass.HUMIDITY,
SensorDeviceClass.ILLUMINANCE,
SensorDeviceClass.IRRADIANCE,
SensorDeviceClass.MOISTURE,
SensorDeviceClass.NITROGEN_DIOXIDE,
SensorDeviceClass.NITROGEN_MONOXIDE,
SensorDeviceClass.NITROUS_OXIDE,
SensorDeviceClass.OZONE,
SensorDeviceClass.PM1,
SensorDeviceClass.PM10,
SensorDeviceClass.PM25,
SensorDeviceClass.POWER_FACTOR,
SensorDeviceClass.POWER,
SensorDeviceClass.PRECIPITATION_INTENSITY,
SensorDeviceClass.PRECIPITATION,
SensorDeviceClass.PRESSURE,
SensorDeviceClass.REACTIVE_POWER,
SensorDeviceClass.SIGNAL_STRENGTH,
SensorDeviceClass.SOUND_PRESSURE,
SensorDeviceClass.SPEED,
SensorDeviceClass.SULPHUR_DIOXIDE,
SensorDeviceClass.TEMPERATURE,
SensorDeviceClass.VOLATILE_ORGANIC_COMPOUNDS,
SensorDeviceClass.VOLTAGE,
SensorDeviceClass.VOLUME,
SensorDeviceClass.WATER,
SensorDeviceClass.WEIGHT,
SensorDeviceClass.WIND_SPEED,
),
)
async def test_device_classes_with_invalid_unit_of_measurement(
hass: HomeAssistant,
caplog: pytest.LogCaptureFixture,
enable_custom_integrations: None,
device_class: SensorDeviceClass,
):
"""Test error when unit of measurement is not valid for used device class."""
platform = getattr(hass.components, "test.sensor")
platform.init(empty=True)
platform.ENTITIES["0"] = platform.MockSensor(
name="Test",
native_value=None,
device_class=device_class,
native_unit_of_measurement="INVALID!",
)

assert await async_setup_component(hass, "sensor", {"sensor": {"platform": "test"}})
await hass.async_block_till_done()

assert (
"is using native unit of measurement 'INVALID!' which is not a valid "
f"unit for the device class ('{device_class}') it is using"
) in caplog.text