Skip to content
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.

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

Support for hierarchy of individual energy devices #23185

Merged
merged 12 commits into from
Mar 24, 2025

Conversation

karwosts
Copy link
Contributor

@karwosts karwosts commented Dec 6, 2024

Proposed change

When adding individual devices to energy configuration, allow an entry to designate another entry as its parent. This will have the following effects:

  • When displaying the energy consumption of the parent in the stacked bar, subtract the energy consumption of the children, so that nested devices are not double (or triple) counted. This will also correct the untracked consumption.
  • When arranging the stack order for the bar chart, children will always be stacked adjacent to their parent.
  • When hiding the child entry in the bar chart, the parent item then fills the chart space of the hidden child.

Consider the following example, which has a montior on the breaker, but also monitors on individual items downstream of the breaker. In the current implementation the child items would be double counted, significantly throwing off the untracked calculation, and making the total height of the energy stack much higher than the true consumption.

With this change they would be "nested" within the parent item, and therefore the remaining area of each parent item then becomes the "untracked energy" remaining in that parent, or the energy not consumed by known tracked children.

424062374-ae64fcb9-b9f5-4573-b513-2c39922f49ed

I realize the actual visualization of the hierarchy is a bit weak here, there may be additional options in chartjs to improve this, or maybe can do something in the tooltip, not sure yet.

This also works for >1 level nesting, so you could have breaker -> smart power strip -> individual item if you wanted.

There hasn't been any work done yet on visualizing this for the summary bar graph, but that doesn't strictly need to be implemented for this to be correct. Could do more in the future.

This has still a bit more work to do polishing the configuration UI, but just wanted an early feedback if this is something of interest.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (thank you!)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example configuration

Additional information

Checklist

  • The code change is tested and works locally.
  • There is no commented out code in this PR.
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

@karwosts karwosts added wip Backend Change Required Requires a Backend Core Code Change panel: energy labels Dec 6, 2024
@home-assistant home-assistant bot added cla-signed WTH Issues & PRs generated from the "Month of What the Heck?" labels Dec 6, 2024
@karwosts
Copy link
Contributor Author

karwosts commented Dec 6, 2024

@MindFreeze - saw you were also working on some energy charts, was curious if you think this is a worthwhile idea or would be something you're interested in leveraging as well.

@MindFreeze
Copy link
Contributor

I would definitely leverage this in a Sankey Chart if we have it. We have even discussed something like this for the future. We'll need to review it from a UX perspective though

@MindFreeze MindFreeze added the Needs UX Pull requests requiring a review from the Home Assistant design team label Dec 6, 2024
@karwosts
Copy link
Contributor Author

karwosts commented Dec 6, 2024

I haven't added it yet, but would probably just add a select selector in the device configuration dialog, to select one of the other devices (and filter it to prevent from selecting a cyclic loop).

@MindFreeze
Copy link
Contributor

We could also link a power entity this way

@karwosts
Copy link
Contributor Author

karwosts commented Dec 6, 2024

We could also link a power entity this way

For what purpose?

@MindFreeze
Copy link
Contributor

To display real time power consumption. There is a WTH thread for this.

@karwosts
Copy link
Contributor Author

karwosts commented Dec 7, 2024

Config UI:

image

@karwosts karwosts marked this pull request as draft February 11, 2025 00:21
@karwosts
Copy link
Contributor Author

Draft while blocked on architecture

@karwosts karwosts marked this pull request as ready for review February 28, 2025 15:33
@karwosts
Copy link
Contributor Author

I got the go-ahead from architecture review to implement this, but since switch to echarts it will need to be significantly rewritten I'm sure.

@MindFreeze - was there a specific aspect of this you were asking about the UX for, can I get any feedback there before I rewrite the code?

@MartinHjelmare
Copy link
Member

Core PR is approved.

@karwosts karwosts marked this pull request as draft March 1, 2025 14:26
@karwosts karwosts marked this pull request as ready for review March 3, 2025 14:44
@karwosts
Copy link
Contributor Author

karwosts commented Mar 3, 2025

I've mostly got this back to functional with echarts, could be ready for a closer look.

@MindFreeze
Copy link
Contributor

Sadly, we haven't had a chance to discuss the UX for this yet.
My opinion is that the hierarchy shouldn't be displayed in this chart as there is no simple way to show it. Would rather show only the children here or only the parents.

@karwosts
Copy link
Contributor Author

karwosts commented Mar 4, 2025

Would rather show only the children here or only the parents.

Hmm, I think that's tough as there's not necessarily perfect coverage. Maybe a device reports 1kWh of consumption, and it has one child that has 0.25kWh.

My current idea was to draw the parent device as 0.75kWh and the child as 0.25kWh, since that makes the total bar height correct and makes the untracked correct. Maybe the tooltip can give extra information.

If we only show the children, we're missing most of the energy.
If we only show the parents, then there was no point to doing this, since it adds no value.

Anyway I'll pause further work here until you get a chance to come to a conclusion.

@marcinbauer85
Copy link
Contributor

This is basically what the Sankey chart shows the best, because it doesn't just stack all inputs into a single bar, but can show splits with labeling of those splits. Stacked barchart isn't the ideal to display this, and we would have to change a lot of a default layout of a stacked chart to achieve it.
One idea that comes to mind is to have the legend vertical on the side with the sub-entries as branches (so a tree-view). That way there Is a clear visual cue that there is a hierarchy. As for the chart, the breaker parent and children could have maybe a dashed border, and different background to differentiate it from the rest. Legend also should have the same border/fill applied, not only color.

@karwosts
Copy link
Contributor Author

Stacked barchart isn't the ideal to display this, and we would have to change a lot of a default layout of a stacked chart to achieve it.

Even if this isn't the ideal graph to convey this, it's currently completely wrong.

The untracked energy is wrong, and the height of the stack is wrong.

Hopefully there can be some way to fix those issues.

@MindFreeze
Copy link
Contributor

Stacked barchart isn't the ideal to display this, and we would have to change a lot of a default layout of a stacked chart to achieve it.

Even if this isn't the ideal graph to convey this, it's currently completely wrong.

Indeed but we could use the hierarchy info to just fix the wrong data in the bar chart and leave displaying the hierarchy to the sankey card

@karwosts
Copy link
Contributor Author

If I have a device (A) with a child (B). A says 3kWh and B says 1kWh, what should I draw?

A as 3 and B as 1? In that case the height of the stack will be much taller than my actual consumption, so I don't think i want that.

A as 2 and B as 1?

I can label the first one as "A (untracked)" or something like that to make it clear that it's just the remainder not already accounted for?

That's kind of what I'm doing now except with some extra tricks to grow A when B is hidden, but I can just remove those for simplicity.

@marcinbauer85
Copy link
Contributor

I have some amendments to the copy of that dialog:

Textfield label: Parent upstream device source (optional)
Helper: “If this device is already counted by another device (such as a smart switch measured by a smart breaker), selecting the upstream device prevents duplicate energy tracking.”

As for the chart, untracked energy could use a different kind of style. Could we use the echarts decal for untracked energy?
https://echarts.apache.org/en/option.html#series-bar.itemStyle.decal

@karwosts
Copy link
Contributor Author

I have some amendments to the copy of that dialog:

Textfield label: Parent upstream device source (optional) Helper: “If this device is already counted by another device (such as a smart switch measured by a smart breaker), selecting the upstream device prevents duplicate energy tracking.”

I'll note in Architecture I was requested not to use the word "parent" to describe this field.

@marcinbauer85
Copy link
Contributor

@karwosts do you need anything else to continue with this PR?

@karwosts
Copy link
Contributor Author

@karwosts do you need anything else to continue with this PR?

A bit more clarity on what we want this feature to do would be helpful. I get the sense people don't like my current implementation, but I'm not sure I have guidance what to replace it with.

@MindFreeze
Copy link
Contributor

I can label the first one as "A (untracked)" or something like that to make it clear that it's just the remainder not already accounted for?

Replace it with this. Let's get the configuration changes in, fix the incorrect data in the bar chart, and worry about displaying it better after that.
We can then update the sankey card to use this data and potentially introduce new charts. Perhaps a sunburst chart card for floor/area?

@karwosts
Copy link
Contributor Author

Ok I think it's ready to go.

image

@MindFreeze MindFreeze removed the Needs UX Pull requests requiring a review from the Home Assistant design team label Mar 18, 2025
@MartinHjelmare
Copy link
Member

Core PR is merged.

@MindFreeze MindFreeze merged commit 6fbc7b2 into home-assistant:dev Mar 24, 2025
15 checks passed
@frenck frenck added the Noteworthy Marks a PR as noteworthy and should be in the release notes (in case it normally would not appear) label Mar 24, 2025
@karwosts karwosts deleted the nested-energy-devices branch March 24, 2025 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed Noteworthy Marks a PR as noteworthy and should be in the release notes (in case it normally would not appear) panel: energy WTH Issues & PRs generated from the "Month of What the Heck?"
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants