Add native unit types for weather entities#59533
Add native unit types for weather entities#59533emontnemery merged 17 commits intohome-assistant:devfrom
Conversation
|
Hey there @fabaff, mind taking a look at this pull request as it has been labeled with an integration ( |
|
I think this looks good! To prevent the |
|
Tests are needed too, I just noticed it's missing from sensor tests. |
|
Edit: I'm going to add the precision change in a different PR. |
|
I'm changing the names of the new methods to no longer have the Plus, everything gets unit-converted anyways in |
The vibility values given by metoffice are formatted into strings, which means they can't automatically be converted.
|
The code should be ready to go now! |
| if (wind_speed := self.wind_speed) is not None: | ||
| if (unit := self.wind_speed_unit) is not None: | ||
| wind_speed = round( | ||
| self.hass.config.units.wind_speed(wind_speed, unit), |
There was a problem hiding this comment.
Why is hass config doing the unit conversion?
wind_seed should be imported directly from util.unit_system instead.
The same applies to all other unit conversions happening here.
|
This PR is causing massive breaking changes. While I think the idea is good, I think the way it is set up is not fitting nor correct. For that reason:
I think it is cool to support unit conversion on an entity or device level in the future, but we cannot rely on core configuration for e.g., unit of mass or pressure. Which unit to use for those cases heavily relies on the use case. |
Proposed change
I'm attempting to port @emontnemery's addition of
native_unitsto SensorEntity to theWeatherEntityclass.This will allow integrations publishing weather data results to convert their data to local units, without needing to write unit conversion routines in each integration. This can also be used as a base to implement unit conversion for weather templates, like in #48641.
It is very unlikely this change should break custom components. However, I did find one component (
met_office) that already defined avisibility_unitmethod but did not return floats in itsvisibilitymethod. If any other components pull similar shenanigans, they will break.Type of change
Additional information
Checklist
black --fast homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all..coveragerc.The integration reached or maintains the following Integration Quality Scale:
To help with the load of incoming pull requests: