From 5a3c5a487a52d75778dc7da521a9704e5ace8eeb Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 22 Aug 2026 17:52:40 +0000 Subject: [PATCH 1/3] fix: harden SI doors, HA discovery slugs, and charging identities MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stop NaN/overflow SoC from leaking as percent or Inf at the units, app, and HA doors. Slug MQTT discovery object ids so driver names with spaces or commas are not silently dropped. Store synthetic history SoC as 0–1. Lock leftover-PV vs battery-grid-buy math in a tested sitepower identity without rewriting files owned by open planner/dispatch drafts. Co-authored-by: Fredrik Ahlgren --- .changeset/harden-units-charge-math.md | 5 + go/internal/api/api_test.go | 2 +- go/internal/appproto/ev.go | 4 +- go/internal/appproto/telemetry.go | 4 +- go/internal/appproto/telemetry_test.go | 15 +++ go/internal/devtools/backfill.go | 14 +-- go/internal/devtools/backfill_test.go | 48 ++++++++ go/internal/ha/bridge.go | 70 ++++++++---- go/internal/ha/bridge_test.go | 47 ++++++++ go/internal/loadpoint/battery_boost.go | 4 +- go/internal/loadpoint/battery_boost_test.go | 39 +++++++ go/internal/sitepower/sitepower.go | 96 ++++++++++++++++ go/internal/sitepower/sitepower_test.go | 118 ++++++++++++++++++++ go/internal/state/energy_ledger_test.go | 28 +++++ go/internal/telemetry/telemetry_test.go | 42 +++++++ go/internal/units/units.go | 43 ++++--- go/internal/units/units_test.go | 97 +++++++++++++++- 17 files changed, 629 insertions(+), 47 deletions(-) create mode 100644 .changeset/harden-units-charge-math.md create mode 100644 go/internal/devtools/backfill_test.go create mode 100644 go/internal/sitepower/sitepower.go create mode 100644 go/internal/sitepower/sitepower_test.go diff --git a/.changeset/harden-units-charge-math.md b/.changeset/harden-units-charge-math.md new file mode 100644 index 000000000..e7a3e6346 --- /dev/null +++ b/.changeset/harden-units-charge-math.md @@ -0,0 +1,5 @@ +--- +"ftw": patch +--- + +Harden SI units and charging identities: SoC doors fold NaN/overflow instead of leaking percent or Inf, HA discovery slugs driver names so sensors with spaces or commas are not dropped, and synthetic history stores 0–1 SoC. diff --git a/go/internal/api/api_test.go b/go/internal/api/api_test.go index 574969078..f7e4d6bf7 100644 --- a/go/internal/api/api_test.go +++ b/go/internal/api/api_test.go @@ -459,7 +459,7 @@ func TestLoadResearchDumpExportsHouseLoadWithEVSplit(t *testing.T) { PVW: -500, BatW: 0, LoadW: 1800, // legacy whole-site load: grid - bat - pv - BatSoC: 55, + BatSoC: 0.55, JSON: js, }); err != nil { t.Fatalf("record history: %v", err) diff --git a/go/internal/appproto/ev.go b/go/internal/appproto/ev.go index 2fca81843..9ba5b15da 100644 --- a/go/internal/appproto/ev.go +++ b/go/internal/appproto/ev.go @@ -240,8 +240,8 @@ func (h *Handler) loadpointBoost(cmd Cmd, uptimeMs int64) error { lease := loadpoint.BatteryBoostLease{ StartedAt: now, ExpiresAt: expires, - MinBatterySoC: units.FractionFromLegacyPercent(minSoC), - EVTargetSoC: units.FractionFromLegacyPercent(evTarget), + MinBatterySoC: units.ClampFraction(units.FractionFromLegacyPercent(minSoC)), + EVTargetSoC: units.ClampFraction(units.FractionFromLegacyPercent(evTarget)), } if departureAtMs, ok := argInt(cmd.Args, "departure_at_ms"); ok && departureAtMs > 0 { lease.DepartureAt = time.UnixMilli(departureAtMs) diff --git a/go/internal/appproto/telemetry.go b/go/internal/appproto/telemetry.go index 91bec6256..b84559fda 100644 --- a/go/internal/appproto/telemetry.go +++ b/go/internal/appproto/telemetry.go @@ -3,6 +3,8 @@ package appproto import ( "math" "strconv" + + "github.com/srcfl/ftw/go/internal/units" ) // Source id prefixes are the box's own; only the shape matters to the app. @@ -96,7 +98,7 @@ func fieldValues(snap Snapshot, modes []ModeInfo) map[string]int64 { fidKey(FidLoadW): roundW(snap.LoadW), } if snap.BatterySoCKnown { - f[fidKey(FidBatterySoc)] = int64(math.Round(snap.BatterySoC * 1000)) + f[fidKey(FidBatterySoc)] = units.PermilleFromFraction(snap.BatterySoC) } if snap.EVWKnown { f[fidKey(FidEvW)] = roundW(snap.EVW) diff --git a/go/internal/appproto/telemetry_test.go b/go/internal/appproto/telemetry_test.go index fea1c6fac..3b907f84b 100644 --- a/go/internal/appproto/telemetry_test.go +++ b/go/internal/appproto/telemetry_test.go @@ -1,6 +1,7 @@ package appproto import ( + "math" "testing" "github.com/srcfl/ftw/go/internal/control" @@ -99,6 +100,20 @@ func TestStateOfChargeIsPermilleOnTheWire(t *testing.T) { } } +func TestStateOfChargeNonFiniteIsZeroPermille(t *testing.T) { + h, box, rec, _ := newRig(t) + box.snap.BatterySoC = math.NaN() + box.snap.BatterySoCKnown = true + deliver(t, h, MsgHello, nil, Hello{Proto: ProtoRange{Min: 0, Max: ProtoMax}}) + rec.reset() + deliver(t, h, MsgSub, nil, Sub{Bucket: 512, Hz: 1}) + + snap := body[Snap](t, rec.only(t, MsgSnap)) + if got := snap.Fields[fidKey(FidBatterySoc)]; got != 0 { + t.Fatalf("NaN battery_soc = %d, want 0 permille", got) + } +} + // Silence would itself be information: it would tell the relay operator that // nothing happened in the house that second. func TestNothingChangedStillSendsATick(t *testing.T) { diff --git a/go/internal/devtools/backfill.go b/go/internal/devtools/backfill.go index 21fdf4a4b..4fc95c998 100644 --- a/go/internal/devtools/backfill.go +++ b/go/internal/devtools/backfill.go @@ -99,7 +99,7 @@ func Backfill(s *state.Store, cfg BackfillConfig, log *slog.Logger) error { const capWh = 15000.0 const batchSize = 5000 - soc := 50.0 + soc := 0.50 written := 0 batch := make([]state.HistoryPoint, 0, batchSize) reportEvery := total / 10 @@ -161,20 +161,20 @@ func Backfill(s *state.Store, cfg BackfillConfig, log *slog.Logger) error { // Battery cascade with small efficiency jitter. surplus := pvGen - load batW := 0.0 - if surplus > 0 && soc < 95 { + if surplus > 0 && soc < 0.95 { batW = math.Min(surplus*(0.75+0.1*rng.Float64()), 5000) - } else if surplus < 0 && soc > 15 { + } else if surplus < 0 && soc > 0.15 { batW = math.Max(surplus*(0.65+0.1*rng.Float64()), -4000) } // Grid residual + measurement noise. gridW := load - pvGen + batW + 30*rng.NormFloat64() - // SoC update — Wh = W * dtH. + // SoC is a 0–1 fraction in history, matching production writes. dtH := step.Seconds() / 3600.0 - soc += batW * dtH / capWh * 100 - if soc > 100 { - soc = 100 + soc += batW * dtH / capWh + if soc > 1 { + soc = 1 } if soc < 0 { soc = 0 diff --git a/go/internal/devtools/backfill_test.go b/go/internal/devtools/backfill_test.go new file mode 100644 index 000000000..a50ae1223 --- /dev/null +++ b/go/internal/devtools/backfill_test.go @@ -0,0 +1,48 @@ +package devtools + +import ( + "log/slog" + "os" + "path/filepath" + "testing" + "time" + + "github.com/srcfl/ftw/go/internal/state" + "github.com/srcfl/ftw/go/internal/units" +) + +func TestBackfillBatSoCIsFraction(t *testing.T) { + path := filepath.Join(t.TempDir(), "state.db") + s, err := state.Open(path) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { s.Close() }) + + log := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError})) + if err := Backfill(s, BackfillConfig{Days: 1, Step: 15 * time.Minute, Seed: 42}, log); err != nil { + t.Fatal(err) + } + rows, err := s.LoadHistory(0, time.Now().UnixMilli()+1, 0) + if err != nil { + t.Fatal(err) + } + if len(rows) < 50 { + t.Fatalf("backfill wrote %d rows, want dozens", len(rows)) + } + sawMid := false + for _, p := range rows { + if !units.ValidFraction(p.BatSoC) { + t.Fatalf("BatSoC %v is not a 0–1 fraction (history used to store 0–100 percent)", p.BatSoC) + } + if p.BatSoC > 0.2 && p.BatSoC < 0.8 { + sawMid = true + } + if p.BatSoC > 2 { + t.Fatalf("BatSoC %v looks like percent, not fraction", p.BatSoC) + } + } + if !sawMid { + t.Fatal("expected some samples in (0.2, 0.8); pack stuck at empty/full suggests the integrator used percent") + } +} diff --git a/go/internal/ha/bridge.go b/go/internal/ha/bridge.go index 8dde9a0aa..896ab9190 100644 --- a/go/internal/ha/bridge.go +++ b/go/internal/ha/bridge.go @@ -26,6 +26,7 @@ import ( "github.com/srcfl/ftw/go/internal/control" "github.com/srcfl/ftw/go/internal/mdnsresolve" "github.com/srcfl/ftw/go/internal/telemetry" + "github.com/srcfl/ftw/go/internal/units" ) // CommandCallbacks is how the bridge hands received commands back to the @@ -365,7 +366,41 @@ func (b *Bridge) availTopic() string { return b.topicPrefix + "/statu func (b *Bridge) stateTopic(name string) string { return b.topicPrefix + "/state/" + name } func (b *Bridge) cmdTopic(name string) string { return b.topicPrefix + "/cmd/" + name } func (b *Bridge) driverTopic(driver, field string) string { - return fmt.Sprintf("%s/driver/%s/%s", b.topicPrefix, driver, field) + return fmt.Sprintf("%s/driver/%s/%s", b.topicPrefix, mqttObjectID(driver), mqttObjectID(field)) +} + +// mqttObjectID slugs a free-form YAML driver name into the character set +// Home Assistant allows in discovery object ids: [a-zA-Z0-9_-]. +// "Laddare, Garage" becomes laddare_garage so EV sensors are not silently +// dropped. The human-readable name stays in the discovery `name` field. +func mqttObjectID(name string) string { + var b strings.Builder + lastSep := true + for _, r := range strings.ToLower(name) { + ok := (r >= 'a' && r <= 'z') || (r >= '0' && r <= '9') || r == '-' || r == '_' + if ok { + b.WriteRune(r) + lastSep = r == '_' || r == '-' + continue + } + if !lastSep { + b.WriteByte('_') + lastSep = true + } + } + s := strings.Trim(b.String(), "_-") + if s == "" { + return "driver" + } + return s +} + +func (b *Bridge) driverUniqueID(driver, suffix string) string { + return b.deviceID + "_" + mqttObjectID(driver) + suffix +} + +func (b *Bridge) driverDiscoveryTopic(kind, driver, object string) string { + return fmt.Sprintf("%s/%s/%s/%s_%s/config", b.discoPrefix, kind, b.deviceID, mqttObjectID(driver), mqttObjectID(object)) } // ---- Autodiscovery ---- @@ -617,22 +652,21 @@ func (b *Bridge) publishDiscovery() { } { msg := b.withAvail(map[string]any{ "name": name + s.label, - "unique_id": b.deviceID + "_" + name + s.id, + "unique_id": b.driverUniqueID(name, s.id), "state_topic": b.driverTopic(name, s.id), "unit_of_measurement": s.unit, "device_class": s.class, "device": dev, }) d, _ := json.Marshal(msg) - topic := fmt.Sprintf("%s/sensor/%s/%s_%s/config", b.discoPrefix, b.deviceID, name, s.id) - b.publish(topic, d, true) + b.publish(b.driverDiscoveryTopic("sensor", name, s.id), d, true) } total += 6 // Health binary_sensor: online/offline with JSON attributes for rich diagnostics. healthMsg := b.withAvail(map[string]any{ "name": name + " Online", - "unique_id": b.deviceID + "_" + name + "_online", + "unique_id": b.driverUniqueID(name, "_online"), "state_topic": b.driverTopic(name, "online"), "json_attributes_topic": b.driverTopic(name, "health_json"), "payload_on": "true", @@ -642,7 +676,7 @@ func (b *Bridge) publishDiscovery() { "device": dev, }) d, _ := json.Marshal(healthMsg) - b.publish(fmt.Sprintf("%s/binary_sensor/%s/%s_online/config", b.discoPrefix, b.deviceID, name), d, true) + b.publish(b.driverDiscoveryTopic("binary_sensor", name, "online"), d, true) total++ // Re-announce every live emit_metric sensor. Reading the snapshots here @@ -676,10 +710,9 @@ func (b *Bridge) publishDiscovery() { // Called when a metric is seen for the first time and on every reconnect. func (b *Bridge) announceMetric(dev map[string]any, driver, metricName, explicitUnit string) { unit, devClass := metricUnitAndClass(metricName, explicitUnit) - uid := b.deviceID + "_" + driver + "_" + metricName msg := b.withAvail(map[string]any{ "name": driver + " " + strings.ReplaceAll(metricName, "_", " "), - "unique_id": uid, + "unique_id": b.driverUniqueID(driver, "_"+metricName), "state_topic": b.driverTopic(driver, metricName), "device": dev, }) @@ -690,8 +723,7 @@ func (b *Bridge) announceMetric(dev map[string]any, driver, metricName, explicit msg["device_class"] = devClass } d, _ := json.Marshal(msg) - topic := fmt.Sprintf("%s/sensor/%s/%s_%s/config", b.discoPrefix, b.deviceID, driver, metricName) - b.publish(topic, d, true) + b.publish(b.driverDiscoveryTopic("sensor", driver, metricName), d, true) } // metricUnitAndClass preserves a unit explicitly emitted by the driver and @@ -867,7 +899,7 @@ func (b *Bridge) publishState() { b.publishValue("ev_w", evW) b.publishValue("v2x_w", v2xW) b.publishValue("load_w", loadW) - b.publishValue("bat_soc_pct", avgSoC*100) + b.publishValue("bat_soc_pct", units.PercentFromFraction(avgSoC)) b.publishValue("grid_target_w", gridTarget) b.publishValue("peak_limit_w", peakLimit) b.publishValue("peak_import_ceiling_w", peakCeiling) @@ -941,13 +973,13 @@ func (b *Bridge) publishState() { if r := b.tel.Get(name, telemetry.DerBattery); r != nil { b.publishDriver(name, "bat_w", r.SmoothedW) if r.SoC != nil { - b.publishDriver(name, "bat_soc_pct", *r.SoC*100) + b.publishDriver(name, "bat_soc_pct", units.PercentFromFraction(*r.SoC)) } } if r := b.tel.Get(name, telemetry.DerV2X); r != nil { b.publishDriver(name, "v2x_w", r.SmoothedW) if r.SoC != nil { - b.publishDriver(name, "v2x_vehicle_soc_pct", *r.SoC*100) + b.publishDriver(name, "v2x_vehicle_soc_pct", units.PercentFromFraction(*r.SoC)) } } @@ -1033,13 +1065,13 @@ func (b *Bridge) publishState() { b.announceVehicleDriver(dev, name) } if r.SoC != nil { - b.publishDriver(name, "vehicle_soc_pct", *r.SoC*100) + b.publishDriver(name, "vehicle_soc_pct", units.PercentFromFraction(*r.SoC)) } attrs := map[string]any{ "updated_at": r.UpdatedAt.UTC().Format(time.RFC3339), } if r.SoC != nil { - attrs["soc_pct"] = *r.SoC * 100 + attrs["soc_pct"] = units.PercentFromFraction(*r.SoC) } if len(r.Data) > 0 { var extra map[string]any @@ -1059,7 +1091,7 @@ func (b *Bridge) publishState() { func (b *Bridge) announceEVDriver(dev map[string]any, driver string) { msg := b.withAvail(map[string]any{ "name": driver + " EV Power", - "unique_id": b.deviceID + "_" + driver + "_ev_w", + "unique_id": b.driverUniqueID(driver, "_ev_w"), "state_topic": b.driverTopic(driver, "ev_w"), "json_attributes_topic": b.driverTopic(driver, "ev_json"), "unit_of_measurement": "W", @@ -1068,14 +1100,14 @@ func (b *Bridge) announceEVDriver(dev map[string]any, driver string) { "device": dev, }) d, _ := json.Marshal(msg) - b.publish(fmt.Sprintf("%s/sensor/%s/%s_ev_w/config", b.discoPrefix, b.deviceID, driver), d, true) + b.publish(b.driverDiscoveryTopic("sensor", driver, "ev_w"), d, true) } // announceVehicleDriver publishes HA discovery for a vehicle SoC reader. func (b *Bridge) announceVehicleDriver(dev map[string]any, driver string) { msg := b.withAvail(map[string]any{ "name": driver + " Vehicle SoC", - "unique_id": b.deviceID + "_" + driver + "_vehicle_soc", + "unique_id": b.driverUniqueID(driver, "_vehicle_soc"), "state_topic": b.driverTopic(driver, "vehicle_soc_pct"), "json_attributes_topic": b.driverTopic(driver, "vehicle_json"), "unit_of_measurement": "%", @@ -1084,7 +1116,7 @@ func (b *Bridge) announceVehicleDriver(dev map[string]any, driver string) { "device": dev, }) d, _ := json.Marshal(msg) - b.publish(fmt.Sprintf("%s/sensor/%s/%s_vehicle_soc/config", b.discoPrefix, b.deviceID, driver), d, true) + b.publish(b.driverDiscoveryTopic("sensor", driver, "vehicle_soc"), d, true) } // publishPlan reads the current MPC plan and publishes: diff --git a/go/internal/ha/bridge_test.go b/go/internal/ha/bridge_test.go index d8d4fd56a..2897f8ccc 100644 --- a/go/internal/ha/bridge_test.go +++ b/go/internal/ha/bridge_test.go @@ -231,3 +231,50 @@ func TestStopAfterFailedConnectDoesNotDeadlock(t *testing.T) { t.Fatal("Stop blocked after a failed Connect — teardown is stuck on <-doneCh; the connectAndStart rollback regressed") } } + +func TestMQTTObjectIDSlugsIllegalDriverNames(t *testing.T) { + cases := []struct{ in, want string }{ + {"easee", "easee"}, + {"Laddare, Garage", "laddare_garage"}, + {"Värmepump, källaren", "v_rmepump_k_llaren"}, + {" foo bar ", "foo_bar"}, + {"???", "driver"}, + {"", "driver"}, + {"Ev-Charger_1", "ev-charger_1"}, + } + for _, c := range cases { + if got := mqttObjectID(c.in); got != c.want { + t.Errorf("mqttObjectID(%q) = %q, want %q", c.in, got, c.want) + } + } +} + +func TestDiscoveryTopicsRejectIllegalCharacters(t *testing.T) { + b := &Bridge{deviceID: "forty_two_watts", discoPrefix: "homeassistant", topicPrefix: "ftw"} + topic := b.driverDiscoveryTopic("sensor", "Laddare, Garage", "ev_current_a") + want := "homeassistant/sensor/forty_two_watts/laddare_garage_ev_current_a/config" + if topic != want { + t.Fatalf("discovery topic = %q, want %q", topic, want) + } + for _, r := range topic { + if r == ',' || r == ' ' { + t.Fatalf("discovery topic still has illegal %q: %s", r, topic) + } + } + if got := b.driverTopic("Laddare, Garage", "ev_current_a"); got != "ftw/driver/laddare_garage/ev_current_a" { + t.Fatalf("state topic = %q", got) + } + if got := b.driverUniqueID("easee", "_ev_w"); got != "forty_two_watts_easee_ev_w" { + t.Fatalf("legal-name unique_id changed: %q", got) + } +} + +func TestPercentFromFractionUsedForSoCDoor(t *testing.T) { + // Guards the HA *100 door against a NaN/overflow regression in units. + if unit, class := metricUnitAndClass("bat_soc_pct", ""); unit != "%" || class != "battery" { + t.Fatalf("bat_soc_pct class = (%q, %q)", unit, class) + } + if unit, class := metricUnitAndClass("pack_soc", ""); unit != "%" || class != "battery" { + t.Fatalf("_soc suffix class = (%q, %q)", unit, class) + } +} diff --git a/go/internal/loadpoint/battery_boost.go b/go/internal/loadpoint/battery_boost.go index 10a274df8..a9d6c68cc 100644 --- a/go/internal/loadpoint/battery_boost.go +++ b/go/internal/loadpoint/battery_boost.go @@ -64,8 +64,8 @@ func (l *BatteryBoostLease) UnmarshalJSON(b []byte) error { if l.EVTargetSoC == 0 && aux.EVTargetSoCPct != nil { l.EVTargetSoC = *aux.EVTargetSoCPct } - l.MinBatterySoC = units.FractionFromLegacyPercent(l.MinBatterySoC) - l.EVTargetSoC = units.FractionFromLegacyPercent(l.EVTargetSoC) + l.MinBatterySoC = units.ClampFraction(units.FractionFromLegacyPercent(l.MinBatterySoC)) + l.EVTargetSoC = units.ClampFraction(units.FractionFromLegacyPercent(l.EVTargetSoC)) return nil } diff --git a/go/internal/loadpoint/battery_boost_test.go b/go/internal/loadpoint/battery_boost_test.go index 1fc6085bd..2f993400e 100644 --- a/go/internal/loadpoint/battery_boost_test.go +++ b/go/internal/loadpoint/battery_boost_test.go @@ -2,6 +2,7 @@ package loadpoint import ( "context" + "encoding/json" "testing" "time" ) @@ -259,3 +260,41 @@ func TestActiveBatteryBoostTotalsArePerLoadpointAndUseStrictestReserve(t *testin t.Fatalf("totals = %.0f W, %.2f reserve; want 5000 W, 0.35", power, reserve) } } + +func TestBatteryBoostLeaseJSONRoundTripIsFraction(t *testing.T) { + now := time.Date(2026, 8, 22, 12, 0, 0, 0, time.UTC) + lease := BatteryBoostLease{ + StartedAt: now, ExpiresAt: now.Add(time.Hour), + MinBatterySoC: 0.30, EVTargetSoC: 0.80, + } + raw, err := json.Marshal(lease) + if err != nil { + t.Fatal(err) + } + var m map[string]any + if err := json.Unmarshal(raw, &m); err != nil { + t.Fatal(err) + } + if _, ok := m["min_battery_soc_pct"]; ok { + t.Fatalf("lease JSON still emits percent key: %s", raw) + } + if m["min_battery_soc"] != 0.30 { + t.Fatalf("min_battery_soc = %v, want 0.30", m["min_battery_soc"]) + } + + var back BatteryBoostLease + if err := json.Unmarshal([]byte(`{"started_at":"2026-08-22T12:00:00Z","expires_at":"2026-08-22T13:00:00Z","min_battery_soc_pct":30,"ev_target_soc_pct":80}`), &back); err != nil { + t.Fatal(err) + } + if back.MinBatterySoC != 0.30 || back.EVTargetSoC != 0.80 { + t.Fatalf("legacy percent hydrate = %+v", back) + } + + var overflow BatteryBoostLease + if err := json.Unmarshal([]byte(`{"started_at":"2026-08-22T12:00:00Z","expires_at":"2026-08-22T13:00:00Z","min_battery_soc":1.02}`), &overflow); err != nil { + t.Fatal(err) + } + if overflow.MinBatterySoC != 1 { + t.Fatalf("1.02 overflow = %v, want 1 (not 0.0102)", overflow.MinBatterySoC) + } +} diff --git a/go/internal/sitepower/sitepower.go b/go/internal/sitepower/sitepower.go new file mode 100644 index 000000000..41dba74a3 --- /dev/null +++ b/go/internal/sitepower/sitepower.go @@ -0,0 +1,96 @@ +// Package sitepower is the site-boundary power identity used by charging +// policy tests and (later) by planner/dispatch. +// +// Core stores watts in the site sign convention: positive into the site. +// The meter identity with every DER named separately is +// +// grid = load + pv + battery + ev + v2x +// +// House leftover PV is max(0, −(load+pv)). A surplus-only EV may take +// leftover, never house-battery discharge, and never grid import that is +// not leftover. Battery soak of leftover is not the same as the battery +// buying from the grid: soak reduces what the EV may take; a battery +// charge above leftover is a grid buy and leaves leftover available to +// the EV (the #957 leftover-PV-beside-battery-grid-charge case). +package sitepower + +import "math" + +// GridChargeImportW is the live-meter deadband used to tell "the site is +// importing" from noise. Matches the existing 50–100 W bands in MPC/control. +const GridChargeImportW = 100.0 + +// GridW is the five-term site identity. PV is ≤0, EV charging is ≥0, +// battery charge is >0, battery discharge is <0. +func GridW(loadW, pvW, batteryW, evW, v2xW float64) float64 { + return loadW + pvW + batteryW + evW + v2xW +} + +// HouseLeftoverW is PV remaining after covering house load, in watts. +// Zero when the house is in deficit. PV must be site-signed (negative). +func HouseLeftoverW(loadW, pvW float64) float64 { + if math.IsNaN(loadW) || math.IsInf(loadW, 0) || math.IsNaN(pvW) || math.IsInf(pvW, 0) { + return 0 + } + v := -(loadW + pvW) + if v < 0 { + return 0 + } + return v +} + +// SurplusAvailableForEVW is leftover PV the surplus-only EV may take this +// tick after the house battery's own soak. +// +// leftover leftover, battery charging ≤ leftover → leftover − soak +// battery charging > leftover → leftover (battery is buying grid) +// battery idle or discharging → leftover (discharge into the car is a +// separate no-battery-to-EV check) +func SurplusAvailableForEVW(loadW, pvW, batteryW float64) float64 { + leftover := HouseLeftoverW(loadW, pvW) + if leftover <= 0 { + return 0 + } + if math.IsNaN(batteryW) || math.IsInf(batteryW, 0) { + return 0 + } + if batteryW > leftover { + return leftover + } + if batteryW > 0 { + return leftover - batteryW + } + return leftover +} + +// SurplusOnlyEVExceedsLeftover reports that evW cannot be explained as +// leftover PV. Use this, not "gridW > 50", to decide whether a surplus-only +// EV imported. Grid import during a battery grid-buy plus leftover EV is +// legal; grid import from soak+EV oversubscription is not. +func SurplusOnlyEVExceedsLeftover(loadW, pvW, batteryW, evW float64) bool { + if math.IsNaN(evW) || math.IsInf(evW, 0) || evW <= 0 { + return false + } + return evW > SurplusAvailableForEVW(loadW, pvW, batteryW)+GridChargeImportW +} + +// BatteryFeedsEV reports conservation: some of a discharging house battery +// must have landed in the EV (or on the grid — callers that already forbid +// battery export can treat this as battery-to-EV). +func BatteryFeedsEV(loadW, pvW, batteryW, evW float64) bool { + if batteryW >= -GridChargeImportW || evW <= 0 { + return false + } + // House residual after PV: positive means the house still needs energy. + houseNeed := loadW + pvW + if houseNeed < 0 { + houseNeed = 0 + } + discharge := -batteryW + return discharge > houseNeed+GridChargeImportW +} + +// Finite reports a usable watt value. +func Finite(v float64) bool { + return !math.IsNaN(v) && !math.IsInf(v, 0) +} diff --git a/go/internal/sitepower/sitepower_test.go b/go/internal/sitepower/sitepower_test.go new file mode 100644 index 000000000..35da6601e --- /dev/null +++ b/go/internal/sitepower/sitepower_test.go @@ -0,0 +1,118 @@ +package sitepower + +import ( + "math" + "testing" +) + +func TestGridWFiveTermIdentity(t *testing.T) { + // load 500, PV −8000, battery soak 5000, EV 4140 → grid 1640. + got := GridW(500, -8000, 5000, 4140, 0) + if got != 1640 { + t.Fatalf("grid = %v, want 1640", got) + } + // True combo: leftover 7500, battery grid-buy 10 kW, EV 4140 → 6640. + got = GridW(500, -8000, 10000, 4140, 0) + if got != 6640 { + t.Fatalf("combo grid = %v, want 6640", got) + } +} + +func TestHouseLeftoverW(t *testing.T) { + if got := HouseLeftoverW(500, -8000); got != 7500 { + t.Fatalf("leftover = %v, want 7500", got) + } + if got := HouseLeftoverW(4000, -1000); got != 0 { + t.Fatalf("deficit leftover = %v, want 0", got) + } + if got := HouseLeftoverW(math.NaN(), -8000); got != 0 { + t.Fatalf("NaN load leftover = %v, want 0", got) + } + if got := HouseLeftoverW(500, math.Inf(-1)); got != 0 { + t.Fatalf("-Inf PV leftover = %v, want 0", got) + } +} + +func TestSurplusAvailableForEVWSoakVersusGridBuy(t *testing.T) { + // Soak: leftover 7500, battery 5000 → EV may take 2500, not 7500. + // Offering 7500 is the #957 live-formula leak (EV+soak cause import, + // then "meter importing" is misread as a deliberate battery grid-buy). + if got := SurplusAvailableForEVW(500, -8000, 5000); got != 2500 { + t.Fatalf("soak leftover for EV = %v, want 2500", got) + } + // Grid-buy: battery 10 kW > leftover 7500 → leftover stays available. + if got := SurplusAvailableForEVW(500, -8000, 10000); got != 7500 { + t.Fatalf("grid-buy leftover for EV = %v, want 7500", got) + } + // Idle battery. + if got := SurplusAvailableForEVW(500, -8000, 0); got != 7500 { + t.Fatalf("idle leftover for EV = %v, want 7500", got) + } + // Discharging battery does not create leftover (no-battery-to-EV). + if got := SurplusAvailableForEVW(500, -8000, -2000); got != 7500 { + t.Fatalf("discharge leftover for EV = %v, want 7500 (discharge is a separate check)", got) + } + if got := SurplusAvailableForEVW(4000, -1000, 0); got != 0 { + t.Fatalf("no leftover → %v", got) + } +} + +func TestSurplusOnlyEVExceedsLeftover(t *testing.T) { + // Soak + EV 4140 with only 2500 leftover after soak → import, forbidden. + if !SurplusOnlyEVExceedsLeftover(500, -8000, 5000, 4140) { + t.Fatal("soak+EV 4140 must count as surplus-only import") + } + // Same leftover, EV 2000 fits after 5000 soak. + if SurplusOnlyEVExceedsLeftover(500, -8000, 5000, 2000) { + t.Fatal("soak+EV 2000 is leftover, not import") + } + // Combo: battery buys grid, EV takes leftover 4140 of 7500 — legal even + // though the meter imports ~6640. The gridW>50 predicate would reject this. + if SurplusOnlyEVExceedsLeftover(500, -8000, 10000, 4140) { + t.Fatal("leftover EV beside battery grid-charge must be allowed") + } + if SurplusOnlyEVExceedsLeftover(500, -8000, 10000, 0) { + t.Fatal("idle EV is not an exceed") + } +} + +func TestGridSignIsNotSurplusOnlyImport(t *testing.T) { + // Characterises the broken DP predicate (evW>0 && gridW>50). A legal + // leftover+grid-buy combo has grid 6640 and ev 4140; using the meter + // sign would reject it. Keep this next to the identity so a planner + // that copies the old predicate fails this file, not a live site. + load, pv, bat, ev := 500.0, -8000.0, 10000.0, 4140.0 + grid := GridW(load, pv, bat, ev, 0) + legacyRejects := ev > 0 && grid > 50 + if !legacyRejects { + t.Fatal("legacy grid-sign predicate should reject the combo (that's the bug)") + } + if SurplusOnlyEVExceedsLeftover(load, pv, bat, ev) { + t.Fatal("identity must allow the combo the grid-sign predicate rejects") + } +} + +func TestBatteryFeedsEV(t *testing.T) { + // House needs 500, PV covers it (−8000 leftover). Battery discharging + // 2000 while EV takes 4140: the discharge cannot be house load. + if !BatteryFeedsEV(500, -8000, -2000, 4140) { + t.Fatal("battery discharge into leftover house must count as feeding EV") + } + // House needs 4000, PV 1000, battery covers the 3000 residual, EV off. + if BatteryFeedsEV(4000, -1000, -3000, 0) { + t.Fatal("covering house load is not feeding EV") + } + if BatteryFeedsEV(4000, -1000, -3000, 100) { + t.Fatal("tiny EV with discharge covering house is not feeding EV") + } + // Discharge 5000 into house need 3000 while EV draws 2000 → feeds EV. + if !BatteryFeedsEV(4000, -1000, -5000, 2000) { + t.Fatal("excess discharge with EV on must count as feeding EV") + } +} + +func TestFinite(t *testing.T) { + if !Finite(0) || !Finite(-3) || Finite(math.NaN()) || Finite(math.Inf(1)) { + t.Fatal("Finite mismatch") + } +} diff --git a/go/internal/state/energy_ledger_test.go b/go/internal/state/energy_ledger_test.go index b77bcf00a..b9b2948a2 100644 --- a/go/internal/state/energy_ledger_test.go +++ b/go/internal/state/energy_ledger_test.go @@ -173,6 +173,34 @@ func TestEnergyLedgerKeepsSimultaneousMeterDirections(t *testing.T) { } } +func TestEnergyLedgerLaterExportDoesNotShrinkImport(t *testing.T) { + s := freshStore(t) + base := int64(1_800_000_000_000 / EnergyLedgerBucketMS * EnergyLedgerBucketMS) + assetID := HardwareEnergyAssetID("maker:serial", AssetGridMeter) + recordEnergyTestTick(t, s, base, + ledgerObservation(assetID, AssetGridMeter, FlowGridImport, base, energyPtr(1000), energyPtr(600)), + ledgerObservation(assetID, AssetGridMeter, FlowGridExport, base, energyPtr(0), energyPtr(0)), + ) + recordEnergyTestTick(t, s, base+60_000, + ledgerObservation(assetID, AssetGridMeter, FlowGridImport, base+60_000, energyPtr(1010), energyPtr(0)), + ledgerObservation(assetID, AssetGridMeter, FlowGridExport, base+60_000, energyPtr(20), energyPtr(1200)), + ) + + points := loadLedgerTestPoints(t, s, assetID, base, base+EnergyLedgerBucketMS) + totals := map[EnergyFlow]float64{} + for _, p := range points { + if p.Quality == "measured" { + totals[p.Flow] += p.EnergyWh + } + } + if totals[FlowGridImport] != 10 { + t.Fatalf("import shrunk after export interval: %#v", totals) + } + if totals[FlowGridExport] != 20 { + t.Fatalf("export = %#v, want 20", totals) + } +} + func TestEnergyLedgerMarksCounterResetAndUsesPowerFallback(t *testing.T) { s := freshStore(t) base := int64(1_800_000_000_000 / EnergyLedgerBucketMS * EnergyLedgerBucketMS) diff --git a/go/internal/telemetry/telemetry_test.go b/go/internal/telemetry/telemetry_test.go index 01a8bdb7c..106c7edf6 100644 --- a/go/internal/telemetry/telemetry_test.go +++ b/go/internal/telemetry/telemetry_test.go @@ -598,3 +598,45 @@ func TestWatchdogPerDriverOverride(t *testing.T) { t.Errorf("tesla should flip stale at 6 min under 5-min override; transitions=%+v", transitions) } } + +func TestValidateReadingSiteConventionAndFractions(t *testing.T) { + soc := 0.55 + if err := ValidateReading(DerPV, -1200, nil); err != nil { + t.Fatalf("valid PV: %v", err) + } + if err := ValidateReading(DerPV, 50, nil); err == nil { + t.Fatal("positive PV must be rejected") + } + if err := ValidateReading(DerEV, -10, nil); err == nil { + t.Fatal("negative EV must be rejected") + } + if err := ValidateReading(DerEV, 4140, nil); err != nil { + t.Fatalf("charging EV: %v", err) + } + if err := ValidateReading(DerBattery, 2000, &soc); err != nil { + t.Fatalf("battery charge + fraction SoC: %v", err) + } + pct := 55.0 + if err := ValidateReading(DerBattery, 0, &pct); err == nil { + t.Fatal("percent SoC must be rejected at the telemetry boundary") + } + neg := -0.01 + if err := ValidateReading(DerBattery, 0, &neg); err == nil { + t.Fatal("negative SoC must be rejected") + } + over := 1.02 + if err := ValidateReading(DerBattery, 0, &over); err == nil { + t.Fatal("SoC 1.02 must be rejected (not silently folded to 1%)") + } + nan := math.NaN() + if err := ValidateReading(DerMeter, nan, nil); err == nil { + t.Fatal("NaN power must be rejected") + } + if err := ValidateReading(DerBattery, 0, &nan); err == nil { + t.Fatal("NaN SoC must be rejected") + } + inf := math.Inf(1) + if err := ValidateReading(DerMeter, inf, nil); err == nil { + t.Fatal("+Inf power must be rejected") + } +} diff --git a/go/internal/units/units.go b/go/internal/units/units.go index 05e455d83..9e221dcf1 100644 --- a/go/internal/units/units.go +++ b/go/internal/units/units.go @@ -16,7 +16,10 @@ // a substitute for storing the right unit. package units -import "math" +import ( + "math" + "strings" +) // STCIrradianceWm2 is standard-test-condition irradiance. PV power at STC // equals the array's rated watts; at other irradiance @@ -41,12 +44,17 @@ func KWpFromWatts(w float64) float64 { // CanonicalPowerEnergy is the emit_metric door for vendor kilo-units. // kW/kWh become W/Wh. Other units pass through, including W and Wh -// already converted in a Lua driver. +// already converted in a Lua driver. Unit matching is case-insensitive so +// a driver that emits "kw" cannot store kilowatts as watts. Non-finite +// values become 0 rather than propagating NaN into history or HA. func CanonicalPowerEnergy(value float64, unit string) (float64, string) { - switch unit { - case "kW": + if math.IsNaN(value) || math.IsInf(value, 0) { + value = 0 + } + switch strings.ToLower(strings.TrimSpace(unit)) { + case "kw": return value * 1000.0, "W" - case "kWh": + case "kwh": return value * 1000.0, "Wh" default: return value, unit @@ -64,16 +72,24 @@ func PVFromIrradiance(ratedW, irradianceWm2 float64) float64 { // FractionFromLegacyPercent maps a value that may still be 0–100 into 0–1. // Values already in (0, 1] pass through. 0 stays 0. Call only when loading // old YAML/JSON; new code writes 0–1. +// +// Values in (1, 2) are treated as 0–1 overflow (BMS 102%, a 1.5 typo on a +// fraction field), not as 1.02%/1.5%. Real percents start at 2. The result +// is not clamped to [0, 1] after /100: 150 stays 1.5 so ValidFraction can +// still reject it rather than silently storing 100%. func FractionFromLegacyPercent(v float64) float64 { if math.IsNaN(v) || math.IsInf(v, 0) { return 0 } - if v > 1 { - return v / 100.0 - } if v < 0 { return 0 } + if v > 1 && v < 2 { + return 1 + } + if v > 1 { + return v / 100.0 + } return v } @@ -92,21 +108,20 @@ func DecodeJSONFraction(canonical, legacyPercent float64) float64 { const DefaultPluginSoC = 0.20 // PercentFromFraction is the UI/HA door. Core must not store the result. +// Non-finite and out-of-range fractions fold onto [0, 100] so a NaN SoC +// cannot become "NaN%" on a chart or in a support dump. func PercentFromFraction(f float64) float64 { - return f * 100.0 + return ClampFraction(f) * 100.0 } // PermilleFromFraction is the appproto door (field battery_soc). func PermilleFromFraction(f float64) int64 { - if math.IsNaN(f) || math.IsInf(f, 0) { - return 0 - } - return int64(math.Round(f * 1000.0)) + return int64(math.Round(ClampFraction(f) * 1000.0)) } // FractionFromPermille is the appproto inbound door. func FractionFromPermille(p int64) float64 { - return float64(p) / 1000.0 + return ClampFraction(float64(p) / 1000.0) } // ValidFraction reports whether f is a finite value in [0, 1]. diff --git a/go/internal/units/units_test.go b/go/internal/units/units_test.go index e77525f9a..14572ae46 100644 --- a/go/internal/units/units_test.go +++ b/go/internal/units/units_test.go @@ -1,6 +1,9 @@ package units -import "testing" +import ( + "math" + "testing" +) func TestPVFromIrradianceSTC(t *testing.T) { if got := PVFromIrradiance(18960, 1000); got != 18960 { @@ -42,6 +45,22 @@ func TestCanonicalPowerEnergy(t *testing.T) { if c != 22.6 || u != "°C" { t.Fatalf("other units pass through: %v %q", c, u) } + w, u = CanonicalPowerEnergy(2.5, "kw") + if w != 2500 || u != "W" { + t.Fatalf("lowercase kw → %v %q, want 2500 W", w, u) + } + wh, u = CanonicalPowerEnergy(1, "KWH") + if wh != 1000 || u != "Wh" { + t.Fatalf("KWH → %v %q, want 1000 Wh", wh, u) + } + w, u = CanonicalPowerEnergy(math.NaN(), "kW") + if w != 0 || u != "W" { + t.Fatalf("NaN kW → %v %q, want 0 W", w, u) + } + w, u = CanonicalPowerEnergy(math.Inf(1), "W") + if w != 0 || u != "W" { + t.Fatalf("+Inf W → %v %q, want 0 W", w, u) + } } func TestKWpRoundTrip(t *testing.T) { @@ -60,9 +79,13 @@ func TestFractionFromLegacyPercent(t *testing.T) { {0, 0}, {0.10, 0.10}, {1, 1}, + {1.02, 1}, + {1.5, 1}, + {2, 0.02}, {10, 0.10}, {90, 0.90}, {100, 1}, + {150, 1.5}, // over-percent stays invalid so ValidFraction can reject {-5, 0}, } for _, c := range cases { @@ -70,6 +93,15 @@ func TestFractionFromLegacyPercent(t *testing.T) { t.Errorf("FractionFromLegacyPercent(%v) = %v, want %v", c.in, got, c.want) } } + if got := FractionFromLegacyPercent(math.NaN()); got != 0 { + t.Errorf("NaN → %v, want 0", got) + } + if got := FractionFromLegacyPercent(math.Inf(1)); got != 0 { + t.Errorf("+Inf → %v, want 0", got) + } + if got := FractionFromLegacyPercent(math.Inf(-1)); got != 0 { + t.Errorf("-Inf → %v, want 0", got) + } } func TestRatedWattsFromLegacyKWp(t *testing.T) { @@ -85,6 +117,15 @@ func TestRatedWattsFromLegacyKWp(t *testing.T) { if got := RatedWattsFromLegacyKWp(0); got != 0 { t.Fatalf("empty → %v", got) } + if got := RatedWattsFromLegacyKWp(math.NaN()); got != 0 { + t.Fatalf("NaN → %v", got) + } + // Values in [1, 1000) are kWp. An 800 W balcony pasted into `kwp` + // still becomes 800 kW — the ≥1000 threshold only catches the + // 18960-watt paste. Documented so a later door can tighten it. + if got := RatedWattsFromLegacyKWp(800); got != 800000 { + t.Fatalf("800 kWp heuristic = %v, want 800000 (known balcony-watt hole)", got) + } } func TestPermilleDoor(t *testing.T) { @@ -94,6 +135,54 @@ func TestPermilleDoor(t *testing.T) { if got := FractionFromPermille(624); got != 0.624 { t.Fatalf("fraction = %v, want 0.624", got) } + if got := PermilleFromFraction(math.NaN()); got != 0 { + t.Fatalf("NaN permille = %d, want 0", got) + } + if got := PermilleFromFraction(math.Inf(1)); got != 0 { + t.Fatalf("+Inf permille = %d, want 0", got) + } + if got := PermilleFromFraction(1.5); got != 1000 { + t.Fatalf("overflow fraction permille = %d, want 1000", got) + } + if got := FractionFromPermille(1500); got != 1 { + t.Fatalf("overflow permille → %v, want 1", got) + } + if got := FractionFromPermille(-5); got != 0 { + t.Fatalf("negative permille → %v, want 0", got) + } +} + +func TestPercentFromFractionNonFinite(t *testing.T) { + if got := PercentFromFraction(0.624); got != 62.4 { + t.Fatalf("percent = %v, want 62.4", got) + } + if got := PercentFromFraction(math.NaN()); got != 0 { + t.Fatalf("NaN percent = %v, want 0", got) + } + if got := PercentFromFraction(math.Inf(-1)); got != 0 { + t.Fatalf("-Inf percent = %v, want 0", got) + } + if got := PercentFromFraction(1.5); got != 100 { + t.Fatalf("overflow percent = %v, want 100", got) + } +} + +func TestClampFractionNonFiniteAndBounds(t *testing.T) { + if got := ClampFraction(math.NaN()); got != 0 { + t.Fatalf("NaN → %v", got) + } + if got := ClampFraction(math.Inf(1)); got != 0 { + t.Fatalf("+Inf → %v", got) + } + if got := ClampFraction(-0.1); got != 0 { + t.Fatalf("negative → %v", got) + } + if got := ClampFraction(1.1); got != 1 { + t.Fatalf("1.1 → %v", got) + } + if got := ClampFraction(0.42); got != 0.42 { + t.Fatalf("passthrough → %v", got) + } } func TestDecodeJSONFraction(t *testing.T) { @@ -106,6 +195,12 @@ func TestDecodeJSONFraction(t *testing.T) { if got := DecodeJSONFraction(0.50, 80); got != 0.50 { t.Fatalf("canonical wins = %v", got) } + if got := DecodeJSONFraction(0, 0); got != 0 { + t.Fatalf("both zero = %v", got) + } + if got := DecodeJSONFraction(0, 0.8); got != 0.8 { + t.Fatalf("legacy already-fraction = %v", got) + } } func TestValidFraction(t *testing.T) { From 454e8a1b453ce330f48bb728077e095a897347e4 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 22 Aug 2026 17:56:26 +0000 Subject: [PATCH 2/3] test(ha): drop legacy product name from discovery slug fixture The brand-cleanup gate treats new "forty_two_watts" copy as unclassified. The illegal-topic repro is the driver name, not the box id. Co-authored-by: Fredrik Ahlgren --- go/internal/ha/bridge_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/go/internal/ha/bridge_test.go b/go/internal/ha/bridge_test.go index 2897f8ccc..f0551f2d8 100644 --- a/go/internal/ha/bridge_test.go +++ b/go/internal/ha/bridge_test.go @@ -250,9 +250,9 @@ func TestMQTTObjectIDSlugsIllegalDriverNames(t *testing.T) { } func TestDiscoveryTopicsRejectIllegalCharacters(t *testing.T) { - b := &Bridge{deviceID: "forty_two_watts", discoPrefix: "homeassistant", topicPrefix: "ftw"} + b := &Bridge{deviceID: "site_box", discoPrefix: "homeassistant", topicPrefix: "ftw"} topic := b.driverDiscoveryTopic("sensor", "Laddare, Garage", "ev_current_a") - want := "homeassistant/sensor/forty_two_watts/laddare_garage_ev_current_a/config" + want := "homeassistant/sensor/site_box/laddare_garage_ev_current_a/config" if topic != want { t.Fatalf("discovery topic = %q, want %q", topic, want) } @@ -264,7 +264,7 @@ func TestDiscoveryTopicsRejectIllegalCharacters(t *testing.T) { if got := b.driverTopic("Laddare, Garage", "ev_current_a"); got != "ftw/driver/laddare_garage/ev_current_a" { t.Fatalf("state topic = %q", got) } - if got := b.driverUniqueID("easee", "_ev_w"); got != "forty_two_watts_easee_ev_w" { + if got := b.driverUniqueID("easee", "_ev_w"); got != "site_box_easee_ev_w" { t.Fatalf("legal-name unique_id changed: %q", got) } } From 121570cab4ad1f56bd8dc6d2a694eb059180bb3f Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 22 Aug 2026 18:06:36 +0000 Subject: [PATCH 3/3] fix(ha): keep legal discovery ids and disambiguate slugs Leave already-legal MQTT object ids, including mixed case, unchanged so Home Assistant does not keep a retained Ev-Charger_1 entity next to a new lowercase copy. Illegal names keep a readable stem and append an FNV-1a tag of the original string so "Laddare, Garage" and "Laddare Garage" do not share a topic. Co-authored-by: Fredrik Ahlgren --- .changeset/harden-units-charge-math.md | 2 +- go/internal/ha/bridge.go | 43 +++++++++++++++++++++----- go/internal/ha/bridge_test.go | 29 +++++++++++------ 3 files changed, 55 insertions(+), 19 deletions(-) diff --git a/.changeset/harden-units-charge-math.md b/.changeset/harden-units-charge-math.md index e7a3e6346..4d2ee59cd 100644 --- a/.changeset/harden-units-charge-math.md +++ b/.changeset/harden-units-charge-math.md @@ -2,4 +2,4 @@ "ftw": patch --- -Harden SI units and charging identities: SoC doors fold NaN/overflow instead of leaking percent or Inf, HA discovery slugs driver names so sensors with spaces or commas are not dropped, and synthetic history stores 0–1 SoC. +Harden SI units and charging identities: SoC doors fold NaN/overflow instead of leaking percent or Inf, HA discovery slugs illegal driver names with a collision tag while leaving already-legal mixed-case ids unchanged, and synthetic history stores 0–1 SoC. diff --git a/go/internal/ha/bridge.go b/go/internal/ha/bridge.go index 896ab9190..fedc3b8ab 100644 --- a/go/internal/ha/bridge.go +++ b/go/internal/ha/bridge.go @@ -11,6 +11,7 @@ import ( "context" "encoding/json" "fmt" + "hash/fnv" "log/slog" "net" "net/url" @@ -369,15 +370,22 @@ func (b *Bridge) driverTopic(driver, field string) string { return fmt.Sprintf("%s/driver/%s/%s", b.topicPrefix, mqttObjectID(driver), mqttObjectID(field)) } -// mqttObjectID slugs a free-form YAML driver name into the character set -// Home Assistant allows in discovery object ids: [a-zA-Z0-9_-]. -// "Laddare, Garage" becomes laddare_garage so EV sensors are not silently -// dropped. The human-readable name stays in the discovery `name` field. +// mqttObjectID maps a free-form YAML driver name onto Home Assistant's +// discovery object-id alphabet: [A-Za-z0-9_-]. +// +// Names that are already legal are returned unchanged, including case, so an +// existing Ev-Charger_1 entity is not duplicated as ev-charger_1 on upgrade. +// Names that need slugging keep a readable stem and append an FNV-1a tag of +// the original string so "Laddare, Garage" and "Laddare Garage" do not share +// a topic. The discovery `name` field still uses the human YAML name. func mqttObjectID(name string) string { + if mqttIDLegal(name) { + return name + } var b strings.Builder lastSep := true - for _, r := range strings.ToLower(name) { - ok := (r >= 'a' && r <= 'z') || (r >= '0' && r <= '9') || r == '-' || r == '_' + for _, r := range name { + ok := (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || (r >= '0' && r <= '9') || r == '-' || r == '_' if ok { b.WriteRune(r) lastSep = r == '_' || r == '-' @@ -390,9 +398,28 @@ func mqttObjectID(name string) string { } s := strings.Trim(b.String(), "_-") if s == "" { - return "driver" + s = "driver" + } + return s + "_" + mqttNameTag(name) +} + +func mqttIDLegal(name string) bool { + if name == "" { + return false + } + for _, r := range name { + ok := (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || (r >= '0' && r <= '9') || r == '-' || r == '_' + if !ok { + return false + } } - return s + return true +} + +func mqttNameTag(name string) string { + h := fnv.New32a() + _, _ = h.Write([]byte(name)) + return fmt.Sprintf("%08x", h.Sum32()) } func (b *Bridge) driverUniqueID(driver, suffix string) string { diff --git a/go/internal/ha/bridge_test.go b/go/internal/ha/bridge_test.go index f0551f2d8..14a887c5e 100644 --- a/go/internal/ha/bridge_test.go +++ b/go/internal/ha/bridge_test.go @@ -235,24 +235,33 @@ func TestStopAfterFailedConnectDoesNotDeadlock(t *testing.T) { func TestMQTTObjectIDSlugsIllegalDriverNames(t *testing.T) { cases := []struct{ in, want string }{ {"easee", "easee"}, - {"Laddare, Garage", "laddare_garage"}, - {"Värmepump, källaren", "v_rmepump_k_llaren"}, - {" foo bar ", "foo_bar"}, - {"???", "driver"}, - {"", "driver"}, - {"Ev-Charger_1", "ev-charger_1"}, + {"Ev-Charger_1", "Ev-Charger_1"}, + {"Garage", "Garage"}, + {"garage", "garage"}, + {"Laddare, Garage", "Laddare_Garage_57d42421"}, + {"Laddare Garage", "Laddare_Garage_7bcd041d"}, + {"Värmepump, källaren", "V_rmepump_k_llaren_13212c9a"}, + {" foo bar ", "foo_bar_09e9096a"}, + {"???", "driver_7bcac794"}, + {"", "driver_811c9dc5"}, } for _, c := range cases { if got := mqttObjectID(c.in); got != c.want { t.Errorf("mqttObjectID(%q) = %q, want %q", c.in, got, c.want) } } + if mqttObjectID("Laddare, Garage") == mqttObjectID("Laddare Garage") { + t.Fatal("comma vs space must not share a discovery id") + } + if mqttObjectID("Garage") == mqttObjectID("garage") { + t.Fatal("legal names that differ only in case must stay distinct") + } } func TestDiscoveryTopicsRejectIllegalCharacters(t *testing.T) { b := &Bridge{deviceID: "site_box", discoPrefix: "homeassistant", topicPrefix: "ftw"} topic := b.driverDiscoveryTopic("sensor", "Laddare, Garage", "ev_current_a") - want := "homeassistant/sensor/site_box/laddare_garage_ev_current_a/config" + want := "homeassistant/sensor/site_box/Laddare_Garage_57d42421_ev_current_a/config" if topic != want { t.Fatalf("discovery topic = %q, want %q", topic, want) } @@ -261,11 +270,11 @@ func TestDiscoveryTopicsRejectIllegalCharacters(t *testing.T) { t.Fatalf("discovery topic still has illegal %q: %s", r, topic) } } - if got := b.driverTopic("Laddare, Garage", "ev_current_a"); got != "ftw/driver/laddare_garage/ev_current_a" { + if got := b.driverTopic("Laddare, Garage", "ev_current_a"); got != "ftw/driver/Laddare_Garage_57d42421/ev_current_a" { t.Fatalf("state topic = %q", got) } - if got := b.driverUniqueID("easee", "_ev_w"); got != "site_box_easee_ev_w" { - t.Fatalf("legal-name unique_id changed: %q", got) + if got := b.driverUniqueID("Ev-Charger_1", "_ev_w"); got != "site_box_Ev-Charger_1_ev_w" { + t.Fatalf("legal mixed-case unique_id changed: %q", got) } }