Calculate derivative unit instantly on creation if possible #147527
Conversation
|
Hey there @afaucogney, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
There was a problem hiding this comment.
Pull Request Overview
This PR improves the derivative sensor’s behavior by immediately calculating and setting its unit_of_measurement upon creation, ensuring consistency between the state and its unit.
- Fixes an issue where the derivative sensor’s unit was delayed in updating.
- Adds tests to restore state and verifies dynamic updates through the options flow.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/components/derivative/test_sensor.py | Adds test_restore to verify state and unit restoration on boot |
| tests/components/derivative/test_config_flow.py | Adds awaits and assertions to ensure proper unit update after state changes |
| homeassistant/components/derivative/sensor.py | Introduces new methods for unit derivation and state restoration, and refines state update handling |
Comments suppressed due to low confidence (1)
tests/components/derivative/test_config_flow.py:115
- [nitpick] Consider adding further assertions after updating the source sensor state to verify that unit_of_measurement is dynamically updated as expected.
hass.states.async_set("sensor.valid", 10, {"unit_of_measurement": "cat"})
|
When addressing the unit of measurement we could share some patterns with "rienmann sum" integration https://github.com/home-assistant/core/blob/dev/homeassistant/components/integration/sensor.py#L349 |
|
Yes I had been following that one a bit. It seems there the unit is recalculated on every source update. Absent any further input I will probably change this to do the same, I just know this had been originally written to snap the unit only once, and without knowing why that was done was going to try to preserve that behavior, but perhaps that's undesirable anyway (and difficult to do). |
|
Integral went through the same issue, calculate once and they more recently do it on every source change. The decision came from the fact that some sources change unit |
|
This has been changed to update the unit on every source state change. |
66908c1 to
9742db9
Compare
Proposed change
Fix some issues with the derivative unit_of_measurement.
Fixes Derivative sensor shows wrong unit #136419, wherein once a derivative sensor was created, it's displayed unit of measurement could not be changed, even when user modifies it through options flow, making the actual state internally calculate using a different unit, leading to wildly incorrect value.
Improves derivative to calculate and set it's unit instantly when created based on the source sensor, instead of waiting for 2 updates of the source sensor when the first derivative could be calculated. The existing behavior is confusing, as after creating a sensor it could have no unit for many minutes or hours depending on how long it takes the source to change state. Also while it was waiting, it would ultimately be a problem for statistics, which would start recording data with unit
"", and then when the real unit updated later, it would generate a statistics issue due to change of native unit.(added late) Update the derivative unit of measurement anytime the source sensor changes unit_of_measurement. Reset the derivative to 0 if the unit changes, since we don't want to calculate huge jumps if a sensor changes from e.g. Wh to kWh.
Type of change
Additional information
Checklist
ruff format 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.To help with the load of incoming pull requests: