From 76643ea128c133433cef67f7541197eb155c44ab Mon Sep 17 00:00:00 2001 From: Fredrik Ahlgren Date: Thu, 1 Oct 2026 11:16:40 +0200 Subject: [PATCH] fix(history): keep the chart running when optional PV never reports Adding a Zap P1 meter stopped history_dashboard and history_site_energy for good. The Zap catalog lists pv, but PV reading is off unless read_pv is set. The forecast topology still expects a PV reading from it, and since #1089 the history point uses those same forecast options. The Zap never reports PV, so every tick wrote no history point, and the same config rebuilt the same block after each restart. History now uses its own options: the same flows, minus PV that config does not declare, and without the identity gate. PV that a source does report is still checked for freshness. Forecast learning is unchanged. Closes #1441 Co-Authored-By: Claude Opus 5.5 Signed-off-by: Fredrik Ahlgren --- .changeset/history-optional-pv.md | 5 ++ go/cmd/ftw/forecast_measurements.go | 31 +++++++++--- go/cmd/ftw/forecast_site.go | 3 +- go/cmd/ftw/forecast_tracking.go | 1 + go/cmd/ftw/history_options_test.go | 73 +++++++++++++++++++++++++++++ go/cmd/ftw/main.go | 4 +- 6 files changed, 107 insertions(+), 10 deletions(-) create mode 100644 .changeset/history-optional-pv.md create mode 100644 go/cmd/ftw/history_options_test.go diff --git a/.changeset/history-optional-pv.md b/.changeset/history-optional-pv.md new file mode 100644 index 000000000..e0ec013e3 --- /dev/null +++ b/.changeset/history-optional-pv.md @@ -0,0 +1,5 @@ +--- +"ftw": patch +--- + +Keep the history chart and site energy running when a device can report PV but does not, such as a Zap P1 meter with PV reading off, or while a device identity is still unconfirmed. Before, adding such a device stopped history until it was removed, even across restarts. Forecast learning keeps its stricter checks. diff --git a/go/cmd/ftw/forecast_measurements.go b/go/cmd/ftw/forecast_measurements.go index 9bc4bcc03..064a22a8a 100644 --- a/go/cmd/ftw/forecast_measurements.go +++ b/go/cmd/ftw/forecast_measurements.go @@ -14,7 +14,23 @@ import ( // forecastMeasurementOptions declares known significant sources. FlowIDs remain // unset: driver names and inverter groups do not identify physical measurements. func forecastMeasurementOptions(cfg *config.Config, catalog []drivers.CatalogEntry) telemetry.ForecastOptions { - opts, _ := forecastMeasurementTopology(cfg, catalog) + opts, _, _ := forecastMeasurementTopology(cfg, catalog) + return opts +} + +// historyMeasurementOptions qualifies the stored chart and site energy. They +// need a measured balance, not the learner's proof that an optional source is +// absent: a source that reports PV is still checked, but one that never +// reports it does not stop history (#1441). Identity checks stay with learning. +func historyMeasurementOptions(cfg *config.Config, catalog []drivers.CatalogEntry) telemetry.ForecastOptions { + opts, _, optional := forecastMeasurementTopology(cfg, catalog) + flows := opts.ExpectedFlows[:0:0] + for _, flow := range opts.ExpectedFlows { + if !optional[flow] { + flows = append(flows, flow) + } + } + opts.ExpectedFlows = flows return opts } @@ -22,17 +38,17 @@ func forecastMeasurementOptions(cfg *config.Config, catalog []drivers.CatalogEnt // establish. Callers must require fresh observations of these optional flows // before qualifying a complete household balance; this is not only a log hint. func forecastMeasurementTopologyUnknown(cfg *config.Config, catalog []drivers.CatalogEntry) []string { - _, unknown := forecastMeasurementTopology(cfg, catalog) + _, unknown, _ := forecastMeasurementTopology(cfg, catalog) return unknown } -func forecastMeasurementTopology(cfg *config.Config, catalog []drivers.CatalogEntry) (telemetry.ForecastOptions, []string) { - opts := telemetry.ForecastOptions{} +// optional holds PV flows that config does not declare; see historyMeasurementOptions. +func forecastMeasurementTopology(cfg *config.Config, catalog []drivers.CatalogEntry) (opts telemetry.ForecastOptions, unknown []string, optional map[telemetry.ForecastFlow]bool) { + optional = make(map[telemetry.ForecastFlow]bool) if cfg == nil { opts.HouseholdInvalidReason = "missing_config" - return opts, []string{"missing_config"} + return opts, []string{"missing_config"}, optional } - var unknown []string type configured struct { driver config.Driver entry drivers.CatalogEntry @@ -97,6 +113,7 @@ func forecastMeasurementTopology(cfg *config.Config, catalog []drivers.CatalogEn add(d, telemetry.DerPV) if !declared { unknown = append(unknown, d.Name+":optional_pv_requires_measurement") + optional[telemetry.ForecastFlow{Driver: d.Name, DerType: telemetry.DerPV}] = true } } } @@ -117,7 +134,7 @@ func forecastMeasurementTopology(cfg *config.Config, catalog []drivers.CatalogEn opts.HouseholdInvalidReason = "configured_pv_without_measurement_source" unknown = append(unknown, "site:configured_pv_without_measurement_source") } - return opts, unknown + return opts, unknown, optional } func forecastCapability(e drivers.CatalogEntry, kind string) bool { diff --git a/go/cmd/ftw/forecast_site.go b/go/cmd/ftw/forecast_site.go index 2ebc2b654..1b27d770f 100644 --- a/go/cmd/ftw/forecast_site.go +++ b/go/cmd/ftw/forecast_site.go @@ -76,11 +76,12 @@ func (s *forecastSiteConfig) Snapshot() forecastSite { defer s.mu.RUnlock() v := s.value v.Options.ExpectedFlows = append([]telemetry.ForecastFlow(nil), v.Options.ExpectedFlows...) + v.HistoryOptions.ExpectedFlows = append([]telemetry.ForecastFlow(nil), v.HistoryOptions.ExpectedFlows...) return v } func (s *forecastSiteConfig) Configure(cfg *config.Config, catalog []drivers.CatalogEntry) bool { - v := forecastSite{Meter: cfg.SiteMeterDriver(), Timezone: forecastTimezone(), Options: forecastMeasurementOptions(cfg, catalog)} + v := forecastSite{Meter: cfg.SiteMeterDriver(), Timezone: forecastTimezone(), Options: forecastMeasurementOptions(cfg, catalog), HistoryOptions: historyMeasurementOptions(cfg, catalog)} if cfg.Weather != nil && cfg.Weather.Provider != "" && cfg.Weather.Provider != "none" { v.Latitude, v.Longitude = cfg.Weather.Latitude, cfg.Weather.Longitude v.HasLocation = !math.IsNaN(v.Latitude) && !math.IsNaN(v.Longitude) && math.Abs(v.Latitude) <= 90 && math.Abs(v.Longitude) <= 180 diff --git a/go/cmd/ftw/forecast_tracking.go b/go/cmd/ftw/forecast_tracking.go index b2e4bb208..a17c3d043 100644 --- a/go/cmd/ftw/forecast_tracking.go +++ b/go/cmd/ftw/forecast_tracking.go @@ -35,6 +35,7 @@ type forecastSite struct { HasPVScale bool Timezone string Options telemetry.ForecastOptions + HistoryOptions telemetry.ForecastOptions // chart and site energy; outside every revision PVLearningStartedMS int64 LoadLearningStartedMS int64 } diff --git a/go/cmd/ftw/history_options_test.go b/go/cmd/ftw/history_options_test.go new file mode 100644 index 000000000..2c511e503 --- /dev/null +++ b/go/cmd/ftw/history_options_test.go @@ -0,0 +1,73 @@ +package main + +import ( + "path/filepath" + "testing" + "time" + + "github.com/srcfl/ftw/go/internal/config" + "github.com/srcfl/ftw/go/internal/drivers" + "github.com/srcfl/ftw/go/internal/state" + "github.com/srcfl/ftw/go/internal/telemetry" +) + +// Issue #1441: adding a Zap P1 meter (catalog meter+pv+battery, PV reading +// off by default) froze the chart for good. The Zap never reports PV, so the +// forecast topology kept the household balance invalid on every tick and +// after every restart. The chart needs a measured balance, not proof that an +// optional source is absent or that device identities are confirmed. +func TestHistoryKeepsRunningWithOptionalPVAndUnconfirmedIdentity(t *testing.T) { + st, err := state.Open(filepath.Join(t.TempDir(), "state.db")) + if err != nil { + t.Fatal(err) + } + defer st.Close() + catalog := []drivers.CatalogEntry{ + {Path: "drivers/zap.lua", Filename: "zap.lua", Capabilities: []string{"meter", "pv", "battery"}}, + {Path: "drivers/sma.lua", Filename: "sma.lua", Capabilities: []string{"pv"}}, + {Path: "drivers/pixii.lua", Filename: "pixii.lua", Capabilities: []string{"battery"}}, + } + cfg := &config.Config{Drivers: []config.Driver{ + {Name: "sourceful-zap", Lua: "drivers/zap.lua", IsSiteMeter: true, Config: map[string]any{"host": "zap.local"}}, + {Name: "sma", Lua: "drivers/sma.lua"}, + {Name: "pixii", Lua: "drivers/pixii.lua", BatteryCapacityWh: 10000}, + }} + ids := map[string]string{"sourceful-zap": "zap:sn", "sma": "sma:sn"} // pixii identity not yet known + site := newForecastSiteConfig(st) + site.identity = func(name string) (string, bool) { id := ids[name]; return id, id != "" } + site.Configure(cfg, catalog) + + tel := telemetry.NewStore() + for name, kind := range map[string]telemetry.DerType{"sourceful-zap": telemetry.DerMeter, "sma": telemetry.DerPV, "pixii": telemetry.DerBattery} { + tel.EnsureDriverHealth(name) + w := map[telemetry.DerType]float64{telemetry.DerMeter: 1500, telemetry.DerPV: -2000, telemetry.DerBattery: 500}[kind] + tel.Update(name, kind, w, nil, nil) + tel.RecordDriverSuccess(name) + } + snap := site.Snapshot() + if r := tel.ForecastMeasurementNow("sourceful-zap", snap.Options); r.Valid { + t.Fatalf("forecast learning must stay strict: %+v", r) + } + point, ok := buildHistoryPoint(tel, tickPersistControl{SiteMeterDriver: "sourceful-zap"}, + time.Now().UnixMilli(), time.Minute, snap.HistoryOptions) + if !ok || point.LoadW != 3000 || point.PVW != -2000 || point.BatW != 500 { + t.Fatalf("history stopped: point=%+v available=%v", point, ok) + } + + // A known source that goes silent still stops history; only optional, + // never-reported PV is treated as absent. + tel.DriverHealthMut("pixii").SetOffline() + if _, ok := buildHistoryPoint(tel, tickPersistControl{SiteMeterDriver: "sourceful-zap"}, + time.Now().UnixMilli(), time.Minute, snap.HistoryOptions); ok { + t.Fatal("offline battery treated as zero in history") + } + tel.Update("pixii", telemetry.DerBattery, 500, nil, nil) + tel.RecordDriverSuccess("pixii") + // Once the Zap does report PV, a stale PV reading stops history again. + tel.Update("sourceful-zap", telemetry.DerPV, -100, nil, nil) + tel.Get("sourceful-zap", telemetry.DerPV).UpdatedAt = time.Now().Add(-2 * time.Minute) + if _, ok := buildHistoryPoint(tel, tickPersistControl{SiteMeterDriver: "sourceful-zap"}, + time.Now().UnixMilli(), time.Minute, snap.HistoryOptions); ok { + t.Fatal("stale reported PV treated as zero in history") + } +} diff --git a/go/cmd/ftw/main.go b/go/cmd/ftw/main.go index 29f2db0fc..1eb0b8b83 100644 --- a/go/cmd/ftw/main.go +++ b/go/cmd/ftw/main.go @@ -2954,7 +2954,7 @@ func main() { // ctrl, so the stored tick has to show the hold already // released rather than one the blocked tick never executed. clearBatteryManualHoldForDispatchBlock(ctrl, ctrlMu) - sampleCount, err := persistTelemetryTick(st, tel, snapshotTickPersistControl(ctrl, ctrlMu), nowMs, watchdogTimeout, energyIdentity, forecastSettings.Snapshot().Options) + sampleCount, err := persistTelemetryTick(st, tel, snapshotTickPersistControl(ctrl, ctrlMu), nowMs, watchdogTimeout, energyIdentity, forecastSettings.Snapshot().HistoryOptions) if err != nil { slog.Warn("tick persistence failed", "samples", sampleCount, "err", err) } @@ -3207,7 +3207,7 @@ func main() { // ---- Persist the tick: history snapshot + flushed metrics ---- // One transaction for both — separate commits doubled the WAL // commit rate for no isolation benefit (SD-card wear). - sampleCount, err := persistTelemetryTick(st, tel, snapshotTickPersistControl(ctrl, ctrlMu), nowMs, watchdogTimeout, energyIdentity, forecastSettings.Snapshot().Options) + sampleCount, err := persistTelemetryTick(st, tel, snapshotTickPersistControl(ctrl, ctrlMu), nowMs, watchdogTimeout, energyIdentity, forecastSettings.Snapshot().HistoryOptions) if err != nil { slog.Warn("tick persistence failed", "samples", sampleCount, "err", err) }