From ed36b17a6c2e8c51a200c61740975efd2a266143 Mon Sep 17 00:00:00 2001 From: Fredrik Ahlgren Date: Wed, 5 Aug 2026 08:04:15 +0200 Subject: [PATCH] fix(control): the peak limit is checked against the site's fuse MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SetPeakLimit stored whatever the operator sent. A threshold above the breaker can never be the first thing to bind — every import clamp already stops at fuse minus margin — so the setting read as armed and did nothing. A negative one is worse: the shaving arm treats it as an error to correct and commands discharge to force export, from a knob named for import. Both are refused now, in one place both operator paths share (POST /api/peak_limit and the Home Assistant number), with a message naming the value sent and the ceiling that beat it. The HA bridge stops discarding that error. Zero keeps the meaning dispatch already gives it — correct everything above 0 W of import. It is deliberately not the zero-means-disabled convention PeakImportCeilingW and MaxExportW use: peak shaving is a mode, switched off by leaving it, not by zeroing its threshold. The comparison is against the fuse rather than effectiveImportCeilingW so acceptance does not depend on the order two operator knobs were typed. A site whose fuse is not described gets the sign check only. A config reload that lowers the fuse can strand a limit that was legal when set; that path warns instead of going quiet. Golden corpus: predicted zero movement before running — no dispatch arithmetic changes and the harness writes PeakLimitW directly, bypassing the setter. Replay passes, no fixture touched. Co-Authored-By: Claude Opus 5 --- .../peak-limit-validated-against-fuse.md | 19 +++ go/cmd/ftw/main.go | 22 ++- go/internal/api/api.go | 13 +- go/internal/api/api_peak_limit_test.go | 100 ++++++++++++ go/internal/control/dispatch.go | 101 ++++++++++++ go/internal/control/peak_limit_test.go | 153 ++++++++++++++++++ go/internal/ha/bridge.go | 9 +- 7 files changed, 412 insertions(+), 5 deletions(-) create mode 100644 .changeset/peak-limit-validated-against-fuse.md create mode 100644 go/internal/api/api_peak_limit_test.go create mode 100644 go/internal/control/peak_limit_test.go diff --git a/.changeset/peak-limit-validated-against-fuse.md b/.changeset/peak-limit-validated-against-fuse.md new file mode 100644 index 00000000..1a350fa0 --- /dev/null +++ b/.changeset/peak-limit-validated-against-fuse.md @@ -0,0 +1,19 @@ +--- +"ftw": patch +--- + +The peak-shaving import limit is now checked against the site's fuse. It +used to be stored exactly as sent, from the API and from the Home +Assistant number alike, so a limit above the breaker was accepted and +then never bound: every import clamp already stops at the fuse, so the +operator read their number back from the status page and believed a +tariff peak was defended that nothing was defending. A negative limit was +worse than useless — the shaving arm treats it as an error to correct and +commands the battery to push power out, from a setting named for import. +Both are refused now, with a message naming the value sent and the +ceiling that beat it. A limit of 0 still means what it always meant in +peak shaving, "correct everything above zero import"; peak shaving is +switched off by leaving the mode, not by zeroing its threshold. A site +whose fuse is not described in the config keeps the old behaviour. When a +config reload lowers the fuse under a limit that was legal when it was +set, the log says so. diff --git a/go/cmd/ftw/main.go b/go/cmd/ftw/main.go index c0933a79..d4736a44 100644 --- a/go/cmd/ftw/main.go +++ b/go/cmd/ftw/main.go @@ -807,7 +807,20 @@ func main() { // explicit 0 → disabled. See EffectiveSafetyMarginA. ctrl.SiteFuseSafetyA = newCfg.Fuse.EffectiveSafetyMarginA() ctrl.MaxExportW = newCfg.Site.MaxExportW + // Lowering the fuse can strand a peak limit that was legal when it + // was set. SetPeakLimit refuses to create that state; only a reload + // can arrive at it from the other side, and it is silent — the + // threshold simply stops being the first thing to bind. Say so + // while the operator is still looking at the config they just saved. + // Peak shaving only: in every other mode the threshold is unread, + // so warning about it would be noise. + peakCeilingW, peakDead := ctrl.PeakLimitIsDead() + peakLimitW, peakMode := ctrl.PeakLimitW, ctrl.Mode == control.ModePeakShaving ctrlMu.Unlock() + if peakDead && peakMode { + slog.Warn("peak limit is now above the site's import ceiling and cannot bind", + "peak_limit_w", peakLimitW, "import_ceiling_w", peakCeilingW) + } // Keep the loadpoint controller's per-phase EV fuse clamp in // sync with hot-reloaded fuse params — previously startup-only, @@ -3893,11 +3906,16 @@ func haCallbacks(ctx context.Context, ctrl *control.State, ctrlMu *sync.Mutex, s ctrl.SetGridTarget(w) return st.SaveConfig("grid_target_w", strconv.FormatFloat(w, 'f', 1, 64)) }, + // Same validation as POST /api/peak_limit — an HA number can be + // dragged past the site's fuse just as easily as an API caller can + // post past it, and the two setters must not diverge (#mode-drift + // again, one field down). The returned error reaches the bridge, + // which logs it; HA's own state topic republishes the value FTW + // actually holds, so the operator sees the number snap back. SetPeakLimit: func(w float64) error { ctrlMu.Lock() defer ctrlMu.Unlock() - ctrl.PeakLimitW = w - return nil + return ctrl.SetPeakLimit(w) }, SetEVCharging: func(w float64, active bool) error { ctrlMu.Lock() diff --git a/go/internal/api/api.go b/go/internal/api/api.go index b66f46d7..48cf553c 100644 --- a/go/internal/api/api.go +++ b/go/internal/api/api.go @@ -1412,7 +1412,11 @@ func (s *Server) handleSetTarget(w http.ResponseWriter, r *http.Request) { } // ---- /api/peak_limit ---- - +// +// Peak-shaving mode's import threshold. Validated against the site's +// fuse — a limit above the breaker can never bind, and a negative one +// would order export. See control.State.SetPeakLimit for the rules and +// why 0 stays a real threshold here rather than "disabled". func (s *Server) handleSetPeakLimit(w http.ResponseWriter, r *http.Request) { var req struct { PeakLimitW float64 `json:"peak_limit_w"` @@ -1422,8 +1426,13 @@ func (s *Server) handleSetPeakLimit(w http.ResponseWriter, r *http.Request) { return } s.deps.CtrlMu.Lock() - s.deps.Ctrl.PeakLimitW = req.PeakLimitW + err := s.deps.Ctrl.SetPeakLimit(req.PeakLimitW) s.deps.CtrlMu.Unlock() + if err != nil { + writeJSON(w, 400, map[string]string{"error": err.Error()}) + return + } + slog.Info("peak limit changed", "w", req.PeakLimitW) writeJSON(w, 200, map[string]any{"status": "ok", "peak_limit_w": req.PeakLimitW}) } diff --git a/go/internal/api/api_peak_limit_test.go b/go/internal/api/api_peak_limit_test.go new file mode 100644 index 00000000..7abbb305 --- /dev/null +++ b/go/internal/api/api_peak_limit_test.go @@ -0,0 +1,100 @@ +package api + +import ( + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" + + "github.com/srcfl/ftw/go/internal/control" + "github.com/srcfl/ftw/go/internal/telemetry" +) + +// POST /api/peak_limit is one of the two operator paths into the +// peak-shaving threshold (the other is the Home Assistant number). Both +// share control.State.SetPeakLimit; these tests pin that the endpoint +// actually routes through it and reports a rejection instead of storing +// a limit the fuse guard would make meaningless. + +// 16 A × 230 V × 3 phases = 11040 W, less the 0.5 A margin = 10695 W. +func newPeakLimitServer(t *testing.T) (*Server, *control.State) { + t.Helper() + st := control.NewState(0, 50, "ferroamp") + st.SiteFuseAmps = 16 + st.SiteFuseVoltage = 230 + st.SiteFusePhases = 3 + st.SiteFuseSafetyA = 0.5 + srv := New(&Deps{Ctrl: st, CtrlMu: &sync.Mutex{}, Tel: telemetry.NewStore()}) + return srv, st +} + +func postPeakLimit(t *testing.T, srv *Server, body string) *httptest.ResponseRecorder { + t.Helper() + req := httptest.NewRequest(http.MethodPost, "/api/peak_limit", strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + rr := httptest.NewRecorder() + srv.Handler().ServeHTTP(rr, req) + return rr +} + +func TestPeakLimitEndpointAcceptsValueUnderTheFuse(t *testing.T) { + srv, st := newPeakLimitServer(t) + if rr := postPeakLimit(t, srv, `{"peak_limit_w":7000}`); rr.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body=%s)", rr.Code, rr.Body.String()) + } + if st.PeakLimitW != 7000 { + t.Errorf("PeakLimitW = %.0f, want 7000", st.PeakLimitW) + } +} + +func TestPeakLimitEndpointRejectsValueAboveTheFuse(t *testing.T) { + srv, st := newPeakLimitServer(t) + rr := postPeakLimit(t, srv, `{"peak_limit_w":20000}`) + if rr.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400 (body=%s)", rr.Code, rr.Body.String()) + } + // The caller has to learn why, here and not from dispatch later. + if !strings.Contains(rr.Body.String(), "10695") { + t.Errorf("response must name the ceiling that beat it, got %s", rr.Body.String()) + } + if st.PeakLimitW != 5000 { + t.Errorf("rejected value must not land: PeakLimitW = %.0f, want the untouched 5000", st.PeakLimitW) + } +} + +func TestPeakLimitEndpointRejectsNegative(t *testing.T) { + srv, st := newPeakLimitServer(t) + if rr := postPeakLimit(t, srv, `{"peak_limit_w":-2000}`); rr.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400 (body=%s)", rr.Code, rr.Body.String()) + } + if st.PeakLimitW != 5000 { + t.Errorf("rejected value must not land: PeakLimitW = %.0f, want 5000", st.PeakLimitW) + } +} + +// Zero is a real threshold for peak shaving ("shave everything above +// 0 W"), not the disabled sentinel PeakImportCeilingW uses. The endpoint +// takes it. +func TestPeakLimitEndpointAcceptsZero(t *testing.T) { + srv, st := newPeakLimitServer(t) + if rr := postPeakLimit(t, srv, `{"peak_limit_w":0}`); rr.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body=%s)", rr.Code, rr.Body.String()) + } + if st.PeakLimitW != 0 { + t.Errorf("PeakLimitW = %.0f, want 0", st.PeakLimitW) + } +} + +// A site whose fuse the operator never described keeps the old +// permissive behaviour rather than inheriting an invented breaker. +func TestPeakLimitEndpointWithoutFuseAcceptsAnyNonNegative(t *testing.T) { + st := control.NewState(0, 50, "ferroamp") + srv := New(&Deps{Ctrl: st, CtrlMu: &sync.Mutex{}, Tel: telemetry.NewStore()}) + if rr := postPeakLimit(t, srv, `{"peak_limit_w":20000}`); rr.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body=%s)", rr.Code, rr.Body.String()) + } + if st.PeakLimitW != 20000 { + t.Errorf("PeakLimitW = %.0f, want 20000", st.PeakLimitW) + } +} diff --git a/go/internal/control/dispatch.go b/go/internal/control/dispatch.go index bd702a85..e7454fdb 100644 --- a/go/internal/control/dispatch.go +++ b/go/internal/control/dispatch.go @@ -2,6 +2,7 @@ package control import ( "encoding/json" + "fmt" "log/slog" "math" "time" @@ -996,6 +997,106 @@ func (s *State) SetGridTarget(w float64) { s.PI.Setpoint = w } +// SetPeakLimit applies the peak-shaving import threshold, rejecting the +// values a site can never act on. Both operator paths — POST +// /api/peak_limit and the Home Assistant peak_limit_w number — go +// through here, so the rule has one home; there is no YAML key for it. +// Callers hold the control mutex. +// +// Two rejections, both about a setting that reads as armed and is not: +// +// - Negative. The threshold is the import level above which shaving +// starts correcting. Below zero, dispatch's `gridW > PeakLimitW` +// branch turns a site sitting at zero grid into a positive error and +// commands discharge to force export; the band between the limit and +// zero meanwhile falls into the `gridW < 0` charge arm, so the two +// halves of the same setting disagree. A knob named for an import +// peak must not be able to order export. +// +// - Above the fuse. Every import-side clamp already binds at the fuse +// less its safety margin, so a threshold above that can never be the +// first thing to bind: peak-shaving mode would do nothing the fuse +// guard was not already doing, while the operator reads their number +// back from /api/status and believes the tariff is defended. +// +// Zero is accepted and keeps the meaning dispatch already gives it — +// "correct everything above 0 W of import". This is deliberately NOT the +// zero-means-disabled convention that PeakImportCeilingW and MaxExportW +// use. Peak shaving is a mode: it is switched off by leaving the mode, +// not by zeroing the threshold. Reading 0 as "disabled" here would let a +// site running peak_shaving import without limit, and a wire value that +// means two things is how the Ferroamp pplim=0 lock bit us. +// +// The comparison is against the fuse, not effectiveImportCeilingW, even +// though PeakImportCeilingW can bind lower. The fuse is a property of the +// site; the ceiling is another operator knob that may be set after this +// one, and validating a knob against a knob makes acceptance depend on +// the order the two were typed. A peak limit under the fuse but over a +// tighter tariff ceiling is redundant, not misleading — the tighter +// number is already doing the operator's stated job. +// +// There is no lower bound beyond zero. A threshold under the site's base +// load binds hard rather than silently, the battery covers what it can, +// and the rest shows up as import over the limit — visible, and the +// operator's business. We have no quantified hardware or control risk to +// point at, so we do not clamp it. +// +// A site whose fuse is not described (SiteFuseAmps <= 0, as in test and +// e2e harnesses) gets the negative check only, matching fuseSafetyMarginW +// and perPhaseOverageW: an incomplete fuse description yields no clamp +// rather than an invented one. +func (s *State) SetPeakLimit(w float64) error { + if w < 0 { + return fmt.Errorf("peak_limit_w must be ≥ 0, got %.0f W", w) + } + if ceiling := s.peakLimitCeilingW(); ceiling > 0 && w > ceiling { + return fmt.Errorf( + "peak_limit_w %.0f W is above the site's import ceiling %.0f W "+ + "(fuse %.0f A × %.0f V × %d phases, less a %.1f A safety margin) "+ + "— a peak limit above the fuse can never bind", + w, ceiling, s.SiteFuseAmps, s.SiteFuseVoltage, s.SiteFusePhases, s.SiteFuseSafetyA) + } + s.PeakLimitW = w + return nil +} + +// PeakLimitIsDead reports whether the current peak-shaving threshold sits +// above the site's import ceiling, i.e. can never bind. SetPeakLimit +// refuses to create that state, but a config reload that lowers +// fuse.max_amps can arrive at it from the other direction, under a value +// that was legal when it was set. Returns false when the fuse is not +// described. Caller holds the control mutex. +func (s *State) PeakLimitIsDead() (float64, bool) { + ceiling := s.peakLimitCeilingW() + return ceiling, ceiling > 0 && s.PeakLimitW > ceiling +} + +// peakLimitCeilingW is the highest peak limit that can still be the first +// thing to bind: the fuse less the margin every import clamp already +// keeps. 0 means "the site's fuse is not described", not "no headroom". +func (s *State) peakLimitCeilingW() float64 { + fuseW := s.siteFuseMaxW() + if fuseW <= 0 { + return 0 + } + ceiling := fuseW - s.fuseSafetyMarginW() + if ceiling < 0 { + return 0 + } + return ceiling +} + +// siteFuseMaxW is the aggregate breaker budget in watts, from the same +// three fields the control tick multiplies together. Returns 0 when any +// of them is unset — no invented 230 V / 3 phases here, matching +// fuseSafetyMarginW. +func (s *State) siteFuseMaxW() float64 { + if s == nil || s.SiteFuseAmps <= 0 || s.SiteFuseVoltage <= 0 || s.SiteFusePhases <= 0 { + return 0 + } + return s.SiteFuseAmps * s.SiteFuseVoltage * float64(s.SiteFusePhases) +} + // batteryInfo is internal state read from telemetry per dispatch cycle. type batteryInfo struct { driver string diff --git a/go/internal/control/peak_limit_test.go b/go/internal/control/peak_limit_test.go new file mode 100644 index 00000000..6a37eb6f --- /dev/null +++ b/go/internal/control/peak_limit_test.go @@ -0,0 +1,153 @@ +package control + +import ( + "strings" + "testing" +) + +// SetPeakLimit is the one place the peak-shaving threshold is validated +// against the site's fuse. A 16 A / 230 V / 3-phase site is 11040 W; the +// default 0.5 A safety margin costs 345 W, so the highest limit that can +// still be the first thing to bind is 10695 W. +func fusedState() *State { + st := NewState(0, 50, "ferroamp") + st.SiteFuseAmps = 16 + st.SiteFuseVoltage = 230 + st.SiteFusePhases = 3 + st.SiteFuseSafetyA = 0.5 + return st +} + +func TestSetPeakLimitAcceptsValueUnderTheFuse(t *testing.T) { + st := fusedState() + if err := st.SetPeakLimit(7000); err != nil { + t.Fatalf("7000 W under an 10695 W ceiling must be accepted, got %v", err) + } + if st.PeakLimitW != 7000 { + t.Errorf("PeakLimitW = %.0f, want 7000", st.PeakLimitW) + } +} + +// The whole point of the rule: a limit above the fuse is not merely +// useless, it is a lie the operator reads back from /api/status. +func TestSetPeakLimitRejectsValueAboveTheFuse(t *testing.T) { + st := fusedState() + st.PeakLimitW = 5000 + err := st.SetPeakLimit(20000) + if err == nil { + t.Fatal("20000 W on an 11040 W fuse must be rejected — it can never bind") + } + // The operator has to be able to act on the message: it names the + // value they sent and the ceiling that beat it. + for _, want := range []string{"20000", "10695"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error must name %s, got %q", want, err.Error()) + } + } + if st.PeakLimitW != 5000 { + t.Errorf("a rejected value must not land: PeakLimitW = %.0f, want 5000", st.PeakLimitW) + } +} + +// The safety margin is part of the ceiling, because every import clamp +// already binds at fuse − margin. A limit in the 345 W the margin holds +// back is dead for exactly the same reason. +func TestSetPeakLimitRejectsValueInsideTheSafetyMargin(t *testing.T) { + st := fusedState() + if err := st.SetPeakLimit(10695); err != nil { + t.Fatalf("the ceiling itself must be accepted, got %v", err) + } + if err := st.SetPeakLimit(10800); err == nil { + t.Fatal("10800 W is above fuse−margin (10695 W) and must be rejected") + } +} + +// Zero is a real threshold in peak shaving — "correct everything above +// 0 W of import" — not the zero-means-disabled convention that +// PeakImportCeilingW and MaxExportW use. Dispatch has always read it that +// way; validation must not quietly redefine it. +func TestSetPeakLimitAcceptsZeroAsARealThreshold(t *testing.T) { + st := fusedState() + st.PeakLimitW = 5000 + if err := st.SetPeakLimit(0); err != nil { + t.Fatalf("zero must be accepted, got %v", err) + } + if st.PeakLimitW != 0 { + t.Fatalf("PeakLimitW = %.0f, want 0", st.PeakLimitW) + } + + // And dispatch still shaves against it rather than treating the site + // as unlimited. 3 kW of import over a 0 W limit must produce a target. + store := seedStore(3000, []struct { + name string + currentW, soc float64 + }{ + {"ferroamp", 0, 0.5}, + }) + st.Mode = ModePeakShaving + st.SlewRateW = 100000 + targets := ComputeDispatch(store, st, caps(map[string]float64{"ferroamp": 15200}), 11040) + if len(targets) == 0 { + t.Fatal("peak limit 0 must still shave 3 kW of import, got no targets") + } +} + +// Negative would order export from a knob named for an import peak. +func TestSetPeakLimitRejectsNegative(t *testing.T) { + st := fusedState() + st.PeakLimitW = 5000 + if err := st.SetPeakLimit(-2000); err == nil { + t.Fatal("a negative peak limit must be rejected") + } + if st.PeakLimitW != 5000 { + t.Errorf("a rejected value must not land: PeakLimitW = %.0f, want 5000", st.PeakLimitW) + } +} + +// An undescribed fuse yields no ceiling rather than an invented one — +// the same back-compat rule fuseSafetyMarginW and perPhaseOverageW keep +// for harnesses that wire only some of the fuse fields. +func TestSetPeakLimitWithoutFuseChecksOnlyTheSign(t *testing.T) { + st := NewState(0, 50, "ferroamp") // no SiteFuse* wired + if err := st.SetPeakLimit(99000); err != nil { + t.Fatalf("no fuse described → no ceiling to enforce, got %v", err) + } + if err := st.SetPeakLimit(-1); err == nil { + t.Fatal("the sign check does not depend on the fuse") + } +} + +// An unset limit is whatever NewState chose; validation runs on operator +// input only. A site must never fail to boot over a shaving threshold. +func TestSetPeakLimitLeavesTheDefaultAlone(t *testing.T) { + if got := NewState(0, 50, "ferroamp").PeakLimitW; got != 5000 { + t.Errorf("default PeakLimitW = %.0f, want the unchanged 5000", got) + } +} + +// A config reload that lowers the fuse can strand a limit that was legal +// when it was set. SetPeakLimit cannot see that coming; PeakLimitIsDead +// is what main.go asks after it re-wires the fuse fields. +func TestPeakLimitIsDeadAfterTheFuseShrinks(t *testing.T) { + st := fusedState() + if err := st.SetPeakLimit(9000); err != nil { + t.Fatalf("9000 W under a 10695 W ceiling: %v", err) + } + if _, dead := st.PeakLimitIsDead(); dead { + t.Fatal("9000 W under the ceiling is not dead") + } + + st.SiteFuseAmps = 10 // 6900 W fuse, 6555 W ceiling + ceiling, dead := st.PeakLimitIsDead() + if !dead { + t.Fatal("9000 W against a 6900 W fuse is dead and must be reported") + } + if ceiling != 6555 { + t.Errorf("reported ceiling = %.0f, want 6555", ceiling) + } + + st.SiteFuseAmps = 0 // fuse no longer described + if _, dead := st.PeakLimitIsDead(); dead { + t.Error("no fuse described → nothing to be dead against") + } +} diff --git a/go/internal/ha/bridge.go b/go/internal/ha/bridge.go index c39e6f57..04c288c2 100644 --- a/go/internal/ha/bridge.go +++ b/go/internal/ha/bridge.go @@ -756,7 +756,14 @@ func (b *Bridge) subscribeCommands() { return } if b.cb.SetPeakLimit != nil { - _ = b.cb.SetPeakLimit(f) + // A rejected peak limit (negative, or above the site's fuse) + // must not vanish here — discarding it left the operator with + // an HA slider showing a number FTW never took. Log it the way + // SetMode does; the retained state topic republishes the value + // FTW actually holds. + if err := b.cb.SetPeakLimit(f); err != nil { + slog.Warn("HA set peak limit failed", "w", f, "err", err) + } } }) b.client.Subscribe(b.cmdTopic("ev_charging_w"), 0, func(_ paho.Client, m paho.Message) {