fix(config): a battery soc_max of 0 no longer blocks charging

.changeset/battery-soc-max-zero.md

@@ -0,0 +1,9 @@
+---
+"ftw": patch
+---
+
+A battery whose Max SoC was saved as 0 charges again. Settings used to save a
+blank number field as 0, and since 0.157.1 that 0 was the battery's charge
+ceiling, so charging stopped at any level, manual Charge included. Core now
+treats a battery soc_max of 0 or less as unset and uses the planner's ceiling,
+and Settings saves a cleared number field as unset.

go/cmd/ftw/battery_soc_max_test.go

@@ -64,7 +64,32 @@ func TestSoCMaxConfigPrecedenceAndReload(t *testing.T) {
 	zero := 0.0
 	cfg.Batteries = map[string]config.Battery{"battery": {SoCMax: &zero}}
 	ctrl.DriverLimits = driverLimitsFrom(cfg.ModbusControlDrivers(), cfg.Batteries, cfg.Planner)
-	check(.5, false) // An explicit zero is not an absent limit.
+	check(.5, false) // Validate drops a configured zero; one that bypasses it still blocks charging.
+}
+
+// Settings saved a blank Max SoC as 0. Loading that config must not leave
+// a 0 % ceiling that blocks every charge order, manual Charge included.
+func TestSavedZeroSoCMaxDoesNotBlockCharge(t *testing.T) {
+	cfg := parseBatteryLimitConfig(t, "", "    soc_min: 0\n    soc_max: 0\n    max_charge_w: 0\n    max_discharge_w: 0\n")
+	if cfg.Batteries["battery"].SoCMax != nil {
+		t.Fatalf("soc_max = %v after load, want unset", *cfg.Batteries["battery"].SoCMax)
+	}
+	cfg.Planner = &config.Planner{Enabled: true, SoCMax: .9}
+	ctrl := newControlStateFromConfig(cfg)
+	ctrl.Mode, ctrl.SlewEnabled = control.ModeCharge, false
+	tel := batteryLimitStore(1000)
+	capacities := map[string]float64{"battery": 10000}
+	for _, tc := range []struct {
+		soc      float64
+		charging bool
+	}{{.58, true}, {.9, false}} {
+		soc := tc.soc
+		tel.Update("battery", telemetry.DerBattery, 0, &soc, nil)
+		got := control.ComputeDispatch(tel, ctrl, capacities, 40000)
+		if len(got) != 1 || (got[0].TargetW > 0) != tc.charging {
+			t.Fatalf("SoC %.2f: got %+v, want charging=%v under the planner's 90%%", soc, got, tc.charging)
+		}
+	}
 }
 
 // Dispatch keeps the higher of the battery's and the running planner's

go/internal/config/config.go

@@ -1953,6 +1953,7 @@ func isLegacyDefaultDriverRepository(repo DriverRepositorySource) bool {
 // Validate ensures the config is internally consistent and safe to run with.
 func (c *Config) Validate() error {
 	c.NormalizeUnits()
+	c.dropZeroBatteryCeilings()
 	if err := c.validateSetup(); err != nil {
 		return err
 	}

go/internal/config/soc_bounds.go

@@ -1,5 +1,10 @@
 package config
 
+import (
+	"log/slog"
+	"maps"
+)
+
 // WithBatterySoCBounds returns a copy of drivers in which every driver that has
 // a matching batteries[<name>] entry gets charge_ceil_soc / discharge_floor_soc
 // defaulted into its free-form Config from the battery's soc_max / soc_min.
@@ -51,3 +56,26 @@ func WithBatterySoCBounds(drivers []Driver, batteries map[string]Battery) []Driv
 	}
 	return out
 }
+
+// dropZeroBatteryCeilings treats a battery soc_max of 0 or less as unset.
+// Settings saved a cleared number field as 0, and an older Settings saved
+// every field on the tab. Since soc_max became the dispatch charge ceiling,
+// that 0 blocked all charging, manual Charge included. A battery that must
+// not charge sets max_charge_w: 0.
+func (c *Config) dropZeroBatteryCeilings() {
+	cloned := false
+	for name, b := range c.Batteries {
+		if b.SoCMax == nil || *b.SoCMax > 0 {
+			continue
+		}
+		slog.Warn("config: ignoring battery soc_max of 0 or less; the planner's ceiling applies",
+			"battery", name, "soc_max", *b.SoCMax)
+		// Callers validate shallow copies of the live config; never write
+		// into the map they share.
+		if !cloned {
+			c.Batteries, cloned = maps.Clone(c.Batteries), true
+		}
+		b.SoCMax = nil
+		c.Batteries[name] = b
+	}
+}

go/internal/config/soc_bounds_test.go

@@ -1,6 +1,9 @@
 package config
 
-import "testing"
+import (
+	"encoding/json"
+	"testing"
+)
 
 func f64(v float64) *float64 { return &v }
 
@@ -96,3 +99,67 @@ func TestWithBatterySoCBoundsDoesNotMutateInput(t *testing.T) {
 		t.Errorf("merged copy lost existing key foo: got %v", got)
 	}
 }
