home-assistant / core

:house_with_garden: Open source home automation that puts local control and privacy first.
https://www.home-assistant.io
Apache License 2.0
72.76k stars 30.47k forks source link

Sensor group with sum does not consistently add unit #114835

Closed ISNL-PimP closed 4 months ago

ISNL-PimP commented 6 months ago

The problem

I have a sum sensor group with 4 members, of which 1 is a Restful sensor and 3 are sum sensor groups as well. All member sensors have unit_of_measurement set to kW

The history graph does not show a graph, even though the output is numeric. image

image

After toggling 'ignore non-numeric' off and on, the unit is added back and the graph is rendered correctly: image

What version of Home Assistant Core has the issue?

core-2024.4.0

What was the last working version of Home Assistant Core?

No response

What type of installation are you running?

Home Assistant OS

Integration causing the issue

group

Link to integration documentation on our website

https://www.home-assistant.io/integrations/group/

Diagnostics information

No response

Example YAML snippet

No response

Anything in the logs that might be useful for us?

The unit of sensor.[snip]_total_power (None) cannot be converted to the unit of previously compiled statistics (kW). Generation of long term statistics will be suppressed unless the unit changes back to kW or a compatible unit. Go to https://my.home-assistant.io/redirect/developer_statistics to fix this

Additional information

No response

home-assistant[bot] commented 6 months ago

Hey there @home-assistant/core, mind taking a look at this issue as it has been labeled with an integration (group) you are listed as a code owner for? Thanks!

Code owner commands Code owners of `group` can trigger bot actions by commenting: - `@home-assistant close` Closes the issue. - `@home-assistant rename Awesome new title` Renames the issue. - `@home-assistant reopen` Reopen the issue. - `@home-assistant unassign group` Removes the current integration label and assignees on the issue, add the integration domain after the command. - `@home-assistant add-label needs-more-information` Add a label (needs-more-information, problem in dependency, problem in custom component) to the issue. - `@home-assistant remove-label needs-more-information` Remove a label (needs-more-information, problem in dependency, problem in custom component) on the issue.

(message by CodeOwnersMention)


group documentation group source (message by IssueLinks)

mvonweis commented 5 months ago

I have the same problem, but with temperature sensors. The unit of the group sensor is normally degrees Celsius, but when one sensor in the group is lost for a moment, the group sensor becomes unitless. I haven't had time to test this properly, so I don't know whether the unit information is restored again when the missing sensor returns.

It might make more sense to let the user set this manually when editing the group, and make a sanity check that all members of the group have the same unit.

catsmanac commented 5 months ago

Looks like I have the same problem after upgrading from 2024.4.1 to 2024.5.4. At startup I got 7 like these, one for each of my summing groups:

2024-05-24 09:26:02.870 WARNING (MainThread) [homeassistant.components.sensor] Entity sensor.envoy_no_dagproductie (<class 'homeassistant.components.group.sensor.SensorGroup'>) is using native unit of measurement 'None' which is not a valid unit for the device class ('energy') it is using; expected one of ['kWh', 'Wh', 'MWh', 'GJ', 'MJ']; Please update your configuration if your entity is manually configured, otherwise create a bug report at https://github.com/home-assistant/core/issues?q=is%3Aopen+is%3Aissue+label%3A%22integration%3A+group%22

Each accompanied with

2024-05-24 09:26:08.627 WARNING (Recorder) [homeassistant.components.sensor.recorder] The unit of sensor.envoy_no_dagproductie (None) cannot be converted to the unit of previously compiled statistics (kWh). Generation of long term statistics will be suppressed unless the unit changes back to kWh or a compatible unit. Go to https://my.home-assistant.io/redirect/developer_statistics to fix this

All of the affected groups are summing either other groups, utility_meter sensors or Riemann sensors. One would think that when layering, unit don't get forwarded.

catsmanac commented 5 months ago

After each restart of HA these errors are reported and units have disappeared from the group. Dashboard example shows 3 entities,2 group entities with missing UOM and 1 kWH one. afbeelding When I then change Ignore non-numeric option of the group, either on to off or the other way around, the UOM shows up. Below after cahnging one of the 2: afbeelding and after changing 3th one as well all are happy back in same trend showing all kWh: afbeelding Then after restart same occurs, flip Ignore non-numeric again and same happens. Issue is consistent but UOM not very.

catsmanac commented 5 months ago

Scanning through my backups this started with upgrade to 2024.2.1 (from 2024.1.4) and I clearly missed it then. In that update Groups summing Riemann sensors were affected, but Groups summing utility meters not. Groups with utility meters seemed affected after first HA restart after upgrade to 2024.3.0.dev20240228.

catsmanac commented 4 months ago

Reproduced the issue in a dev environment.

That last one triggered the problem to happen. After restart no units are displayed on the groups, the

2024-05-24 09:26:02.870 WARNING (MainThread) [homeassistant.components.sensor] Entity sensor.envoy_no_dagproductie (<class 'homeassistant.components.group.sensor.SensorGroup'>) is using native unit of measurement 'None' which is not a valid unit for the device class ('energy') it is using; expected one of ['kWh', 'Wh', 'MWh', 'GJ', 'MJ']; Please update your configuration if your entity is manually configured, otherwise create a bug report at https://github.com/home-assistant/core/issues?q=is%3Aopen+is%3Aissue+label%3A%22integration%3A+group%22

showed, followed by

