Optimizer: expose forecasted highest/lowest battery SOC#29564
Merged
Conversation
Replaces the boolean Full/Empty timestamps with two BatteryForecastPoint
entries (Highest/Lowest) carrying the predicted SOC, time and a Limit
flag indicating whether the configured SMax/SMin boundary is reached
(within 1% tolerance). The frontend uses Limit to render either
"full/empty {time}" or "{soc}% {time}" so the next high/low point
remains visible even when the battery is not forecasted to reach a
boundary.
Fixes #29004
Contributor
There was a problem hiding this comment.
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="core/site_optimizer.go" line_range="312-316" />
<code_context>
+ return &types.BatteryForecastPoint{Soc: p.soc, Time: ts, Limit: p.limit}
}
+ res := types.BatteryForecast{
+ Highest: point(high),
+ Lowest: point(low),
+ }
+ if res.Highest == nil && res.Lowest == nil {
+ return nil
+ }
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Avoid calling time.Now() multiple times when computing forecast timestamps to prevent subtle boundary inconsistencies.
In `addBatteryForecastTotals`, `now` is computed once, but `point` still calls `time.Now()` via `ts.After(time.Now())`. This can make borderline slots non-deterministic due to tiny differences between the two timestamps. Capture a single `now := time.Now()` (or `cutoff := time.Now()`) once and reuse it for both rounding and the `After` check to ensure consistent boundary behavior.
Suggested implementation:
```golang
ts := now.Add(time.Duration(p.slot) * tariff.SlotDuration)
if !ts.After(now) {
return nil
}
return &types.BatteryForecastPoint{Soc: p.soc, Time: ts, Limit: p.limit}
}
```
This change assumes that a single `now := time.Now()` is already computed earlier in `addBatteryForecastTotals` and reused for the slot rounding. If that is not yet the case, introduce a `now := time.Now()` (or `cutoff := time.Now()`) once at the start of the function and use that same variable both where `now` is used for computing `ts` and in this `After` check.
</issue_to_address>
### Comment 2
<location path="assets/js/components/Energyflow/Energyflow.vue" line_range="690-693" />
<code_context>
- fmtForecast(
- forecast: { full?: string | null; empty?: string | null } | undefined,
- full: boolean
+ fmtForecastPoint(
+ point: { soc: number; time: string; limit?: boolean } | undefined,
+ high: boolean
): string | undefined {
- const isoString = full ? forecast?.full : forecast?.empty;
- if (!isoString) return undefined;
</code_context>
<issue_to_address>
**suggestion:** Reuse the shared BatteryForecastPoint type instead of an ad-hoc inline shape for better type safety.
The `fmtForecastPoint` param is currently typed as `{ soc: number; time: string; limit?: boolean } | undefined`. Since `BatteryForecastPoint` is already defined in `assets/js/types/evcc.ts`, update this signature to use `BatteryForecastPoint | undefined` to avoid duplicating the shape and reduce drift if the shared type changes (fields or nullability).
Suggested implementation:
```
import type { BatteryForecastPoint } from "@/types/evcc";
export default {
```
```
fmtForecastPoint(
point: BatteryForecastPoint | undefined,
high: boolean
): string | undefined {
```
1. If your project does not use the `@` alias for `assets/js`, adjust the import path to match existing imports in this file, e.g.:
- `import type { BatteryForecastPoint } from "../types/evcc";` or
- `import type { BatteryForecastPoint } from "../../types/evcc";`
2. Ensure `BatteryForecastPoint` in `assets/js/types/evcc.ts` is exported as a TypeScript type/interface and matches the `{ soc: number; time: string; limit?: boolean }` shape currently used by `fmtForecastPoint`.
</issue_to_address>
### Comment 3
<location path="core/site_optimizer.go" line_range="328" />
<code_context>
+ limit bool // true when SMax (highest) or SMin (lowest) boundary reached
+}
+
+// batteryForecastSocExtremes returns the highest and lowest aggregate SOC
+// points across home batteries (SCapacity > 0) over the forecast horizon.
+// The Limit flag indicates whether the SOC reached the configured SMax (for
</code_context>
<issue_to_address>
**issue (complexity):** Consider splitting the SOC aggregation and extreme-selection logic into smaller helpers and making the limit-selection rules explicit to simplify understanding and testing.
You can keep the new behavior but reduce complexity by separating aggregation from selection and by making the “limit” semantics explicit.
### 1. Split `batteryForecastSocExtremes` responsibilities
Right now it:
- aggregates capacities,
- builds the aggregate SOC over time,
- decides full/empty with tolerance, and
- tracks high/low with subtle limit semantics in a single loop.
You can make this easier to follow and test by extracting the aggregation and selection into helpers:
```go
func aggregateSocSeries(req []optimizer.BatteryConfig, resp []optimizer.BatteryResult) (
socSeries []float64, fullReached, emptyReached []bool,
) {
var totalCapacity, totalSMax, totalSMin float32
for _, b := range req {
if b.SCapacity > 0 {
totalCapacity += b.SCapacity
totalSMax += b.SMax
totalSMin += b.SMin
}
}
if totalCapacity == 0 || len(resp) == 0 {
return nil, nil, nil
}
tolerance := 0.01 * totalCapacity
slotCount := len(resp[0].StateOfCharge)
socSeries = make([]float64, slotCount)
fullReached = make([]bool, slotCount)
emptyReached = make([]bool, slotCount)
for i := 0; i < slotCount; i++ {
var sum float32
for batIdx := range req {
if req[batIdx].SCapacity > 0 {
sum += resp[batIdx].StateOfCharge[i]
}
}
socSeries[i] = float64(sum/totalCapacity) * 100
fullReached[i] = totalSMax > 0 && sum >= totalSMax-tolerance
emptyReached[i] = sum <= totalSMin+tolerance
}
return socSeries, fullReached, emptyReached
}
func selectExtremes(socSeries []float64, fullReached, emptyReached []bool) (
high, low *batteryForecastSlot,
) {
for i, soc := range socSeries {
fr, er := fullReached[i], emptyReached[i]
// first slot at SMax wins for highest
if high == nil || (!high.limit && (fr || soc > high.soc)) {
high = &batteryForecastSlot{slot: i, soc: soc, limit: fr}
}
// first slot at SMin wins for lowest
if low == nil || (!low.limit && (er || soc < low.soc)) {
low = &batteryForecastSlot{slot: i, soc: soc, limit: er}
}
}
return
}
func batteryForecastSocExtremes(req []optimizer.BatteryConfig, resp []optimizer.BatteryResult) (high, low *batteryForecastSlot) {
socSeries, fullReached, emptyReached := aggregateSocSeries(req, resp)
if socSeries == nil {
return nil, nil
}
return selectExtremes(socSeries, fullReached, emptyReached)
}
```
This preserves the behavior (including the 1% tolerance and “first at limit wins”) but separates:
- “what is the SOC/time series and limit flags?” from
- “given a series and flags, how do we pick high/low?”.
Both parts can then be unit-tested independently.
### 2. Make the selection logic self-documenting
You can further clarify the domain rules by centralizing the “first boundary wins” policy in small helpers, reducing mental parsing of `||`/`&&`:
```go
func betterHigh(current *batteryForecastSlot, soc float64, reachedLimit bool) bool {
if current == nil {
return true
}
if current.limit {
// once at limit, never override
return false
}
// prefer boundary over non-boundary, otherwise higher SOC
if reachedLimit && !current.limit {
return true
}
return soc > current.soc
}
func betterLow(current *batteryForecastSlot, soc float64, reachedLimit bool) bool {
if current == nil {
return true
}
if current.limit {
return false
}
if reachedLimit && !current.limit {
return true
}
return soc < current.soc
}
// then in selectExtremes:
if betterHigh(high, soc, fr) {
high = &batteryForecastSlot{slot: i, soc: soc, limit: fr}
}
if betterLow(low, soc, er) {
low = &batteryForecastSlot{slot: i, soc: soc, limit: er}
}
```
This keeps the same semantics but makes the intent (“once at limit, don’t override; prefer limit over non-limit; otherwise use max/min”) explicit and easy to review.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Replaces the boolean BatteryForecastPoint.Limit with a typed string so the JSON payload is self-describing and the frontend formatter no longer needs a separate 'high' parameter to map the flag onto the right label.
naltatis
reviewed
May 2, 2026
naltatis
reviewed
May 2, 2026
Per review feedback, swaps the 'full'/'empty' string back to a plain bool. The frontend already knows whether it is rendering the highest or lowest point, so it passes that as a flag to fmtForecastPoint and selects the right i18n key from there.
Member
Author
Agent-Logs-Url: https://github.com/evcc-io/evcc/sessions/2fed810b-6777-480a-b514-5276c1ce1661 Co-authored-by: andig <184815+andig@users.noreply.github.com>
Member
Member
Member
Author
|
Lets check is this is really next low or forecasted low, i.e. the absolute low even after next? Which one would be prefer- I'd say the latter? |
Member
I'd say "the next" since our forecast horizon is subject to change and if we are picking longer horizons this info gets less meaningful/actionable for the users. The next one is clear and in line with our solar forecast communication. We could evolve this to showing "the next peaks" later. |
jeffborg
added a commit
to jeffborg/evcc
that referenced
this pull request
May 8, 2026
Cherry-picks the PR-only changes from upstream/feat/optimizer-soc-extremes-29004 without the unrelated upstream history that came from merging the whole branch. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.


Summary
Extracts the predicted highest and lowest battery SOC from the optimizer forecast so the energy-flow widget can show the next peak/trough even when the battery is not forecasted to reach a full/empty boundary.
Details
BatteryForecast.Full/Emptytimestamps withHighest/Lowestpoints carrying SOC %, time and aLimitflag.Limitis set when the aggregated SOC reaches the configuredSMax(full) orSMin(empty) exactly.SCapacity > 0); vehicles are ignored.batteryForecastFullAndEmptySlotshelper.{soc} {time}for non-limit points andfull/empty {time}when the boundary is reached.main.energyflow.batteryForecastSocfor the percentage format.Fixes #29004
Test plan
go test ./core/...npm test(vitest)vue-tsc --noEmitBatteryForecastSocExtremesstory)