From 99793642424a3338b2200779a95cbd23c68e37d5 Mon Sep 17 00:00:00 2001 From: SolarPower2024 Date: Sun, 11 Oct 2026 22:02:06 +0200 Subject: [PATCH 1/8] Peak: Home Assistant ohne Antwort kostet einen Timeout je Zyklus, ungelesene Entitaeten werden nicht geschrieben (Review D4) Ein haengendes Home Assistant verlaengerte den Zyklus um einen Timeout je Aufruf (Energiesensor, Sollwert, Ladeleistung, Marstek-Entitaeten, EEG-Zaehler: 40 s und mehr). Der Waechter im Transport der gemeinsamen Verbindung (core/site_ha_guard.go) lehnt nach dem ersten Timeout die weiteren Aufrufe des Zyklus ab, am Ende des Zyklus (updateBatteryModePeakAware) fragt der naechste wieder. Der EEG-Zaehler wird nach einem eigenen Timeout eine Minute nicht gelesen. numberSetter und omniSwitchOff schreiben nach einem Lesefehler nicht mehr blind, sondern geben den Fehler zurueck; die Freigabe versucht es im naechsten Zyklus erneut und behaelt solange, was evcc haelt. Co-Authored-By: Claude Sonnet 5.5 --- core/lm/README.md | 13 ++- core/site_feedin_eeg.go | 23 +++- core/site_ha_guard.go | 110 +++++++++++++++++++ core/site_ha_guard_test.go | 211 ++++++++++++++++++++++++++++++++++++ core/site_peak_omni.go | 11 +- core/site_peak_omni_test.go | 8 +- core/site_peakshaving.go | 30 ++++- 7 files changed, 394 insertions(+), 12 deletions(-) create mode 100644 core/site_ha_guard.go create mode 100644 core/site_ha_guard_test.go diff --git a/core/lm/README.md b/core/lm/README.md index b853062199..0e527b1a77 100644 --- a/core/lm/README.md +++ b/core/lm/README.md @@ -264,6 +264,17 @@ error once; the same error then only at debug level until a write lands again a general 500 of Home Assistant the hint to look into its log (`TestPeakWriteErrorLoggedOnce`, `TestPeakWriteErrorBody`). +The calls to Home Assistant run inside the cycle, each limited by the client +timeout (10 s). One that does not answer would cost a timeout per call and +stretch the cycle by 40 s and more, so after the first timeout the other calls +of the cycle are refused and the next cycle asks again: a cycle costs one +timeout (`TestHAGuardOneTimeoutPerCycle`, `TestHAGuardOneTimeoutPerCycleBYD`). +The EEG counter is not read for a minute after a timeout of its own +(`TestHAGuardEegCounter`). An entity that cannot be read (timeout, 404, 5xx) is +not written to: Home Assistant answers a call to an entity it has not loaded with +success and does nothing (`TestNumberSetterReadFailure`); a release that cannot +be read keeps what evcc holds and tries again (`TestOmniReleaseHangs`). + The limit applies to the clock-aligned 15 minute window: `allowed` = `(limit × 15 min − energy drawn so far) / time left`, so energy left unused earlier allows more and a short spike is only covered when the window would end @@ -839,7 +850,7 @@ Every change in an evcc file. Check these when merging a new evcc version. | switch devices, stages | `charger/switchsocket_lm.go`, `charger/switchstages.go`, `core/loadpoint_stages.go`, `server/http_config_custom.go`, `templates/definition/charger/homeassistant-stages.yaml` | | phase switching | `core/loadpoint_phasecurrents.go`, `core/loadpoint/config_custom.go`, `core/keys/loadpoint_custom.go` | | grid charging | `core/site_lm_once.go` (soc-based in `core/site_lm.go`) | -| peak shaving | `core/peak/`, `core/site_peakshaving.go`, `core/site_peak_omni.go`, `core/site_peak_follow.go`, `core/site_peak_stats.go`, `core/site_peak_tariff.go` | +| peak shaving | `core/peak/`, `core/site_peakshaving.go`, `core/site_peak_omni.go`, `core/site_ha_guard.go`, `core/site_peak_follow.go`, `core/site_peak_stats.go`, `core/site_peak_tariff.go` | | profiles | `core/lm/profile/`, `core/site_lm_profiles.go` | | forecast | `core/site_load_weekday.go`, `core/site_load_manual.go`, `core/metrics/profile_custom.go` | | battery identification | `core/site_battery_ident.go`, `core/metrics/slots_custom.go` | diff --git a/core/site_feedin_eeg.go b/core/site_feedin_eeg.go index 923b326d5a..281cfb7ad3 100644 --- a/core/site_feedin_eeg.go +++ b/core/site_feedin_eeg.go @@ -16,6 +16,7 @@ package core // tariff. Without a counter nothing here runs and evcc behaves as upstream. import ( + "errors" "fmt" "strings" "sync" @@ -37,6 +38,7 @@ type feedInEegState struct { collector *metrics.Collector // records the counter per slot priceSlot time.Time // last slot whose price was persisted failing bool // a failing read was logged + retryAt time.Time // no read before, after a timeout clock clock.Clock // for tests, nil = real time } @@ -163,24 +165,41 @@ func (site *Site) applyFeedInEegEntity(entity string, changed bool) error { return nil } +// eegRetry is how long the counter is not read after a read timed out, Home +// Assistant does not answer then, see haGuard +const eegRetry = time.Minute + // updateFeedInEeg records the EEG counter and the EEG price of the slot. Called // every cycle, does nothing without a counter. func (site *Site) updateFeedInEeg() { s := site.eeg() + now := time.Now() + if s.clock != nil { + now = s.clock.Now() + } + s.mu.Lock() - entity, get, c := s.entity, s.get, s.collector + entity, get, c, wait := s.entity, s.get, s.collector, now.Before(s.retryAt) s.mu.Unlock() - if get == nil || c == nil { + if get == nil || c == nil || wait { return } v, err := get() + // refused after a call timed out in this cycle, that call logged the cause + if errors.Is(err, errHAHung) { + return + } + s.mu.Lock() warn, recovered := err != nil && !s.failing, err == nil && s.failing s.failing = err != nil + if err != nil && isTimeout(err) { + s.retryAt = now.Add(eegRetry) + } s.mu.Unlock() switch { diff --git a/core/site_ha_guard.go b/core/site_ha_guard.go new file mode 100644 index 0000000000..a98ff1abdd --- /dev/null +++ b/core/site_ha_guard.go @@ -0,0 +1,110 @@ +package core + +// Custom extension: a Home Assistant that does not answer must not stretch the +// site cycle. +// +// Peak shaving, grid charging and the EEG counter call Home Assistant inside the +// cycle, one after the other, each call limited by the client timeout only (10 s). +// A Home Assistant that takes the connection but does not answer (start of the +// core, an overloaded host) would cost one timeout per call: the energy sensor, the +// setpoint, the charge power, the entities of the battery type Marstek, the EEG +// counter. 40 s and more per cycle, and load management and wallboxes would be +// regulated that much later. +// +// haGuard sits in the transport of the shared connection (see haConnection). After +// the first call timing out in a cycle it refuses the further calls with errHAHung +// until the cycle ends (haCycleEnd); the first call of the next cycle asks again. +// So a cycle costs one timeout at most. + +import ( + "context" + "errors" + "net" + "net/http" + "sync" + + "github.com/evcc-io/evcc/util" + "github.com/evcc-io/evcc/util/homeassistant" +) + +// errHAHung refuses a call after a call in the same cycle timed out +var errHAHung = errors.New("home assistant did not answer earlier in this cycle") + +// haGuard is the transport of the connection to Home Assistant +type haGuard struct { + base http.RoundTripper + log *util.Logger + + mu sync.Mutex + hung bool // a call timed out in this cycle +} + +// guardHA puts the guard in the transport of the connection +func guardHA(conn *homeassistant.Connection, log *util.Logger) { + conn.Client.Transport = &haGuard{base: conn.Client.Transport, log: log} +} + +// RoundTrip implements http.RoundTripper +func (g *haGuard) RoundTrip(req *http.Request) (*http.Response, error) { + g.mu.Lock() + hung := g.hung + g.mu.Unlock() + + if hung { + // a round tripper closes the body also when it does not send it + if req.Body != nil { + _ = req.Body.Close() + } + + return nil, errHAHung + } + + resp, err := g.base.RoundTrip(req) + + if err != nil && isTimeout(err) { + g.mu.Lock() + first := !g.hung + g.hung = true + g.mu.Unlock() + + if first { + g.log.DEBUG.Println("home assistant: no answer, no further calls in this cycle") + } + } + + return resp, err +} + +// endCycle lets the next call through again +func (g *haGuard) endCycle() { + g.mu.Lock() + defer g.mu.Unlock() + + g.hung = false +} + +// isTimeout reports whether the call timed out, rather than failed +func isTimeout(err error) bool { + var ne net.Error + + return errors.Is(err, context.DeadlineExceeded) || (errors.As(err, &ne) && ne.Timeout()) +} + +// haCycleEnd ends the cycle for the guard of the connection: the next call to +// Home Assistant is made again even if one timed out. Called at the end of the +// fork's part of the cycle, see updateBatteryModePeakAware. +func (site *Site) haCycleEnd() { + s := site.peak() + + s.mu.Lock() + conn := s.conn + s.mu.Unlock() + + if conn == nil { + return + } + + if g, ok := conn.Client.Transport.(*haGuard); ok { + g.endCycle() + } +} diff --git a/core/site_ha_guard_test.go b/core/site_ha_guard_test.go new file mode 100644 index 0000000000..9994e8a017 --- /dev/null +++ b/core/site_ha_guard_test.go @@ -0,0 +1,211 @@ +package core + +import ( + "net/http" + "sync/atomic" + "testing" + "time" + + "github.com/benbjohnson/clock" + "github.com/evcc-io/evcc/db" + "github.com/evcc-io/evcc/util" + "github.com/evcc-io/evcc/util/homeassistant" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// hangingHA is a Home Assistant that answers like fakeHA, or, while it hangs, +// takes the connection and does not answer: the call waits for the timeout of +// the client. +type hangingHA struct { + base http.RoundTripper + hang atomic.Bool + requests atomic.Int32 +} + +func (h *hangingHA) RoundTrip(req *http.Request) (*http.Response, error) { + h.requests.Add(1) + + if h.hang.Load() { + <-req.Context().Done() + return nil, req.Context().Err() + } + + return h.base.RoundTrip(req) +} + +// hangHA puts a Home Assistant that can hang below the guard of the scenario's +// connection and shortens the timeout of the client. It answers until hang is set. +func hangHA(sc *scenario) *hangingHA { + conn := sc.site.peak().conn + g := conn.Client.Transport.(*haGuard) + + h := &hangingHA{base: g.base} + g.base = h + conn.Client.Timeout = 20 * time.Millisecond + + return h +} + +// TestHAGuardOneTimeoutPerCycle verifies that a Home Assistant that does not +// answer costs one timeout per cycle: the first call times out, the rest of the +// cycle is not called (grid charging and Omnibattery's peak shaving, each of them +// would run into a timeout of its own), and the next cycle asks again. +func TestHAGuardOneTimeoutPerCycle(t *testing.T) { + sc, ha := newOmni(t) + sc.site.lms().socChargeEnabled = true + h := hangHA(sc) + h.hang.Store(true) + + require.True(t, sc.cycle(20, 1000, 0)) + assert.Equal(t, int32(1), h.requests.Load(), "one request in the cycle") + assert.Empty(t, ha.take()) + + sc.cycle(20, 1000, 0) + assert.Equal(t, int32(2), h.requests.Load(), "the next cycle asks again") + + // back: everything is done in the cycle + h.hang.Store(false) + sc.cycle(20, 1000, 0) + assert.Equal(t, append([]string{ + "turn_on " + haSwitch, + "select_option " + haSelect + " Charge", + "set_value " + haChargeNum + " 4000", + }, protOn("30", "5000")...), ha.take()) + assert.True(t, sc.site.peakOwned()) + assert.True(t, sc.site.peak().protOwned) +} + +// TestHAGuardOneTimeoutPerCycleBYD is the same for the battery type BYD, which +// writes the setpoint and the charge power in the middle of the cycle. +func TestHAGuardOneTimeoutPerCycleBYD(t *testing.T) { + sc, ha := newOmni(t) + sc.site.lms().socChargeEnabled = true + + s := sc.site.peak() + s.batteryType = batteryTypeBYD + require.NoError(t, sc.site.rebuildPeakSetter()) + require.NoError(t, sc.site.rebuildChargeSetter()) + + h := hangHA(sc) + h.hang.Store(true) + + require.True(t, sc.cycle(20, 1000, 0)) + assert.Equal(t, int32(1), h.requests.Load(), "the setpoint and the charge power: one request") + + h.hang.Store(false) + sc.cycle(20, 1000, 0) + assert.Contains(t, ha.take(), "set_value "+haChargeNum+" 4000") +} + +// TestHAGuardEegCounter verifies the EEG counter, read in the middle of the cycle: +// it is not read after a timeout in the same cycle, and a timeout of its own is +// not asked again for a minute. +func TestHAGuardEegCounter(t *testing.T) { + sc, _ := newOmni(t) // keeps the settings in memory, the collector needs a database + require.NoError(t, db.NewInstance("sqlite", ":memory:")) + t.Cleanup(func() { db.Instance = nil }) + + site := sc.site + h := hangHA(sc) + + require.NoError(t, site.applyFeedInEegEntity("", false)) + + conn := site.peak().conn + clk := clock.NewMock() + clk.Set(time.Date(2026, 9, 10, 12, 0, 0, 0, time.UTC)) + + s := site.eeg() + s.clock = clk + s.entity, s.get = "sensor.eeg", func() (float64, error) { return conn.GetFloatState("sensor.eeg") } + + // cycle: the counter times out, Omnibattery's peak shaving at the end is not called + h.hang.Store(true) + site.updateFeedInEeg() + site.updateBatteryModePeakAware(false, false, sc.rate) + assert.Equal(t, int32(1), h.requests.Load(), "the counter timed out, nothing else was called") + + // the next cycles: no read of the counter for a minute, Omnibattery asks + clk.Add(15 * time.Second) + site.updateFeedInEeg() + assert.Equal(t, int32(1), h.requests.Load(), "the counter is not read again") + site.updateBatteryModePeakAware(false, false, sc.rate) + assert.Equal(t, int32(2), h.requests.Load()) + + clk.Add(time.Minute) + site.updateFeedInEeg() + assert.Equal(t, int32(3), h.requests.Load(), "read again after a minute") +} + +// TestHAGuardRefusedCallsAreNotLogged verifies that a call refused after a timeout +// is no error of its own: the call that timed out reported it. +func TestHAGuardRefusedCallsAreNotLogged(t *testing.T) { + sc, _ := newOmni(t) + sc.site.lms().socChargeEnabled = true + h := hangHA(sc) + h.hang.Store(true) + + sc.cycle(20, 1000, 0) + + s := sc.site.peak() + s.mu.Lock() + defer s.mu.Unlock() + + assert.Len(t, s.writeErr, 1, "only the call that timed out") +} + +// TestHAGuardInstalled verifies that the connection built for the add-on has the +// guard: the tests above set it up by hand. +func TestHAGuardInstalled(t *testing.T) { + t.Setenv(homeassistant.SupervisorToken, "token") + + site := &Site{log: util.NewLogger("test")} + + conn, err := site.haConnection() + require.NoError(t, err) + assert.IsType(t, &haGuard{}, conn.Client.Transport) +} + +// TestNumberSetterReadFailure verifies that an entity that cannot be read is not +// written to: the range is unknown, and an entity Home Assistant has not loaded +// answers a call with success and does nothing. +func TestNumberSetterReadFailure(t *testing.T) { + sc, ha := newOmni(t) + + ha.mu.Lock() + delete(ha.states, haDischargeNum) + ha.mu.Unlock() + + set, err := sc.site.numberSetter(haDischargeNum, true, nil) + require.NoError(t, err) + + assert.Error(t, set(1000)) + assert.Empty(t, ha.take(), "nothing written") + + // timeout + h := hangHA(sc) + h.hang.Store(true) + + assert.Error(t, set(1000)) + assert.Equal(t, int32(1), h.requests.Load(), "a read, no write") +} + +// TestOmniReleaseHangs verifies that a release that cannot be read keeps what +// evcc holds: the next cycle tries again. +func TestOmniReleaseHangs(t *testing.T) { + sc, ha := charging(t) + + h := hangHA(sc) + h.hang.Store(true) + + // the charge ends: the charge power to zero, then the switch + sc.cycle(90, 1000, 0) + assert.Equal(t, int32(1), h.requests.Load()) + assert.True(t, sc.site.peakOwned(), "still held") + assert.True(t, sc.site.peak().protOwned, "still held") + + h.hang.Store(false) + sc.cycle(90, 1000, 0) + assert.False(t, sc.site.peakOwned()) + assert.Equal(t, "off", ha.get(haSwitch)) +} diff --git a/core/site_peak_omni.go b/core/site_peak_omni.go index a21504912f..2a9ea11e8a 100644 --- a/core/site_peak_omni.go +++ b/core/site_peak_omni.go @@ -408,14 +408,21 @@ func (site *Site) releaseOmni() bool { } // omniSwitchOff switches the entity off unless it is already, and reports -// whether it is. An entity not readable is written to anyway. +// whether it is. An entity not readable is not written to: Home Assistant answers +// a call to an entity it has not loaded with success and does nothing, so the +// release would count as done. The next cycle tries again. func (site *Site) omniSwitchOff(name, entity string) bool { conn, err := site.haConnection() if !site.logWrite("marstek connect", "connect", err) { return false } - if state, _, err := omniRead(conn, entity); err == nil && strings.EqualFold(state, "off") { + state, _, err := omniRead(conn, entity) + if !site.logWrite(name+" read", "read "+entity, err) { + return false + } + + if strings.EqualFold(state, "off") { return true } diff --git a/core/site_peak_omni_test.go b/core/site_peak_omni_test.go index 1fb3eb17e6..37e65e31d7 100644 --- a/core/site_peak_omni_test.go +++ b/core/site_peak_omni_test.go @@ -145,7 +145,12 @@ func (f *fakeHA) RoundTrip(req *http.Request) (*http.Response, error) { return reply(code, "500 Internal Server Error\n\nServer got itself in trouble") } - e := f.states[id] + // an entity Home Assistant has not loaded: the call is answered with success + // and does nothing + e, ok := f.states[id] + if !ok { + return reply(http.StatusOK, "[]") + } switch name { case "turn_on": @@ -182,6 +187,7 @@ func newOmni(t *testing.T) (*scenario, *fakeHA) { conn, err := homeassistant.NewConnection(util.NewLogger("test"), homeassistant.SupervisorURI, "", false) require.NoError(t, err) conn.Client.Transport = ha + guardHA(conn, util.NewLogger("test")) s := sc.site.peak() s.clock = clock.NewMock() diff --git a/core/site_peakshaving.go b/core/site_peakshaving.go index 38a027f474..9fa5f49c5d 100644 --- a/core/site_peakshaving.go +++ b/core/site_peakshaving.go @@ -359,11 +359,15 @@ func (site *Site) haConnection() (*homeassistant.Connection, error) { return nil, err } - conn, err = homeassistant.NewConnection(util.NewLogger("peakshaving"), uri, "", false) + log := util.NewLogger("peakshaving") + + conn, err = homeassistant.NewConnection(log, uri, "", false) if err != nil { return nil, err } + guardHA(conn, log) // one timeout per cycle at most, see site_ha_guard.go + s.mu.Lock() s.conn = conn s.mu.Unlock() @@ -379,7 +383,9 @@ func (site *Site) haConnection() (*homeassistant.Connection, error) { // peak.Range.Unchanged: a device may store every write. As the comparison is // against the entity, a value changed by hand is still corrected. tolerance // returns the smallest change written; nil for an entity that is not a power, the -// write tolerance is in W. +// write tolerance is in W. An entity that cannot be read is not written: its +// range is unknown, and a write to an entity Home Assistant has not loaded (yet) +// is answered with success and does nothing. func (site *Site) numberSetter(entity string, up bool, tolerance func() float64) (func(float64) error, error) { conn, err := site.haConnection() if err != nil { @@ -389,7 +395,7 @@ func (site *Site) numberSetter(entity string, up bool, tolerance func() float64) return func(val float64) error { r, current, err := numberState(conn, entity) if err != nil { - return conn.CallNumberService(entity, val) + return fmt.Errorf("read %s: %w", entity, err) } var tol float64 @@ -753,8 +759,14 @@ func (site *Site) writeOutput(name string, set func(float64) error, value float6 // logWrite reports the result of a write to an output and returns whether it // landed. The same error is logged once as an error and then at debug level until // a write succeeds again, so a rejected write does not fill the log with one line -// per cycle. +// per cycle. A call refused by the guard, see haGuard, is not logged. func (site *Site) logWrite(name, what string, err error) bool { + // refused after a call timed out in this cycle, that call logged the cause + if errors.Is(err, errHAHung) { + site.log.TRACE.Printf("%s: %s: %v", name, what, err) + return false + } + s := site.peak() s.mu.Lock() @@ -834,7 +846,10 @@ func (site *Site) peakEnergy() (float64, string) { if err == nil { return v, peak.SourceEntity } - site.log.WARN.Printf("peak shaving: energy %s: %v, using the grid power", entity, err) + + if !errors.Is(err, errHAHung) { + site.log.WARN.Printf("peak shaving: energy %s: %v, using the grid power", entity, err) + } } return 0, peak.SourcePower @@ -979,7 +994,10 @@ func (site *Site) peakChargeHeadroom() (headroom float64, ok bool) { // running peak (see batteryGridChargeRequested), and a mode set from outside // through the api stays the caller's decision. func (site *Site) updateBatteryModePeakAware(gridCharge, gridDischarge bool, rate api.Rate) { - // the last hook of the cycle: everything the overview shows is decided now + // the last hook of the cycle: the calls to Home Assistant are done + defer site.haCycleEnd() + + // everything the overview shows is decided now defer site.publishLmStatus(gridCharge) defer site.publishLmWallboxes() defer site.checkLmFollowing() From 6acebe4cf150836cdbeb5b27d0cdc099bd4ef95c Mon Sep 17 00:00:00 2001 From: SolarPower2024 Date: Sun, 11 Oct 2026 22:04:08 +0200 Subject: [PATCH 2/8] Peak: Dienstaufruf an nicht verfuegbare Entitaet zaehlt nicht mehr als Freigabe (Review D3) omniRead und numberState melden unknown/unavailable wie GetState als api.ErrNotAvailable. Dann wird nicht geschrieben (Home Assistant antwortet mit 200 und tut nichts), peakShavingOwned und peakShavingProtOwned bleiben gesetzt, und die Freigabe wird im naechsten Zyklus wiederholt, auch waehrend Home Assistant startet und die Entitaeten noch nicht kennt. Nur ein Schalter, den Home Assistant gar nicht kennt (404), gilt als aus, sonst blockierte die Freigabe jede Aenderung der Entitaeten. fakeHA kennt jetzt nicht verfuegbare und fehlende Entitaeten. Co-Authored-By: Claude Sonnet 5.5 --- core/lm/README.md | 14 +++-- core/site_peak_omni.go | 31 ++++++++-- core/site_peak_omni_test.go | 118 ++++++++++++++++++++++++++++++++++-- core/site_peakshaving.go | 13 +++- 4 files changed, 162 insertions(+), 14 deletions(-) diff --git a/core/lm/README.md b/core/lm/README.md index 0e527b1a77..a14d96c38b 100644 --- a/core/lm/README.md +++ b/core/lm/README.md @@ -270,10 +270,16 @@ stretch the cycle by 40 s and more, so after the first timeout the other calls of the cycle are refused and the next cycle asks again: a cycle costs one timeout (`TestHAGuardOneTimeoutPerCycle`, `TestHAGuardOneTimeoutPerCycleBYD`). The EEG counter is not read for a minute after a timeout of its own -(`TestHAGuardEegCounter`). An entity that cannot be read (timeout, 404, 5xx) is -not written to: Home Assistant answers a call to an entity it has not loaded with -success and does nothing (`TestNumberSetterReadFailure`); a release that cannot -be read keeps what evcc holds and tries again (`TestOmniReleaseHangs`). +(`TestHAGuardEegCounter`). An entity that cannot be read (timeout, 404, 5xx) or +is `unavailable` or `unknown` is not written to: Home Assistant answers a call +to such an entity with success and does nothing (`TestNumberSetterReadFailure`, +`TestOmniNotAvailableKeepsHold`). A release that meets one keeps what evcc holds +(`peakShavingOwned`, `peakShavingProtOwned`) and tries again in the next cycle, +also while Home Assistant starts and does not know the entities yet +(`TestOmniReleaseHangs`, `TestOmniEntityNotLoaded`). Only a switch that Home +Assistant does not know at all (deleted, renamed; 404) counts as off, as the +release would otherwise never land and block every change of the entities +(`TestOmniSwitchGone`). The limit applies to the clock-aligned 15 minute window: `allowed` = `(limit × 15 min − energy drawn so far) / time left`, so energy left unused diff --git a/core/site_peak_omni.go b/core/site_peak_omni.go index 2a9ea11e8a..5f0efc3978 100644 --- a/core/site_peak_omni.go +++ b/core/site_peak_omni.go @@ -25,14 +25,17 @@ import ( "errors" "fmt" "math" + "net/http" "net/url" "slices" "strconv" "strings" + "github.com/evcc-io/evcc/api" "github.com/evcc-io/evcc/core/keys" "github.com/evcc-io/evcc/db/settings" "github.com/evcc-io/evcc/util/homeassistant" + "github.com/evcc-io/evcc/util/request" ) const ( @@ -408,9 +411,12 @@ func (site *Site) releaseOmni() bool { } // omniSwitchOff switches the entity off unless it is already, and reports -// whether it is. An entity not readable is not written to: Home Assistant answers -// a call to an entity it has not loaded with success and does nothing, so the -// release would count as done. The next cycle tries again. +// whether it is. An entity not readable (not available, timeout, error) is not +// written to: Home Assistant answers a call to such an entity with success and does +// nothing, so the release would count as done while the switch stays on. What evcc +// holds stays held and the next cycle tries again. Only an entity Home Assistant +// does not know (404) counts as off: nothing is left to switch off, and the release +// would otherwise never land and block every change of the entities. func (site *Site) omniSwitchOff(name, entity string) bool { conn, err := site.haConnection() if !site.logWrite("marstek connect", "connect", err) { @@ -418,6 +424,11 @@ func (site *Site) omniSwitchOff(name, entity string) bool { } state, _, err := omniRead(conn, entity) + if notFound(err) { + site.log.WARN.Printf("%s: %s not found, nothing to switch off", name, entity) + return true + } + if !site.logWrite(name+" read", "read "+entity, err) { return false } @@ -444,7 +455,16 @@ func (site *Site) omniShow(mode string) { } } -// omniRead reads the state and the options of a switch or select entity +// notFound reports whether Home Assistant does not know the entity +func notFound(err error) bool { + var se *request.StatusError + + return errors.As(err, &se) && se.StatusCode() == http.StatusNotFound +} + +// omniRead reads the state and the options of a switch or select entity. As +// GetState does, it reports an entity that is unknown or not available as +// api.ErrNotAvailable: a call to it is answered with success and does nothing. func omniRead(conn *homeassistant.Connection, entity string) (string, []string, error) { var res struct { State string `json:"state"` @@ -454,6 +474,9 @@ func omniRead(conn *homeassistant.Connection, entity string) (string, []string, } err := conn.GetJSON(fmt.Sprintf("%s/api/states/%s", conn.URI(), url.PathEscape(entity)), &res) + if err == nil && notAvailable(res.State) { + err = api.ErrNotAvailable + } return res.State, res.Attributes.Options, err } diff --git a/core/site_peak_omni_test.go b/core/site_peak_omni_test.go index 37e65e31d7..e460ea146f 100644 --- a/core/site_peak_omni_test.go +++ b/core/site_peak_omni_test.go @@ -35,7 +35,9 @@ const ( // fakeHA is a Home Assistant answering state reads and service calls, and // recording the calls as "service entity [value]". A failing service answers -// with the status in fail, e.g. "switch/turn_on". +// with the status in fail, e.g. "switch/turn_on". A call to an entity that is +// unavailable, unknown or missing is answered with success and does nothing, as +// Home Assistant does. type fakeHA struct { mu sync.Mutex states map[string]*haEntity @@ -145,10 +147,8 @@ func (f *fakeHA) RoundTrip(req *http.Request) (*http.Response, error) { return reply(code, "500 Internal Server Error\n\nServer got itself in trouble") } - // an entity Home Assistant has not loaded: the call is answered with success - // and does nothing e, ok := f.states[id] - if !ok { + if !ok || e.state == "unavailable" || e.state == "unknown" { return reply(http.StatusOK, "[]") } @@ -1299,3 +1299,113 @@ func TestOmniNoHysteresis(t *testing.T) { assert.Equal(t, peak.DefaultHysteresis, sc.site.peakHysteresis()) }) } + +// TestOmniNotAvailableKeepsHold verifies that nothing is written to an entity that +// is not available or unknown and that what evcc holds stays held: Home Assistant +// answers such a call with success and does nothing, the battery would stay in +// manual control with nothing left to release it. +func TestOmniNotAvailableKeepsHold(t *testing.T) { + for _, state := range []string{"unavailable", "unknown"} { + t.Run("manual control "+state, func(t *testing.T) { + sc, ha := charging(t) + ha.set(haSwitch, state) + + // the charge ends: the power to zero lands, the switch cannot be read + assert.False(t, sc.cycle(90, 1000, 0)) + assert.Equal(t, []string{"set_value " + haChargeNum + " 0"}, ha.take()) + + for range 2 { + assert.False(t, sc.cycle(90, 1000, 0)) + assert.Empty(t, ha.take(), "no call to the switch") + assert.True(t, sc.site.peakOwned(), "still held") + } + + // back: released + ha.set(haSwitch, "on") + assert.False(t, sc.cycle(90, 1000, 0)) + assert.Equal(t, []string{"turn_off " + haSwitch}, ha.take()) + assert.False(t, sc.site.peakOwned()) + }) + + t.Run("peak shaving switch "+state, func(t *testing.T) { + sc, ha := protecting(t) + ha.set(haProtSwitch, state) + + require.NoError(t, sc.site.SetPeakShaving(false)) + assert.Empty(t, ha.take(), "no call to the switch") + assert.True(t, sc.site.peak().protOwned, "still held") + + ha.set(haProtSwitch, "on") + sc.cycle(25, 8000, 0) + assertProtReleased(t, sc, ha) + }) + + t.Run("limit "+state, func(t *testing.T) { + sc, ha := newOmni(t) + ha.set(haProtLimit, state) + + sc.cycle(25, 8000, 0) + assert.Equal(t, []string{"set_value " + haProtSoc + " 30"}, ha.take(), "the threshold only, no limit and no switch") + assert.False(t, sc.site.peak().protOwned) + + ha.set(haProtLimit, "20000") + sc.cycle(25, 8000, 0) + assert.Equal(t, []string{"set_value " + haProtLimit + " 5000", "turn_on " + haProtSwitch}, ha.take()) + assert.True(t, sc.site.peak().protOwned) + }) + + t.Run("number "+state, func(t *testing.T) { + sc, ha := newOmni(t) + ha.set(haDischargeNum, state) + + set, err := sc.site.numberSetter(haDischargeNum, true, nil) + require.NoError(t, err) + + assert.ErrorIs(t, set(1000), api.ErrNotAvailable) + assert.Empty(t, ha.take(), "nothing written") + }) + } +} + +// TestOmniEntityNotLoaded verifies the start of Home Assistant, which does not +// know the entities yet: a release writes nothing and keeps what evcc holds until +// they are back, as does a missing entity (no panic in between). +func TestOmniEntityNotLoaded(t *testing.T) { + sc, ha := charging(t) + + ha.mu.Lock() + charge := ha.states[haChargeNum] + delete(ha.states, haChargeNum) + ha.mu.Unlock() + + for range 2 { + assert.False(t, sc.cycle(90, 1000, 0)) + assert.Empty(t, ha.take()) + assert.True(t, sc.site.peakOwned(), "still held") + assert.Equal(t, "on", ha.get(haSwitch)) + } + + ha.mu.Lock() + ha.states[haChargeNum] = charge + ha.mu.Unlock() + + assert.False(t, sc.cycle(90, 1000, 0)) + assert.Equal(t, []string{"set_value " + haChargeNum + " 0", "turn_off " + haSwitch}, ha.take()) + assert.False(t, sc.site.peakOwned()) +} + +// TestOmniSwitchGone verifies that a switch Home Assistant does not know any more +// (deleted, renamed) counts as released: nothing is left to switch off, and the +// release would otherwise never land and block every change of the entities. +func TestOmniSwitchGone(t *testing.T) { + sc, ha := charging(t) + + ha.mu.Lock() + delete(ha.states, haSwitch) + ha.mu.Unlock() + + require.NoError(t, sc.site.SetPeakShavingManualEntity("")) + assert.Equal(t, []string{"set_value " + haChargeNum + " 0"}, ha.take()) + assert.False(t, sc.site.peakOwned()) + assert.True(t, sc.site.peak().protOwned, "the peak shaving of Omnibattery is not part of it") +} diff --git a/core/site_peakshaving.go b/core/site_peakshaving.go index 9fa5f49c5d..ad716d01a5 100644 --- a/core/site_peakshaving.go +++ b/core/site_peakshaving.go @@ -417,7 +417,7 @@ func (site *Site) numberSetter(entity string, up bool, tolerance func() float64) func numberWrite(r peak.Range, current string, val float64, up bool, tolerance float64) (float64, bool) { val = r.Fit(val, up) - // an unavailable entity has no current value and gets the write + // a state that is no number has no current value and gets the write v, err := strconv.ParseFloat(current, 64) return val, err != nil || !r.Unchanged(v, val, tolerance) @@ -431,8 +431,14 @@ func (site *Site) peakWriteTolerance() float64 { return 0 } +// notAvailable reports whether the state is that of an entity without a value +func notAvailable(state string) bool { + return state == "unknown" || state == "unavailable" +} + // numberState reads the min, max and step attributes and the state of a number -// entity +// entity. An entity that is unknown or not available is reported as +// api.ErrNotAvailable, as GetState does. func numberState(conn *homeassistant.Connection, entity string) (peak.Range, string, error) { var res struct { State string `json:"state"` @@ -445,6 +451,9 @@ func numberState(conn *homeassistant.Connection, entity string) (peak.Range, str uri := fmt.Sprintf("%s/api/states/%s", conn.URI(), url.PathEscape(entity)) err := conn.GetJSON(uri, &res) + if err == nil && notAvailable(res.State) { + err = api.ErrNotAvailable + } return peak.Range(res.Attributes), res.State, err } From 96fb75ac7c6536a379cce30514f6327980ae1431 Mon Sep 17 00:00:00 2001 From: SolarPower2024 Date: Sun, 11 Oct 2026 22:06:55 +0200 Subject: [PATCH 3/8] Peak: Batterie beim Stoppen von evcc zurueckgeben (Review B3) Beim Stoppen gab evcc nur seinen Batteriemodus zurueck. Was der Fork ueber Home Assistant haelt, blieb bis zum ersten Zyklus nach dem naechsten Start: Marstek mit Ladeleistung und Handsteuerung (Netzladen bis voll ohne Peak-Gate), das Limit von Omnibatterys Peak Shaving auf dem letzten Fensterwert, BYD mit Sollwert und Ladeleistung. Eine Shutdown-Funktion (aus restorePeakSettings, in core/site_peak_shutdown.go, kein neuer Upstream-Hook) setzt ein Flag, gegen das der noch laufende Zyklus nichts mehr schreibt, und gibt in der Reihenfolge Sicherheit zuerst zurueck: Marstek Ladeleistung 0 und Handsteuerung aus, Omnibattery behaelt Peak Shaving mit dem Peak-Limit, BYD Ladeleistung 0 und freier Wert. Best Effort in hoechstens 8 s, blockiert den Stopp nicht; was nicht ankommt, bleibt als gehalten markiert. Abgelehnte Aufrufe waehrend des Stopps (Supervisor meldet 400/503 not ready) stehen als info im Log. Co-Authored-By: Claude Sonnet 5.5 --- core/lm/README.md | 15 +++- core/site_peak_omni.go | 4 + core/site_peak_shutdown.go | 112 ++++++++++++++++++++++++++ core/site_peak_shutdown_test.go | 135 ++++++++++++++++++++++++++++++++ core/site_peakshaving.go | 21 +++++ 5 files changed, 286 insertions(+), 1 deletion(-) create mode 100644 core/site_peak_shutdown.go create mode 100644 core/site_peak_shutdown_test.go diff --git a/core/lm/README.md b/core/lm/README.md index a14d96c38b..09b296aa41 100644 --- a/core/lm/README.md +++ b/core/lm/README.md @@ -264,6 +264,19 @@ error once; the same error then only at debug level until a write lands again a general 500 of Home Assistant the hint to look into its log (`TestPeakWriteErrorLoggedOnce`, `TestPeakWriteErrorBody`). +Stopping evcc hands back as well, not only evcc's battery mode +(`TestPeakReleasedOnShutdown`): a Marstek gets the charge power 0 and the manual +control off, Omnibattery's peak shaving stays on with the peak limit instead of +the allowed power of the window; a BYD gets the charge power 0 and the free +value. The cycle writes nothing any more from then on. It is best effort and +never holds the stop up (`TestPeakShutdownDoesNotBlock`): stop budget add-on 30 s +(`timeout` in its manifest), evcc waits `max(interval, 5 s)` for the shutdown +functions, the hand back takes 8 s at most. When the host shuts down, the +supervisor refuses the calls (400 or 503 "not ready", logged at info), and +Omnibattery keeps its last value until evcc runs again. A crash or a kill is not +covered either; there the stored mark hands back with the first cycle after the +next start. + The calls to Home Assistant run inside the cycle, each limited by the client timeout (10 s). One that does not answer would cost a timeout per call and stretch the cycle by 40 s and more, so after the first timeout the other calls @@ -856,7 +869,7 @@ Every change in an evcc file. Check these when merging a new evcc version. | switch devices, stages | `charger/switchsocket_lm.go`, `charger/switchstages.go`, `core/loadpoint_stages.go`, `server/http_config_custom.go`, `templates/definition/charger/homeassistant-stages.yaml` | | phase switching | `core/loadpoint_phasecurrents.go`, `core/loadpoint/config_custom.go`, `core/keys/loadpoint_custom.go` | | grid charging | `core/site_lm_once.go` (soc-based in `core/site_lm.go`) | -| peak shaving | `core/peak/`, `core/site_peakshaving.go`, `core/site_peak_omni.go`, `core/site_ha_guard.go`, `core/site_peak_follow.go`, `core/site_peak_stats.go`, `core/site_peak_tariff.go` | +| peak shaving | `core/peak/`, `core/site_peakshaving.go`, `core/site_peak_omni.go`, `core/site_peak_shutdown.go`, `core/site_ha_guard.go`, `core/site_peak_follow.go`, `core/site_peak_stats.go`, `core/site_peak_tariff.go` | | profiles | `core/lm/profile/`, `core/site_lm_profiles.go` | | forecast | `core/site_load_weekday.go`, `core/site_load_manual.go`, `core/metrics/profile_custom.go` | | battery identification | `core/site_battery_ident.go`, `core/metrics/slots_custom.go` | diff --git a/core/site_peak_omni.go b/core/site_peak_omni.go index 5f0efc3978..fc0f71e484 100644 --- a/core/site_peak_omni.go +++ b/core/site_peak_omni.go @@ -161,6 +161,10 @@ func (site *Site) applyOmni() { s.out.Lock() defer s.out.Unlock() + if site.peakStopped() { + return + } + site.applyOmniCharge() site.applyOmniProtection() } diff --git a/core/site_peak_shutdown.go b/core/site_peak_shutdown.go new file mode 100644 index 0000000000..d96600ac07 --- /dev/null +++ b/core/site_peak_shutdown.go @@ -0,0 +1,112 @@ +package core + +// Custom extension: hand the battery back when evcc stops. +// +// evcc only hands back its own battery mode on stopping. What the fork holds +// through Home Assistant would stay until the next cycle after the next start: +// the grid charge power and the manual control of a Marstek (it would charge from +// the grid up to full, with no peak gate and no price check), the limit of +// Omnibattery's peak shaving at the last value of the window (up to twice the +// limit), the setpoint and the charge power of a BYD. The shutdown function hands +// it back before evcc exits, see releasePeak. +// +// It is best effort: the whole hand back may take peakShutdownBudget, then evcc +// stops anyway, and with a Home Assistant that is stopping itself (the supervisor +// refuses calls with 400 or 503 "not ready" once the host shuts down) nothing +// lands. A crash or a kill is not covered either. In both cases what is stored +// (peakShavingOwned) makes the first cycle after the next start hand back. + +import ( + "time" + + "github.com/evcc-io/evcc/cmd/shutdown" +) + +// peakShutdownBudget is how long the shutdown function may take. The supervisor +// stops the add-on after 30 s (timeout in its manifest), evcc waits for the +// shutdown functions for max(interval, 5 s) and stops the database after them. +var peakShutdownBudget = 8 * time.Second + +// registerShutdown is evcc's registration of a shutdown function, replaced in tests +var registerShutdown = shutdown.Register + +// registerPeakShutdown registers the hand back with evcc's shutdown functions, +// once +func (site *Site) registerPeakShutdown() { + s := site.peak() + + s.shutdownOnce.Do(func() { registerShutdown(site.releasePeakOnShutdown) }) +} + +// peakStopped reports whether evcc is stopping: the cycle writes nothing any more +func (site *Site) peakStopped() bool { + s := site.peak() + + s.mu.Lock() + defer s.mu.Unlock() + + return s.stopped +} + +// releasePeakOnShutdown stops the cycle's writes and hands back what the fork +// holds, within peakShutdownBudget +func (site *Site) releasePeakOnShutdown() { + s := site.peak() + + // the shutdown functions run beside a cycle that may still be running + s.mu.Lock() + s.stopped = true + s.mu.Unlock() + + done := make(chan struct{}) + + go func() { + defer close(done) + site.releasePeak() + }() + + select { + case <-done: + case <-time.After(peakShutdownBudget): + site.log.WARN.Printf("peak shaving: handing back took longer than %s, stopping anyway", peakShutdownBudget) + } +} + +// releasePeak hands back, safety first: the grid charge power and the manual +// control of a Marstek, the limit of Omnibattery's peak shaving (it stays on, with +// the peak limit instead of the allowed power of the window), then the charge power +// and the setpoint of a BYD. What does not land stays marked as held. +func (site *Site) releasePeak() { + s := site.peak() + + // a running cycle finishes its call first + s.out.Lock() + defer s.out.Unlock() + + s.mu.Lock() + typ, limit, limitEntity, protOwned := s.batteryType, s.limit, s.protLimit, s.protOwned + chargeSet, charging, set := s.chargeSet, s.chargeSetpoint > 0, s.set + s.mu.Unlock() + + if typ == batteryTypeMarstek { + if !site.releaseOmni() { + site.log.INFO.Println("peak shaving: the manual control stays on, released with the next start") + } + + if protOwned && limitEntity != "" { + if conn, err := site.haConnection(); err == nil { + site.omniWriteLimit(conn, limitEntity, limit, omniLimitBasis(false, limit)) + } + } + + return + } + + if charging && chargeSet != nil { + site.writeOutput("grid charge power", chargeSet, 0) + } + + if site.peakOwned() && set != nil && site.writeOutput("peak shaving", set, site.peakFreeValue()) { + site.setPeakOwned(false) + } +} diff --git a/core/site_peak_shutdown_test.go b/core/site_peak_shutdown_test.go new file mode 100644 index 0000000000..af0bec521e --- /dev/null +++ b/core/site_peak_shutdown_test.go @@ -0,0 +1,135 @@ +package core + +import ( + "net/http" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// shutdownBudget sets the time the shutdown function may take for the test +func shutdownBudget(t *testing.T, d time.Duration) { + t.Helper() + + prev := peakShutdownBudget + peakShutdownBudget = d + t.Cleanup(func() { peakShutdownBudget = prev }) +} + +// TestPeakReleasedOnShutdown verifies that stopping evcc hands back what the fork +// holds: the grid charge power and the manual control of a Marstek, the limit of +// Omnibattery's peak shaving (it stays on, with the peak limit instead of the +// allowed power of the window), the charge power and the setpoint of a BYD. The +// cycle writes nothing any more afterwards. +func TestPeakReleasedOnShutdown(t *testing.T) { + t.Run("marstek", func(t *testing.T) { + sc, ha := charging(t) + ha.set(haProtLimit, "6700") // the window raised it, the peak limit is 5000 + + sc.site.releasePeakOnShutdown() + assert.Equal(t, []string{ + "set_value " + haChargeNum + " 0", + "turn_off " + haSwitch, + "set_value " + haProtLimit + " 5000", + }, ha.take()) + assert.False(t, sc.site.peakOwned()) + assert.True(t, sc.site.peak().protOwned, "the peak shaving of Omnibattery stays on") + assert.Equal(t, "on", ha.get(haProtSwitch)) + + sc.cycle(20, 1000, 0) + sc.cycle(25, 8000, 0) + assert.Empty(t, ha.take(), "the cycle writes nothing any more") + }) + + t.Run("byd", func(t *testing.T) { + sc, ha := newOmni(t) + sc.site.lms().socChargeEnabled = true + + s := sc.site.peak() + s.batteryType = batteryTypeBYD + require.NoError(t, sc.site.rebuildPeakSetter()) + require.NoError(t, sc.site.rebuildChargeSetter()) + + require.True(t, sc.cycle(20, 1000, 0)) + require.True(t, sc.site.peakOwned()) + ha.take() + + sc.site.releasePeakOnShutdown() + assert.Equal(t, []string{ + "set_value " + haChargeNum + " 0", + "set_value " + haDischargeNum + " 10000", + }, ha.take()) + assert.False(t, sc.site.peakOwned()) + + sc.cycle(20, 1000, 0) + sc.cycle(25, 8000, 0) + assert.Empty(t, ha.take(), "the cycle writes nothing any more") + }) + + t.Run("nothing held", func(t *testing.T) { + sc, ha := newOmni(t) + sc.site.releasePeakOnShutdown() + assert.Empty(t, ha.take()) + + sc, ha = newOmni(t) + sc.site.peak().batteryType = batteryTypeBYD + require.NoError(t, sc.site.rebuildPeakSetter()) + sc.site.releasePeakOnShutdown() + assert.Empty(t, ha.take()) + }) +} + +// TestPeakShutdownRegisteredOnce verifies that the settings restore registers the +// hand back with evcc's shutdown functions, once +func TestPeakShutdownRegisteredOnce(t *testing.T) { + var registered []func() + + prev := registerShutdown + registerShutdown = func(cb func()) { registered = append(registered, cb) } + t.Cleanup(func() { registerShutdown = prev }) + + sc, ha := charging(t) + sc.site.restorePeakSettings() + sc.site.restorePeakSettings() + + require.Len(t, registered, 1) + + registered[0]() + assert.False(t, sc.site.peakOwned()) + assert.Contains(t, ha.take(), "turn_off "+haSwitch) +} + +// TestPeakShutdownDoesNotBlock verifies that the stop is not held up by a Home +// Assistant that does not answer or refuses the call (the supervisor does once the +// host shuts down): the hand back returns within its budget, what was not handed +// back stays marked as held, and the refusal is no error. +func TestPeakShutdownDoesNotBlock(t *testing.T) { + shutdownBudget(t, 100*time.Millisecond) + + t.Run("no answer", func(t *testing.T) { + sc, _ := charging(t) + h := hangHA(sc) + sc.site.peak().conn.Client.Timeout = time.Second // longer than the budget + h.hang.Store(true) + + start := time.Now() + sc.site.releasePeakOnShutdown() + assert.Less(t, time.Since(start), 900*time.Millisecond) + assert.True(t, sc.site.peakOwned(), "still held, released with the next start") + }) + + t.Run("refused", func(t *testing.T) { + sc, ha := charging(t) + ha.fail["switch/turn_off"] = http.StatusBadRequest + + sc.site.releasePeakOnShutdown() + assert.True(t, sc.site.peakOwned(), "still held, released with the next start") + + s := sc.site.peak() + s.mu.Lock() + defer s.mu.Unlock() + assert.Empty(t, s.writeErr, "expected while stopping: no error of an output") + }) +} diff --git a/core/site_peakshaving.go b/core/site_peakshaving.go index ad716d01a5..3f0879d979 100644 --- a/core/site_peakshaving.go +++ b/core/site_peakshaving.go @@ -77,6 +77,11 @@ type peakState struct { owned bool writeErr map[string]string // the last error logged per output, see writeOutput + // evcc is stopping: the cycle writes nothing any more while the shutdown + // function hands back, see site_peak_shutdown.go + stopped bool + shutdownOnce sync.Once + // battery type Marstek (Omnibattery), see site_peak_omni.go: evcc drives // Omnibattery's own peak shaving and switches the battery to manual control // while grid charging @@ -252,6 +257,8 @@ func (site *Site) restorePeakSettings() { site.restorePeakTariff() site.publishPeakSettings() site.publishLmPriorities() + + site.registerPeakShutdown() // hands the battery back on stopping, see site_peak_shutdown.go } // peakURI returns the Home Assistant endpoint. Running as an add-on, the @@ -649,6 +656,10 @@ func (site *Site) writePeakValue(value float64) bool { s.out.Lock() defer s.out.Unlock() + if site.peakStopped() { + return false + } + s.mu.Lock() set := s.set s.mu.Unlock() @@ -719,6 +730,10 @@ func (site *Site) writeChargeValue(value float64) { s.out.Lock() defer s.out.Unlock() + if site.peakStopped() { + return + } + s.mu.Lock() set := s.chargeSet s.chargeSetpoint = value @@ -776,6 +791,12 @@ func (site *Site) logWrite(name, what string, err error) bool { return false } + // stopping, e.g. with the host: Home Assistant may refuse the call, expected + if err != nil && site.peakStopped() { + site.log.INFO.Printf("%s: %s: %s (stopping)", name, what, writeErrorText(err)) + return false + } + s := site.peak() s.mu.Lock() From 495a778db0a98370186db6fd91473b3c00e0aeb7 Mon Sep 17 00:00:00 2001 From: SolarPower2024 Date: Sun, 11 Oct 2026 22:08:14 +0200 Subject: [PATCH 4/8] Peak: Merker fuer gehaltene Batterie sofort in die Datenbank (Review E9) settings.SetBool merkt sich einen Wert nur im Speicher, in die Datenbank kommt er mit dem Minuten-Takt oder beim Stopp. Ein Absturz in der ersten Minute nach der Uebernahme verlor peakShavingOwned und peakShavingProtOwned, die Batterie wurde nach dem Neustart nicht mehr zurueckgegeben. setPeakOwned und setProtOwned speichern bei einer Aenderung sofort; ein Fehler dabei wird einmal je Ursache geloggt. Die Tests lesen die Zeilen aus der Datenbank (SQLite im Speicher) und werden ohne den Aufruf rot. Co-Authored-By: Claude Sonnet 5.5 --- core/lm/README.md | 20 ++++++------ core/site_peak_omni.go | 1 + core/site_peak_owned_test.go | 62 ++++++++++++++++++++++++++++++++++++ core/site_peakshaving.go | 10 ++++++ 4 files changed, 84 insertions(+), 9 deletions(-) create mode 100644 core/site_peak_owned_test.go diff --git a/core/lm/README.md b/core/lm/README.md index 09b296aa41..215aa69cd7 100644 --- a/core/lm/README.md +++ b/core/lm/README.md @@ -254,15 +254,17 @@ gets the free value, a charge power entity 0 W flapping across the reserve (BYD only: the battery type Marstek uses none, see 11). evcc only hands back what it held. It remembers that it wrote a value other than -the free value (`peakShavingOwned`, stored, so a restart in the middle of a -control still hands back) and writes the free value on switching off, a lost -meter or a removed or replaced entity only while the mark is set, then clears -it. A start with peak shaving off writes nothing (`TestPeakNoHandBackWithoutOwned`, -`TestPeakHandBackAfterRestartWhenOwned`). A write that fails is logged as an -error once; the same error then only at debug level until a write lands again -(once at info), with the first 200 characters of the answer on one line, and for -a general 500 of Home Assistant the hint to look into its log -(`TestPeakWriteErrorLoggedOnce`, `TestPeakWriteErrorBody`). +the free value (`peakShavingOwned`, stored at once on every change, not with +the minute flush of the settings, so a restart or a crash in the middle of a +control still hands back, `TestPeakHeldMarksStoredAtOnce`; a power cut of the +host within seconds after the change is not covered) and writes the free value +on switching off, a lost meter or a removed or replaced entity only while the +mark is set, then clears it. A start with peak shaving off writes nothing +(`TestPeakNoHandBackWithoutOwned`, `TestPeakHandBackAfterRestartWhenOwned`). A +write that fails is logged as an error once; the same error then only at debug +level until a write lands again (once at info), with the first 200 characters of +the answer on one line, and for a general 500 of Home Assistant the hint to look +into its log (`TestPeakWriteErrorLoggedOnce`, `TestPeakWriteErrorBody`). Stopping evcc hands back as well, not only evcc's battery mode (`TestPeakReleasedOnShutdown`): a Marstek gets the charge power 0 and the manual diff --git a/core/site_peak_omni.go b/core/site_peak_omni.go index fc0f71e484..33cb06b34e 100644 --- a/core/site_peak_omni.go +++ b/core/site_peak_omni.go @@ -357,6 +357,7 @@ func (site *Site) setProtOwned(owned bool) { if changed { settings.SetBool(keys.PeakShavingProtOwned, owned) + site.persistHeld() } } diff --git a/core/site_peak_owned_test.go b/core/site_peak_owned_test.go new file mode 100644 index 0000000000..e3ae8c2a70 --- /dev/null +++ b/core/site_peak_owned_test.go @@ -0,0 +1,62 @@ +package core + +import ( + "testing" + + "github.com/evcc-io/evcc/core/keys" + "github.com/evcc-io/evcc/db" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// stored returns the value of a setting in the database, empty if it is not there +func stored(t *testing.T, key string) string { + t.Helper() + + var v []string + require.NoError(t, db.Instance.Table("settings").Select("value").Where("key = ?", key).Scan(&v).Error) + + if len(v) == 0 { + return "" + } + + return v[0] +} + +// TestPeakHeldMarksStoredAtOnce verifies that the marks of what evcc holds are in +// the database right away, not with the next flush of the settings: a crash +// within the minute would lose them, and a start would not hand the battery back. +func TestPeakHeldMarksStoredAtOnce(t *testing.T) { + sc, _ := newOmni(t) // keeps the settings in memory, the database follows + require.NoError(t, db.NewInstance("sqlite", ":memory:")) + t.Cleanup(func() { db.Instance = nil }) + + sc.site.lms().socChargeEnabled = true + + require.True(t, sc.cycle(20, 1000, 0)) + assert.Equal(t, "true", stored(t, keys.PeakShavingOwned), "manual control") + assert.Equal(t, "true", stored(t, keys.PeakShavingProtOwned), "peak shaving of Omnibattery") + + // and the release + require.False(t, sc.cycle(90, 1000, 0)) + assert.Equal(t, "false", stored(t, keys.PeakShavingOwned)) + + require.NoError(t, sc.site.SetPeakShaving(false)) + assert.Equal(t, "false", stored(t, keys.PeakShavingProtOwned)) +} + +// TestPeakHeldMarkStoredBYD is the same for the setpoint of the battery type BYD +func TestPeakHeldMarkStoredBYD(t *testing.T) { + sc, _ := newOmni(t) + require.NoError(t, db.NewInstance("sqlite", ":memory:")) + t.Cleanup(func() { db.Instance = nil }) + + sc.site.peak().batteryType = batteryTypeBYD + require.NoError(t, sc.site.rebuildPeakSetter()) + + sc.cycle(25, 9000, 0) + assert.Equal(t, "true", stored(t, keys.PeakShavingOwned)) + + require.NoError(t, sc.site.SetPeakShaving(false)) + assert.Equal(t, "false", stored(t, keys.PeakShavingOwned)) +} diff --git a/core/site_peakshaving.go b/core/site_peakshaving.go index 3f0879d979..136fe225c6 100644 --- a/core/site_peakshaving.go +++ b/core/site_peakshaving.go @@ -685,9 +685,19 @@ func (site *Site) setPeakOwned(owned bool) { if changed { settings.SetBool(keys.PeakShavingOwned, owned) + site.persistHeld() } } +// persistHeld stores the mark of what evcc holds at once. SetBool only keeps it +// in memory until the next flush (every minute and when evcc stops); a crash in +// between would lose the mark, and nothing would hand the battery back after the +// start. Called with s.out held and only on a change, blocks up to the busy +// timeout of the database at worst. A failing save is logged once per cause. +func (site *Site) persistHeld() { + site.logWrite("peak shaving mark", "save", settings.Persist()) +} + // peakOwned reports whether evcc holds the battery back, see peakState.owned func (site *Site) peakOwned() bool { s := site.peak() From 6ccc785c6f31df22cb409ac7f9fd032a9fe88498 Mon Sep 17 00:00:00 2001 From: SolarPower2024 Date: Sun, 11 Oct 2026 22:10:05 +0200 Subject: [PATCH 5/8] Hook: restoreCustom als defer am Anfang von restoreSettings (Review B13) Der Aufruf stand am Ende von evccs restoreSettings, hinter neun fruehen Returns. Scheitert ein Restore-Schritt von evcc (oder ein kuenftiges neues return err), lud nichts vom Fork: Peak Shaving still aus, kein Kreis, eine gehaltene Batterie wurde nie zurueckgegeben. Der Hook steht jetzt als defer direkt hinter der Pruefung fuer Tests, die alte Zeile entfaellt. Neuer Vertragstest TestForkDeferHooks prueft per Syntaxbaum, dass restoreCustom ein defer im Funktionsrumpf ist und kein evcc-Return davor steht (vor der Aenderung rot). Tabelle und Kommentar angepasst. Co-Authored-By: Claude Sonnet 5.5 --- core/fork_hooks_test.go | 102 ++++++++++++++++++++++++++++++++++++---- core/lm/README.md | 7 ++- core/site.go | 3 +- core/site_lm.go | 6 ++- 4 files changed, 105 insertions(+), 13 deletions(-) diff --git a/core/fork_hooks_test.go b/core/fork_hooks_test.go index 88390f2d9a..581a97c1b0 100644 --- a/core/fork_hooks_test.go +++ b/core/fork_hooks_test.go @@ -58,6 +58,16 @@ var forkHooks = []struct { {"cmd/root.go", "runRoot", []string{"CloseFile"}}, } +// forkDeferHooks lists the hooks that have to run on every way out of an evcc +// function: a deferred call in the function body, before any return of evcc's +// own. A call at the end would be skipped by the first return added above it. +var forkDeferHooks = []struct { + file, fn string + name string +}{ + {"core/site.go", "Site.restoreSettings", "restoreCustom"}, +} + // forkUiHooks lists the fork's mounts in evcc's ui files var forkUiHooks = []struct { file string @@ -86,6 +96,21 @@ var forkUiHooks = []struct { func referenced(t *testing.T, file, fn string) map[string]bool { t.Helper() + res := make(map[string]bool) + ast.Inspect(funcBody(t, file, fn), func(n ast.Node) bool { + if id, ok := n.(*ast.Ident); ok { + res[id.Name] = true + } + return true + }) + + return res +} + +// funcBody returns the body of a function +func funcBody(t *testing.T, file, fn string) *ast.BlockStmt { + t.Helper() + path := filepath.Join("..", filepath.FromSlash(file)) f, err := parser.ParseFile(token.NewFileSet(), path, nil, 0) require.NoError(t, err) @@ -108,14 +133,7 @@ func referenced(t *testing.T, file, fn string) map[string]bool { continue } - res := make(map[string]bool) - ast.Inspect(fd.Body, func(n ast.Node) bool { - if id, ok := n.(*ast.Ident); ok { - res[id.Name] = true - } - return true - }) - return res + return fd.Body } require.Failf(t, "function not found", "%s in %s", fn, file) @@ -133,6 +151,74 @@ func recvName(e ast.Expr) string { return "" } +// returns reports whether a statement contains a return of its function +func returns(st ast.Stmt) bool { + found := false + + ast.Inspect(st, func(n ast.Node) bool { + switch n.(type) { + case *ast.FuncLit: + return false // returns from its own function + case *ast.ReturnStmt: + found = true + } + return !found + }) + + return found +} + +// isTestingGuard reports whether a statement is "if testing.Testing() {...}" +func isTestingGuard(st ast.Stmt) bool { + ifs, ok := st.(*ast.IfStmt) + if !ok { + return false + } + + found := false + ast.Inspect(ifs.Cond, func(n ast.Node) bool { + if id, ok := n.(*ast.Ident); ok && id.Name == "Testing" { + found = true + } + return !found + }) + + return found +} + +// TestForkDeferHooks fails when a hook that has to run on every way out of an evcc +// function is no deferred call of the function body any more, or an evcc return +// stands before it: restoreSettings returns on the first failing restore step of +// evcc, and nothing of the fork would be loaded (peak shaving off, no circuit, the +// battery held by evcc never handed back). +func TestForkDeferHooks(t *testing.T) { + for _, h := range forkDeferHooks { + t.Run(h.file+" "+h.fn, func(t *testing.T) { + body := funcBody(t, h.file, h.fn) + + at := -1 + for i, st := range body.List { + d, ok := st.(*ast.DeferStmt) + if !ok { + continue + } + if sel, ok := d.Call.Fun.(*ast.SelectorExpr); ok && sel.Sel.Name == h.name { + at = i + break + } + } + require.GreaterOrEqual(t, at, 0, "%s: %s is no deferred call of the function body", h.fn, h.name) + + // only the guard for tests may return before it + for _, st := range body.List[:at] { + if !isTestingGuard(st) { + assert.False(t, returns(st), "%s: a return of evcc stands before the deferred %s", h.fn, h.name) + } + } + }) + } +} + // TestForkHooksInPlace fails when an evcc update or a merge conflict dropped a // hook: evcc's own tests and the fork's tests of its functions would both stay // green, while the function is simply no longer called. diff --git a/core/lm/README.md b/core/lm/README.md index 215aa69cd7..e8488012b6 100644 --- a/core/lm/README.md +++ b/core/lm/README.md @@ -831,7 +831,7 @@ Every change in an evcc file. Check these when merging a new evcc version. | File | Change | | --- | --- | -| `core/site.go` | `custom` field; `meteredPower` in `updateLoadpoints` (an assumed heater power stays in the home consumption); `restoreCustom` in `restoreSettings`; `updateCustom` after `updatePower`; `setPeakGridEnergy` in `updateGridMeter`; `batteryGridChargeRequested` and `updateBatteryModePeakAware` in place of evcc's calls | +| `core/site.go` | `custom` field; `meteredPower` in `updateLoadpoints` (an assumed heater power stays in the home consumption); `restoreCustom` deferred at the start of `restoreSettings` (after the guard for tests, so it also runs when an evcc restore step returns an error); `updateCustom` after `updatePower`; `setPeakGridEnergy` in `updateGridMeter`; `batteryGridChargeRequested` and `updateBatteryModePeakAware` in place of evcc's calls | | `core/site_circuits.go` | `circuitLoads()` instead of `loadpointsAsCircuitDevices()` (adds the battery) | | `core/site_load_predictor.go` | `homeProfileCustom` call in `homeProfile` | | `core/site_optimizer.go` | `optimizerGridTariff` for the grid price, `applyLmOptimizerInputs` where the request is assembled, `lmOptimizerResult` after the solve, `lmForecastLowest` for the forecast, `lmOptimizeLater`/`lmOptimizeAgain` in `optimizerUpdateAsync` | @@ -916,6 +916,11 @@ unused: hook from the table above is still there (Go: in the right function; ui: the mount). A hook lost in a merge fails here, while all other tests would stay green. A new hook gets a line in its table. +- `TestForkDeferHooks`: the hook in `restoreSettings` is a deferred call of the + function body, with no return of evcc before it. A call at the end would be + skipped by the first failing restore step, and the fork would start without its + settings (`restoreSettings` does nothing in tests, so only the syntax tree can + tell). - `TestForkInertWhenUnused`, `TestEqualPrioritiesAreUpstream`, `TestCurrents1pInertWhenUnused`, `TestLmSMaxFromUpstream`: without configuration the fork changes nothing. diff --git a/core/site.go b/core/site.go index 5a9257b625..63a4cf656d 100644 --- a/core/site.go +++ b/core/site.go @@ -452,6 +452,7 @@ func (site *Site) restoreSettings() error { if testing.Testing() { return nil } + defer site.restoreCustom() // custom: runs also when a restore step below fails, see core/site_lm.go if v, err := settings.Float(keys.BufferSoc); err == nil { if err := site.SetBufferSoc(v); err != nil && !errors.Is(err, ErrBatteryNotConfigured) { return err @@ -510,8 +511,6 @@ func (site *Site) restoreSettings() error { site.publish(keys.OptimizerChargingStrategy, site.GetOptimizerChargingStrategy()) site.publish(keys.OptimizerChargingStrategies, optimizerChargingStrategies) - site.restoreCustom() // custom: see core/site_lm.go - // drop legacy accumulator-based forecast settings (now stored via metrics collector) settings.Delete("solarAccForecast") settings.Delete("solarAccYield") diff --git a/core/site_lm.go b/core/site_lm.go index bb363aa87b..175f69e327 100644 --- a/core/site_lm.go +++ b/core/site_lm.go @@ -96,8 +96,10 @@ type siteCustom struct { export exportForecastState // planned grid export as forecast in Home Assistant, see site_lm_export_forecast.go } -// restoreCustom restores the fork's settings, called at the end of upstream's -// restoreSettings +// restoreCustom restores the fork's settings, deferred at the start of upstream's +// restoreSettings: evcc's own restore returns on the first step failing, which +// must not leave the fork without its settings (peak shaving off, the battery held +// by evcc never handed back) func (site *Site) restoreCustom() { site.restoreLmSettings() site.restorePeakSettings() From 8b248c6f88a68b909621d708ce257baa4ca2ef33 Mon Sep 17 00:00:00 2001 From: SolarPower2024 Date: Sun, 11 Oct 2026 22:11:06 +0200 Subject: [PATCH 6/8] Peak: Omnibattery bekommt nach Fensterende nicht mehr den alten erlaubten Wert (Review B20) Das Fenster mit dem erlaubten Wert wird nur in einem Zyklus mit Zaehlerwerten erneuert. Faellt der Zaehler am Ende eines Fensters aus, behielt Omnibattery bis zum Zaehlerausfall (2 min) den im alten Fenster eingefrorenen Wert, bis zum Doppelten des Limits. applyOmniProtection nimmt Allowed jetzt nur, wenn das Fenster das laufende ist, sonst das Peak-Limit wie bei Zaehlerausfall. Test mit Allowed 7000, Uhr ueber der Grenze und ohne Zaehlerwerte (ohne die Aenderung rot). Co-Authored-By: Claude Sonnet 5.5 --- core/lm/README.md | 7 ++++++- core/site_peak_omni.go | 8 ++++++-- core/site_peak_omni_test.go | 28 ++++++++++++++++++++++++++++ 3 files changed, 40 insertions(+), 3 deletions(-) diff --git a/core/lm/README.md b/core/lm/README.md index e8488012b6..2f86a7099e 100644 --- a/core/lm/README.md +++ b/core/lm/README.md @@ -389,7 +389,12 @@ At the end of each cycle (after the battery mode, `updateBatteryModePeakAware`) (`TestOmniProtectionLimitFollowsWindow`). Below the entity's minimum (500 W) the minimum is written, Omnibattery takes no lower limit (`TestOmniProtectionLimitMinimum`). Omnibattery has no window of its own, - this is how evcc passes the 15 minutes on. + this is how evcc passes the 15 minutes on. The allowed power counts for the + running window only: a meter failing at the end of a window leaves the window + of the last cycle behind, and its frozen value (up to twice the limit) would go + on into the next one for the 2 minutes until the meters count as lost; from + the first cycle in the new window the peak limit applies + (`TestOmniWindowEnded`). - **Soc threshold:** the reserve, at least 20 % (the lowest value of the entity), rounded up to the step (`TestOmniProtectionThreshold`), every change written: the write tolerance is in W and does not apply diff --git a/core/site_peak_omni.go b/core/site_peak_omni.go index 33cb06b34e..dbaed095b8 100644 --- a/core/site_peak_omni.go +++ b/core/site_peak_omni.go @@ -33,6 +33,7 @@ import ( "github.com/evcc-io/evcc/api" "github.com/evcc-io/evcc/core/keys" + "github.com/evcc-io/evcc/core/peak" "github.com/evcc-io/evcc/db/settings" "github.com/evcc-io/evcc/util/homeassistant" "github.com/evcc-io/evcc/util/request" @@ -244,9 +245,12 @@ func (site *Site) applyOmniProtection() { owned, sw, limitEntity, socSet := s.protOwned, s.protSwitch, s.protLimit, s.protSocSet threshold := s.effectiveReserve() - // the window is there once a cycle metered it + // the window of the allowed power is the running one: it is renewed by a cycle + // with meter values only, and past its end (a meter failing at the end of the + // window, up to metersLost) it would go on with the allowed power frozen in + // the old window, up to twice the limit limit, basis := s.limit, omniLimitBasis(false, s.limit) - if !s.metersLost && !s.window.Start.IsZero() { + if !s.metersLost && s.window.Start.Equal(s.clock.Now().Truncate(peak.Window)) { limit, basis = s.window.Allowed, omniLimitBasis(true, s.limit) } s.mu.Unlock() diff --git a/core/site_peak_omni_test.go b/core/site_peak_omni_test.go index e460ea146f..44e3f032d6 100644 --- a/core/site_peak_omni_test.go +++ b/core/site_peak_omni_test.go @@ -1409,3 +1409,31 @@ func TestOmniSwitchGone(t *testing.T) { assert.False(t, sc.site.peakOwned()) assert.True(t, sc.site.peak().protOwned, "the peak shaving of Omnibattery is not part of it") } + +// TestOmniWindowEnded verifies that the allowed power is that of the running +// window only: a meter failing at the end of a window leaves the window of the +// last cycle behind, and its allowed power, up to twice the limit, must not go on +// into the next one before the meters count as lost. +func TestOmniWindowEnded(t *testing.T) { + sc, ha := newOmni(t) + clk := mockClock(sc) + + // the last cycle of a window with little drawn: it allows more than the limit + clk.Add(peak.Window - 30*time.Second) + sc.cycle(25, 1000, 0) + ha.take() + + s := sc.site.peak() + s.mu.Lock() + s.window.Allowed = 7000 + s.mu.Unlock() + + sc.site.updateBatteryModePeakAware(false, false, sc.rate) + assert.Equal(t, []string{"set_value " + haProtLimit + " 7000"}, ha.take(), "the running window") + + // the next window, the meters failed: the peak limit, not the old window + clk.Add(40 * time.Second) + sc.site.updateBatteryModePeakAware(false, false, sc.rate) + assert.Equal(t, []string{"set_value " + haProtLimit + " 5000"}, ha.take()) + assert.Equal(t, "5000", ha.get(haProtLimit)) +} From 1216cb43ad361e2c4e290eebe597fd5c95ed41a8 Mon Sep 17 00:00:00 2001 From: SolarPower2024 Date: Sun, 11 Oct 2026 22:20:51 +0200 Subject: [PATCH 7/8] =?UTF-8?q?Review=20P4:=20R=C3=BCckgabe=20beim=20Stopp?= =?UTF-8?q?en=20wird=20vom=20HA-W=C3=A4chter=20still=20verweigert?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Der Wächter lehnt nach einem Timeout alle Aufrufe ab, bis der Zyklus endet. Die Shutdown-Funktion läuft neben dem Zyklus: lief dessen erster HA-Aufruf in einen Timeout, bekamen Ladeleistung, Schalter und Sollwert der Rückgabe errHAHung, und logWrite schrieb das nur auf TRACE. Die Batterie blieb gehalten, ohne Zeile im Log, und das gerade bei langsamem Home Assistant. releasePeakOnShutdown sagt dem Wächter jetzt zuerst, dass evcc stoppt (haStop, baut die Verbindung bei Bedarf): er lehnt dann nichts mehr ab. logWrite prüft das Stoppen vor errHAHung, eine Verweigerung beim Stoppen steht auf INFO. Test mit einem Timeout im laufenden Zyklus für Marstek und BYD, Test für den Wächter einer erst beim Stoppen gebauten Verbindung. Co-Authored-By: Claude Sonnet 5.5 --- core/lm/README.md | 5 ++- core/site_ha_guard.go | 37 ++++++++++++++--- core/site_peak_shutdown.go | 4 ++ core/site_peak_shutdown_test.go | 72 +++++++++++++++++++++++++++++++++ core/site_peakshaving.go | 13 +++--- 5 files changed, 119 insertions(+), 12 deletions(-) diff --git a/core/lm/README.md b/core/lm/README.md index 2f86a7099e..e5ba4efc87 100644 --- a/core/lm/README.md +++ b/core/lm/README.md @@ -273,7 +273,10 @@ the allowed power of the window; a BYD gets the charge power 0 and the free value. The cycle writes nothing any more from then on. It is best effort and never holds the stop up (`TestPeakShutdownDoesNotBlock`): stop budget add-on 30 s (`timeout` in its manifest), evcc waits `max(interval, 5 s)` for the shutdown -functions, the hand back takes 8 s at most. When the host shuts down, the +functions, the hand back takes 8 s at most. The shutdown function runs beside the +cycle; the guard below refuses nothing from then on, so a call that timed out +earlier in the running cycle does not have the hand back refused +(`TestPeakReleasedAfterTimeout`). When the host shuts down, the supervisor refuses the calls (400 or 503 "not ready", logged at info), and Omnibattery keeps its last value until evcc runs again. A crash or a kill is not covered either; there the stored mark hands back with the first cycle after the diff --git a/core/site_ha_guard.go b/core/site_ha_guard.go index a98ff1abdd..956075b761 100644 --- a/core/site_ha_guard.go +++ b/core/site_ha_guard.go @@ -14,7 +14,9 @@ package core // haGuard sits in the transport of the shared connection (see haConnection). After // the first call timing out in a cycle it refuses the further calls with errHAHung // until the cycle ends (haCycleEnd); the first call of the next cycle asks again. -// So a cycle costs one timeout at most. +// So a cycle costs one timeout at most. Once evcc is stopping the guard refuses +// nothing (stop): the hand back of the battery has to try every call, also when a +// call timed out earlier in the cycle still running beside the shutdown function. import ( "context" @@ -35,8 +37,9 @@ type haGuard struct { base http.RoundTripper log *util.Logger - mu sync.Mutex - hung bool // a call timed out in this cycle + mu sync.Mutex + hung bool // a call timed out in this cycle + stopping bool // evcc is stopping, nothing is refused any more } // guardHA puts the guard in the transport of the connection @@ -47,7 +50,7 @@ func guardHA(conn *homeassistant.Connection, log *util.Logger) { // RoundTrip implements http.RoundTripper func (g *haGuard) RoundTrip(req *http.Request) (*http.Response, error) { g.mu.Lock() - hung := g.hung + hung := g.hung && !g.stopping g.mu.Unlock() if hung { @@ -63,7 +66,7 @@ func (g *haGuard) RoundTrip(req *http.Request) (*http.Response, error) { if err != nil && isTimeout(err) { g.mu.Lock() - first := !g.hung + first := !g.hung && !g.stopping g.hung = true g.mu.Unlock() @@ -83,6 +86,15 @@ func (g *haGuard) endCycle() { g.hung = false } +// stop ends the refusals for good: evcc is stopping +func (g *haGuard) stop() { + g.mu.Lock() + defer g.mu.Unlock() + + g.stopping = true + g.hung = false +} + // isTimeout reports whether the call timed out, rather than failed func isTimeout(err error) bool { var ne net.Error @@ -108,3 +120,18 @@ func (site *Site) haCycleEnd() { g.endCycle() } } + +// haStop ends the refusals of the guard for good, see haGuard.stop: called when +// evcc stops, before the hand back. The connection is built if it does not exist +// yet, so the guard is there to be told; without Home Assistant nothing is held +// through it. +func (site *Site) haStop() { + conn, err := site.haConnection() + if err != nil { + return + } + + if g, ok := conn.Client.Transport.(*haGuard); ok { + g.stop() + } +} diff --git a/core/site_peak_shutdown.go b/core/site_peak_shutdown.go index d96600ac07..7bf92a165e 100644 --- a/core/site_peak_shutdown.go +++ b/core/site_peak_shutdown.go @@ -58,6 +58,10 @@ func (site *Site) releasePeakOnShutdown() { s.stopped = true s.mu.Unlock() + // a call that timed out in that cycle would have every call of the hand back + // refused until the cycle ends, see site_ha_guard.go + site.haStop() + done := make(chan struct{}) go func() { diff --git a/core/site_peak_shutdown_test.go b/core/site_peak_shutdown_test.go index af0bec521e..9de933285a 100644 --- a/core/site_peak_shutdown_test.go +++ b/core/site_peak_shutdown_test.go @@ -5,6 +5,8 @@ import ( "testing" "time" + "github.com/evcc-io/evcc/util" + "github.com/evcc-io/evcc/util/homeassistant" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -133,3 +135,73 @@ func TestPeakShutdownDoesNotBlock(t *testing.T) { assert.Empty(t, s.writeErr, "expected while stopping: no error of an output") }) } + +// timedOut lets the first call of a cycle time out, as one to a Home Assistant +// that is slow does, and checks that the guard then refuses the calls. The +// scenario's Home Assistant answers again afterwards. +func timedOut(t *testing.T, sc *scenario) { + t.Helper() + + h := hangHA(sc) + conn := sc.site.peak().conn + + h.hang.Store(true) + _, _, err := numberState(conn, haChargeNum) + require.Error(t, err) + h.hang.Store(false) + + _, _, err = numberState(conn, haChargeNum) + require.ErrorIs(t, err, errHAHung) +} + +// TestPeakReleasedAfterTimeout verifies that the hand back is not refused by the +// guard when evcc stops after a call of the running cycle timed out: the shutdown +// function runs beside the cycle, which ends the refusals only at its end. +func TestPeakReleasedAfterTimeout(t *testing.T) { + t.Run("marstek", func(t *testing.T) { + sc, ha := charging(t) + timedOut(t, sc) + + sc.site.releasePeakOnShutdown() + assert.Equal(t, []string{ + "set_value " + haChargeNum + " 0", + "turn_off " + haSwitch, + }, ha.take()) + assert.False(t, sc.site.peakOwned()) + assert.Equal(t, "off", ha.get(haSwitch)) + }) + + t.Run("byd", func(t *testing.T) { + sc, ha := newOmni(t) + sc.site.lms().socChargeEnabled = true + + s := sc.site.peak() + s.batteryType = batteryTypeBYD + require.NoError(t, sc.site.rebuildPeakSetter()) + require.NoError(t, sc.site.rebuildChargeSetter()) + + require.True(t, sc.cycle(20, 1000, 0)) + require.True(t, sc.site.peakOwned()) + ha.take() + + timedOut(t, sc) + + sc.site.releasePeakOnShutdown() + assert.Equal(t, []string{ + "set_value " + haChargeNum + " 0", + "set_value " + haDischargeNum + " 10000", + }, ha.take()) + assert.False(t, sc.site.peakOwned()) + }) + + t.Run("guard of a connection built while stopping", func(t *testing.T) { + t.Setenv(homeassistant.SupervisorToken, "token") + + site := &Site{log: util.NewLogger("test")} + site.haStop() + + g, ok := site.peak().conn.Client.Transport.(*haGuard) + require.True(t, ok) + assert.True(t, g.stopping) + }) +} diff --git a/core/site_peakshaving.go b/core/site_peakshaving.go index 136fe225c6..0637cc88e8 100644 --- a/core/site_peakshaving.go +++ b/core/site_peakshaving.go @@ -795,15 +795,16 @@ func (site *Site) writeOutput(name string, set func(float64) error, value float6 // a write succeeds again, so a rejected write does not fill the log with one line // per cycle. A call refused by the guard, see haGuard, is not logged. func (site *Site) logWrite(name, what string, err error) bool { - // refused after a call timed out in this cycle, that call logged the cause - if errors.Is(err, errHAHung) { - site.log.TRACE.Printf("%s: %s: %v", name, what, err) + // stopping, e.g. with the host: Home Assistant may refuse the call, expected; + // also a call the guard refused, the hand back would otherwise fail unnoticed + if err != nil && site.peakStopped() { + site.log.INFO.Printf("%s: %s: %s (stopping)", name, what, writeErrorText(err)) return false } - // stopping, e.g. with the host: Home Assistant may refuse the call, expected - if err != nil && site.peakStopped() { - site.log.INFO.Printf("%s: %s: %s (stopping)", name, what, writeErrorText(err)) + // refused after a call timed out in this cycle, that call logged the cause + if errors.Is(err, errHAHung) { + site.log.TRACE.Printf("%s: %s: %v", name, what, err) return false } From 73c70e56aae891b49259c0791ffbd8e0835d32ba Mon Sep 17 00:00:00 2001 From: SolarPower2024 Date: Sun, 11 Oct 2026 22:22:43 +0200 Subject: [PATCH 8/8] =?UTF-8?q?Review=20P4:=20Lese-404=20einer=20number-En?= =?UTF-8?q?tit=C3=A4t=20verhindert=20die=20Freigabe,=20der=20Marstek=20ble?= =?UTF-8?q?ibt=20in=20manueller=20Steuerung?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Seit numberSetter bei jedem Lesefehler einen Fehler zurückgibt (statt blind zu schreiben), scheiterte die Freigabe an einer Ladeleistungs- oder Sollwert-Entität, die Home Assistant nicht kennt (gelöscht, umbenannt): Bei einem Marstek blieb die manuelle Steuerung im Modus Charge an und lud weiter aus dem Netz, in jedem Zyklus, beim Stoppen und bei Typ- oder Entitätswechsel (errHandBack). Bei BYD blockierte der Sollwert den Typwechsel. Das Konzept (D3) sieht vor, dass ein Lese-404 im Freigabepfad als freigegeben gilt, umgesetzt war es nur für Schalter. releaseOutput schreibt wie writeOutput, wertet einen 404 aber als zurückgegeben (WARN im Log). Es gilt für die Ladeleistung in releaseOmni, für releasePeak (Marstek und BYD), den BYD-Zweig von SetPeakShavingBatteryType und die Freigabe des alten Ziels in SetPeakShavingEntity. Jeder andere Fehler hält weiter fest. Die Reihenfolge Ladeleistung 0, dann Schalter aus bleibt: Omnibattery nimmt die Leistung nur in manueller Steuerung an (TestOmniGridChargeEndZeroFails). TestOmniEntityNotLoaded prüft jetzt eine nicht verfügbare statt eine fehlende Entität, der 404-Fall steht in TestOmniReleaseEntityGone. Co-Authored-By: Claude Sonnet 5.5 --- core/lm/README.md | 13 ++-- core/site_peak_omni.go | 30 ++++++++- core/site_peak_omni_test.go | 119 ++++++++++++++++++++++++++++++++---- core/site_peak_shutdown.go | 4 +- core/site_peakshaving.go | 2 +- 5 files changed, 146 insertions(+), 22 deletions(-) diff --git a/core/lm/README.md b/core/lm/README.md index e5ba4efc87..7a9bbdfbba 100644 --- a/core/lm/README.md +++ b/core/lm/README.md @@ -294,10 +294,12 @@ to such an entity with success and does nothing (`TestNumberSetterReadFailure`, `TestOmniNotAvailableKeepsHold`). A release that meets one keeps what evcc holds (`peakShavingOwned`, `peakShavingProtOwned`) and tries again in the next cycle, also while Home Assistant starts and does not know the entities yet -(`TestOmniReleaseHangs`, `TestOmniEntityNotLoaded`). Only a switch that Home -Assistant does not know at all (deleted, renamed; 404) counts as off, as the -release would otherwise never land and block every change of the entities -(`TestOmniSwitchGone`). +(`TestOmniReleaseHangs`, `TestOmniEntityNotLoaded`). Only an entity that Home +Assistant does not know at all (deleted, renamed; 404 when reading it) counts as +handed back in a release, with a warning in the log: a switch as off, the charge +power and the setpoint as written. The release would otherwise never land, keep +the manual control of a Marstek on in the mode Charge and block every change of +the entities (`TestOmniSwitchGone`, `TestOmniReleaseEntityGone`). The limit applies to the clock-aligned 15 minute window: `allowed` = `(limit × 15 min − energy drawn so far) / time left`, so energy left unused @@ -432,7 +434,8 @@ At the end of each cycle (after the battery mode, `updateBatteryModePeakAware`) (`TestOmniGridChargeWithoutManualWarns`). The manual switch is released only if evcc held it (`peakShavingOwned`): the charge power to 0 first, written only where the entity shows another value, then the switch off; if the 0 fails the switch stays on and the next cycle - tries again (`TestOmniGridChargeEndZeroFails`, `TestOmniGridCharge`). A switch + tries again (`TestOmniGridChargeEndZeroFails`, `TestOmniGridCharge`); a charge + power entity Home Assistant does not know counts as written. A switch turned on by hand stays while evcc does not control (`TestOmniLeavesManualAloneWhenNotOwned`). - **Changes:** removing or replacing a switch, removing the limit or the diff --git a/core/site_peak_omni.go b/core/site_peak_omni.go index dbaed095b8..aa034c4fa2 100644 --- a/core/site_peak_omni.go +++ b/core/site_peak_omni.go @@ -391,7 +391,8 @@ func (site *Site) releaseProt() bool { // if evcc switched it on, and reports whether it is released. The charge power // would otherwise stay on its last value; the zero is only written where the // entity shows another value, see numberSetter. A zero that fails keeps the -// switch on, the next cycle tries again. The mode stays: Omnibattery overwrites +// switch on, the next cycle tries again; an entity Home Assistant does not know +// counts as written, see releaseOutput. The mode stays: Omnibattery overwrites // it in automatic operation anyway. Called with s.out held. func (site *Site) releaseOmni() bool { if !site.omniOwned() { @@ -405,7 +406,7 @@ func (site *Site) releaseOmni() bool { manual, chargeSet := s.manualEntity, s.chargeSet s.mu.Unlock() - if chargeSet != nil && !site.writeOutput("grid charge power", chargeSet, 0) { + if chargeSet != nil && !site.releaseOutput("grid charge power", chargeSet, 0) { return false } @@ -449,6 +450,29 @@ func (site *Site) omniSwitchOff(name, entity string) bool { return site.logWrite(name, "switch off", conn.CallSwitchService(entity, false)) } +// releaseOutput writes the value that hands an output back, like writeOutput, and +// reports whether it is handed back. Home Assistant not knowing the entity (404 +// when reading it, deleted or renamed) counts as handed back: nothing is left to +// hand back, and the release would otherwise never land and keep the manual +// control of a Marstek on, or block every change of the entities. Any other +// failure leaves it held, the next cycle tries again. +func (site *Site) releaseOutput(name string, set func(float64) error, value float64) bool { + if set == nil { + return false + } + + return site.writeOutput(name, func(v float64) error { + err := set(v) + + if notFound(err) { + site.log.WARN.Printf("%s: %v, nothing to hand back", name, err) + return nil + } + + return err + }, value) +} + // omniShow publishes the forced mode evcc holds, empty = not controlling func (site *Site) omniShow(mode string) { s := site.peak() @@ -575,7 +599,7 @@ func (site *Site) SetPeakShavingBatteryType(typ string) error { released = omni && prot case site.peakOwned(): - if released = site.writeOutput("peak shaving", set, site.peakFreeValue()); released { + if released = site.releaseOutput("peak shaving", set, site.peakFreeValue()); released { site.setPeakOwned(false) } } diff --git a/core/site_peak_omni_test.go b/core/site_peak_omni_test.go index 44e3f032d6..ab10eb5f2a 100644 --- a/core/site_peak_omni_test.go +++ b/core/site_peak_omni_test.go @@ -1367,16 +1367,12 @@ func TestOmniNotAvailableKeepsHold(t *testing.T) { } } -// TestOmniEntityNotLoaded verifies the start of Home Assistant, which does not -// know the entities yet: a release writes nothing and keeps what evcc holds until -// they are back, as does a missing entity (no panic in between). +// TestOmniEntityNotLoaded verifies the start of Home Assistant, whose entities +// are not available yet: a release writes nothing and keeps what evcc holds until +// they are back. func TestOmniEntityNotLoaded(t *testing.T) { sc, ha := charging(t) - - ha.mu.Lock() - charge := ha.states[haChargeNum] - delete(ha.states, haChargeNum) - ha.mu.Unlock() + ha.set(haChargeNum, "unavailable") for range 2 { assert.False(t, sc.cycle(90, 1000, 0)) @@ -1385,9 +1381,7 @@ func TestOmniEntityNotLoaded(t *testing.T) { assert.Equal(t, "on", ha.get(haSwitch)) } - ha.mu.Lock() - ha.states[haChargeNum] = charge - ha.mu.Unlock() + ha.set(haChargeNum, "4000") assert.False(t, sc.cycle(90, 1000, 0)) assert.Equal(t, []string{"set_value " + haChargeNum + " 0", "turn_off " + haSwitch}, ha.take()) @@ -1410,6 +1404,109 @@ func TestOmniSwitchGone(t *testing.T) { assert.True(t, sc.site.peak().protOwned, "the peak shaving of Omnibattery is not part of it") } +// forget drops the entities from Home Assistant: it answers a read with 404 +func (f *fakeHA) forget(entities ...string) { + f.mu.Lock() + defer f.mu.Unlock() + + for _, e := range entities { + delete(f.states, e) + } +} + +// bydHolding returns a scenario of the battery type BYD that holds the battery +// back through the discharge setpoint +func bydHolding(t *testing.T) (*scenario, *fakeHA) { + t.Helper() + + sc, ha := newOmni(t) + sc.site.lms().socChargeEnabled = true + + s := sc.site.peak() + s.batteryType = batteryTypeBYD + require.NoError(t, sc.site.rebuildPeakSetter()) + require.NoError(t, sc.site.rebuildChargeSetter()) + + require.True(t, sc.cycle(20, 1000, 0)) + require.True(t, sc.site.peakOwned()) + ha.take() + + return sc, ha +} + +// TestOmniReleaseEntityGone verifies that a number entity Home Assistant does not +// know any more (deleted, renamed) counts as handed back, like a switch does: the +// read answered with 404, nothing is left to write. The manual control would +// otherwise stay on in the mode Charge, charging from the grid, in every cycle, on +// stopping and on every change of the entities. +func TestOmniReleaseEntityGone(t *testing.T) { + t.Run("charge power, grid charging ends", func(t *testing.T) { + sc, ha := charging(t) + ha.forget(haChargeNum) + + sc.cycle(90, 1000, 0) + assert.Equal(t, []string{"turn_off " + haSwitch}, ha.take(), "no write to the entity, the manual control off") + assert.Equal(t, "off", ha.get(haSwitch)) + assert.False(t, sc.site.peakOwned()) + }) + + t.Run("charge power, stopping", func(t *testing.T) { + sc, ha := charging(t) + ha.forget(haChargeNum) + + sc.site.releasePeakOnShutdown() + assert.Equal(t, []string{"turn_off " + haSwitch}, ha.take()) + assert.Equal(t, "off", ha.get(haSwitch)) + assert.False(t, sc.site.peakOwned()) + }) + + t.Run("charge power, battery type changed", func(t *testing.T) { + sc, ha := charging(t) + ha.forget(haChargeNum) + + require.NoError(t, sc.site.SetPeakShavingBatteryType(batteryTypeBYD)) + assert.Contains(t, ha.take(), "turn_off "+haSwitch) + assert.Equal(t, "off", ha.get(haSwitch)) + assert.False(t, sc.site.peakOwned()) + assert.False(t, sc.site.peak().protOwned) + }) + + t.Run("charge power, manual control removed", func(t *testing.T) { + sc, ha := charging(t) + ha.forget(haChargeNum) + + require.NoError(t, sc.site.SetPeakShavingManualEntity("")) + assert.Equal(t, []string{"turn_off " + haSwitch}, ha.take()) + assert.False(t, sc.site.peakOwned()) + }) + + t.Run("discharge setpoint, stopping", func(t *testing.T) { + sc, ha := bydHolding(t) + ha.forget(haDischargeNum) + + sc.site.releasePeakOnShutdown() + assert.Equal(t, []string{"set_value " + haChargeNum + " 0"}, ha.take()) + assert.False(t, sc.site.peakOwned()) + }) + + t.Run("discharge setpoint, battery type changed", func(t *testing.T) { + sc, ha := bydHolding(t) + ha.forget(haDischargeNum) + + require.NoError(t, sc.site.SetPeakShavingBatteryType(batteryTypeMarstek)) + assert.Equal(t, batteryTypeMarstek, sc.site.GetPeakShavingBatteryType()) + assert.False(t, sc.site.peakOwned()) + }) + + t.Run("discharge setpoint, entity removed", func(t *testing.T) { + sc, ha := bydHolding(t) + ha.forget(haDischargeNum) + + require.NoError(t, sc.site.SetPeakShavingEntity("")) + assert.False(t, sc.site.peakOwned()) + }) +} + // TestOmniWindowEnded verifies that the allowed power is that of the running // window only: a meter failing at the end of a window leaves the window of the // last cycle behind, and its allowed power, up to twice the limit, must not go on diff --git a/core/site_peak_shutdown.go b/core/site_peak_shutdown.go index 7bf92a165e..2b4569a6fc 100644 --- a/core/site_peak_shutdown.go +++ b/core/site_peak_shutdown.go @@ -107,10 +107,10 @@ func (site *Site) releasePeak() { } if charging && chargeSet != nil { - site.writeOutput("grid charge power", chargeSet, 0) + site.releaseOutput("grid charge power", chargeSet, 0) } - if site.peakOwned() && set != nil && site.writeOutput("peak shaving", set, site.peakFreeValue()) { + if site.peakOwned() && set != nil && site.releaseOutput("peak shaving", set, site.peakFreeValue()) { site.setPeakOwned(false) } } diff --git a/core/site_peakshaving.go b/core/site_peakshaving.go index 0637cc88e8..5c3cfdb34a 100644 --- a/core/site_peakshaving.go +++ b/core/site_peakshaving.go @@ -1163,7 +1163,7 @@ func (site *Site) SetPeakShavingEntity(entity string) error { case err != nil || omni || !site.peakOwned(): default: // the previous target would otherwise keep the last setpoint - if site.writeOutput("peak shaving", previousSet, site.peakFreeValue()) { + if site.releaseOutput("peak shaving", previousSet, site.peakFreeValue()) { site.setPeakOwned(false) } }