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 b853062199..7a9bbdfbba 100644 --- a/core/lm/README.md +++ b/core/lm/README.md @@ -254,15 +254,52 @@ 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 +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. 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 +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 +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) 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 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 @@ -357,7 +394,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 @@ -392,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 @@ -799,7 +842,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` | @@ -839,7 +882,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_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` | @@ -884,6 +927,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_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..956075b761 --- /dev/null +++ b/core/site_ha_guard.go @@ -0,0 +1,137 @@ +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. 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" + "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 + stopping bool // evcc is stopping, nothing is refused any more +} + +// 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.stopping + 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.stopping + 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 +} + +// 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 + + 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() + } +} + +// 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_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_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() diff --git a/core/site_peak_omni.go b/core/site_peak_omni.go index a21504912f..aa034c4fa2 100644 --- a/core/site_peak_omni.go +++ b/core/site_peak_omni.go @@ -25,14 +25,18 @@ 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/core/peak" "github.com/evcc-io/evcc/db/settings" "github.com/evcc-io/evcc/util/homeassistant" + "github.com/evcc-io/evcc/util/request" ) const ( @@ -158,6 +162,10 @@ func (site *Site) applyOmni() { s.out.Lock() defer s.out.Unlock() + if site.peakStopped() { + return + } + site.applyOmniCharge() site.applyOmniProtection() } @@ -237,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() @@ -350,6 +361,7 @@ func (site *Site) setProtOwned(owned bool) { if changed { settings.SetBool(keys.PeakShavingProtOwned, owned) + site.persistHeld() } } @@ -379,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() { @@ -393,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 } @@ -408,20 +421,58 @@ 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 (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) { return false } - if state, _, err := omniRead(conn, entity); err == nil && strings.EqualFold(state, "off") { + 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 + } + + if strings.EqualFold(state, "off") { return true } 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() @@ -437,7 +488,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"` @@ -447,6 +507,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 } @@ -536,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 1fb3eb17e6..ab10eb5f2a 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,7 +147,10 @@ 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] + e, ok := f.states[id] + if !ok || e.state == "unavailable" || e.state == "unknown" { + 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() @@ -1293,3 +1299,238 @@ 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, 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.set(haChargeNum, "unavailable") + + 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.set(haChargeNum, "4000") + + 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") +} + +// 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 +// 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)) +} 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_peak_shutdown.go b/core/site_peak_shutdown.go new file mode 100644 index 0000000000..2b4569a6fc --- /dev/null +++ b/core/site_peak_shutdown.go @@ -0,0 +1,116 @@ +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() + + // 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() { + 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.releaseOutput("grid charge power", chargeSet, 0) + } + + if site.peakOwned() && set != nil && site.releaseOutput("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..9de933285a --- /dev/null +++ b/core/site_peak_shutdown_test.go @@ -0,0 +1,207 @@ +package core + +import ( + "net/http" + "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" +) + +// 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") + }) +} + +// 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 38a027f474..5c3cfdb34a 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 @@ -359,11 +366,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 +390,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 +402,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 @@ -411,7 +424,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) @@ -425,8 +438,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"` @@ -439,6 +458,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 } @@ -634,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() @@ -659,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() @@ -704,6 +740,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 @@ -753,8 +793,21 @@ 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 { + // 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 + } + + // 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 +887,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 +1035,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() @@ -1104,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) } }