From 1c1bf32bfa62ba66652e2978f17dbdaab2e2fe4a Mon Sep 17 00:00:00 2001 From: Leitet Date: Mon, 3 Aug 2026 15:34:15 +0200 Subject: [PATCH] fix(api): apply a POSTed config through the same path as a file edit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit POST /api/config hot-applied a hand-picked subset of control fields and swapped the shared config pointer itself, which left the configreload watcher diffing new against new — everything the handler didn't copy, starting with a first-time site-meter designation, never reached the running controller until restart. configreload.Apply is now the one apply path; the handler calls it with main.go's applier closure (Deps.ConfigApplier), the same closure the watcher runs. Fixes #760. Co-Authored-By: Claude Fable 5 --- .changeset/760-one-config-apply-path.md | 5 + go/cmd/ftw/main.go | 416 ++++++++++++----------- go/internal/api/api.go | 34 +- go/internal/api/api_config_apply_test.go | 98 ++++++ go/internal/configreload/watcher.go | 64 ++-- go/internal/configreload/watcher_test.go | 51 +++ 6 files changed, 425 insertions(+), 243 deletions(-) create mode 100644 .changeset/760-one-config-apply-path.md create mode 100644 go/internal/api/api_config_apply_test.go diff --git a/.changeset/760-one-config-apply-path.md b/.changeset/760-one-config-apply-path.md new file mode 100644 index 00000000..9aa28d95 --- /dev/null +++ b/.changeset/760-one-config-apply-path.md @@ -0,0 +1,5 @@ +--- +"ftw": patch +--- + +Saving a config through the API now applies it exactly like a file edit. Previously POST /api/config hot-applied only a hand-picked subset of control fields and swapped the shared config pointer itself, which blinded the config watcher's own diff — so a site meter set for the first time (the setup wizard's normal path) never reached the running controller: the dashboard showed Grid 0 W and an inflated Load, and dispatch had no site boundary until a process restart. Both paths now run one shared apply, so hot-reload of the site meter, slew enable, DC-link protection, inverter groups, fuse parameters and the mpc/loadmodel sync all work from the UI too. diff --git a/go/cmd/ftw/main.go b/go/cmd/ftw/main.go index 6b989e5e..72ceaa93 100644 --- a/go/cmd/ftw/main.go +++ b/go/cmd/ftw/main.go @@ -711,223 +711,226 @@ func main() { var calSvc *calendar.Service // ---- Config hot-reload watcher ---- - watcher, err := configreload.New(*configPath, cfgMu, cfg, ctrlMu, ctrl, - func(newCfg, oldCfg *config.Config) { - // Restore EV charger password from state.db (not in YAML). - if newCfg.EVCharger != nil { - if pw, ok := st.LoadConfig("ev_charger_password"); ok { - newCfg.EVCharger.Password = pw - } - } - // Restore CalDAV password from state.db (not in YAML). Any CalDAV - // change is restart-gated because the native server and client must - // switch credentials, paths, and listeners atomically. - if newCfg.CalDAV != nil { - if pw, ok := st.LoadConfig("caldav_password"); ok { - newCfg.CalDAV.Password = pw - } - } - // Driver paths are already resolved by config.Load; no extra - // work needed here. Re-apply the battery SoC-window → driver - // config mapping so a hot-edited soc_max reaches the driver too. - reg.Reload(ctx, - config.WithBatterySoCBounds(newCfg.Drivers, newCfg.Batteries), - newCfg.Site.TroubleshootingMode) - // Refresh capacities — mutate the existing map in place so - // Deps.Capacities (a map header captured at init) sees the - // update. Rebinding the local variable would orphan the - // reference the api server still holds. - capMu.Lock() - for k := range capacities { - delete(capacities, k) - } - for k := range telemetryCapacities { - delete(telemetryCapacities, k) - } - // Re-scan the catalog so a hot-edited Lua driver's - // capability change is picked up by the EV-classification - // filter on the very next reload tick. - reloadCatalog, err := drivers.LoadCatalogMulti(*userDriversDirFlag, resolveDriverDir()) - if err != nil || len(reloadCatalog) == 0 { - slog.Warn("driver catalog reload failed; retaining last known catalog", - "err", err, "entries", len(reloadCatalog)) - reloadCatalog = driverCatalog + // Named because two callers share it: the fsnotify watcher created + // below and POST /api/config (Deps.ConfigApplier), so a config saved + // through the API is applied exactly like an edit of the file (#760). + applyConfigChange := func(newCfg, oldCfg *config.Config) { + // Restore EV charger password from state.db (not in YAML). + if newCfg.EVCharger != nil { + if pw, ok := st.LoadConfig("ev_charger_password"); ok { + newCfg.EVCharger.Password = pw + } + } + // Restore CalDAV password from state.db (not in YAML). Any CalDAV + // change is restart-gated because the native server and client must + // switch credentials, paths, and listeners atomically. + if newCfg.CalDAV != nil { + if pw, ok := st.LoadConfig("caldav_password"); ok { + newCfg.CalDAV.Password = pw + } + } + // Driver paths are already resolved by config.Load; no extra + // work needed here. Re-apply the battery SoC-window → driver + // config mapping so a hot-edited soc_max reaches the driver too. + reg.Reload(ctx, + config.WithBatterySoCBounds(newCfg.Drivers, newCfg.Batteries), + newCfg.Site.TroubleshootingMode) + // Refresh capacities — mutate the existing map in place so + // Deps.Capacities (a map header captured at init) sees the + // update. Rebinding the local variable would orphan the + // reference the api server still holds. + capMu.Lock() + for k := range capacities { + delete(capacities, k) + } + for k := range telemetryCapacities { + delete(telemetryCapacities, k) + } + // Re-scan the catalog so a hot-edited Lua driver's + // capability change is picked up by the EV-classification + // filter on the very next reload tick. + reloadCatalog, err := drivers.LoadCatalogMulti(*userDriversDirFlag, resolveDriverDir()) + if err != nil || len(reloadCatalog) == 0 { + slog.Warn("driver catalog reload failed; retaining last known catalog", + "err", err, "entries", len(reloadCatalog)) + reloadCatalog = driverCatalog + } else { + driverCatalog = reloadCatalog + } + for k, v := range driverCapacitiesFrom(newCfg.Drivers, newCfg.Loadpoints, reloadCatalog, true) { + capacities[k] = v + } + for k, v := range driverCapacitiesFrom(newCfg.Drivers, newCfg.Loadpoints, reloadCatalog, false) { + telemetryCapacities[k] = v + } + observeOnly = config.ObserveOnlyDriverSet(newCfg) + capMu.Unlock() + warnIfEVHasBatteryCapacity(newCfg.Drivers, newCfg.Loadpoints, reloadCatalog) + + // Swap inverter-group tags (#143) and per-driver power + // limits (#145) together. Taken under ctrlMu because + // ComputeDispatch reads State.InverterGroups + .DriverLimits; + // a bare replace would race with the control loop's 5 s tick. + ctrlMu.Lock() + ctrl.InverterGroups = inverterGroupsFrom(newCfg.Drivers) + ctrl.SupportsPVCurtail = supportsPVCurtailFrom(newCfg.Drivers) + ctrl.DriverLimits = driverLimitsFrom(newCfg.Drivers, newCfg.Batteries) + // Fuse params + safety margin: previously startup-only. + // Hot-reload them so operators can tune the per-phase margin + // from the UI without restarting (e.g. raising it after the + // inverter's own protection trips, lowering it to recover + // last few hundred W of arbitrage headroom). + ctrl.SiteFuseAmps = newCfg.Fuse.MaxAmps + ctrl.SiteFuseVoltage = newCfg.Fuse.Voltage + ctrl.SiteFusePhases = newCfg.Fuse.Phases + // Mirror the startup-path default semantics — nil → 0.5, + // explicit 0 → disabled. See EffectiveSafetyMarginA. + ctrl.SiteFuseSafetyA = newCfg.Fuse.EffectiveSafetyMarginA() + ctrl.MaxExportW = newCfg.Site.MaxExportW + ctrlMu.Unlock() + + // Keep the loadpoint controller's per-phase EV fuse clamp in + // sync with hot-reloaded fuse params — previously startup-only, + // so an operator tuning max_amps / margin from the UI updated + // the control-package battery lever (above) but left the EV + // clamp on the stale startup value until restart. SetSiteFuse + // takes its own lock; call it outside ctrlMu. + if lpController != nil { + lpController.SetSiteFuse(loadpoint.SiteFuse{ + MaxAmps: newCfg.Fuse.MaxAmps, + Voltage: newCfg.Fuse.Voltage, + PhaseCnt: newCfg.Fuse.Phases, + }) + } + + // Site-meter swap propagation. The configreload watcher + // already updated ctrl.SiteMeterDriver under ctrlMu before + // this applier ran, so the dispatch loop reads from the + // right driver from the next tick. Two more sites cached + // the meter at construction and need the same hot-update + // treatment: + // - mpc.Service.SiteMeter — used by reactive replan to + // compute actual site load (grid − pv − bat). + // - loadmodel.Service.SiteMeter — drives twin learning; + // leaving it stale teaches the load model from a meter + // that may not even be emitting any more. + if newCfg.SiteMeterDriver() != oldCfg.SiteMeterDriver() { + if mpcSvc != nil { + mpcSvc.SetSiteMeter(newCfg.SiteMeterDriver()) + } + if loadSvc != nil { + loadSvc.SetSiteMeter(newCfg.SiteMeterDriver()) + } + slog.Info("site-meter hot-reloaded into mpc + loadmodel", + "driver", newCfg.SiteMeterDriver()) + } + + // Push the new pool totals into the planner so its next + // replan uses the right CapacityWh / MaxChargeW / + // MaxDischargeW. Without this the MPC keeps the snapshot + // it took at buildMPC time; SoC % and terminal credit go + // stale after an EV loadpoint is added/removed. Codex P1 + // on PR #121. + if mpcSvc != nil { + fleet := mpcBatteryFleetFromConfig(newCfg, capacities) + totalCap, maxChg, maxDis := aggregateBatteryFleetLimits(newCfg, fleet) + mpcSvc.UpdateBatteryFleet(fleet, totalCap, maxChg, maxDis) + slog.Info("mpc: capacity updated via hot-reload", + "capacity_wh", totalCap, "max_charge_w", maxChg, "max_discharge_w", maxDis) + } + + // Hot-reload EV loadpoints so operators can add / remove / + // retune them without restarting. Manager preserves + // observed state across reloads (plug status, session + // anchor, current SoC estimate) — see loadpoint.Manager.Load. + lpMgr.Load(buildLoadpointConfigs(newCfg.Loadpoints)) + hydrateLoadpointSurplusOnly() + + // Notifications: rebuild the provider from fresh config + // (handles the cold-start case where the initial config + // had no notifications: block and notifProvider was nil), + // wire it onto the service, then reset the rule-engine + // per-outage latch. All calls are nil-safe. + newProv := notifications.NewProvider(newCfg.Notifications) + notifProvider = newProv + var newPub notifications.Publisher + if newProv != nil { + newPub = newProv + } + notifSvc.SetPublisher(newPub) + notifSvc.Reload(newCfg.Notifications) + + // Home Assistant: hot-reload broker / credentials / publish + // interval / driver list. Bridge.Reload tears down the paho + // client and re-publishes discovery so an operator changing + // the broker IP from Settings sees HA reconnect within a + // second — no process restart required. + // + // Three transitions to handle: + // running → running: Bridge.Reload swaps connection. + // running → disabled: Stop the existing bridge. + // disabled → enabled: Start a fresh bridge (handles both + // the "previously toggled off" case and + // the "Start failed at boot, operator + // fixed the broker" recovery path). + haEnabled := newCfg.HomeAssistant != nil && newCfg.HomeAssistant.Enabled + switch { + case haBridge != nil && haEnabled: + if err := haBridge.Reload(newCfg.HomeAssistant, reg.Names()); err != nil { + slog.Warn("HA bridge reload failed", "err", err) } else { - driverCatalog = reloadCatalog - } - for k, v := range driverCapacitiesFrom(newCfg.Drivers, newCfg.Loadpoints, reloadCatalog, true) { - capacities[k] = v - } - for k, v := range driverCapacitiesFrom(newCfg.Drivers, newCfg.Loadpoints, reloadCatalog, false) { - telemetryCapacities[k] = v + slog.Info("HA bridge reloaded", "broker", newCfg.HomeAssistant.Broker) } - observeOnly = config.ObserveOnlyDriverSet(newCfg) - capMu.Unlock() - warnIfEVHasBatteryCapacity(newCfg.Drivers, newCfg.Loadpoints, reloadCatalog) - - // Swap inverter-group tags (#143) and per-driver power - // limits (#145) together. Taken under ctrlMu because - // ComputeDispatch reads State.InverterGroups + .DriverLimits; - // a bare replace would race with the control loop's 5 s tick. - ctrlMu.Lock() - ctrl.InverterGroups = inverterGroupsFrom(newCfg.Drivers) - ctrl.SupportsPVCurtail = supportsPVCurtailFrom(newCfg.Drivers) - ctrl.DriverLimits = driverLimitsFrom(newCfg.Drivers, newCfg.Batteries) - // Fuse params + safety margin: previously startup-only. - // Hot-reload them so operators can tune the per-phase margin - // from the UI without restarting (e.g. raising it after the - // inverter's own protection trips, lowering it to recover - // last few hundred W of arbitrage headroom). - ctrl.SiteFuseAmps = newCfg.Fuse.MaxAmps - ctrl.SiteFuseVoltage = newCfg.Fuse.Voltage - ctrl.SiteFusePhases = newCfg.Fuse.Phases - // Mirror the startup-path default semantics — nil → 0.5, - // explicit 0 → disabled. See EffectiveSafetyMarginA. - ctrl.SiteFuseSafetyA = newCfg.Fuse.EffectiveSafetyMarginA() - ctrl.MaxExportW = newCfg.Site.MaxExportW - ctrlMu.Unlock() - - // Keep the loadpoint controller's per-phase EV fuse clamp in - // sync with hot-reloaded fuse params — previously startup-only, - // so an operator tuning max_amps / margin from the UI updated - // the control-package battery lever (above) but left the EV - // clamp on the stale startup value until restart. SetSiteFuse - // takes its own lock; call it outside ctrlMu. - if lpController != nil { - lpController.SetSiteFuse(loadpoint.SiteFuse{ - MaxAmps: newCfg.Fuse.MaxAmps, - Voltage: newCfg.Fuse.Voltage, - PhaseCnt: newCfg.Fuse.Phases, - }) + case haBridge != nil && !haEnabled: + haBridge.Stop() + haBridge = nil + deps.HA = nil + slog.Info("HA bridge stopped (disabled in config)") + case haBridge == nil && haEnabled: + if bridge, err := ha.Start(newCfg.HomeAssistant, tel, ctrl, ctrlMu, reg.Names(), haCallbacks(ctx, ctrl, ctrlMu, st, mpcSvc), mpcPlanSource(mpcSvc), haEnergySource(st)); err != nil { + slog.Warn("HA bridge start failed", "err", err) + } else { + haBridge = bridge + deps.HA = bridge + slog.Info("HA bridge started", "broker", newCfg.HomeAssistant.Broker) } + } - // Site-meter swap propagation. The configreload watcher - // already updated ctrl.SiteMeterDriver under ctrlMu before - // this applier ran, so the dispatch loop reads from the - // right driver from the next tick. Two more sites cached - // the meter at construction and need the same hot-update - // treatment: - // - mpc.Service.SiteMeter — used by reactive replan to - // compute actual site load (grid − pv − bat). - // - loadmodel.Service.SiteMeter — drives twin learning; - // leaving it stale teaches the load model from a meter - // that may not even be emitting any more. - if newCfg.SiteMeterDriver() != oldCfg.SiteMeterDriver() { - if mpcSvc != nil { - mpcSvc.SetSiteMeter(newCfg.SiteMeterDriver()) - } - if loadSvc != nil { - loadSvc.SetSiteMeter(newCfg.SiteMeterDriver()) - } - slog.Info("site-meter hot-reloaded into mpc + loadmodel", - "driver", newCfg.SiteMeterDriver()) + // Weather diff → push live into the PV twin + forecast + // fetcher without a process restart. Users adjust rated PV + // + lat/lon from Settings and expect the change to take + // effect right away. + if newCfg.Weather != nil { + oldLat, oldLon, oldRated := 0.0, 0.0, 0.0 + if oldCfg.Weather != nil { + oldLat = oldCfg.Weather.Latitude + oldLon = oldCfg.Weather.Longitude + oldRated = oldCfg.Weather.PVRatedW } - - // Push the new pool totals into the planner so its next - // replan uses the right CapacityWh / MaxChargeW / - // MaxDischargeW. Without this the MPC keeps the snapshot - // it took at buildMPC time; SoC % and terminal credit go - // stale after an EV loadpoint is added/removed. Codex P1 - // on PR #121. - if mpcSvc != nil { - fleet := mpcBatteryFleetFromConfig(newCfg, capacities) - totalCap, maxChg, maxDis := aggregateBatteryFleetLimits(newCfg, fleet) - mpcSvc.UpdateBatteryFleet(fleet, totalCap, maxChg, maxDis) - slog.Info("mpc: capacity updated via hot-reload", - "capacity_wh", totalCap, "max_charge_w", maxChg, "max_discharge_w", maxDis) - } - - // Hot-reload EV loadpoints so operators can add / remove / - // retune them without restarting. Manager preserves - // observed state across reloads (plug status, session - // anchor, current SoC estimate) — see loadpoint.Manager.Load. - lpMgr.Load(buildLoadpointConfigs(newCfg.Loadpoints)) - hydrateLoadpointSurplusOnly() - - // Notifications: rebuild the provider from fresh config - // (handles the cold-start case where the initial config - // had no notifications: block and notifProvider was nil), - // wire it onto the service, then reset the rule-engine - // per-outage latch. All calls are nil-safe. - newProv := notifications.NewProvider(newCfg.Notifications) - notifProvider = newProv - var newPub notifications.Publisher - if newProv != nil { - newPub = newProv - } - notifSvc.SetPublisher(newPub) - notifSvc.Reload(newCfg.Notifications) - - // Home Assistant: hot-reload broker / credentials / publish - // interval / driver list. Bridge.Reload tears down the paho - // client and re-publishes discovery so an operator changing - // the broker IP from Settings sees HA reconnect within a - // second — no process restart required. - // - // Three transitions to handle: - // running → running: Bridge.Reload swaps connection. - // running → disabled: Stop the existing bridge. - // disabled → enabled: Start a fresh bridge (handles both - // the "previously toggled off" case and - // the "Start failed at boot, operator - // fixed the broker" recovery path). - haEnabled := newCfg.HomeAssistant != nil && newCfg.HomeAssistant.Enabled - switch { - case haBridge != nil && haEnabled: - if err := haBridge.Reload(newCfg.HomeAssistant, reg.Names()); err != nil { - slog.Warn("HA bridge reload failed", "err", err) - } else { - slog.Info("HA bridge reloaded", "broker", newCfg.HomeAssistant.Broker) + newRated := newCfg.Weather.PVRatedW + if newRated > 0 && newRated != oldRated { + if pvSvc != nil { + pvSvc.SetRated(newRated) } - case haBridge != nil && !haEnabled: - haBridge.Stop() - haBridge = nil - deps.HA = nil - slog.Info("HA bridge stopped (disabled in config)") - case haBridge == nil && haEnabled: - if bridge, err := ha.Start(newCfg.HomeAssistant, tel, ctrl, ctrlMu, reg.Names(), haCallbacks(ctx, ctrl, ctrlMu, st, mpcSvc), mpcPlanSource(mpcSvc), haEnergySource(st)); err != nil { - slog.Warn("HA bridge start failed", "err", err) - } else { - haBridge = bridge - deps.HA = bridge - slog.Info("HA bridge started", "broker", newCfg.HomeAssistant.Broker) + if forecastSvc != nil { + forecastSvc.RatedPVW = newRated } } - - // Weather diff → push live into the PV twin + forecast - // fetcher without a process restart. Users adjust rated PV - // + lat/lon from Settings and expect the change to take - // effect right away. - if newCfg.Weather != nil { - oldLat, oldLon, oldRated := 0.0, 0.0, 0.0 - if oldCfg.Weather != nil { - oldLat = oldCfg.Weather.Latitude - oldLon = oldCfg.Weather.Longitude - oldRated = oldCfg.Weather.PVRatedW + newLat := newCfg.Weather.Latitude + newLon := newCfg.Weather.Longitude + if newLat != oldLat || newLon != oldLon { + if pvSvc != nil { + pvSvc.ClearSky = func(t time.Time) float64 { return forecast.ClearSkyW(newLat, newLon, t) } } - newRated := newCfg.Weather.PVRatedW - if newRated > 0 && newRated != oldRated { - if pvSvc != nil { - pvSvc.SetRated(newRated) - } - if forecastSvc != nil { - forecastSvc.RatedPVW = newRated - } - } - newLat := newCfg.Weather.Latitude - newLon := newCfg.Weather.Longitude - if newLat != oldLat || newLon != oldLon { - if pvSvc != nil { - pvSvc.ClearSky = func(t time.Time) float64 { return forecast.ClearSkyW(newLat, newLon, t) } - } - if forecastSvc != nil { - forecastSvc.Lat = newLat - forecastSvc.Lon = newLon - } - slog.Info("weather location updated", "lat", newLat, "lon", newLon) + if forecastSvc != nil { + forecastSvc.Lat = newLat + forecastSvc.Lon = newLon } + slog.Info("weather location updated", "lat", newLat, "lon", newLon) } - }) + } + } + watcher, err := configreload.New(*configPath, cfgMu, cfg, ctrlMu, ctrl, applyConfigChange) if err != nil { slog.Warn("could not start config watcher", "err", err) } else { @@ -2066,6 +2069,7 @@ func main() { State: st, CapMu: capMu, Capacities: capacities, TelemetryCapacities: telemetryCapacities, CfgMu: cfgMu, Cfg: cfg, ConfigPath: *configPath, + ConfigApplier: applyConfigChange, DriverDir: resolveDriverDir(), UserDriverDir: *userDriversDirFlag, DriverMQTTFactory: reg.MQTTFactory, diff --git a/go/internal/api/api.go b/go/internal/api/api.go index 969d7542..3e13313c 100644 --- a/go/internal/api/api.go +++ b/go/internal/api/api.go @@ -27,6 +27,7 @@ import ( "github.com/srcfl/ftw/go/internal/battery" "github.com/srcfl/ftw/go/internal/calendar" "github.com/srcfl/ftw/go/internal/config" + "github.com/srcfl/ftw/go/internal/configreload" "github.com/srcfl/ftw/go/internal/control" "github.com/srcfl/ftw/go/internal/driverrepo" "github.com/srcfl/ftw/go/internal/drivers" @@ -88,6 +89,14 @@ type Deps struct { SelfTune *selftune.Coordinator DtS float64 // control interval seconds (for model τ / age displays) SaveConfig func(path string, c *config.Config) error // injection for testability + // ConfigApplier is main.go's config-applied callback — the same + // closure the configreload watcher runs (registry reload with SoC + // bounds, capacities, inverter groups, fuse and mpc/loadmodel + // site-meter sync). Injected so POST /api/config applies a saved + // config exactly like a file edit would. Nil (tests, minimal + // embeddings) still applies control-level fields via + // configreload.Apply; only the callback's extras are skipped. + ConfigApplier configreload.Applier WebDir string // static assets root (default "web") ColdDir string // cold-storage root for parquet rolloff; empty disables cold fallback DataDir string // complete persistent-data root used by portable backups @@ -1272,23 +1281,18 @@ func (s *Server) handlePostConfig(w http.ResponseWriter, r *http.Request) { slog.Warn("failed to persist ev_charger_password", "err", err) } } - // Apply control-level changes immediately (file watcher will also pick - // this up but we're snappier). - s.deps.CtrlMu.Lock() - s.deps.Ctrl.SetGridTarget(newCfg.Site.GridTargetW) - s.deps.Ctrl.GridToleranceW = newCfg.Site.GridToleranceW - s.deps.Ctrl.SlewRateW = newCfg.Site.SlewRateW - s.deps.Ctrl.MinDispatchIntervalS = newCfg.Site.MinDispatchIntervalS - s.deps.Ctrl.PVSurplusAbsorbSoCCapPct = newCfg.Site.PVSurplusAbsorbSoCCapPct - s.deps.Ctrl.PVSurplusAbsorbThresholdW = newCfg.Site.PVSurplusAbsorbThresholdW - s.deps.CtrlMu.Unlock() - if s.deps.Registry != nil { + // One apply path, shared with the file watcher. Hand-applying a + // subset here and swapping the shared pointer is what #760 was: the + // watcher then diffed new against new, so everything this handler + // didn't copy — starting with the site-meter designation — never + // reached the running controller until a restart. + configreload.Apply(s.deps.CfgMu, s.deps.Cfg, s.deps.CtrlMu, s.deps.Ctrl, + &newCfg, s.deps.ConfigApplier) + if s.deps.ConfigApplier == nil && s.deps.Registry != nil { + // Minimal embeddings without main.go's callback still need the + // new driver set running. s.deps.Registry.Reload(r.Context(), newCfg.Drivers, newCfg.Site.TroubleshootingMode) } - // Update shared cfg pointer - s.deps.CfgMu.Lock() - *s.deps.Cfg = newCfg - s.deps.CfgMu.Unlock() slog.Info("config updated via API", "restart_required", len(restartReasons) > 0) writeJSON(w, 200, map[string]any{ "status": "ok", diff --git a/go/internal/api/api_config_apply_test.go b/go/internal/api/api_config_apply_test.go new file mode 100644 index 00000000..6802843f --- /dev/null +++ b/go/internal/api/api_config_apply_test.go @@ -0,0 +1,98 @@ +package api + +import ( + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" + + "github.com/srcfl/ftw/go/internal/config" + "github.com/srcfl/ftw/go/internal/control" +) + +// A config saved through the API must reach the running controller the +// same way a file edit does. The regression this guards (#760): the +// handler applied a hand-picked subset of control fields and swapped the +// shared config pointer itself, which left the configreload watcher +// diffing new against new — so a site meter set for the first time never +// reached the controller, grid_w pegged at 0 and load_w inflated to +// |pv_w| until a process restart. + +const firstSiteMeterConfig = `{ + "site": {"name": "Test", "smoothing_alpha": 0.3}, + "fuse": {"max_amps": 16, "phases": 3, "voltage": 230}, + "api": {"port": 8080}, + "drivers": [{ + "name": "foxess", + "lua": "drivers/foxess_h3_smart.lua", + "is_site_meter": true, + "capabilities": {"modbus": {"host": "192.0.2.10", "port": 502, "unit_id": 247}} + }] +}` + +func postConfigServer(t *testing.T, applier func(newCfg, oldCfg *config.Config)) (*Server, *control.State, *config.Config) { + t.Helper() + var cfgMu sync.RWMutex + var ctrlMu sync.Mutex + cfg := &config.Config{} + ctrl := control.NewState(0, 42, cfg.SiteMeterDriver()) + srv := New(&Deps{ + Ctrl: ctrl, CtrlMu: &ctrlMu, + Cfg: cfg, CfgMu: &cfgMu, + ConfigPath: t.TempDir() + "/config.yaml", + DriverDir: t.TempDir(), + UserDriverDir: t.TempDir(), + SaveConfig: func(string, *config.Config) error { return nil }, + ConfigApplier: applier, + }) + return srv, ctrl, cfg +} + +func postConfig(t *testing.T, srv *Server, body string) int { + t.Helper() + req := httptest.NewRequest(http.MethodPost, "/api/config", strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + rr := httptest.NewRecorder() + srv.Handler().ServeHTTP(rr, req) + if rr.Code != 200 { + t.Logf("response: %s", rr.Body.String()) + } + return rr.Code +} + +func TestPostConfigFirstSiteMeterReachesControl(t *testing.T) { + srv, ctrl, cfg := postConfigServer(t, nil) + + if code := postConfig(t, srv, firstSiteMeterConfig); code != 200 { + t.Fatalf("POST /api/config = %d, want 200", code) + } + + if got := ctrl.SiteMeterDriver; got != "foxess" { + t.Fatalf("Ctrl.SiteMeterDriver = %q after POST, want %q — the saved site meter never reached the running controller", got, "foxess") + } + if got := cfg.SiteMeterDriver(); got != "foxess" { + t.Fatalf("shared config SiteMeterDriver() = %q, want %q", got, "foxess") + } +} + +func TestPostConfigRunsTheSharedApplierWithOldSnapshot(t *testing.T) { + var gotNew, gotOld *config.Config + srv, _, _ := postConfigServer(t, func(newCfg, oldCfg *config.Config) { + gotNew, gotOld = newCfg, oldCfg + }) + + if code := postConfig(t, srv, firstSiteMeterConfig); code != 200 { + t.Fatalf("POST /api/config = %d, want 200", code) + } + + if gotNew == nil { + t.Fatal("ConfigApplier was never called — registry/capacities/mpc sync all silently skipped") + } + if got := gotNew.SiteMeterDriver(); got != "foxess" { + t.Fatalf("applier newCfg.SiteMeterDriver() = %q, want %q", got, "foxess") + } + if got := gotOld.SiteMeterDriver(); got != "" { + t.Fatalf("applier oldCfg.SiteMeterDriver() = %q, want the pre-POST snapshot %q", got, "") + } +} diff --git a/go/internal/configreload/watcher.go b/go/internal/configreload/watcher.go index 3ea410c6..d92ba6f3 100644 --- a/go/internal/configreload/watcher.go +++ b/go/internal/configreload/watcher.go @@ -122,22 +122,43 @@ func (w *Watcher) reload() { slog.Warn("config reload failed", "err", err) return } + Apply(w.cfgMu, w.cfg, w.ctrlMu, w.ctrl, newCfg, w.applier) + slog.Info("config reload: applied") +} + +// Apply is the single apply path for a changed config: diff newCfg +// against the shared snapshot, hot-apply the control-level fields, swap +// the shared config pointer, then run the applier callback with +// (new, old). The fsnotify watcher calls it after loading the file, and +// POST /api/config calls it directly with the config it just saved. +// +// It has to be one function. The API handler used to apply a hand-picked +// subset of fields and swap the pointer itself, which left this +// package's watcher diffing new against new when the fsnotify event +// arrived — so everything the handler didn't copy, starting with the +// site-meter designation, never reached the running controller until a +// restart (#760). +func Apply( + cfgMu *sync.RWMutex, cfg *config.Config, + ctrlMu *sync.Mutex, ctrl *control.State, + newCfg *config.Config, applier Applier, +) { // Snapshot old - w.cfgMu.RLock() - oldCfg := *w.cfg - w.cfgMu.RUnlock() + cfgMu.RLock() + oldCfg := *cfg + cfgMu.RUnlock() // Apply control-level changes - w.ctrlMu.Lock() + ctrlMu.Lock() if newCfg.Site.GridTargetW != oldCfg.Site.GridTargetW { slog.Info("config reload: grid_target_w", "old", oldCfg.Site.GridTargetW, "new", newCfg.Site.GridTargetW) - w.ctrl.SetGridTarget(newCfg.Site.GridTargetW) + ctrl.SetGridTarget(newCfg.Site.GridTargetW) } if newCfg.Site.GridToleranceW != oldCfg.Site.GridToleranceW { - w.ctrl.GridToleranceW = newCfg.Site.GridToleranceW + ctrl.GridToleranceW = newCfg.Site.GridToleranceW } if newCfg.Site.SlewRateW != oldCfg.Site.SlewRateW { - w.ctrl.SlewRateW = newCfg.Site.SlewRateW + ctrl.SlewRateW = newCfg.Site.SlewRateW } newEnabled := true if newCfg.Site.SlewEnabled != nil { @@ -149,31 +170,31 @@ func (w *Watcher) reload() { } if newEnabled != oldEnabled { slog.Info("config reload: slew_enabled", "old", oldEnabled, "new", newEnabled) - w.ctrl.SlewEnabled = newEnabled + ctrl.SlewEnabled = newEnabled } if newCfg.Site.MinDispatchIntervalS != oldCfg.Site.MinDispatchIntervalS { - w.ctrl.MinDispatchIntervalS = newCfg.Site.MinDispatchIntervalS + ctrl.MinDispatchIntervalS = newCfg.Site.MinDispatchIntervalS } if newCfg.Site.PVSurplusAbsorbSoCCapPct != oldCfg.Site.PVSurplusAbsorbSoCCapPct { slog.Info("config reload: pv_surplus_absorb_soc_cap_pct", "old", oldCfg.Site.PVSurplusAbsorbSoCCapPct, "new", newCfg.Site.PVSurplusAbsorbSoCCapPct) - w.ctrl.PVSurplusAbsorbSoCCapPct = newCfg.Site.PVSurplusAbsorbSoCCapPct + ctrl.PVSurplusAbsorbSoCCapPct = newCfg.Site.PVSurplusAbsorbSoCCapPct } if newCfg.Site.PVSurplusAbsorbThresholdW != oldCfg.Site.PVSurplusAbsorbThresholdW { - w.ctrl.PVSurplusAbsorbThresholdW = newCfg.Site.PVSurplusAbsorbThresholdW + ctrl.PVSurplusAbsorbThresholdW = newCfg.Site.PVSurplusAbsorbThresholdW } if newCfg.Site.DCLinkProtectionEnabled != oldCfg.Site.DCLinkProtectionEnabled { slog.Info("config reload: dc_link_protection_enabled", "old", oldCfg.Site.DCLinkProtectionEnabled, "new", newCfg.Site.DCLinkProtectionEnabled) - w.ctrl.DCLinkProtectionEnabled = newCfg.Site.DCLinkProtectionEnabled + ctrl.DCLinkProtectionEnabled = newCfg.Site.DCLinkProtectionEnabled } if newCfg.Site.DCLinkProtectionSoCThreshold != oldCfg.Site.DCLinkProtectionSoCThreshold { - w.ctrl.DCLinkProtectionSoCThreshold = newCfg.Site.DCLinkProtectionSoCThreshold + ctrl.DCLinkProtectionSoCThreshold = newCfg.Site.DCLinkProtectionSoCThreshold } if newCfg.Site.DCLinkProtectionMarginW != oldCfg.Site.DCLinkProtectionMarginW { - w.ctrl.DCLinkProtectionMarginW = newCfg.Site.DCLinkProtectionMarginW + ctrl.DCLinkProtectionMarginW = newCfg.Site.DCLinkProtectionMarginW } // Site-meter swap (operator moved `is_site_meter: true` from one // driver to another, or set it for the first time). Without this @@ -188,18 +209,17 @@ func (w *Watcher) reload() { if newCfg.SiteMeterDriver() != oldCfg.SiteMeterDriver() { slog.Info("config reload: site_meter", "old", oldCfg.SiteMeterDriver(), "new", newCfg.SiteMeterDriver()) - w.ctrl.SiteMeterDriver = newCfg.SiteMeterDriver() + ctrl.SiteMeterDriver = newCfg.SiteMeterDriver() } - w.ctrlMu.Unlock() + ctrlMu.Unlock() // Swap global pointer - w.cfgMu.Lock() - *w.cfg = *newCfg - w.cfgMu.Unlock() + cfgMu.Lock() + *cfg = *newCfg + cfgMu.Unlock() // Let caller handle driver registry etc. - if w.applier != nil { - w.applier(newCfg, &oldCfg) + if applier != nil { + applier(newCfg, &oldCfg) } - slog.Info("config reload: applied") } diff --git a/go/internal/configreload/watcher_test.go b/go/internal/configreload/watcher_test.go index 63827446..87d884b9 100644 --- a/go/internal/configreload/watcher_test.go +++ b/go/internal/configreload/watcher_test.go @@ -311,3 +311,54 @@ api: w.Stop() w.Stop() } + +// Apply is the one shared apply path (#760): POST /api/config calls it +// directly with the config it just saved, so a site meter set for the +// first time must reach the controller without any fsnotify round trip. +func TestApplyFirstSiteMeterWithoutAWatcher(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.yaml") + + const noDriversYAML = ` +site: + name: Test + grid_target_w: 0 +fuse: + max_amps: 16 +drivers: [] +api: + port: 8080 +` + writeConfig(t, path, noDriversYAML) + cfg, err := config.Load(path) + if err != nil { + t.Fatal(err) + } + writeConfig(t, path, minimalYAML) + newCfg, err := config.Load(path) + if err != nil { + t.Fatal(err) + } + + var cfgMu sync.RWMutex + var ctrlMu sync.Mutex + ctrl := control.NewState(0, 0, cfg.SiteMeterDriver()) + + var gotNew, gotOld *config.Config + Apply(&cfgMu, cfg, &ctrlMu, ctrl, newCfg, func(n, o *config.Config) { + gotNew, gotOld = n, o + }) + + if ctrl.SiteMeterDriver != "ferroamp" { + t.Fatalf("Ctrl.SiteMeterDriver = %q, want %q", ctrl.SiteMeterDriver, "ferroamp") + } + if cfg.SiteMeterDriver() != "ferroamp" { + t.Fatalf("shared cfg not swapped: SiteMeterDriver() = %q", cfg.SiteMeterDriver()) + } + if gotNew == nil || gotNew.SiteMeterDriver() != "ferroamp" { + t.Fatal("applier did not receive the new config") + } + if gotOld == nil || gotOld.SiteMeterDriver() != "" { + t.Fatal("applier did not receive the pre-apply snapshot as old") + } +}