2024-05-24 09:26:08.627 WARNING (Recorder) [homeassistant.components.sensor.recorder] The unit of sensor.envoy_no_dagproductie (None) cannot be converted to the unit of previously compiled statistics (kWh). Generation of long term statistics will be suppressed unless the unit changes back to kWh or a compatible unit. Go to https://my.home-assistant.io/redirect/developer_statistics to fix this

Needed to flip the Ignore non-numeric on each group to get units back and statistic logging going again. BUT THE STATISTICS GAP WAS NOT FILLED, so it's loosing statistics in this situation.

Seems a timing thing, race condition occurring?

catsmanac commented 4 months ago

The Chain of sensor, integration and group: Multiple (24) power sensors (Enphase_envoy integration) defined as:

        native_unit_of_measurement=UnitOfPower.WATT,
        state_class=SensorStateClass.MEASUREMENT,
        device_class=SensorDeviceClass.POWER,

Showing in entity registry as

"state_class":"measurement"
"device_class":null
"original_device_class":"power"
"unit_of_measurement":"W"

Each (24) power sensor integrated by a Riemann (Yaml config in prod, ui config in dev) as

- platform: integration
  source:  sensor.envoy_122302045041_inverter_122219084557
  name: envoy_122302045041_inverter_122219084557_productie
  unique_id: sensor.envoy_122302045041_inverter_122219084557_productie
  unit_prefix: k
  round: 3
  method: left

Showing in the entity registry as:

"state_class":"total"
"device_class":null
"original_device_class":"energy"
"platform":"integration"
"unit_of_measurement":null

2 sets of 12 of these integrators each into a group sum showing in the entity registry as:

"state_class":"total"
"device_class":null
"original_device_class":"energy"
"platform":"group"
"unit_of_measurement":null
catsmanac commented 4 months ago

Updated production HA to 2024.5.5 to no avail. Still no units and statistics loss at HA restart.

catsmanac commented 4 months ago

Some troubleshooting

Did a big of digging:

Too early to pinpoint what is happening, for me at least, but if the fix is to reload the group sum immediately when HA has started, one would think that something in timing during HA load has changed over the versions. Maybe the size of the groups now triggers some race condition, as with only a single small group sum the statistic loss does not seem to occur. The missing unit however does.

Have the HA log with info and debug details available for each test run.

Work around

This is no solution to the problem, but as the flipping of the switch on the group basically leads to a reload of the group, triggering a reload through an automation is a stopgap (for me) for the time being.

Create an automation that triggers on home assistant start and for action use Home Assistant Core Integration: Reload config entry targeting the groups having the issue. In below example I used a label assigned to each group sum to trigger them, created in the automation UI:

alias: Reload Group sums 1
description: Reload summing groups to set units
trigger:
  - platform: homeassistant
    event: start
condition: []
action:
  - service: homeassistant.reload_config_entry
    data: {}
    target:
      label_id: group_2_reload_1
mode: single

For the group sum summing other groups sums, a separate automation is needed that should run AFTER the first one completed.

catsmanac commented 4 months ago

Found the solution, group/manifest.json needs

  "after_dependencies": ["integration", "utility_meter"],

added. I'll create pr for it.

ISNL-PimP commented 4 months ago

Good find! However, I'm wondering if this fix will also work for all sensor types. Judging by your findings, this is a race condition where if the Group integration starts before all of it's subsensors, the unit of measurement will break. E.g. I'm using REST and Template sensors. If the Group integration starts faster than REST, it will still break.

Edit: Just noticed comments with the same sentiment in the PR. I'll continue the conversation there

catsmanac commented 4 months ago

At least it confirms the dependency. But if this would require changing utility_meter then I would think in your case it would also require changes to REST and Template. What common element can be changed to solve this in one go.

catsmanac commented 4 months ago

E.g. I'm using REST and Template sensors. If the Group integration starts faster than REST, it will still break.

The strange part is my Group sum of Enphase_envoy integration device sensors is working. But the its the 2nd or 3th layer sums that fail. (See picture at the bottom of my last PR reply). Think the REST would compare to the Enphase_Integration.

Think these device entities store their uom in the entity registry and are readily available when the group starts. Integration, utility_meter nor group do that.

And it looks like you have 2nd level groups sum as well?

As for Template, that has a after_dependency on group in the manifest file for quit some time already. So there must be another reason why it started to fail.

ISNL-PimP commented 4 months ago

My issue might just be related to nested group entities. Looks like the issue is not the source entities (template/rest), but the top group entity not calculating UOM due to the underlying group entity not having UOM.

sensor.group_1 (no unit)

catsmanac commented 4 months ago

So that's dependency within group. Sounds like group needs to find out internally it's own dependencies and startup order as well when starting.

Where these newly added groups or did they work before?

ISNL-PimP commented 4 months ago

They did work before, although it's hard to say exactly when. Looking back through the statistics, I see a big gap of missing statistics around February 8th, which is when I upgraded tot 2024.02.0. So that matches up with your findings above.

catsmanac commented 4 months ago

While building the construct you described for testing on current development version, which now is 2024.6.0, I found that the old group we used for summing, is now named combine state of sensors. The group helper has no longer the sum and other features anymore. afbeelding So I build some and it seems these don't have the issue. Haven't searched for more details and what is under the hood, or how to migrate. But it might be worthwhile looking into it.

catsmanac commented 4 months ago

Combine (Min-Max integration) helper entities work fine. All combine helpers show UOM as HA restart and no sign of statistics loss.

Now only find a way to connect history of group sensors to combine sensors to switch to the combine sensors.

afbeelding