Repository navigation
fix: harden SI doors, HA discovery slugs, and charging identities #970
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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") | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
| } | ||
|
Comment on lines
+425
to
+427
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For an existing driver whose name is already legal but contains uppercase characters, such as the test's Useful? React with 👍 / 👎. |
||
|
|
||
| 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: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When two valid configured driver names normalize identically, such as
GarageandgarageorLaddare, GarageandLaddare Garage, this lossy slug assigns both drivers the same discovery IDs and state topics. Their Home Assistant entities overwrite one another and subsequent state publications become indistinguishable; include a stable collision-resistant component instead of using the slug as the identity.Useful? React with 👍 / 👎.