+
+// Settings saved a blank Max SoC as 0. Only soc_max loses it: a soc_min of 0
+// and the power limits are real choices.
+func TestDropZeroBatteryCeilings(t *testing.T) {
+	c := &Config{Batteries: map[string]Battery{
+		"blank":    {SoCMin: f64(0), SoCMax: f64(0), MaxChargeW: f64(7500)},
+		"negative": {SoCMax: f64(-0.1)},
+		"set":      {SoCMax: f64(0.9)},
+		"unset":    {SoCMin: f64(0.1)},
+	}}
+
+	c.dropZeroBatteryCeilings()
+
+	for name, b := range c.Batteries {
+		if name != "set" && b.SoCMax != nil {
+			t.Errorf("%s: soc_max = %v, want unset", name, *b.SoCMax)
+		}
+	}
+	if b := c.Batteries["set"]; b.SoCMax == nil || *b.SoCMax != 0.9 {
+		t.Errorf("set: soc_max = %v, want 0.9", b.SoCMax)
+	}
+	if b := c.Batteries["blank"]; b.SoCMin == nil || *b.SoCMin != 0 || b.MaxChargeW == nil || *b.MaxChargeW != 7500 {
+		t.Errorf("blank: other limits changed: %+v", b)
+	}
+	if out := WithBatterySoCBounds([]Driver{{Name: "blank"}}, c.Batteries); out[0].Config["charge_ceil_soc"] != nil {
+		t.Errorf("a dropped soc_max still reached the driver: %v", out[0].Config)
+	}
+}
+
+// A box's settings live in the config database. A zero an older Core stored
+// there must load as unset, and the copy being validated must not write into
+// the map it shares with the live config.
+func TestStoredZeroSoCMaxLoadsAsUnset(t *testing.T) {
+	path, _, st := importedLiveSettings(t)
+	doc, found, err := st.Configuration()
+	if err != nil || !found {
+		t.Fatalf("stored configuration: found=%v err=%v", found, err)
+	}
+	var stored map[string]any
+	if err := json.Unmarshal(doc.Document, &stored); err != nil {
+		t.Fatal(err)
+	}
+	stored["config"].(map[string]any)["batteries"] = map[string]any{
+		"pixii": map[string]any{"soc_min": 0, "soc_max": 0, "max_charge_w": 0, "max_discharge_w": 0},
+	}
+	raw, _ := json.Marshal(stored)
+	if _, err := st.SaveConfiguration(raw, doc.Revision, nil); err != nil {
+		t.Fatal(err)
+	}
+	got, err := Load(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if b := got.Batteries["pixii"]; b.SoCMax != nil || b.SoCMin == nil || b.MaxChargeW == nil {
+		t.Fatalf("loaded %+v, want soc_max unset and the other limits kept", b)
+	}
+
+	live := Config{Batteries: map[string]Battery{"pixii": {SoCMax: f64(0)}}}
+	next := live
+	next.dropZeroBatteryCeilings()
+	if live.Batteries["pixii"].SoCMax == nil || next.Batteries["pixii"].SoCMax != nil {
+		t.Fatalf("live %+v, copy %+v: want only the copy changed", live.Batteries, next.Batteries)
+	}
+}

web/settings-shell.test.mjs

@@ -361,6 +361,21 @@ describe("unchanged settings fields", () => {
     assert.equal(rig.saved().planner.soc_max, 0.95);
   });
 
+  // A blank Max SoC saved as 0 became a 0 % charge ceiling in Core.
+  it("saves a cleared number as unset, not 0", async () => {
+    const rig = await formShell({ site: { name: "Home" }, batteries: { pixii: { soc_max: 0, max_charge_w: 7.5 } } });
+    const number = (path, value, unitScale) => Object.assign(stubElement(), {
+      dataset: unitScale ? { path, unitScale } : { path }, type: "number", defaultValue: value, value });
+    const max = number("batteries.pixii.soc_max", "0");
+    const charge = number("batteries.pixii.max_charge_w", "7.5", "1000");
+    rig.addField(max);
+    rig.addField(charge);
+    max.value = "";
+    charge.value = "";
+    await rig.context.saveConfig();
+    assert.deepEqual(rig.saved().batteries, { pixii: { soc_max: null, max_charge_w: null } });
+  });
+
   // A select shows the first option when the stored value is none of them,
   // and an unchanged save then never stores what the form showed (#1436).
   it("shows a select's default when the stored value is not an option", async () => {

web/settings.js

@@ -362,8 +362,10 @@
       if (fieldValue(input, input.defaultValue) === input.value) return;
       var path = input.dataset.path;
       var val = input.type === "number" ? parseFloat(input.value) : input.value;
-      if (input.type === "number" && isNaN(val)) val = 0;
-      if (input.type === "number" && input.dataset.unitScale) {
+      // A blank number is unset, so Core uses its default. Saving 0 instead
+      // gave a battery a 0 % charge ceiling.
+      if (input.type === "number" && isNaN(val)) val = null;
+      else if (input.type === "number" && input.dataset.unitScale) {
         val = val * parseFloat(input.dataset.unitScale);
       }
       // Preserve a stored password when the user hasn't typed over it.