From 924312153b26e422040b1754fa3a7f5a317ff28a Mon Sep 17 00:00:00 2001 From: Fredrik Ahlgren Date: Mon, 19 Oct 2026 06:49:08 +0200 Subject: [PATCH] fix(chargers): surface missing saved connections --- .changeset/charger-connection-diagnostics.md | 5 ++ go/internal/api/api_agent_catalog.go | 12 +++- .../api/api_agent_charger_connections_test.go | 30 ++++++++ go/internal/api/api_charger_steps_test.go | 4 +- go/internal/config/loadpoint_connections.go | 55 +++++++++++++++ .../config/loadpoint_connections_test.go | 68 +++++++++++++++++++ go/internal/config/loadpoint_steps.go | 1 + go/internal/config/loadpoint_steps_test.go | 9 +-- .../onboarding/connection_warnings_test.go | 57 ++++++++++++++++ go/internal/onboarding/draft.go | 20 +++++- web/app.css | 1 + web/settings-shell.test.mjs | 25 ++++++- web/settings.js | 9 ++- .../tabs/loadpoint-connections.test.mjs | 49 +++++++++++++ web/settings/tabs/loadpoints.js | 30 ++++++-- 15 files changed, 356 insertions(+), 19 deletions(-) create mode 100644 .changeset/charger-connection-diagnostics.md create mode 100644 go/internal/api/api_agent_charger_connections_test.go create mode 100644 go/internal/config/loadpoint_connections.go create mode 100644 go/internal/config/loadpoint_connections_test.go create mode 100644 go/internal/onboarding/connection_warnings_test.go create mode 100644 web/settings/tabs/loadpoint-connections.test.mjs diff --git a/.changeset/charger-connection-diagnostics.md b/.changeset/charger-connection-diagnostics.md new file mode 100644 index 000000000..d4805dc79 --- /dev/null +++ b/.changeset/charger-connection-diagnostics.md @@ -0,0 +1,5 @@ +--- +"ftw": patch +--- + +Warn about missing saved charger connections in Settings and setup drafts. Include exact charger-binding evidence in technical diagnostics so assistants can verify connections without guessing from redacted names. diff --git a/go/internal/api/api_agent_catalog.go b/go/internal/api/api_agent_catalog.go index 2c44c2685..8bc6bf73f 100644 --- a/go/internal/api/api_agent_catalog.go +++ b/go/internal/api/api_agent_catalog.go @@ -134,7 +134,7 @@ func (s *Server) agentDebugContext() map[string]any { ref, kind = "v"+strings.TrimPrefix(s.deps.Version, "v"), "declared release tag" } tree := "https://github.com/srcfl/ftw/tree/" + url.PathEscape(ref) - return map[string]any{ + out := map[string]any{ "core": map[string]any{"version": s.deps.Version, "revision": revision, "modified": modified, "source_ref_kind": kind, "repository": "https://github.com/srcfl/ftw", "source": tree, "api": tree + "/go/internal/api", "energy_history": tree + "/go/internal/state", "planner": tree + "/go/internal/mpc", @@ -151,8 +151,18 @@ func (s *Server) agentDebugContext() map[string]any { "Read /api/health, /api/support/report and /api/components for versions, clock, freshness and failures.", "Use /api/energy/history for Wh and provenance, /api/series for original measurements, and /api/mpc/diagnose/history then /api/mpc/diagnose/at for the plan active at the incident.", "Inspect configuration, driver details, driver source and logs as needed. Compare with source for the running version; default-branch code may differ.", + "For charger faults, read charger_connections below and use /api/config, /api/drivers and /api/loadpoints to compare loadpoints[].driver_name with drivers[].name. If OCPP is enabled, also check /api/ocpp/chargers. A configured name does not prove online equipment or successful control.", + "Basic config_get and drivers_status hide technical names and cannot prove these bindings. Use these full diagnostic API reads. If a read is refused, record the path, refusal and missing grant or version; do not infer a driver fault from unavailable or redacted evidence.", "For a bug report, include the Core version and source reference, equipment and driver versions, reproduction steps, expected and observed results, incident time, exact API queries and relevant logs or snapshots. Mark data gaps and whether evidence came from real equipment, a simulator or a test.", "Use the existing control and goal tools when an action is needed. Reading a diagnostic view does not issue a command. Prepare a bug report for the user; do not publish it without their request.", }, } + if s.deps.Cfg != nil { + if s.deps.CfgMu != nil { + s.deps.CfgMu.RLock() + defer s.deps.CfgMu.RUnlock() + } + out["charger_connections"] = s.deps.Cfg.LoadpointConnections() + } + return out } diff --git a/go/internal/api/api_agent_charger_connections_test.go b/go/internal/api/api_agent_charger_connections_test.go new file mode 100644 index 000000000..613ae79e9 --- /dev/null +++ b/go/internal/api/api_agent_charger_connections_test.go @@ -0,0 +1,30 @@ +package api + +import ( + "encoding/json" + "sync" + "testing" + + "github.com/srcfl/ftw/go/internal/config" + "github.com/srcfl/ftw/go/protocol" +) + +func TestDiagnosticCatalogueIncludesExactSavedChargerConnections(t *testing.T) { + cfg := &config.Config{Drivers: []config.Driver{{Name: "charger-current"}}, + Loadpoints: []config.Loadpoint{{ID: "garage", DriverName: "charger-old"}, {ID: "driveway", DriverName: "charger-current"}}} + srv := New(&Deps{Cfg: cfg, CfgMu: &sync.RWMutex{}, Version: "0.153.0-beta.1"}) + answer, _ := diagnosticRead(t, srv, protocol.AgentAPICatalogPath, protocol.APIRead{}) + var page struct { + Troubleshooting struct { + Connections []config.LoadpointConnection `json:"charger_connections"` + } `json:"troubleshooting"` + } + if err := json.Unmarshal(answer.Body.Bytes(), &page); err != nil { + t.Fatal(err) + } + connections := page.Troubleshooting.Connections + if len(connections) != 2 || connections[0].DriverName != "charger-old" || connections[0].Status != "missing" || + connections[1].DriverName != "charger-current" || connections[1].Status != "configured" { + t.Fatal("diagnostic evidence lost the saved name or confused a name binding with device health") + } +} diff --git a/go/internal/api/api_charger_steps_test.go b/go/internal/api/api_charger_steps_test.go index 41508c4da..3fc6509e1 100644 --- a/go/internal/api/api_charger_steps_test.go +++ b/go/internal/api/api_charger_steps_test.go @@ -31,7 +31,9 @@ func TestPostConfigSavesLimitedStepsAndReturnsWarning(t *testing.T) { if err := json.Unmarshal(rr.Body.Bytes(), &response); err != nil { t.Fatal(err) } - if len(response.Warnings) != 1 || len(response.Loadpoints) != 1 { + // The missing connection is separate from the corrected power steps. + if len(response.Warnings) != 2 || len(response.Loadpoints) != 1 || + !strings.Contains(strings.Join(response.Warnings, " "), "Missing charger connection") { t.Fatalf("missing save feedback: %s", rr.Body.String()) } if response.Loadpoints[0].AllowedStepsW[3] != 11000 { diff --git a/go/internal/config/loadpoint_connections.go b/go/internal/config/loadpoint_connections.go new file mode 100644 index 000000000..3652a44c5 --- /dev/null +++ b/go/internal/config/loadpoint_connections.go @@ -0,0 +1,55 @@ +package config + +import ( + "fmt" + "strings" +) + +// LoadpointConnection describes a saved name binding, not device health. +type LoadpointConnection struct { + ID string `json:"id"` + DriverName string `json:"driver_name"` + Status string `json:"status"` +} + +func (c *Config) LoadpointConnections() []LoadpointConnection { + drivers := make(map[string]bool, len(c.Drivers)) + for _, driver := range c.Drivers { + drivers[driver.Name] = true + } + connections := make([]LoadpointConnection, 0, len(c.Loadpoints)) + for _, lp := range c.Loadpoints { + status := "missing" + if lp.DriverName != "" && drivers[lp.DriverName] { + status = "configured" + } else if lp.DriverName != "" && c.OCPP != nil && c.OCPP.Enabled { + // OCPP identities have no driver entry and may not have connected yet. + status = "unverified_ocpp" + } + connections = append(connections, LoadpointConnection{ID: lp.ID, DriverName: lp.DriverName, Status: status}) + } + return connections +} + +const missingChargerConnection = "Missing charger connection: " + +func (c *Config) warnMissingLoadpointConnections() { + // Recompute these warnings after a repair while retaining step corrections. + warnings := make([]string, 0, len(c.LoadpointWarnings)) + for _, warning := range c.LoadpointWarnings { + if !strings.HasPrefix(warning, missingChargerConnection) { + warnings = append(warnings, warning) + } + } + for _, connection := range c.LoadpointConnections() { + if connection.Status != "missing" { + continue + } + detail := fmt.Sprintf("charger %q has no charger selected.", connection.ID) + if connection.DriverName != "" { + detail = fmt.Sprintf("charger %q refers to %q, which is not a configured driver.", connection.ID, connection.DriverName) + } + warnings = append(warnings, missingChargerConnection+detail+" Select a configured charger or remove this charger in Settings > Chargers.") + } + c.LoadpointWarnings = warnings +} diff --git a/go/internal/config/loadpoint_connections_test.go b/go/internal/config/loadpoint_connections_test.go new file mode 100644 index 000000000..c5693ef76 --- /dev/null +++ b/go/internal/config/loadpoint_connections_test.go @@ -0,0 +1,68 @@ +package config + +import ( + "slices" + "strings" + "testing" +) + +func TestMissingChargerConnectionWarnsWithoutRebinding(t *testing.T) { + cfg := &Config{Loadpoints: []Loadpoint{{ID: "garage", DriverName: "old-charger", MinChargeW: 1400, MaxChargeW: 7000}}} + applyDefaults(cfg) + cfg.Fuse.MaxAmps, cfg.Fuse.Phases, cfg.Fuse.Voltage = 20, 3, 230 + if err := cfg.Validate(); err != nil { + t.Fatal("valid settings should remain repairable") + } + if len(cfg.LoadpointWarnings) != 1 || !strings.Contains(cfg.LoadpointWarnings[0], "old-charger") { + t.Fatal("missing charger connection was silent") + } + if cfg.Loadpoints[0].DriverName != "old-charger" { + t.Fatal("validation chose a replacement") + } + cfg.LoadpointWarnings = append(cfg.LoadpointWarnings, "Existing charging-step correction") + if err := cfg.Validate(); err != nil { + t.Fatal(err) + } + if len(cfg.LoadpointWarnings) != 2 { + t.Fatal("validation duplicated its warning") + } + cfg.Loadpoints = nil + if err := cfg.Validate(); err != nil { + t.Fatal(err) + } + if !slices.Equal(cfg.LoadpointWarnings, []string{"Existing charging-step correction"}) { + t.Fatal("repair kept a stale connection warning or lost a charging-step correction") + } +} + +func TestChargerConnectionEvidenceDistinguishesConfiguredMissingAndOCPP(t *testing.T) { + for _, tc := range []struct { + name, bound, driver string + ocpp bool + status string + warnings int + }{ + {"configured name", "charger-current", "charger-current", false, "configured", 0}, + {"old name", "charger-old", "charger-current", false, "missing", 1}, + {"no selection", "", "charger-current", false, "missing", 1}, + {"OCPP may be offline", "ocpp-garage", "", true, "unverified_ocpp", 0}, + {"OCPP does not replace an empty selection", "", "", true, "missing", 1}, + } { + t.Run(tc.name, func(t *testing.T) { + cfg := &Config{Loadpoints: []Loadpoint{{ID: "garage", DriverName: tc.bound}}, OCPP: &OCPP{Enabled: tc.ocpp}} + if tc.driver != "" { + cfg.Drivers = []Driver{{Name: tc.driver}} + } + if err := cfg.normalizeLoadpointSteps(); err != nil { + t.Fatal(err) + } + if len(cfg.LoadpointWarnings) != tc.warnings { + t.Fatal("unexpected connection warnings") + } + evidence := cfg.LoadpointConnections() + if len(evidence) != 1 || evidence[0].Status != tc.status || evidence[0].DriverName != tc.bound { + t.Fatal("connection evidence lost the exact saved name or its status") + } + }) + } +} diff --git a/go/internal/config/loadpoint_steps.go b/go/internal/config/loadpoint_steps.go index 90bf29b68..445738fb9 100644 --- a/go/internal/config/loadpoint_steps.go +++ b/go/internal/config/loadpoint_steps.go @@ -34,5 +34,6 @@ func (c *Config) normalizeLoadpointSteps() error { } } c.Loadpoints = loadpoints + c.warnMissingLoadpointConnections() return nil } diff --git a/go/internal/config/loadpoint_steps_test.go b/go/internal/config/loadpoint_steps_test.go index c5edb20a2..cc7c80959 100644 --- a/go/internal/config/loadpoint_steps_test.go +++ b/go/internal/config/loadpoint_steps_test.go @@ -27,7 +27,8 @@ loadpoints: if !reflect.DeepEqual(cfg.Loadpoints[0].AllowedStepsW, want) { t.Fatal(cfg.Loadpoints[0].AllowedStepsW) } - if len(cfg.LoadpointWarnings) != 1 || !strings.Contains(cfg.LoadpointWarnings[0], `"Easee"`) { + // This fixture also has no configured charger for its saved connection. + if len(cfg.LoadpointWarnings) != 2 || !strings.Contains(cfg.LoadpointWarnings[0], `"Easee"`) { t.Fatal(cfg.LoadpointWarnings) } old := cfg.Loadpoints[0].AllowedStepsW @@ -48,8 +49,8 @@ loadpoints: t.Fatalf("persisted step %v exceeds max", step) } } - if len(reloaded.LoadpointWarnings) != 0 { - t.Fatal("corrected persisted config still warns", reloaded.LoadpointWarnings) + if len(reloaded.LoadpointWarnings) != 1 || !strings.HasPrefix(reloaded.LoadpointWarnings[0], missingChargerConnection) { + t.Fatal("step correction should leave only the unresolved connection warning") } } @@ -79,7 +80,7 @@ func TestLoadpointStepsAreLimitedInStoredSettings(t *testing.T) { if err := json.Unmarshal(doc.Document, &saved); err != nil { t.Fatal(err) } - if saved.Config.Loadpoints[0].AllowedStepsW[1] != 11000 || len(cfg.LoadpointWarnings) != 1 { + if saved.Config.Loadpoints[0].AllowedStepsW[1] != 11000 || len(cfg.LoadpointWarnings) != 2 { t.Fatalf("stored steps=%v warnings=%v", saved.Config.Loadpoints[0].AllowedStepsW, cfg.LoadpointWarnings) } } diff --git a/go/internal/onboarding/connection_warnings_test.go b/go/internal/onboarding/connection_warnings_test.go new file mode 100644 index 000000000..1ddb89824 --- /dev/null +++ b/go/internal/onboarding/connection_warnings_test.go @@ -0,0 +1,57 @@ +package onboarding + +import ( + "bytes" + "encoding/json" + "errors" + "testing" + + "github.com/srcfl/ftw/go/internal/config" +) + +func TestSetupValidationPersistsConnectionWarningWithoutChangingSettings(t *testing.T) { + live, err := config.Parse([]byte("fuse:\n max_amps: 20\n phases: 3\n voltage: 230\n"), t.TempDir()) + if err != nil { + t.Fatal(err) + } + live.Revision = 1 + live.Loadpoints = []config.Loadpoint{{ID: "garage", DriverName: "removed-charger", MinChargeW: 1400, MaxChargeW: 7000}} + store := draftStore(t) + service := New(store) + original, err := service.Edit(live, 0, json.RawMessage("{}")) + if err != nil { + t.Fatal(err) + } + if _, err := service.Checked(live.Revision, original.Revision, original.SHA256); !errors.Is(err, ErrConflict) { + t.Fatal("unchecked charger warning was not bound to review") + } + checked, err := service.Validate(live, original.Revision, original.SHA256) + if err != nil { + t.Fatal(err) + } + if len(checked.ValidationWarnings) != 1 || checked.Revision != original.Revision+1 { + t.Fatal("warning-only validation was not saved for review") + } + if !bytes.Equal(checked.Config, original.Config) { + t.Fatal("warning changed the saved charger choice") + } + resumed, err := New(store).Read(live) + if err != nil || len(resumed.ValidationWarnings) != 1 || resumed.Revision != checked.Revision { + t.Fatal("warning disappeared after reopening the draft") + } + again, err := service.Validate(live, checked.Revision, checked.SHA256) + if err != nil || again.Revision != checked.Revision { + t.Fatal("revalidation changed a reviewed warning") + } + if _, err := service.Checked(live.Revision, checked.Revision, checked.SHA256); err != nil { + t.Fatal(err) + } + repaired, err := service.Edit(live, checked.Revision, json.RawMessage(`{"loadpoints":[]}`)) + if err != nil { + t.Fatal(err) + } + repaired, err = service.Validate(live, repaired.Revision, repaired.SHA256) + if err != nil || len(repaired.ValidationWarnings) != 0 { + t.Fatal("repair left an old warning") + } +} diff --git a/go/internal/onboarding/draft.go b/go/internal/onboarding/draft.go index 70091e66c..25358aaa2 100644 --- a/go/internal/onboarding/draft.go +++ b/go/internal/onboarding/draft.go @@ -10,6 +10,7 @@ import ( "errors" "fmt" "io" + "slices" "strings" "sync" @@ -258,12 +259,20 @@ func (s *Service) ValidateWithCheck(live *config.Config, revision int64, sha str if err != nil { return Draft{}, err } - if bytes.Equal(normalized, d.Config) { + warnings := slices.Clone(d.ValidationWarnings) + for _, warning := range cfg.LoadpointWarnings { + if !slices.Contains(warnings, warning) { + warnings = append(warnings, warning) + } + } + if bytes.Equal(normalized, d.Config) && slices.Equal(warnings, d.ValidationWarnings) { return d, nil } + if !bytes.Equal(normalized, d.Config) { + d.NormalizedFromRevision = d.Revision + } d.Config = normalized - d.NormalizedFromRevision = d.Revision - d.ValidationWarnings = append([]string(nil), cfg.LoadpointWarnings...) + d.ValidationWarnings = warnings d.Revision++ d.SHA256 = digest(d.Config, d.BaseRevision) raw, err := json.Marshal(d) @@ -318,6 +327,11 @@ func (s *Service) checkedLocked(liveRevision, revision int64, sha string) (*conf if !bytes.Equal(normalized, d.Config) { return nil, fmt.Errorf("%w: run setup_validate and review its returned draft before saving", ErrConflict) } + for _, warning := range cfg.LoadpointWarnings { + if !slices.Contains(d.ValidationWarnings, warning) { + return nil, fmt.Errorf("%w: run setup_validate and review its returned warnings before saving", ErrConflict) + } + } return cfg, nil } diff --git a/web/app.css b/web/app.css index 7eb947ef2..920be11ee 100644 --- a/web/app.css +++ b/web/app.css @@ -2190,6 +2190,7 @@ body.ftw-app .settings-status { font-family: var(--mono); } body.ftw-app .settings-status.success { color: var(--green-e); } +body.ftw-app .settings-status.warning { color: var(--amber); } body.ftw-app .settings-status.error { color: var(--red-e); } body.ftw-app .creds-badge { border-radius: 999px; diff --git a/web/settings-shell.test.mjs b/web/settings-shell.test.mjs index e989b851b..6f940efbc 100644 --- a/web/settings-shell.test.mjs +++ b/web/settings-shell.test.mjs @@ -61,6 +61,7 @@ function loadShell(saveResponse, ok = true) { const requests = []; const responses = {}; const alerts = []; + const timers = []; const sandbox = { window: { FTWSettings: { tabs: {} } }, document, @@ -73,13 +74,13 @@ function loadShell(saveResponse, ok = true) { }, // The shell only uses timers to clear the "Saved" status and to poll after // a restart; neither is what these tests are about. - setTimeout: () => 0, + setTimeout: fn => { timers.push(fn); return timers.length; }, Date, JSON, Array, Object, String, console, }; sandbox.globalThis = sandbox; vm.createContext(sandbox); vm.runInContext(source, sandbox); - return { elements, document, requests, responses, alerts, tabs: sandbox.window.FTWSettings.tabs, + return { elements, document, requests, responses, alerts, timers, tabs: sandbox.window.FTWSettings.tabs, loadTab: file => vm.runInContext(readFileSync(new URL(file, import.meta.url), "utf8"), sandbox) }; } @@ -440,6 +441,26 @@ describe("late driver profile fields", () => { }); describe("the settings shell after a save", () => { + it("keeps charger warnings visible after a successful save", async () => { + const rig = loadShell({warnings: ["Missing charger connection: select a configured charger."]}); + rig.elements["settings-save"].handlers.click(); + await settled(); + const status = rig.elements["settings-status"]; + assert.match(status.textContent, /Saved.*Missing charger connection/); + assert.equal(status.className, "settings-status warning"); + assert.equal(status.attributes.role, "alert"); + assert.equal(rig.timers.length, 0, "a warning must not disappear on a timer"); + }); + it("does not let an earlier success timer clear a later warning", async () => { + const rig = loadShell({}); + rig.elements["settings-save"].handlers.click(); + await settled(); + rig.responses["/api/config"] = {warnings: ["Missing charger connection."]}; + rig.elements["settings-save"].handlers.click(); + await settled(); + rig.timers.forEach(fn => fn()); + assert.match(rig.elements["settings-status"].textContent, /Missing charger connection/); + }); it("calls the tab back so it can ask the box again", async () => { const { elements, tabs } = loadShell({ restart_required: false, restart_reasons: [] }); let called = 0; diff --git a/web/settings.js b/web/settings.js index 901b2b4be..0d6c39619 100644 --- a/web/settings.js +++ b/web/settings.js @@ -158,8 +158,12 @@ return r.json(); }) .then(function (res) { - setStatus("Saved โœ“", "success"); - setTimeout(function () { setStatus(""); }, 2000); + var warnings = res && Array.isArray(res.warnings) ? res.warnings : []; + var savedText = "Saved โœ“" + (warnings.length ? " " + warnings.join(" ") : ""); + setStatus(savedText, warnings.length ? "warning" : "success"); + if (!warnings.length) setTimeout(function () { + if (statusEl.textContent === savedText) setStatus(""); + }, 2000); // A tab that shows what the box says about itself, rather than what // the form holds, is stale from here until it asks again. var saved = S.tabs[currentTab]; @@ -315,6 +319,7 @@ function setStatus(msg, kind) { statusEl.textContent = msg || ""; statusEl.className = "settings-status" + (kind ? " " + kind : ""); + statusEl.setAttribute("role", kind === "warning" || kind === "error" ? "alert" : "status"); } // Async tabs register a field when they insert it, before the user can edit it. diff --git a/web/settings/tabs/loadpoint-connections.test.mjs b/web/settings/tabs/loadpoint-connections.test.mjs new file mode 100644 index 000000000..0a16a8d4b --- /dev/null +++ b/web/settings/tabs/loadpoint-connections.test.mjs @@ -0,0 +1,49 @@ +import assert from 'node:assert/strict'; +import { describe, it } from 'node:test'; +globalThis.window = {}; +await import('./loadpoints.js'); +const S = window.FTWSettings; +const escHtml = s => String(s).replaceAll('&', '&').replaceAll('<', '<').replaceAll('>', '>').replaceAll('"', '"'); +const render = config => S.tabs.loadpoints.render({config, escHtml, help: () => ''}); +const config = () => ({ + drivers: [{name: 'charger-current', lua: 'drivers/charger.lua'}], + loadpoints: [{id: 'garage', driver_name: 'charger-old'}] +}); + +describe('saved charger connections', () => { + it('shows the missing saved connection without selecting a replacement', () => { + S.catalogByLua = {'drivers/charger.lua': {capabilities: ['ev']}}; + S.ocppStatus = {enabled: false, chargers: []}; + const cfg = config(), html = render(cfg); + assert.match(html, /charger-old \(missing connection\)/); + assert.match(html, /data-charger-connection-status="0" role="alert"/); + assert.match(html, /Select a configured charger or remove this charger/); + assert.equal(cfg.loadpoints[0].driver_name, 'charger-old'); + const savedChoice = html.match(/