Metrics: track loadpoint enabled state per slot - #32096
Open
andig wants to merge 2 commits into
Open
Conversation
Record the time-weighted fraction (0..1) of each 15min slot during which a loadpoint was enabled, stored in a new nullable meters.enabled column. Non-loadpoint entities never sample it, so the column stays null for them. Enabled is integrated over time like power (AddEnabled), sampled in the same accumulator interval as the energy update so it shares the slot's timing. Recovered downtime slots store null since enabled was not sampled during the gap.
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="core/metrics/accumulator.go" line_range="62-70" />
<code_context>
+
+// EnabledFraction returns the 0..1 fraction of the slot the loadpoint was
+// enabled, or nil when the state was never sampled (non-loadpoint entities).
+func (m *Accumulator) EnabledFraction() *float64 {
+ if m.totalTime <= 0 {
+ return nil
+ }
+ f := m.enabledTime.Seconds() / m.totalTime.Seconds()
+ return &f
+}
</code_context>
<issue_to_address>
**suggestion:** Clamping the enabled fraction into [0,1] would make the metric robust against unexpected time accounting anomalies.
If `enabledTime` ever slightly exceeds `totalTime` (or becomes negative due to subtle timing bugs), this could yield fractions outside [0,1]. Clamping `f` into that range before returning (e.g. `if f < 0 { f = 0 } else if f > 1 { f = 1 }`) would make the result safer for downstream consumers without changing behavior in the normal case.
```suggestion
// EnabledFraction returns the 0..1 fraction of the slot the loadpoint was
// enabled, or nil when the state was never sampled (non-loadpoint entities).
func (m *Accumulator) EnabledFraction() *float64 {
if m.totalTime <= 0 {
return nil
}
f := m.enabledTime.Seconds() / m.totalTime.Seconds()
// Clamp into [0,1] to guard against timing anomalies.
if f < 0 {
f = 0
} else if f > 1 {
f = 1
}
return &f
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Member
Author
|
@naltatis this PR would provide the basis for creating the kind of "enabled over time" chart other HEMSs seem to have. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Records how much of each 15min metrics slot a loadpoint was enabled.
A new nullable
meters.enabledcolumn stores the time-weighted fraction (0..1) of the slot during which the loadpoint's charging was enabled. It is integrated over time like power (AddEnabled), sampled in the same accumulator interval as the energy update so it shares the slot's timing.Only loadpoints sample the state — grid/pv/battery/home/forecast entities leave the column null. Recovered downtime-catchup slots also store null, since enabled was not observed during the gap.
AutoMigrateadds the column to existing databases; no manual migration.This only persists the value; wiring it into history/export/CLI is out of scope here.
🤖 Generated with Claude Code