use a simpler calculation in the SolarGaugeCard#51410
use a simpler calculation in the SolarGaugeCard#51410eringerli wants to merge 1 commit intohome-assistant:devfrom
Conversation
eringerli
force-pushed
the
solar-gauge
branch
6 times, most recently
from
1711a67 to
0424810
Compare
eringerli
force-pushed
the
solar-gauge
branch
2 times, most recently
from
a1c66be to
55026ff
Compare
|
Can you review the original PR that added this logic, and the associated issues it closed, and make sure you aren't just bringing back old issues? I feel like we maybe used to have this formula and changed it to try to fix some issue, though I don't recall exactly what it was. But we probably shouldn't just delete large swathe of code without understanding what it was for. |
|
Thanks for the suggestions, I will look into it. I will add a test for this and adjust the formula accordingly. I set the PR to draft in the meantime. If I can make a suggestion: next time, please add the description of the problem as comment into the code, instead of what the code does, then I would have tested it already. |
|
That PR was #25326 |
eringerli
force-pushed
the
solar-gauge
branch
3 times, most recently
from
880b169 to
a731eac
Compare
|
@karwosts I think, this works now. I still have to adjust the test suite and experiment some more, but I think the solution with the aggregated data from |
In the tooltip, the formula is described simple as 100% - (total grid export / total solar production) This commit uses this formula instead of the complicated calculations with the stored battery power. That formula reduced the self consumption ratio if the batteries are not 100% efficient.
|
@karwosts I adjusted the tests. I'm not that experienced with test and added |
| }); | ||
| } | ||
|
|
||
| let batteryToGrid = consumption.battery_to_grid[t] ?? 0; |
There was a problem hiding this comment.
It's tracked in sum.total.solar and consumption.total.solar_to_grid, so the ratio is the effective solar grid export (explicitly without the export from storage) to the total solar production.
For me, the solar gauge should show exactly what the tooltip says: how much solar energy was used (the solar_to_grid is the opposite of that, hence 1 - ( ... ) * 100) relative to the total production over a certain period if time. If you factor in any storage, then it becomes dependant of the efficiency of that, which gives too low values. Also, storage can be filled with energy from the grid (dependant on the rules about storage systems in your country and if you have dynamic pricing), this shouldn't be shown as 'solar'. If you want to account for that, the algorythm gets even more complicated, and still wouldn't be as described in the tooltip.
There was a problem hiding this comment.
battery_to_grid is not included in solar_to_grid so it is just not tracked and will be treated as self-consumed when it is not. This looks like a regression to me.
This does NOT account for the following scenario (which the old LIFO algorithm explicitly handled): solar charges the battery, then the battery later exports to the grid. In that case, the solar energy was not really "self-consumed" -- it went solar -> battery -> grid. The old algorithm tracked this. The new formula misses it, counting that energy as self-consumed because it was not directly exported from solar to grid.
The previous code was a estimation and not 100% correct but this looks even less correct.
There was a problem hiding this comment.
I understand the need for simplicity if you don't have battery_to_grid which is the most common case, but if you do have battery_to_grid, we shouldn't just ignore it.
3 participants
Proposed change
In the tooltip, the formula is described simply as ratio between used and fed in solar energy, but the calculations included the battery storage. That formula reduced the self consumption ratio artificially if the batteries are not 100% efficient, which they can't ever be.
This commit uses the following formula:
100% - (total grid export / total solar production) * 100%This is more logical (you can calculate this ratio easily for yourself with only the displayed data) and is now consistent with the tooltip. This tripped me, when I tried to calculate the ratio for myself and got different values than displayed in the dashboard. My storage system is about 85% efficient (as they mostly are), so the gauge was consistently about 10% too low. Also, I think this gauge should be specific to one day and shouldn't include stored energy, as the tooltip is right: this should be an indicator if controllable loads and a storage system are a good idea or not.
I also adjusted the tests and prohibited for division-by-zero errors.
Breaking change
This PR propopses a different formula for the solar self consumption gauge. I'm not sure if this constitutes as "Breaking Change". It generally goes up compared to the original formula if you have a storage system, for all other cases it stays the same.
Screenshots
Type of change
Checklist
If user exposed functionality or configuration variables are added/changed: