From 071ccfd9fee764fc08a96b5eda2fbe2763477276 Mon Sep 17 00:00:00 2001 From: hannesb90 Date: Tue, 10 Nov 2026 09:51:14 +0100 Subject: [PATCH 1/3] feat(drivers): let a control declare what it does to heat A control may now declare asset, strategy, zone and write timings in the thermal contract's vocabulary, and a driver may list the metrics that are a heat store's temperature. /api/drivers/{name} exposes both. Nothing plans for or sends a heat command because of them; this is the declaration step of the proposal in #1994. A string control may declare a closed set of options, which the control API now enforces. A misdeclared thermal field drops the control, as an unrenderable declaration already does. nestedBlock now requires the whole field name, so temp_sensors is not read as sensors. Refs #520 Co-Authored-By: Claude Opus 5.5 Signed-off-by: hannesb90 --- .changeset/thermal-control-declaration.md | 7 + go/internal/api/api_driver_control.go | 7 + go/internal/api/api_driver_thermal_test.go | 128 ++++++++++++ go/internal/api/api_drivers_debug.go | 21 +- go/internal/drivers/catalog.go | 5 + go/internal/drivers/catalog_controls.go | 53 ++++- go/internal/drivers/catalog_thermal.go | 194 ++++++++++++++++++ go/internal/drivers/catalog_thermal_test.go | 208 ++++++++++++++++++++ go/internal/thermal/model.go | 11 ++ 9 files changed, 622 insertions(+), 12 deletions(-) create mode 100644 .changeset/thermal-control-declaration.md create mode 100644 go/internal/api/api_driver_thermal_test.go create mode 100644 go/internal/drivers/catalog_thermal.go create mode 100644 go/internal/drivers/catalog_thermal_test.go diff --git a/.changeset/thermal-control-declaration.md b/.changeset/thermal-control-declaration.md new file mode 100644 index 000000000..1f9a16182 --- /dev/null +++ b/.changeset/thermal-control-declaration.md @@ -0,0 +1,7 @@ +--- +"ftw": patch +--- + +A driver can now say what a control does to heat. A control may declare `asset` (space_heat, dhw, cooling or buffer), `strategy` (curve_offset, setpoint, on_off or discrete_mode), `zone`, `min_interval_s`, and for on_off `min_on_s` and `min_off_s`. A driver may also list `sensors`: the metrics that are a heat store's temperature. `/api/drivers/{name}` shows both. Nothing plans for or sends a heat command because of them yet. + +A string control may declare a closed set of `options`. The control API then refuses any other value instead of passing it to the driver. A control that declares these fields wrongly is left out, as an unrenderable control is today. Drivers that declare none of them are unchanged. diff --git a/go/internal/api/api_driver_control.go b/go/internal/api/api_driver_control.go index 592e1f840..c6db6952e 100644 --- a/go/internal/api/api_driver_control.go +++ b/go/internal/api/api_driver_control.go @@ -23,6 +23,7 @@ import ( "log/slog" "math" "net/http" + "slices" "sync" "time" @@ -241,6 +242,12 @@ func decodeControlValue(raw json.RawMessage, in drivers.CatalogControlInput) (an if err := json.Unmarshal(raw, &value); err != nil { return nil, errors.New("control requires a string value") } + // A declared closed set is the whole allowlist, as the declared + // bounds are for a number. Unlike a number there is no nearest + // value to clamp to, so anything else is refused. + if len(in.Options) > 0 && !slices.Contains(in.Options, value) { + return nil, errors.New("control value is not one of the declared options") + } return value, nil default: return nil, fmt.Errorf("control has unsupported input type %q", in.Type) diff --git a/go/internal/api/api_driver_thermal_test.go b/go/internal/api/api_driver_thermal_test.go new file mode 100644 index 000000000..d9e9b2707 --- /dev/null +++ b/go/internal/api/api_driver_thermal_test.go @@ -0,0 +1,128 @@ +package api + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/srcfl/ftw/go/internal/config" +) + +const controlOptionsProbeLua = `DRIVER = { + id = "probe_options", + name = "Probe options", + version = "1.0.0", + controls = { + { + id = "set_mode", asset = "dhw", strategy = "discrete_mode", + input = { type = "string", options = { "eco", "boost" } }, + }, + }, + sensors = { + { metric = "tank_temp_c", asset = "dhw" }, + }, +} + +local applied = 0 + +function driver_init(config) + host.set_make("Probe options") + host.set_poll_interval(100) +end + +function driver_poll() + host.emit_metric("applied", applied, "n") + return 100 +end + +function driver_command(action, power_w, cmd) + if action == "set_mode" then + if cmd.value == "eco" then applied = 1 elseif cmd.value == "boost" then applied = 2 else applied = 9 end + return true + end + return false +end + +function driver_default_mode() + applied = 0 +end +` + +// A declared closed set is the allowlist. A value outside it is a 400 and +// never reaches the driver, which would otherwise have to guess. +func TestDriverControlRefusesValueOutsideDeclaredOptions(t *testing.T) { + srv, tel := controlServerWithLua(t, controlOptionsProbeLua) + + rec := post(t, srv, "/api/drivers/heat/control", `{"control":"set_mode","value":"turbo","duration_s":600}`) + if rec.Code != http.StatusBadRequest { + t.Fatalf("undeclared option = %d, want 400 (body %s)", rec.Code, rec.Body.String()) + } + if hold := srv.activeControlHold("heat"); hold != nil { + t.Fatalf("hold = %+v after a refused value, want none", hold) + } + // Give the driver a few polls; a refused value must not have been sent. + time.Sleep(300 * time.Millisecond) + if v, _, ok := tel.LatestMetric("heat", "applied"); ok && v == 9 { + t.Fatalf("the refused value reached the driver") + } + + rec = post(t, srv, "/api/drivers/heat/control", `{"control":"set_mode","value":"boost","duration_s":600}`) + if rec.Code != http.StatusOK { + t.Fatalf("declared option = %d, body %s", rec.Code, rec.Body.String()) + } + waitMetric(t, tel, "heat", "applied", 2) +} + +func TestDriverDetailSurfacesThermalDeclaration(t *testing.T) { + srv := serverWithDrivers(t, + map[string]string{"probe.lua": controlOptionsProbeLua}, + &config.Config{Drivers: []config.Driver{{Name: "tank", Lua: "probe.lua"}}}) + + got := driverDetail(t, srv, "tank") + if len(got.Controls) != 1 || got.Controls[0].Thermal == nil { + t.Fatalf("controls = %+v, want one thermal control", got.Controls) + } + th := got.Controls[0].Thermal + if th.Asset != "dhw" || th.Strategy != "discrete_mode" { + t.Errorf("thermal = %+v", th) + } + if opts := got.Controls[0].Input.Options; len(opts) != 2 || opts[0] != "eco" || opts[1] != "boost" { + t.Errorf("options = %v", opts) + } + if len(got.Sensors) != 1 || got.Sensors[0].Metric != "tank_temp_c" || got.Sensors[0].Asset != "dhw" { + t.Errorf("sensors = %+v", got.Sensors) + } +} + +// A driver that declares no heat role keeps its JSON exactly as before: no +// sensors key, and no thermal key on its controls. +func TestDriverDetailOmitsThermalKeysWhenNoneDeclared(t *testing.T) { + srv := serverWithDrivers(t, + map[string]string{"probe.lua": controlDriverLua}, + &config.Config{Drivers: []config.Driver{{Name: "heat", Lua: "probe.lua"}}}) + + rec := httptest.NewRecorder() + srv.Handler().ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/api/drivers/heat", nil)) + var raw struct { + Sensors json.RawMessage `json:"sensors"` + Controls []json.RawMessage `json:"controls"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &raw); err != nil { + t.Fatalf("decode: %v", err) + } + if raw.Sensors != nil { + t.Errorf("sensors key present: %s", rec.Body.String()) + } + if len(raw.Controls) != 1 { + t.Fatalf("controls = %d, want 1", len(raw.Controls)) + } + var control map[string]any + if err := json.Unmarshal(raw.Controls[0], &control); err != nil { + t.Fatal(err) + } + if _, present := control["thermal"]; present { + t.Errorf("thermal key present on a control that declares none: %s", raw.Controls[0]) + } +} diff --git a/go/internal/api/api_drivers_debug.go b/go/internal/api/api_drivers_debug.go index 0747bc92e..5f5608bc0 100644 --- a/go/internal/api/api_drivers_debug.go +++ b/go/internal/api/api_drivers_debug.go @@ -42,6 +42,9 @@ type driverDetailResp struct { // Controls are what this driver says an operator may command. Absent // for every driver that only reports. Controls []drivers.CatalogControl `json:"controls,omitempty"` + // Sensors are the metrics this driver says are a heat store's + // temperature. Absent for every driver that declares none. + Sensors []drivers.CatalogSensor `json:"sensors,omitempty"` // Hold is the operator setting in force, if any, and when it ends. Hold *controlHold `json:"hold,omitempty"` // ControlState says whether Core has confirmed autonomous default mode. @@ -121,7 +124,10 @@ func (s *Server) handleDriverDetail(w http.ResponseWriter, r *http.Request) { resp.Identity = driverIdentityDTO{Make: make, SN: sn, MAC: mac, Endpoint: ep} } } - resp.Controls = s.driverControls(name) + if entry := s.driverCatalogEntry(name); entry != nil { + resp.Controls = entry.Controls + resp.Sensors = entry.Sensors + } resp.Hold = s.activeControlHold(name) resp.ControlState = s.driverControlState(name, resp.Hold) writeJSON(w, 200, resp) @@ -160,6 +166,15 @@ func (s *Server) driverControlState(name string, hold *controlHold) driverContro // failure belongs in the catalog endpoint, where an operator is looking at // driver files, not here. func (s *Server) driverControls(name string) []drivers.CatalogControl { + if entry := s.driverCatalogEntry(name); entry != nil { + return entry.Controls + } + return nil +} + +// driverCatalogEntry is the catalog entry for the configured driver `name`, +// or nil. See driverControls for why a parse failure is nil too. +func (s *Server) driverCatalogEntry(name string) *drivers.CatalogEntry { cfg, ok := s.configuredDriver(name) if !ok || cfg.Lua == "" { return nil @@ -173,7 +188,7 @@ func (s *Server) driverControls(name string) []drivers.CatalogControl { if err != nil { return nil } - return entry.Controls + return &entry } dir := s.deps.DriverDir @@ -184,7 +199,7 @@ func (s *Server) driverControls(name string) []drivers.CatalogControl { if err != nil { return nil } - return drivers.ControlsForDriver(entries, lua) + return drivers.EntryForDriver(entries, lua) } func (s *Server) configuredDriver(name string) (config.Driver, bool) { diff --git a/go/internal/drivers/catalog.go b/go/internal/drivers/catalog.go index b8e1c48c7..7e714ac37 100644 --- a/go/internal/drivers/catalog.go +++ b/go/internal/drivers/catalog.go @@ -77,6 +77,10 @@ type CatalogEntry struct { // Controls are the operator-facing commands the driver declares. Empty // for every driver that only reports. See catalog_controls.go. Controls []CatalogControl `json:"controls,omitempty"` + // Sensors are the metrics the driver says describe a heat store's + // temperature. Empty for every driver that declares none. See + // catalog_thermal.go. + Sensors []CatalogSensor `json:"sensors,omitempty"` // ConfigSecrets lists driver-specific config keys that the Settings // UI / setup wizard should render as password inputs and store @@ -240,6 +244,7 @@ func parseCatalogEntry(path string) (CatalogEntry, error) { e.AuthPostPath = pickString(block, "auth_post_path") e.AuthPostPaths = pickList(block, "auth_post_paths") e.Controls = pickControls(block) + e.Sensors = pickSensors(block) return e, nil } diff --git a/go/internal/drivers/catalog_controls.go b/go/internal/drivers/catalog_controls.go index 5247c72c9..b173f38d8 100644 --- a/go/internal/drivers/catalog_controls.go +++ b/go/internal/drivers/catalog_controls.go @@ -23,6 +23,9 @@ type CatalogControl struct { Label string `json:"label,omitempty"` Evidence string `json:"evidence,omitempty"` Input CatalogControlInput `json:"input"` + // Thermal says what this control does to heat, when the driver declares + // it. Nil for every other control. See catalog_thermal.go. + Thermal *CatalogThermalControl `json:"thermal,omitempty"` } // CatalogControlInput describes the single value a control carries. Min, max @@ -34,6 +37,9 @@ type CatalogControlInput struct { Max *float64 `json:"max,omitempty"` Step *float64 `json:"step,omitempty"` Unit string `json:"unit,omitempty"` + // Options is the closed set a string input accepts. Empty means any + // string, as before. + Options []string `json:"options,omitempty"` } var controlEvidence = map[string]bool{"readback": true, "write_ack": true} @@ -51,6 +57,15 @@ func validControlToken(value string) bool { // — and a control that appears for one spelling and not the other would look // like the driver, not the lookup. func ControlsForDriver(catalog []CatalogEntry, luaPath string) []CatalogControl { + if e := EntryForDriver(catalog, luaPath); e != nil { + return e.Controls + } + return nil +} + +// EntryForDriver is the catalog entry for luaPath, found as ControlsForDriver +// describes, or nil. +func EntryForDriver(catalog []CatalogEntry, luaPath string) *CatalogEntry { if luaPath == "" { return nil } @@ -58,14 +73,14 @@ func ControlsForDriver(catalog []CatalogEntry, luaPath string) []CatalogControl wantFilename := filepath.Base(wantPath) // A filename is only a fallback. When two sources contain the same // filename, the configured path is the only safe identity we have. - for _, e := range catalog { - if strings.EqualFold(filepath.ToSlash(filepath.Clean(e.Path)), wantPath) { - return e.Controls + for i := range catalog { + if strings.EqualFold(filepath.ToSlash(filepath.Clean(catalog[i].Path)), wantPath) { + return &catalog[i] } } - for _, e := range catalog { - if strings.EqualFold(e.Filename, wantFilename) { - return e.Controls + for i := range catalog { + if strings.EqualFold(catalog[i].Filename, wantFilename) { + return &catalog[i] } } return nil @@ -110,16 +125,33 @@ func pickControls(block string) []CatalogControl { Max: fieldNumber(input, "max"), Step: fieldNumber(input, "step"), } + options, ok := pickOptions(input) + if !ok { + continue + } + control.Input.Options = options switch control.Input.Type { case "number": if control.Input.Min == nil || control.Input.Max == nil || *control.Input.Min >= *control.Input.Max { continue } - case "boolean", "string": + if options != nil { + continue + } + case "boolean": + if options != nil { + continue + } + case "string": default: continue } + thermal, ok := pickThermalControl(entry, control.Input) + if !ok { + continue + } + control.Thermal = thermal out = append(out, control) } return out @@ -128,11 +160,14 @@ func pickControls(block string) []CatalogControl { // nestedBlock returns the body of `name = { … }`, matching braces so an inner // table does not end the outer one. Braces inside string literals are skipped; // a topic or a label is allowed to contain one. +// +// The name must stand alone: `temp_sensors = {` is not `sensors = {`. func nestedBlock(block, name string) string { - loc := regexp.MustCompile(regexp.QuoteMeta(name) + `\s*=\s*\{`).FindStringIndex(block) - if loc == nil { + m := regexp.MustCompile(`(?:^|[^A-Za-z0-9_])(` + regexp.QuoteMeta(name) + `\s*=\s*\{)`).FindStringSubmatchIndex(block) + if m == nil { return "" } + loc := []int{m[2], m[3]} depth := 0 inString := false for i := loc[1] - 1; i < len(block); i++ { diff --git a/go/internal/drivers/catalog_thermal.go b/go/internal/drivers/catalog_thermal.go new file mode 100644 index 000000000..e6561a16c --- /dev/null +++ b/go/internal/drivers/catalog_thermal.go @@ -0,0 +1,194 @@ +package drivers + +import ( + "math" + "regexp" + "strings" + + "github.com/srcfl/ftw/go/internal/thermal" +) + +// What a heat control does, said by the driver in the thermal contract's own +// vocabulary, so Core can reason about a heat pump, a thermostat and a tank +// without knowing any of them by name. Proposed in docs/adr/0012. +// +// This only describes. Nothing in Core plans for or sends a thermal control +// because of it; the operator path in api_driver_control.go is unchanged. +type CatalogThermalControl struct { + Asset thermal.AssetKind `json:"asset"` + Strategy thermal.InfluenceStrategy `json:"strategy"` + // Zone links the control to the sensors that judge it. Empty means the + // driver does not say. + Zone string `json:"zone,omitempty"` + // MinIntervalS is the shortest time between two writes the device or its + // vendor accepts: a cloud rate limit, or flash wear. + MinIntervalS *int `json:"min_interval_s,omitempty"` + // MinOnS and MinOffS protect a compressor from short cycling. They are + // declared only on an on_off control. + MinOnS *int `json:"min_on_s,omitempty"` + MinOffS *int `json:"min_off_s,omitempty"` +} + +// CatalogSensor names one metric the driver already emits as the temperature +// of a heat store: a room, a hot-water tank, a buffer. +type CatalogSensor struct { + Metric string `json:"metric"` + Asset thermal.AssetKind `json:"asset"` + Zone string `json:"zone,omitempty"` +} + +// The strategies a control may declare. synthetic_price is a signal Core +// would publish, not a lever a driver offers, so it is not one of them. +var controlStrategyInput = map[thermal.InfluenceStrategy]string{ + thermal.InfluenceCurveOffset: "number", + thermal.InfluenceSetpoint: "number", + thermal.InfluenceOnOff: "boolean", + thermal.InfluenceDiscreteMode: "string", +} + +var thermalControlKeys = []string{"asset", "strategy", "zone", "min_interval_s", "min_on_s", "min_off_s"} + +var metricNameRE = regexp.MustCompile(`^[a-z][a-z0-9_]*$`) + +// pickThermalControl reads the thermal fields of one control entry. ok is +// false when the entry tries to declare them and gets them wrong, and the +// caller then drops the whole control, as it does for a bad evidence kind. +// A planner later trusting a misdeclared strategy, or silently missing a +// compressor's minimum off time because it was written as a string, is the +// failure this prevents. +func pickThermalControl(entry string, input CatalogControlInput) (*CatalogThermalControl, bool) { + entry = withoutBlock(entry, "input") + declared := false + for _, key := range thermalControlKeys { + if fieldPresent(entry, key) { + declared = true + break + } + } + if !declared { + return nil, true + } + + out := &CatalogThermalControl{ + Asset: thermal.AssetKind(fieldString(entry, "asset")), + Strategy: thermal.InfluenceStrategy(fieldString(entry, "strategy")), + Zone: fieldString(entry, "zone"), + } + if !thermal.ValidAssetKind(out.Asset) { + return nil, false + } + want, known := controlStrategyInput[out.Strategy] + if !known || input.Type != want { + return nil, false + } + if out.Strategy == thermal.InfluenceDiscreteMode && len(input.Options) < 2 { + return nil, false + } + if fieldPresent(entry, "zone") && !validControlToken(out.Zone) { + return nil, false + } + + var ok bool + if out.MinIntervalS, ok = pickSeconds(entry, "min_interval_s"); !ok { + return nil, false + } + if out.MinOnS, ok = pickSeconds(entry, "min_on_s"); !ok { + return nil, false + } + if out.MinOffS, ok = pickSeconds(entry, "min_off_s"); !ok { + return nil, false + } + if (out.MinOnS != nil || out.MinOffS != nil) && out.Strategy != thermal.InfluenceOnOff { + return nil, false + } + return out, true +} + +// pickSeconds reads a whole, non-negative number of seconds. Absent is nil and +// fine; present and anything else is not. +func pickSeconds(entry, name string) (*int, bool) { + if !fieldPresent(entry, name) { + return nil, true + } + v := fieldNumber(entry, name) + if v == nil || *v < 0 || *v != math.Trunc(*v) || *v > 7*24*3600 { + return nil, false + } + s := int(*v) + return &s, true +} + +// pickOptions reads an input's `options = { "a", "b" }`. Absent is nil and +// fine. A list that holds anything but distinct control tokens is refused: +// an option the API would accept and the driver would not is the gap a +// closed set exists to remove. +func pickOptions(input string) ([]string, bool) { + if !fieldPresent(input, "options") { + return nil, true + } + body := nestedBlock(input, "options") + if strings.TrimSpace(body) == "" { + return nil, false + } + quoted := regexp.MustCompile(`"([^"]*)"`) + if strings.Trim(quoted.ReplaceAllString(body, ""), ", \t\r\n") != "" { + return nil, false + } + var out []string + seen := map[string]bool{} + for _, m := range quoted.FindAllStringSubmatch(body, -1) { + if !validControlToken(m[1]) || seen[m[1]] { + return nil, false + } + seen[m[1]] = true + out = append(out, m[1]) + } + return out, true +} + +// pickSensors reads the DRIVER block's `sensors` list. Each entry stands on +// its own, so one malformed sensor is dropped without hiding the others. +func pickSensors(block string) []CatalogSensor { + body := nestedBlock(stripLuaComments(block), "sensors") + if body == "" { + return nil + } + var out []CatalogSensor + seen := map[string]bool{} + for _, entry := range topLevelTables(body) { + s := CatalogSensor{ + Metric: fieldString(entry, "metric"), + Asset: thermal.AssetKind(fieldString(entry, "asset")), + Zone: fieldString(entry, "zone"), + } + if len(s.Metric) > 128 || !metricNameRE.MatchString(s.Metric) || seen[s.Metric] { + continue + } + if !thermal.ValidAssetKind(s.Asset) { + continue + } + if fieldPresent(entry, "zone") && !validControlToken(s.Zone) { + continue + } + seen[s.Metric] = true + out = append(out, s) + } + return out +} + +// fieldPresent reports whether `name =` appears as a whole field name, +// whatever its value. It is how a malformed value is told apart from an +// absent one. +func fieldPresent(block, name string) bool { + return regexp.MustCompile(`(?:^|[\s,{])` + regexp.QuoteMeta(name) + `\s*=`).MatchString(block) +} + +// withoutBlock blanks `name = { … }` so fields inside it are not read as the +// entry's own: a control's thermal `zone` is not something inside its input. +func withoutBlock(block, name string) string { + inner := nestedBlock(block, name) + if inner == "" { + return block + } + return strings.Replace(block, inner, strings.Repeat(" ", len(inner)), 1) +} diff --git a/go/internal/drivers/catalog_thermal_test.go b/go/internal/drivers/catalog_thermal_test.go new file mode 100644 index 000000000..9c6e8dc20 --- /dev/null +++ b/go/internal/drivers/catalog_thermal_test.go @@ -0,0 +1,208 @@ +package drivers + +import ( + "reflect" + "testing" + + "github.com/srcfl/ftw/go/internal/thermal" +) + +func intp(v int) *int { return &v } + +const thermalDriver = `DRIVER = { + id = "probe", + name = "Probe", + version = "1.0.0", + controls = { + { + id = "set_heat_curve_offset", label = "Heat curve offset", + evidence = "write_ack", + asset = "space_heat", strategy = "curve_offset", zone = "z1", + min_interval_s = 600, + input = { type = "number", min = -3, max = 3, step = 1, unit = "°C" }, + }, + { + id = "set_power", asset = "space_heat", strategy = "on_off", + min_on_s = 1200, min_off_s = 900, + input = { type = "boolean" }, + }, + { + id = "set_dhw_mode", asset = "dhw", strategy = "discrete_mode", + input = { type = "string", options = { "eco", "normal", "boost" } }, + }, + }, + sensors = { + { metric = "hp_indoor_temp_c", asset = "space_heat", zone = "z1" }, + { metric = "hp_dhw_temp_c", asset = "dhw" }, + }, +} +` + +func TestThermalDeclarationParses(t *testing.T) { + entry := writeDriver(t, thermalDriver) + if entry.ID != "probe" || entry.Version != "1.0.0" { + t.Fatalf("thermal fields broke the surrounding block: %+v", entry) + } + if len(entry.Controls) != 3 { + t.Fatalf("controls = %d, want 3: %+v", len(entry.Controls), entry.Controls) + } + want := []*CatalogThermalControl{ + {Asset: thermal.AssetSpaceHeat, Strategy: thermal.InfluenceCurveOffset, Zone: "z1", MinIntervalS: intp(600)}, + {Asset: thermal.AssetSpaceHeat, Strategy: thermal.InfluenceOnOff, MinOnS: intp(1200), MinOffS: intp(900)}, + {Asset: thermal.AssetDHW, Strategy: thermal.InfluenceDiscreteMode}, + } + for i, w := range want { + if got := entry.Controls[i].Thermal; !reflect.DeepEqual(got, w) { + t.Errorf("control %s thermal = %+v, want %+v", entry.Controls[i].ID, got, w) + } + } + if got := entry.Controls[2].Input.Options; !reflect.DeepEqual(got, []string{"eco", "normal", "boost"}) { + t.Errorf("options = %v", got) + } + // The offset control's own input still parses as before. + if in := entry.Controls[0].Input; in.Min == nil || *in.Min != -3 || in.Max == nil || *in.Max != 3 { + t.Errorf("input = %+v", in) + } + + wantSensors := []CatalogSensor{ + {Metric: "hp_indoor_temp_c", Asset: thermal.AssetSpaceHeat, Zone: "z1"}, + {Metric: "hp_dhw_temp_c", Asset: thermal.AssetDHW}, + } + if !reflect.DeepEqual(entry.Sensors, wantSensors) { + t.Errorf("sensors = %+v, want %+v", entry.Sensors, wantSensors) + } +} + +// A control that says nothing about heat keeps exactly the shape it had, so +// no existing driver changes for a field it never declared. +func TestControlWithoutThermalFieldsIsUnchanged(t *testing.T) { + entry := writeDriver(t, offsetControl) + if len(entry.Controls) != 1 || entry.Controls[0].Thermal != nil { + t.Fatalf("controls = %+v, want one with no thermal", entry.Controls) + } + if entry.Sensors != nil { + t.Errorf("sensors = %+v, want nil", entry.Sensors) + } +} + +// Each of these tries to declare a heat role and gets it wrong. The control +// is dropped rather than kept without its thermal fields: a planner trusting +// a misdeclared strategy, or missing a compressor's minimum off time because +// it was written as a string, is worse than the control not appearing. +func TestThermalControlDropsMisdeclarations(t *testing.T) { + num := `input = { type = "number", min = -3, max = 3 }` + cases := map[string]string{ + "asset only": `{ id = "c", asset = "space_heat", ` + num + ` }`, + "strategy only": `{ id = "c", strategy = "curve_offset", ` + num + ` }`, + "zone only": `{ id = "c", zone = "z1", ` + num + ` }`, + "unknown asset": `{ id = "c", asset = "sauna", strategy = "curve_offset", ` + num + ` }`, + "unknown strategy": `{ id = "c", asset = "space_heat", strategy = "vibes", ` + num + ` }`, + "synthetic price": `{ id = "c", asset = "space_heat", strategy = "synthetic_price", ` + num + ` }`, + "offset on boolean": `{ id = "c", asset = "space_heat", strategy = "curve_offset", input = { type = "boolean" } }`, + "on_off on number": `{ id = "c", asset = "space_heat", strategy = "on_off", ` + num + ` }`, + "mode without options": `{ id = "c", asset = "dhw", strategy = "discrete_mode", input = { type = "string" } }`, + "mode with one option": `{ id = "c", asset = "dhw", strategy = "discrete_mode", input = { type = "string", options = { "eco" } } }`, + "bad zone": `{ id = "c", asset = "space_heat", strategy = "curve_offset", zone = "Living Room", ` + num + ` }`, + "interval as string": `{ id = "c", asset = "space_heat", strategy = "curve_offset", min_interval_s = "600", ` + num + ` }`, + "negative interval": `{ id = "c", asset = "space_heat", strategy = "curve_offset", min_interval_s = -1, ` + num + ` }`, + "fractional interval": `{ id = "c", asset = "space_heat", strategy = "curve_offset", min_interval_s = 1.5, ` + num + ` }`, + "min_off as string": `{ id = "c", asset = "space_heat", strategy = "on_off", min_off_s = "900", input = { type = "boolean" } }`, + "min_on on a setpoint": `{ id = "c", asset = "space_heat", strategy = "setpoint", min_on_s = 60, ` + num + ` }`, + "interval over a week": `{ id = "c", asset = "space_heat", strategy = "curve_offset", min_interval_s = 700000, ` + num + ` }`, + } + for name, entry := range cases { + t.Run(name, func(t *testing.T) { + parsed := writeDriver(t, "DRIVER = {\n id = \"probe\",\n controls = {\n "+entry+",\n },\n}\n") + if len(parsed.Controls) != 0 { + t.Errorf("controls = %+v, want none", parsed.Controls) + } + }) + } +} + +// A zone written inside the input belongs to the input, not to the control, +// and must not make an ordinary control look like a half-declared heat one. +func TestThermalFieldsInsideInputAreNotTheControls(t *testing.T) { + entry := writeDriver(t, `DRIVER = { + id = "probe", + controls = { + { id = "c", input = { type = "number", min = 0, max = 1, zone = "z1" } }, + }, +} +`) + if len(entry.Controls) != 1 || entry.Controls[0].Thermal != nil { + t.Fatalf("controls = %+v, want one with no thermal", entry.Controls) + } +} + +func TestControlOptionsValidation(t *testing.T) { + cases := map[string]struct { + entry string + keep bool + }{ + "plain string options": {`{ id = "c", input = { type = "string", options = { "a", "b" } } }`, true}, + "empty options": {`{ id = "c", input = { type = "string", options = { } } }`, false}, + "duplicate option": {`{ id = "c", input = { type = "string", options = { "a", "a" } } }`, false}, + "bad option token": {`{ id = "c", input = { type = "string", options = { "Eco Mode" } } }`, false}, + "non-string option": {`{ id = "c", input = { type = "string", options = { "a", 2 } } }`, false}, + "options on a number": {`{ id = "c", input = { type = "number", min = 0, max = 1, options = { "a" } } }`, false}, + "options on a boolean": {`{ id = "c", input = { type = "boolean", options = { "a" } } }`, false}, + "options not a table": {`{ id = "c", input = { type = "string", options = "a" } }`, false}, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + parsed := writeDriver(t, "DRIVER = {\n id = \"probe\",\n controls = {\n "+tc.entry+",\n },\n}\n") + if got := len(parsed.Controls) == 1; got != tc.keep { + t.Errorf("kept = %v, want %v: %+v", got, tc.keep, parsed.Controls) + } + }) + } +} + +// One bad sensor is dropped on its own; it does not hide the others. +func TestSensorsDropEachMalformedEntry(t *testing.T) { + entry := writeDriver(t, `DRIVER = { + id = "probe", + sensors = { + { metric = "room_temp_c", asset = "space_heat" }, + { metric = "Room Temp", asset = "space_heat" }, + { metric = "tank_temp_c", asset = "sauna" }, + { asset = "dhw" }, + { metric = "room_temp_c", asset = "cooling" }, + { metric = "zone_temp_c", asset = "space_heat", zone = "Bad Zone" }, + { metric = "tank_temp_c", asset = "dhw" }, + }, +} +`) + want := []CatalogSensor{ + {Metric: "room_temp_c", Asset: thermal.AssetSpaceHeat}, + {Metric: "tank_temp_c", Asset: thermal.AssetDHW}, + } + if !reflect.DeepEqual(entry.Sensors, want) { + t.Errorf("sensors = %+v, want %+v", entry.Sensors, want) + } +} + +// A field whose name only ends in "sensors" or "controls" is not that list. +func TestNestedBlockNeedsTheWholeName(t *testing.T) { + entry := writeDriver(t, `DRIVER = { + id = "probe", + temp_sensors = { { metric = "room_temp_c", asset = "space_heat" } }, + my_controls = { { id = "c", input = { type = "boolean" } } }, +} +`) + if entry.Sensors != nil || entry.Controls != nil { + t.Errorf("sensors = %+v, controls = %+v, want neither", entry.Sensors, entry.Controls) + } +} + +func TestSensorsIgnoreCommentedDeclarations(t *testing.T) { + entry := writeDriver(t, `DRIVER = { + id = "probe", + -- sensors = { { metric = "room_temp_c", asset = "space_heat" } }, +} +`) + if entry.Sensors != nil { + t.Errorf("sensors = %+v, want nil", entry.Sensors) + } +} diff --git a/go/internal/thermal/model.go b/go/internal/thermal/model.go index 8642b9158..b5eb71cde 100644 --- a/go/internal/thermal/model.go +++ b/go/internal/thermal/model.go @@ -22,8 +22,19 @@ const ( InfluenceCurveOffset InfluenceStrategy = "curve_offset" InfluenceSetpoint InfluenceStrategy = "setpoint" InfluenceDiscreteMode InfluenceStrategy = "discrete_mode" + // InfluenceOnOff is a device whose only lever is start and stop. + InfluenceOnOff InfluenceStrategy = "on_off" ) +// ValidAssetKind reports whether k is one of the declared asset kinds. +func ValidAssetKind(k AssetKind) bool { + switch k { + case AssetSpaceHeat, AssetCooling, AssetDHW, AssetBuffer: + return true + } + return false +} + // IntentKind is planner intent before the driver maps it to vendor commands. type IntentKind string From f066969b1487426247a2e21fc680e47fb8d48f17 Mon Sep 17 00:00:00 2001 From: hannesb90 Date: Tue, 10 Nov 2026 10:13:06 +0100 Subject: [PATCH 2/3] feat(drivers): declare a device-side hold and a number control's default The owner's answer on #1994: planned control needs the device to fall back on its own through a device-side timed hold; otherwise planned values stay a small step from the device default. A control may now declare device_hold_max_s, and a number input its default, so Core can own that lease rule. A zero or non-numeric hold, or a default outside min..max or on a non-number input, drops the control. Co-Authored-By: Claude Opus 5.5 Signed-off-by: hannesb90 --- .changeset/thermal-control-declaration.md | 2 +- go/internal/drivers/catalog_controls.go | 19 ++++++++++++++++++- go/internal/drivers/catalog_thermal.go | 15 ++++++++++++++- go/internal/drivers/catalog_thermal_test.go | 21 ++++++++++++++++++--- 4 files changed, 51 insertions(+), 6 deletions(-) diff --git a/.changeset/thermal-control-declaration.md b/.changeset/thermal-control-declaration.md index 1f9a16182..28c838a6b 100644 --- a/.changeset/thermal-control-declaration.md +++ b/.changeset/thermal-control-declaration.md @@ -2,6 +2,6 @@ "ftw": patch --- -A driver can now say what a control does to heat. A control may declare `asset` (space_heat, dhw, cooling or buffer), `strategy` (curve_offset, setpoint, on_off or discrete_mode), `zone`, `min_interval_s`, and for on_off `min_on_s` and `min_off_s`. A driver may also list `sensors`: the metrics that are a heat store's temperature. `/api/drivers/{name}` shows both. Nothing plans for or sends a heat command because of them yet. +A driver can now say what a control does to heat. A control may declare `asset` (space_heat, dhw, cooling or buffer), `strategy` (curve_offset, setpoint, on_off or discrete_mode), `zone`, `min_interval_s`, `device_hold_max_s` (how long the device keeps a value before it returns to its own regulation), and for on_off `min_on_s` and `min_off_s`. A number input may declare its `default`, the value the device runs at by itself. A driver may also list `sensors`: the metrics that are a heat store's temperature. `/api/drivers/{name}` shows both. Nothing plans for or sends a heat command because of them yet. A string control may declare a closed set of `options`. The control API then refuses any other value instead of passing it to the driver. A control that declares these fields wrongly is left out, as an unrenderable control is today. Drivers that declare none of them are unchanged. diff --git a/go/internal/drivers/catalog_controls.go b/go/internal/drivers/catalog_controls.go index b173f38d8..ddfbcafc0 100644 --- a/go/internal/drivers/catalog_controls.go +++ b/go/internal/drivers/catalog_controls.go @@ -40,6 +40,9 @@ type CatalogControlInput struct { // Options is the closed set a string input accepts. Empty means any // string, as before. Options []string `json:"options,omitempty"` + // Default is the value a number control's device runs at by itself, + // inside min..max. Nil means the driver does not say. + Default *float64 `json:"default,omitempty"` } var controlEvidence = map[string]bool{"readback": true, "write_ack": true} @@ -130,6 +133,8 @@ func pickControls(block string) []CatalogControl { continue } control.Input.Options = options + hasDefault := fieldPresent(withoutBlock(input, "options"), "default") + control.Input.Default = fieldNumber(input, "default") switch control.Input.Type { case "number": if control.Input.Min == nil || control.Input.Max == nil || @@ -139,11 +144,23 @@ func pickControls(block string) []CatalogControl { if options != nil { continue } + // A default outside the range, or not a number, is no + // default: the device cannot run at it, and a planner + // measuring small steps from it would start in the wrong + // place. + if hasDefault && (control.Input.Default == nil || + *control.Input.Default < *control.Input.Min || + *control.Input.Default > *control.Input.Max) { + continue + } case "boolean": - if options != nil { + if options != nil || hasDefault { continue } case "string": + if hasDefault { + continue + } default: continue } diff --git a/go/internal/drivers/catalog_thermal.go b/go/internal/drivers/catalog_thermal.go index e6561a16c..815afaff6 100644 --- a/go/internal/drivers/catalog_thermal.go +++ b/go/internal/drivers/catalog_thermal.go @@ -27,6 +27,11 @@ type CatalogThermalControl struct { // declared only on an on_off control. MinOnS *int `json:"min_on_s,omitempty"` MinOffS *int `json:"min_off_s,omitempty"` + // DeviceHoldMaxS says the device itself keeps a commanded value for at + // most this long and then returns to its own regulation: a hub's timed + // boost or override. It is the lease that survives FTW stopping. Nil + // means the device holds a value until told otherwise. + DeviceHoldMaxS *int `json:"device_hold_max_s,omitempty"` } // CatalogSensor names one metric the driver already emits as the temperature @@ -46,7 +51,7 @@ var controlStrategyInput = map[thermal.InfluenceStrategy]string{ thermal.InfluenceDiscreteMode: "string", } -var thermalControlKeys = []string{"asset", "strategy", "zone", "min_interval_s", "min_on_s", "min_off_s"} +var thermalControlKeys = []string{"asset", "strategy", "zone", "min_interval_s", "min_on_s", "min_off_s", "device_hold_max_s"} var metricNameRE = regexp.MustCompile(`^[a-z][a-z0-9_]*$`) @@ -101,6 +106,14 @@ func pickThermalControl(entry string, input CatalogControlInput) (*CatalogTherma if (out.MinOnS != nil || out.MinOffS != nil) && out.Strategy != thermal.InfluenceOnOff { return nil, false } + if out.DeviceHoldMaxS, ok = pickSeconds(entry, "device_hold_max_s"); !ok { + return nil, false + } + // A hold of zero seconds is no hold: the device would revert before + // the value took effect, and calling it a lease would be false. + if out.DeviceHoldMaxS != nil && *out.DeviceHoldMaxS == 0 { + return nil, false + } return out, true } diff --git a/go/internal/drivers/catalog_thermal_test.go b/go/internal/drivers/catalog_thermal_test.go index 9c6e8dc20..942648599 100644 --- a/go/internal/drivers/catalog_thermal_test.go +++ b/go/internal/drivers/catalog_thermal_test.go @@ -18,8 +18,8 @@ const thermalDriver = `DRIVER = { id = "set_heat_curve_offset", label = "Heat curve offset", evidence = "write_ack", asset = "space_heat", strategy = "curve_offset", zone = "z1", - min_interval_s = 600, - input = { type = "number", min = -3, max = 3, step = 1, unit = "°C" }, + min_interval_s = 600, device_hold_max_s = 3600, + input = { type = "number", min = -3, max = 3, step = 1, unit = "°C", default = 0 }, }, { id = "set_power", asset = "space_heat", strategy = "on_off", @@ -47,7 +47,7 @@ func TestThermalDeclarationParses(t *testing.T) { t.Fatalf("controls = %d, want 3: %+v", len(entry.Controls), entry.Controls) } want := []*CatalogThermalControl{ - {Asset: thermal.AssetSpaceHeat, Strategy: thermal.InfluenceCurveOffset, Zone: "z1", MinIntervalS: intp(600)}, + {Asset: thermal.AssetSpaceHeat, Strategy: thermal.InfluenceCurveOffset, Zone: "z1", MinIntervalS: intp(600), DeviceHoldMaxS: intp(3600)}, {Asset: thermal.AssetSpaceHeat, Strategy: thermal.InfluenceOnOff, MinOnS: intp(1200), MinOffS: intp(900)}, {Asset: thermal.AssetDHW, Strategy: thermal.InfluenceDiscreteMode}, } @@ -63,6 +63,12 @@ func TestThermalDeclarationParses(t *testing.T) { if in := entry.Controls[0].Input; in.Min == nil || *in.Min != -3 || in.Max == nil || *in.Max != 3 { t.Errorf("input = %+v", in) } + if d := entry.Controls[0].Input.Default; d == nil || *d != 0 { + t.Errorf("default = %v, want 0", d) + } + if entry.Controls[1].Input.Default != nil || entry.Controls[1].Thermal.DeviceHoldMaxS != nil { + t.Errorf("on_off control = %+v, want no default and no device hold", entry.Controls[1]) + } wantSensors := []CatalogSensor{ {Metric: "hp_indoor_temp_c", Asset: thermal.AssetSpaceHeat, Zone: "z1"}, @@ -109,6 +115,9 @@ func TestThermalControlDropsMisdeclarations(t *testing.T) { "min_off as string": `{ id = "c", asset = "space_heat", strategy = "on_off", min_off_s = "900", input = { type = "boolean" } }`, "min_on on a setpoint": `{ id = "c", asset = "space_heat", strategy = "setpoint", min_on_s = 60, ` + num + ` }`, "interval over a week": `{ id = "c", asset = "space_heat", strategy = "curve_offset", min_interval_s = 700000, ` + num + ` }`, + "zero device hold": `{ id = "c", asset = "space_heat", strategy = "curve_offset", device_hold_max_s = 0, ` + num + ` }`, + "device hold string": `{ id = "c", asset = "space_heat", strategy = "curve_offset", device_hold_max_s = "3600", ` + num + ` }`, + "device hold alone": `{ id = "c", device_hold_max_s = 3600, ` + num + ` }`, } for name, entry := range cases { t.Run(name, func(t *testing.T) { @@ -148,6 +157,12 @@ func TestControlOptionsValidation(t *testing.T) { "options on a number": {`{ id = "c", input = { type = "number", min = 0, max = 1, options = { "a" } } }`, false}, "options on a boolean": {`{ id = "c", input = { type = "boolean", options = { "a" } } }`, false}, "options not a table": {`{ id = "c", input = { type = "string", options = "a" } }`, false}, + "default in range": {`{ id = "c", input = { type = "number", min = -3, max = 3, default = -1.5 } }`, true}, + "default at a bound": {`{ id = "c", input = { type = "number", min = 0, max = 1, default = 0 } }`, true}, + "default out of range": {`{ id = "c", input = { type = "number", min = -3, max = 3, default = 5 } }`, false}, + "default as string": {`{ id = "c", input = { type = "number", min = -3, max = 3, default = "0" } }`, false}, + "default on boolean": {`{ id = "c", input = { type = "boolean", default = 0 } }`, false}, + "default on string": {`{ id = "c", input = { type = "string", default = 0 } }`, false}, } for name, tc := range cases { t.Run(name, func(t *testing.T) { From f542adc78ff37159a71412bb40badc47e151a0fd Mon Sep 17 00:00:00 2001 From: hannesb90 Date: Tue, 10 Nov 2026 10:42:23 +0100 Subject: [PATCH 3/3] refactor(drivercontrol): move operator holds out of the API The per-driver lock, hold, expiry and default path move unchanged from internal/api into internal/drivercontrol, so a planned value can later go through the same lock and expiry as an operator's (ADR 0012). The API keeps its routes, responses and SendDriverDefault; the registry lifecycle hook now clears holds through the package. As asked on #1994, the unused thermal policy draft goes with it: DecideIntent, DecisionInput, Intent, TemperatureBand, MarginalPrice and the synthetic_price strategy. The declaration vocabulary stays. Co-Authored-By: Claude Opus 5.5 Signed-off-by: hannesb90 --- .changeset/driver-control-holds-package.md | 7 + go/internal/api/api.go | 23 +- go/internal/api/api_driver_control.go | 318 +++--------------- go/internal/api/api_driver_control_test.go | 104 +++--- go/internal/api/api_driver_thermal_test.go | 2 +- go/internal/api/api_drivers_debug.go | 7 +- go/internal/drivercontrol/holds.go | 360 +++++++++++++++++++++ go/internal/drivercontrol/holds_test.go | 213 ++++++++++++ go/internal/drivers/catalog_thermal.go | 3 +- go/internal/thermal/model.go | 187 +---------- go/internal/thermal/model_test.go | 146 +-------- 11 files changed, 709 insertions(+), 661 deletions(-) create mode 100644 .changeset/driver-control-holds-package.md create mode 100644 go/internal/drivercontrol/holds.go create mode 100644 go/internal/drivercontrol/holds_test.go diff --git a/.changeset/driver-control-holds-package.md b/.changeset/driver-control-holds-package.md new file mode 100644 index 000000000..36c2afced --- /dev/null +++ b/.changeset/driver-control-holds-package.md @@ -0,0 +1,7 @@ +--- +"ftw": patch +--- + +Internal: operator driver-control holds moved from the API into their own +package so a later planner can share the same lock and expiry, and the unused +thermal policy draft (`DecideIntent`) is removed. No behaviour change. diff --git a/go/internal/api/api.go b/go/internal/api/api.go index d6de34afb..90b2a8925 100644 --- a/go/internal/api/api.go +++ b/go/internal/api/api.go @@ -39,6 +39,7 @@ import ( "github.com/srcfl/ftw/go/internal/configreload" "github.com/srcfl/ftw/go/internal/control" "github.com/srcfl/ftw/go/internal/coverage" + "github.com/srcfl/ftw/go/internal/drivercontrol" "github.com/srcfl/ftw/go/internal/driverrepo" "github.com/srcfl/ftw/go/internal/drivers" "github.com/srcfl/ftw/go/internal/energyreport" @@ -296,11 +297,9 @@ type Server struct { savingsCacheMu sync.Mutex savingsCache map[string]daySavings - // controlStates serializes command dispatch, default dispatch, and hold - // transitions per driver. Process-lifetime only, deliberately: a restart - // should leave no device held by a setting nobody remembers making. - controlStateMu sync.Mutex - controlStates map[string]*controlDriverState + // controlHolds serializes command dispatch, default dispatch, and hold + // transitions per driver. Process-lifetime only; see drivercontrol. + controlHolds *drivercontrol.Holds // beforeDriverControlSend is a package-test seam for reproducing a // lifecycle change between request validation and registry dispatch. It is @@ -340,13 +339,13 @@ func New(deps *Deps) *Server { deps.WebDir = "web" } s := &Server{ - deps: deps, - mux: http.NewServeMux(), - dailyCache: make(map[string]state.DayEnergy), - controlStates: make(map[string]*controlDriverState), - drafts: newDriverDrafts(), - marks: make(map[string]routeMark), + deps: deps, + mux: http.NewServeMux(), + dailyCache: make(map[string]state.DayEnergy), + drafts: newDriverDrafts(), + marks: make(map[string]routeMark), } + s.controlHolds = drivercontrol.New(s.controlRegistry) s.setup = onboarding.NewWithSecrets(deps.State, s.preserveSetupSecrets, filepath.Dir(deps.ConfigPath)) if deps.SaveConfigWithValues == nil && deps.State != nil { deps.SaveConfigWithValues = func(path string, cfg *config.Config, values map[string]string, reviewedJSON []byte, beforeSave func() error) error { @@ -356,7 +355,7 @@ func New(deps *Deps) *Server { if deps.Registry != nil { // Registry removal is the lifecycle boundary for a driver generation. // Clear the API hold before a replacement instance can be added. - deps.Registry.SetLifecycleHook(s.clearDriverControl) + deps.Registry.SetLifecycleHook(s.controlHolds.Clear) } s.routes() // A draft's timer died with the previous process, so anything left behind diff --git a/go/internal/api/api_driver_control.go b/go/internal/api/api_driver_control.go index c6db6952e..82006752f 100644 --- a/go/internal/api/api_driver_control.go +++ b/go/internal/api/api_driver_control.go @@ -20,13 +20,12 @@ import ( "encoding/json" "errors" "fmt" - "log/slog" "math" "net/http" "slices" - "sync" "time" + "github.com/srcfl/ftw/go/internal/drivercontrol" "github.com/srcfl/ftw/go/internal/drivers" ) @@ -37,26 +36,8 @@ import ( const ( maxControlHoldSeconds = 24 * 60 * 60 defaultControlHoldSeconds = 4 * 60 * 60 - controlDefaultTimeout = 10 * time.Second ) -type controlDriverState struct { - mu sync.Mutex - hold *controlHold -} - -// controlHold is one active operator setting. The timer is what releases it; -// the fields are what the UI shows meanwhile. -type controlHold struct { - Control string `json:"control"` - Value any `json:"value,omitempty"` - ExpiresAt int64 `json:"expires_at_ms"` - Generation uint64 `json:"-"` - - armedAt time.Time - timer *time.Timer -} - type controlRequest struct { Control string `json:"control"` Value json.RawMessage `json:"value"` @@ -143,53 +124,49 @@ func (s *Server) handleDriverControl(w http.ResponseWriter, r *http.Request) { s.beforeDriverControlSend() } - // Keep command dispatch and hold transitions under one per-driver lock. - // Registry.SendWithGeneration selects the concrete running instance and - // returns its generation, so expiry can never pair a new command with an - // old status snapshot. The state lookup and lock are also one lifecycle - // transaction: clearDriverControlState takes the same locks in the same - // order, so it cannot remove the map entry between these operations. - state := s.lockControlState(name, s.beforeDriverControlStateLock) - defer state.mu.Unlock() - // The lock is held across the device call, so the call gets a deadline: - // a request context has none, and hold expiry waits on this lock. - sendCtx, cancel := context.WithTimeout(r.Context(), controlDefaultTimeout) - defer cancel() - generation, err := s.deps.Registry.SendWithGeneration(sendCtx, name, body) - if err != nil { - s.clearControlHoldLocked(state) - if errors.Is(err, drivers.ErrObserveOnly) { - writeJSON(w, http.StatusForbidden, map[string]any{ - "error": err.Error(), - "observe_only": true, - }) - return - } - if errors.Is(err, drivers.ErrControlBlocked) { - status, _ := s.deps.Registry.ControlStatus(name) - writeJSON(w, http.StatusConflict, map[string]any{ - "error": err.Error(), - "control_blocked": true, - "default_confirmed": false, - "recovery_pending": status.RecoveryPending, + // drivercontrol.Holds.Send keeps dispatch and the hold transition under + // one per-driver lock and binds the hold to the generation that took the + // command. The response is written inside done, still under that lock, so + // the status read that explains an error is the one that caused it. + s.controlHolds.Send(r.Context(), name, control.ID, applied, body, + time.Duration(seconds)*time.Second, s.beforeDriverControlStateLock, + func(hold drivercontrol.Hold, err error) { + if err != nil { + s.writeControlSendError(w, name, err) + return + } + writeJSON(w, 200, map[string]any{ + "control": control.ID, + "applied": applied, + "evidence": control.Evidence, + "expires_at_ms": hold.ExpiresAt, }) - return - } - if _, running := s.deps.Registry.ControlStatus(name); !running { - writeJSON(w, http.StatusNotFound, map[string]string{"error": "driver not running"}) - return - } - writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()}) + }) +} + +func (s *Server) writeControlSendError(w http.ResponseWriter, name string, err error) { + if errors.Is(err, drivers.ErrObserveOnly) { + writeJSON(w, http.StatusForbidden, map[string]any{ + "error": err.Error(), + "observe_only": true, + }) return } - hold := s.armControlHoldLocked(name, state, control.ID, applied, generation, time.Duration(seconds)*time.Second) - - writeJSON(w, 200, map[string]any{ - "control": control.ID, - "applied": applied, - "evidence": control.Evidence, - "expires_at_ms": hold.ExpiresAt, - }) + if errors.Is(err, drivers.ErrControlBlocked) { + status, _ := s.deps.Registry.ControlStatus(name) + writeJSON(w, http.StatusConflict, map[string]any{ + "error": err.Error(), + "control_blocked": true, + "default_confirmed": false, + "recovery_pending": status.RecoveryPending, + }) + return + } + if _, running := s.deps.Registry.ControlStatus(name); !running { + writeJSON(w, http.StatusNotFound, map[string]string{"error": "driver not running"}) + return + } + writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()}) } // DELETE /api/drivers/{name}/control — end the hold now. @@ -264,215 +241,18 @@ func clampToDeclared(value float64, in drivers.CatalogControlInput) float64 { return value } -// armControlHold replaces any existing hold for the driver. One driver holds -// one control at a time: two overlapping holds on the same device would each -// expire into a default that undoes the other. -func (s *Server) controlState(name string) *controlDriverState { - s.controlStateMu.Lock() - defer s.controlStateMu.Unlock() - return s.controlStateLocked(name) -} - -func (s *Server) controlStateLocked(name string) *controlDriverState { - if s.controlStates == nil { - s.controlStates = make(map[string]*controlDriverState) - } - state := s.controlStates[name] - if state == nil { - state = &controlDriverState{} - s.controlStates[name] = state - } - return state -} - -// lockControlState serializes a state map lookup with acquisition of that -// driver's mutex. Lifecycle invalidation uses controlStateMu -> state.mu as -// well, so a request can either own the state before removal starts or see a -// removed state after removal finishes; it cannot keep a pointer that was -// deleted in between. -func (s *Server) lockControlState(name string, beforeStateLock func()) *controlDriverState { - s.controlStateMu.Lock() - state := s.controlStateLocked(name) - if beforeStateLock != nil { - beforeStateLock() - } - state.mu.Lock() - s.controlStateMu.Unlock() - return state -} - -// tryLockExistingControlState locks a state that already exists without -// creating one for a driver that may have been removed concurrently, and -// without waiting: locked is false when another request holds the state. -// Lifecycle invalidation uses the same controlStateMu -> state.mu order. -func (s *Server) tryLockExistingControlState(name string) (state *controlDriverState, locked bool) { - s.controlStateMu.Lock() - defer s.controlStateMu.Unlock() - state = s.controlStates[name] - if state == nil { - return nil, false - } - return state, state.mu.TryLock() -} - -func (s *Server) armControlHoldLocked(name string, state *controlDriverState, control string, value any, generation uint64, d time.Duration) *controlHold { - s.clearControlHoldLocked(state) - now := time.Now() - hold := &controlHold{ - Control: control, - Value: value, - ExpiresAt: now.Add(d).UnixMilli(), - Generation: generation, - armedAt: now, - } - hold.timer = time.AfterFunc(d, func() { - if err := s.expireControlHold(name, state, hold); err != nil { - slog.Error("control hold expiry default failed; registry recovery gate remains active", "driver", name, "err", err) - } - }) - state.hold = hold - return hold -} - -func (s *Server) clearControlHoldLocked(state *controlDriverState) { - if state.hold != nil && state.hold.timer != nil { - state.hold.timer.Stop() - } - state.hold = nil -} - -// expireControlHold hands the device back to itself. It checks identity -// first: a hold that was replaced or released already had its timer stopped, -// but a timer that had begun firing cannot be stopped, and defaulting a -// driver that an operator has just set again is the one wrong answer here. -func (s *Server) expireControlHold(name string, state *controlDriverState, fired *controlHold) error { - state.mu.Lock() - defer state.mu.Unlock() - if state.hold != fired { - return nil - } +// controlRegistry is what the holds see of the registry. A nil +// *drivers.Registry must stay a nil interface, or the holds would call it. +func (s *Server) controlRegistry() drivercontrol.Registry { if s.deps == nil || s.deps.Registry == nil { - s.clearControlHoldLocked(state) - return errors.New("driver registry not available") - } - status, ok := s.deps.Registry.ControlStatus(name) - if !ok || status.Generation != fired.Generation { - // A stopped generation must never default a replacement instance. - s.clearControlHoldLocked(state) return nil } - s.clearControlHoldLocked(state) - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() - return s.sendDefaultLocked(ctx, name) + return s.deps.Registry } -// SendDriverDefault is the shared safety path for watchdogs and API release. -// It clears the operator hold while holding the same per-driver lock used by -// command dispatch, then sends the driver's own default with a bounded -// context. A caller without a deadline gets the 10-second safety deadline. -// -// The control tick calls this, so it never waits for an operator command in -// flight. When one holds the lock, the default goes first: SendDefault cancels -// that command and refuses new ones until the default is confirmed. The hold -// is cleared once the lock is free, but only a hold armed before the default -// returned. A later one belongs to a command sent after the default and keeps -// its own expiry. +// SendDriverDefault is the shared safety path for watchdogs and API release: +// it clears any hold under the per-driver lock and sends the driver's own +// default. See drivercontrol.Holds.SendDefault. func (s *Server) SendDriverDefault(ctx context.Context, name string) error { - if s.deps == nil || s.deps.Registry == nil { - return errors.New("driver registry not available") - } - if _, ok := s.deps.Registry.ControlStatus(name); !ok { - return fmt.Errorf("driver %q not found", name) - } - if s.beforeDriverDefaultStateLock != nil { - s.beforeDriverDefaultStateLock() - } - state, locked := s.tryLockExistingControlState(name) - if locked { - defer state.mu.Unlock() - s.clearControlHoldLocked(state) - return s.sendDefaultLocked(ctx, name) - } - err := s.sendDefaultLocked(ctx, name) - if state != nil { - defaulted := time.Now() - go func() { - state.mu.Lock() - defer state.mu.Unlock() - if state.hold != nil && !state.hold.armedAt.After(defaulted) { - s.clearControlHoldLocked(state) - } - }() - } - return err -} - -func (s *Server) sendDefaultLocked(ctx context.Context, name string) error { - if s.deps == nil || s.deps.Registry == nil { - return errors.New("driver registry not available") - } - if ctx == nil { - ctx = context.Background() - } - if _, ok := ctx.Deadline(); !ok { - var cancel context.CancelFunc - ctx, cancel = context.WithTimeout(ctx, controlDefaultTimeout) - defer cancel() - } - return s.deps.Registry.SendDefault(ctx, name) -} - -func (s *Server) activeControlHold(name string) *controlHold { - state := s.peekControlState(name) - if state == nil { - return nil - } - state.mu.Lock() - defer state.mu.Unlock() - if state.hold == nil { - return nil - } - if s.deps == nil || s.deps.Registry == nil { - s.clearControlHoldLocked(state) - return nil - } - status, ok := s.deps.Registry.ControlStatus(name) - if !ok || status.Generation != state.hold.Generation { - s.clearControlHoldLocked(state) - return nil - } - hold := *state.hold - hold.timer = nil - return &hold -} - -func (s *Server) peekControlState(name string) *controlDriverState { - s.controlStateMu.Lock() - state := s.controlStates[name] - s.controlStateMu.Unlock() - return state -} - -// clearDriverControl is called by Registry before a driver generation is -// removed. It stops the old timer and drops the map entry so a re-added driver -// cannot inherit an operator hold from the previous instance. -func (s *Server) clearDriverControl(name string) { - s.clearDriverControlState(name, nil) -} - -func (s *Server) clearDriverControlState(name string, expected *controlDriverState) { - s.controlStateMu.Lock() - defer s.controlStateMu.Unlock() - state := s.controlStates[name] - if expected != nil && state != expected { - return - } - if state == nil { - return - } - state.mu.Lock() - s.clearControlHoldLocked(state) - delete(s.controlStates, name) - state.mu.Unlock() + return s.controlHolds.SendDefault(ctx, name, s.beforeDriverDefaultStateLock) } diff --git a/go/internal/api/api_driver_control_test.go b/go/internal/api/api_driver_control_test.go index 45ec4194b..e96d26547 100644 --- a/go/internal/api/api_driver_control_test.go +++ b/go/internal/api/api_driver_control_test.go @@ -364,7 +364,7 @@ func TestDriverControlPreservesDeclaredBooleanAndStringValues(t *testing.T) { t.Fatalf("boolean POST = %d, body %s", rec.Code, rec.Body.String()) } waitMetric(t, tel, "heat", "applied", 1) - if hold := srv.activeControlHold("heat"); hold == nil || hold.Value != true { + if hold := srv.controlHolds.Active("heat"); hold == nil || hold.Value != true { t.Fatalf("boolean hold = %+v, want true", hold) } @@ -373,7 +373,7 @@ func TestDriverControlPreservesDeclaredBooleanAndStringValues(t *testing.T) { t.Fatalf("string POST = %d, body %s", rec.Code, rec.Body.String()) } waitMetric(t, tel, "heat", "applied", 3) - if hold := srv.activeControlHold("heat"); hold == nil || hold.Value != "eco" { + if hold := srv.controlHolds.Active("heat"); hold == nil || hold.Value != "eco" { t.Fatalf("string hold = %+v, want eco", hold) } } @@ -409,7 +409,7 @@ func TestDriverControlHoldIsVisibleAndReleasable(t *testing.T) { // Releasing calls the driver's own default mode, not a value this // package invented. waitMetric(t, tel, "heat", "defaulted", 1) - if hold := srv.activeControlHold("heat"); hold != nil { + if hold := srv.controlHolds.Active("heat"); hold != nil { t.Errorf("hold survived release: %+v", hold) } } @@ -425,7 +425,7 @@ func TestDriverControlHoldExpiresIntoDefault(t *testing.T) { } waitMetric(t, tel, "heat", "applied", 2) waitMetric(t, tel, "heat", "defaulted", 1) - if hold := srv.activeControlHold("heat"); hold != nil { + if hold := srv.controlHolds.Active("heat"); hold != nil { t.Errorf("hold survived expiry: %+v", hold) } } @@ -485,7 +485,7 @@ func TestDriverControlBindsHoldToRestartedGeneration(t *testing.T) { t.Fatalf("POST after restart race = %d, body %s", rec.Code, rec.Body.String()) } waitMetric(t, tel, "heat", "applied", 2) - hold := srv.activeControlHold("heat") + hold := srv.controlHolds.Active("heat") if hold == nil { t.Fatal("control request did not create a hold") } @@ -493,19 +493,17 @@ func TestDriverControlBindsHoldToRestartedGeneration(t *testing.T) { t.Fatalf("hold generation = %d, want replacement generation %d", hold.Generation, newStatus.Generation) } - state := srv.peekControlState("heat") + state := srv.controlHolds.Peek("heat") if state == nil { t.Fatal("control state disappeared before expiry") } - state.mu.Lock() - fired := state.hold - state.mu.Unlock() - if err := srv.expireControlHold("heat", state, fired); err != nil { + fired := state.CurrentHold() + if err := srv.controlHolds.Expire("heat", state, fired); err != nil { t.Fatalf("expiry after generation-bound command = %v", err) } waitMetric(t, tel, "heat", "defaulted", 1) waitMetric(t, tel, "heat", "applied", 0) - if got := srv.activeControlHold("heat"); got != nil { + if got := srv.controlHolds.Active("heat"); got != nil { t.Fatalf("hold survived successful expiry: %+v", got) } } @@ -568,7 +566,7 @@ func TestDriverControlLookupRaceCannotOrphanStateOrTimer(t *testing.T) { lifecycleDone := make(chan error, 1) go func() { lifecycleDone <- test.lifecycle(srv, cfg) }() mapProbeDone := make(chan bool, 1) - go func() { mapProbeDone <- srv.peekControlState("heat") == nil }() + go func() { mapProbeDone <- srv.controlHolds.Peek("heat") == nil }() select { case removed := <-mapProbeDone: if removed { @@ -596,7 +594,7 @@ func TestDriverControlLookupRaceCannotOrphanStateOrTimer(t *testing.T) { t.Fatal("lifecycle deadlocked after the request released state.mu") } - if got := srv.activeControlHold("heat"); got != nil { + if got := srv.controlHolds.Active("heat"); got != nil { t.Fatalf("%s left an orphaned hold: %+v", test.name, got) } if got := driverDetail(t, srv, "heat"); got.Hold != nil { @@ -637,21 +635,19 @@ func TestDriverControlExpiryDefaultFailureBlocksUntilRecovery(t *testing.T) { `{"control":"set_offset","value":2,"duration_s":600}`); rec.Code != http.StatusOK { t.Fatalf("POST = %d, body %s", rec.Code, rec.Body.String()) } - state := srv.peekControlState("heat") + state := srv.controlHolds.Peek("heat") if state == nil { t.Fatal("missing control state") } - state.mu.Lock() - hold := state.hold - state.mu.Unlock() + hold := state.CurrentHold() if hold == nil { t.Fatal("missing control hold") } - if err := srv.expireControlHold("heat", state, hold); err == nil { + if err := srv.controlHolds.Expire("heat", state, hold); err == nil { t.Fatal("expiry hid a failed default") } - if got := srv.activeControlHold("heat"); got != nil { + if got := srv.controlHolds.Active("heat"); got != nil { t.Fatalf("failed expiry left hold active: %+v", got) } status, ok := srv.deps.Registry.ControlStatus("heat") @@ -735,7 +731,7 @@ func TestDriverControlAmbiguousCommandRestoresDefault(t *testing.T) { t.Fatalf("ambiguous POST = %d, body %s", rec.Code, rec.Body.String()) } waitMetric(t, tel, "heat", "defaulted", 1) - if hold := srv.activeControlHold("heat"); hold != nil { + if hold := srv.controlHolds.Active("heat"); hold != nil { t.Fatalf("ambiguous command left hold active: %+v", hold) } waitMetric(t, tel, "heat", "applied", 0) @@ -751,10 +747,10 @@ func TestDriverControlDefaultPathClearsHold(t *testing.T) { if err := srv.SendDriverDefault(context.Background(), "heat"); err != nil { t.Fatalf("SendDriverDefault = %v", err) } - if hold := srv.activeControlHold("heat"); hold != nil { + if hold := srv.controlHolds.Active("heat"); hold != nil { t.Fatalf("default path left hold active: %+v", hold) } - if state := srv.peekControlState("heat"); state == nil { + if state := srv.controlHolds.Peek("heat"); state == nil { t.Fatal("default path removed the existing control state") } waitMetric(t, tel, "heat", "defaulted", 1) @@ -834,7 +830,7 @@ func TestSendDriverDefaultDoesNotWaitForAnOperatorCommand(t *testing.T) { case <-time.After(3 * time.Second): t.Fatal("operator command did not return after the default cancelled it") } - if hold := srv.activeControlHold("heat"); hold != nil { + if hold := srv.controlHolds.Active("heat"); hold != nil { t.Fatalf("hold survived the safety default: %+v", hold) } } @@ -857,31 +853,31 @@ func TestSendDriverDefaultClearsOnlyHoldsArmedBeforeIt(t *testing.T) { if !ok { t.Fatal("driver not running") } - state := srv.controlState("heat") - state.mu.Lock() // an operator request in flight + state := srv.controlHolds.State("heat") + state.Lock() // an operator request in flight if tc.armBefore { - srv.armControlHoldLocked("heat", state, "set_offset", 1.0, status.Generation, time.Hour) + srv.controlHolds.ArmLocked("heat", state, "set_offset", 1.0, status.Generation, time.Hour) } done := make(chan error, 1) go func() { done <- srv.SendDriverDefault(context.Background(), "heat") }() select { case err := <-done: if err != nil { - state.mu.Unlock() + state.Unlock() t.Fatalf("SendDriverDefault = %v", err) } case <-time.After(time.Second): - state.mu.Unlock() + state.Unlock() t.Fatal("SendDriverDefault waited for the operator request") } if !tc.armBefore { - srv.armControlHoldLocked("heat", state, "set_offset", 2.0, status.Generation, time.Hour) + srv.controlHolds.ArmLocked("heat", state, "set_offset", 2.0, status.Generation, time.Hour) } - state.mu.Unlock() + state.Unlock() if tc.wantCleared { deadline := time.Now().Add(time.Second) - for srv.activeControlHold("heat") != nil { + for srv.controlHolds.Active("heat") != nil { if time.Now().After(deadline) { t.Fatal("hold armed before the default survived it") } @@ -891,7 +887,7 @@ func TestSendDriverDefaultClearsOnlyHoldsArmedBeforeIt(t *testing.T) { } // Give the deferred clear time to take the lock and decide. time.Sleep(200 * time.Millisecond) - if srv.activeControlHold("heat") == nil { + if srv.controlHolds.Active("heat") == nil { t.Fatal("default cleared a hold armed after it; that command would never expire") } }) @@ -900,7 +896,7 @@ func TestSendDriverDefaultClearsOnlyHoldsArmedBeforeIt(t *testing.T) { func TestSendDriverDefaultDoesNotRecreateStateAfterRemove(t *testing.T) { srv, _ := controlServer(t) - srv.controlState("heat") + srv.controlHolds.State("heat") paused := make(chan struct{}) resume := make(chan struct{}) @@ -930,7 +926,7 @@ func TestSendDriverDefaultDoesNotRecreateStateAfterRemove(t *testing.T) { close(resume) t.Fatal("RemoveProbe deadlocked behind SendDriverDefault") } - if state := srv.peekControlState("heat"); state != nil { + if state := srv.controlHolds.Peek("heat"); state != nil { t.Fatalf("removed driver still has control state before default resumes: %p", state) } close(resume) @@ -943,14 +939,14 @@ func TestSendDriverDefaultDoesNotRecreateStateAfterRemove(t *testing.T) { case <-time.After(time.Second): t.Fatal("SendDriverDefault did not finish after lifecycle removal") } - if state := srv.peekControlState("heat"); state != nil { + if state := srv.controlHolds.Peek("heat"); state != nil { t.Fatalf("default path recreated removed control state: %p", state) } } func TestSendDriverDefaultDoesNotAccumulateRemovedStates(t *testing.T) { srv, _ := controlServer(t) - srv.controlState("heat") + srv.controlHolds.State("heat") base := srv.deps.Cfg.Drivers[0] for i := 0; i < 64; i++ { @@ -960,7 +956,7 @@ func TestSendDriverDefaultDoesNotAccumulateRemovedStates(t *testing.T) { if err := srv.deps.Registry.Add(context.Background(), cfg); err != nil { t.Fatalf("add %s: %v", name, err) } - srv.controlState(name) + srv.controlHolds.State(name) paused := make(chan struct{}) resume := make(chan struct{}) @@ -989,14 +985,12 @@ func TestSendDriverDefaultDoesNotAccumulateRemovedStates(t *testing.T) { t.Fatalf("SendDriverDefault(%s) did not finish", name) } srv.beforeDriverDefaultStateLock = nil - if state := srv.peekControlState(name); state != nil { + if state := srv.controlHolds.Peek(name); state != nil { t.Fatalf("removed driver %s left a control state", name) } } - srv.controlStateMu.Lock() - defer srv.controlStateMu.Unlock() - if got := len(srv.controlStates); got != 1 { + if got := srv.controlHolds.Len(); got != 1 { t.Fatalf("control state map has %d entries after removed names, want only heat", got) } } @@ -1011,17 +1005,15 @@ func TestDriverControlSerializesExpiryAndReplacement(t *testing.T) { `{"control":"set_offset","value":1,"duration_s":600}`); rec.Code != http.StatusOK { t.Fatalf("POST = %d, body %s", rec.Code, rec.Body.String()) } - state := srv.controlState("heat") - state.mu.Lock() - hold := state.hold - state.mu.Unlock() + state := srv.controlHolds.State("heat") + hold := state.CurrentHold() if hold == nil { t.Fatal("missing hold") } done := make(chan struct{}) go func() { - srv.expireControlHold("heat", state, hold) + srv.controlHolds.Expire("heat", state, hold) close(done) }() waitMetric(t, tel, "heat", "default_started", 1) @@ -1124,13 +1116,11 @@ func TestDriverControlClearsHoldAcrossLifecycle(t *testing.T) { t.Fatalf("initial POST = %d, body %s", rec.Code, rec.Body.String()) } waitMetric(t, tel, "heat", "applied", 2) - oldState := srv.peekControlState("heat") + oldState := srv.controlHolds.Peek("heat") if oldState == nil { t.Fatal("initial control state is missing") } - oldState.mu.Lock() - oldHold := oldState.hold - oldState.mu.Unlock() + oldHold := oldState.CurrentHold() if oldHold == nil { t.Fatal("initial control hold is missing") } @@ -1138,10 +1128,10 @@ func TestDriverControlClearsHoldAcrossLifecycle(t *testing.T) { if err := srv.deps.Registry.Restart(context.Background(), cfg); err != nil { t.Fatalf("restart = %v", err) } - if got := srv.activeControlHold("heat"); got != nil { + if got := srv.controlHolds.Active("heat"); got != nil { t.Fatalf("hold survived restart: %+v", got) } - if got := srv.peekControlState("heat"); got != nil { + if got := srv.controlHolds.Peek("heat"); got != nil { t.Fatal("control state map entry survived restart") } if got := driverDetail(t, srv, "heat"); got.Hold != nil { @@ -1156,7 +1146,7 @@ func TestDriverControlClearsHoldAcrossLifecycle(t *testing.T) { waitMetric(t, tel, "heat", "applied", -2) // Simulate a timer callback that was already in flight when the old // generation was removed. It must not default the new instance. - srv.expireControlHold("heat", oldState, oldHold) + srv.controlHolds.Expire("heat", oldState, oldHold) time.Sleep(250 * time.Millisecond) if got, _, ok := tel.LatestMetric("heat", "defaulted"); !ok || got != 0 { t.Fatalf("old timer changed the replacement: defaulted=%v/%v", got, ok) @@ -1168,7 +1158,7 @@ func TestDriverControlClearsHoldAcrossLifecycle(t *testing.T) { disabled := cfg disabled.Disabled = true srv.deps.Registry.Reload(context.Background(), []config.Driver{disabled}, false) - if got := srv.peekControlState("heat"); got != nil { + if got := srv.controlHolds.Peek("heat"); got != nil { t.Fatal("control state map entry survived disable") } if got := driverDetail(t, srv, "heat"); got.Hold != nil { @@ -1176,7 +1166,7 @@ func TestDriverControlClearsHoldAcrossLifecycle(t *testing.T) { } srv.deps.Registry.Reload(context.Background(), []config.Driver{cfg}, false) - if got := srv.peekControlState("heat"); got != nil { + if got := srv.controlHolds.Peek("heat"); got != nil { t.Fatal("control state map entry survived re-add") } if got := driverDetail(t, srv, "heat"); got.Hold != nil { @@ -1194,9 +1184,7 @@ func TestDriverDetailUnknownNamesDoNotCreateControlState(t *testing.T) { t.Fatalf("GET %s = %d, body %s", name, rec.Code, rec.Body.String()) } } - srv.controlStateMu.Lock() - defer srv.controlStateMu.Unlock() - if got := len(srv.controlStates); got != 0 { + if got := srv.controlHolds.Len(); got != 0 { t.Fatalf("unknown GETs created %d control states", got) } } diff --git a/go/internal/api/api_driver_thermal_test.go b/go/internal/api/api_driver_thermal_test.go index d9e9b2707..9fd5bfb35 100644 --- a/go/internal/api/api_driver_thermal_test.go +++ b/go/internal/api/api_driver_thermal_test.go @@ -59,7 +59,7 @@ func TestDriverControlRefusesValueOutsideDeclaredOptions(t *testing.T) { if rec.Code != http.StatusBadRequest { t.Fatalf("undeclared option = %d, want 400 (body %s)", rec.Code, rec.Body.String()) } - if hold := srv.activeControlHold("heat"); hold != nil { + if hold := srv.controlHolds.Active("heat"); hold != nil { t.Fatalf("hold = %+v after a refused value, want none", hold) } // Give the driver a few polls; a refused value must not have been sent. diff --git a/go/internal/api/api_drivers_debug.go b/go/internal/api/api_drivers_debug.go index 5f5608bc0..ab79148c9 100644 --- a/go/internal/api/api_drivers_debug.go +++ b/go/internal/api/api_drivers_debug.go @@ -26,6 +26,7 @@ import ( "time" "github.com/srcfl/ftw/go/internal/config" + "github.com/srcfl/ftw/go/internal/drivercontrol" "github.com/srcfl/ftw/go/internal/drivers" "github.com/srcfl/ftw/go/internal/telemetry" ) @@ -46,7 +47,7 @@ type driverDetailResp struct { // temperature. Absent for every driver that declares none. Sensors []drivers.CatalogSensor `json:"sensors,omitempty"` // Hold is the operator setting in force, if any, and when it ends. - Hold *controlHold `json:"hold,omitempty"` + Hold *drivercontrol.Hold `json:"hold,omitempty"` // ControlState says whether Core has confirmed autonomous default mode. // It must not call a failed default safe or settled. ControlState driverControlStateResp `json:"control_state"` @@ -128,12 +129,12 @@ func (s *Server) handleDriverDetail(w http.ResponseWriter, r *http.Request) { resp.Controls = entry.Controls resp.Sensors = entry.Sensors } - resp.Hold = s.activeControlHold(name) + resp.Hold = s.controlHolds.Active(name) resp.ControlState = s.driverControlState(name, resp.Hold) writeJSON(w, 200, resp) } -func (s *Server) driverControlState(name string, hold *controlHold) driverControlStateResp { +func (s *Server) driverControlState(name string, hold *drivercontrol.Hold) driverControlStateResp { if s.deps == nil || s.deps.Registry == nil { return driverControlStateResp{State: "unknown"} } diff --git a/go/internal/drivercontrol/holds.go b/go/internal/drivercontrol/holds.go new file mode 100644 index 000000000..c4c572719 --- /dev/null +++ b/go/internal/drivercontrol/holds.go @@ -0,0 +1,360 @@ +// Package drivercontrol owns what is held on a driver: one bounded value at a +// time, sent and released under one per-driver lock, and handed back to the +// device's own default when it ends. +// +// It moved here from the API package unchanged so that more than one caller +// can share it. An operator's hold and, later, a planned value must go +// through the same lock and the same expiry, or each would expire into a +// default that undoes the other (docs/adr/0012). +// +// Holds are process-lifetime only, deliberately: a restart should leave no +// device held by a setting nobody remembers making. +package drivercontrol + +import ( + "context" + "errors" + "fmt" + "log/slog" + "sync" + "time" + + "github.com/srcfl/ftw/go/internal/drivers" +) + +// DefaultTimeout bounds a default sent without a caller deadline, and a +// command sent while the per-driver lock is held. +const DefaultTimeout = 10 * time.Second + +// Registry is the part of drivers.Registry holds need. +type Registry interface { + SendWithGeneration(ctx context.Context, name string, payload []byte) (uint64, error) + ControlStatus(name string) (drivers.DriverControlStatus, bool) + SendDefault(ctx context.Context, name string) error +} + +// Hold is one active setting. The timer is what releases it; the fields are +// what the UI shows meanwhile. +type Hold struct { + Control string `json:"control"` + Value any `json:"value,omitempty"` + ExpiresAt int64 `json:"expires_at_ms"` + Generation uint64 `json:"-"` + + armedAt time.Time + timer *time.Timer +} + +// State serializes command dispatch, default dispatch and hold transitions +// for one driver. +type State struct { + mu sync.Mutex + hold *Hold +} + +// Lock takes the per-driver lock. Exported for tests that stand in for a +// command in flight; production callers use Holds' methods. +func (st *State) Lock() { st.mu.Lock() } + +// Unlock releases the per-driver lock. +func (st *State) Unlock() { st.mu.Unlock() } + +// CurrentHold returns the held setting itself, not a copy, or nil. Its +// identity is what Expire compares. +func (st *State) CurrentHold() *Hold { + st.mu.Lock() + defer st.mu.Unlock() + return st.hold +} + +// Holds is every driver's State. +type Holds struct { + registry func() Registry + + mu sync.Mutex + states map[string]*State +} + +// New returns empty holds. registry is read at each use, so a caller whose +// registry is absent gets the same answers it did before this package +// existed. +func New(registry func() Registry) *Holds { + return &Holds{registry: registry, states: make(map[string]*State)} +} + +func (h *Holds) reg() Registry { + if h.registry == nil { + return nil + } + return h.registry() +} + +// Send dispatches body to the driver and holds controlID at value for d. +// The send and the hold transition happen under one per-driver lock, and the +// device call has a deadline because hold expiry waits on that lock. On error +// any existing hold is cleared and nothing is armed. +// +// done runs with the outcome while the lock is still held, so a caller that +// reads registry status to explain an error sees it before any lifecycle +// change waiting on this driver can proceed. +// +// beforeStateLock is a test seam, run after the state lookup and before the +// per-driver lock; nil in production. +func (h *Holds) Send(ctx context.Context, name, controlID string, value any, body []byte, d time.Duration, beforeStateLock func(), done func(Hold, error)) { + reg := h.reg() + if reg == nil { + done(Hold{}, errors.New("driver registry not available")) + return + } + // Keep command dispatch and hold transitions under one per-driver lock. + // Registry.SendWithGeneration selects the concrete running instance and + // returns its generation, so expiry can never pair a new command with an + // old status snapshot. The state lookup and lock are also one lifecycle + // transaction: clearState takes the same locks in the same order, so it + // cannot remove the map entry between these operations. + state := h.lock(name, beforeStateLock) + defer state.mu.Unlock() + sendCtx, cancel := context.WithTimeout(ctx, DefaultTimeout) + defer cancel() + generation, err := reg.SendWithGeneration(sendCtx, name, body) + if err != nil { + h.clearLocked(state) + done(Hold{}, err) + return + } + hold := h.ArmLocked(name, state, controlID, value, generation, d) + out := *hold + out.timer = nil + done(out, nil) +} + +// State returns the driver's state, creating it. +func (h *Holds) State(name string) *State { + h.mu.Lock() + defer h.mu.Unlock() + return h.stateLocked(name) +} + +func (h *Holds) stateLocked(name string) *State { + if h.states == nil { + h.states = make(map[string]*State) + } + state := h.states[name] + if state == nil { + state = &State{} + h.states[name] = state + } + return state +} + +// lock serializes a state map lookup with acquisition of that driver's +// mutex. Lifecycle invalidation uses h.mu -> state.mu as well, so a request +// can either own the state before removal starts or see a removed state +// after removal finishes; it cannot keep a pointer that was deleted in +// between. +func (h *Holds) lock(name string, beforeStateLock func()) *State { + h.mu.Lock() + state := h.stateLocked(name) + if beforeStateLock != nil { + beforeStateLock() + } + state.mu.Lock() + h.mu.Unlock() + return state +} + +// tryLockExisting locks a state that already exists without creating one +// for a driver that may have been removed concurrently, and without waiting: +// locked is false when another request holds the state. Lifecycle +// invalidation uses the same h.mu -> state.mu order. +func (h *Holds) tryLockExisting(name string) (state *State, locked bool) { + h.mu.Lock() + defer h.mu.Unlock() + state = h.states[name] + if state == nil { + return nil, false + } + return state, state.mu.TryLock() +} + +// ArmLocked replaces any existing hold for the driver; the caller holds +// state's lock. One driver holds one control at a time: two overlapping +// holds on the same device would each expire into a default that undoes the +// other. +func (h *Holds) ArmLocked(name string, state *State, control string, value any, generation uint64, d time.Duration) *Hold { + h.clearLocked(state) + now := time.Now() + hold := &Hold{ + Control: control, + Value: value, + ExpiresAt: now.Add(d).UnixMilli(), + Generation: generation, + armedAt: now, + } + hold.timer = time.AfterFunc(d, func() { + if err := h.Expire(name, state, hold); err != nil { + slog.Error("control hold expiry default failed; registry recovery gate remains active", "driver", name, "err", err) + } + }) + state.hold = hold + return hold +} + +func (h *Holds) clearLocked(state *State) { + if state.hold != nil && state.hold.timer != nil { + state.hold.timer.Stop() + } + state.hold = nil +} + +// Expire hands the device back to itself. It checks identity first: a hold +// that was replaced or released already had its timer stopped, but a timer +// that had begun firing cannot be stopped, and defaulting a driver that an +// operator has just set again is the one wrong answer here. +func (h *Holds) Expire(name string, state *State, fired *Hold) error { + state.mu.Lock() + defer state.mu.Unlock() + if state.hold != fired { + return nil + } + reg := h.reg() + if reg == nil { + h.clearLocked(state) + return errors.New("driver registry not available") + } + status, ok := reg.ControlStatus(name) + if !ok || status.Generation != fired.Generation { + // A stopped generation must never default a replacement instance. + h.clearLocked(state) + return nil + } + h.clearLocked(state) + ctx, cancel := context.WithTimeout(context.Background(), DefaultTimeout) + defer cancel() + return h.sendDefaultLocked(ctx, name) +} + +// SendDefault is the shared safety path for watchdogs and release. It clears +// the hold while holding the same per-driver lock used by command dispatch, +// then sends the driver's own default with a bounded context. A caller +// without a deadline gets the 10-second safety deadline. +// +// The control tick calls this, so it never waits for a command in flight. +// When one holds the lock, the default goes first: SendDefault cancels that +// command and refuses new ones until the default is confirmed. The hold is +// cleared once the lock is free, but only a hold armed before the default +// returned. A later one belongs to a command sent after the default and +// keeps its own expiry. +// +// beforeDefaultStateLock is a test seam; nil in production. +func (h *Holds) SendDefault(ctx context.Context, name string, beforeDefaultStateLock func()) error { + reg := h.reg() + if reg == nil { + return errors.New("driver registry not available") + } + if _, ok := reg.ControlStatus(name); !ok { + return fmt.Errorf("driver %q not found", name) + } + if beforeDefaultStateLock != nil { + beforeDefaultStateLock() + } + state, locked := h.tryLockExisting(name) + if locked { + defer state.mu.Unlock() + h.clearLocked(state) + return h.sendDefaultLocked(ctx, name) + } + err := h.sendDefaultLocked(ctx, name) + if state != nil { + defaulted := time.Now() + go func() { + state.mu.Lock() + defer state.mu.Unlock() + if state.hold != nil && !state.hold.armedAt.After(defaulted) { + h.clearLocked(state) + } + }() + } + return err +} + +func (h *Holds) sendDefaultLocked(ctx context.Context, name string) error { + reg := h.reg() + if reg == nil { + return errors.New("driver registry not available") + } + if ctx == nil { + ctx = context.Background() + } + if _, ok := ctx.Deadline(); !ok { + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, DefaultTimeout) + defer cancel() + } + return reg.SendDefault(ctx, name) +} + +// Active returns a copy of the driver's hold, or nil. A hold bound to a +// generation that is no longer running is cleared, not returned. +func (h *Holds) Active(name string) *Hold { + state := h.Peek(name) + if state == nil { + return nil + } + state.mu.Lock() + defer state.mu.Unlock() + if state.hold == nil { + return nil + } + reg := h.reg() + if reg == nil { + h.clearLocked(state) + return nil + } + status, ok := reg.ControlStatus(name) + if !ok || status.Generation != state.hold.Generation { + h.clearLocked(state) + return nil + } + hold := *state.hold + hold.timer = nil + return &hold +} + +// Peek returns the driver's state without creating one. +func (h *Holds) Peek(name string) *State { + h.mu.Lock() + state := h.states[name] + h.mu.Unlock() + return state +} + +// Len is how many drivers have state. For tests of lifecycle cleanup. +func (h *Holds) Len() int { + h.mu.Lock() + defer h.mu.Unlock() + return len(h.states) +} + +// Clear is called by the registry before a driver generation is removed. It +// stops the old timer and drops the map entry so a re-added driver cannot +// inherit a hold from the previous instance. +func (h *Holds) Clear(name string) { + h.clearState(name, nil) +} + +func (h *Holds) clearState(name string, expected *State) { + h.mu.Lock() + defer h.mu.Unlock() + state := h.states[name] + if expected != nil && state != expected { + return + } + if state == nil { + return + } + state.mu.Lock() + h.clearLocked(state) + delete(h.states, name) + state.mu.Unlock() +} diff --git a/go/internal/drivercontrol/holds_test.go b/go/internal/drivercontrol/holds_test.go new file mode 100644 index 000000000..4c6654911 --- /dev/null +++ b/go/internal/drivercontrol/holds_test.go @@ -0,0 +1,213 @@ +package drivercontrol + +import ( + "context" + "errors" + "sync" + "testing" + "time" + + "github.com/srcfl/ftw/go/internal/drivers" +) + +type fakeRegistry struct { + mu sync.Mutex + generation uint64 + running bool + sendErr error + sent int + defaults int +} + +func (f *fakeRegistry) SendWithGeneration(_ context.Context, _ string, _ []byte) (uint64, error) { + f.mu.Lock() + defer f.mu.Unlock() + if f.sendErr != nil { + return 0, f.sendErr + } + f.sent++ + return f.generation, nil +} + +func (f *fakeRegistry) ControlStatus(string) (drivers.DriverControlStatus, bool) { + f.mu.Lock() + defer f.mu.Unlock() + return drivers.DriverControlStatus{Generation: f.generation}, f.running +} + +func (f *fakeRegistry) SendDefault(context.Context, string) error { + f.mu.Lock() + defer f.mu.Unlock() + f.defaults++ + return nil +} + +func (f *fakeRegistry) counts() (sent, defaults int) { + f.mu.Lock() + defer f.mu.Unlock() + return f.sent, f.defaults +} + +func newHolds(reg *fakeRegistry) *Holds { + return New(func() Registry { return reg }) +} + +func send(t *testing.T, h *Holds, value any, d time.Duration) (Hold, error) { + t.Helper() + var hold Hold + var err error + h.Send(context.Background(), "heat", "set_offset", value, []byte(`{}`), d, nil, func(got Hold, e error) { + hold, err = got, e + }) + return hold, err +} + +func TestSendArmsAHoldBoundToTheGeneration(t *testing.T) { + reg := &fakeRegistry{generation: 7, running: true} + h := newHolds(reg) + hold, err := send(t, h, 2.0, time.Hour) + if err != nil { + t.Fatal(err) + } + if hold.Generation != 7 || hold.Control != "set_offset" || hold.Value != 2.0 { + t.Fatalf("hold = %+v", hold) + } + if got := h.Active("heat"); got == nil || got.Generation != 7 { + t.Fatalf("Active = %+v", got) + } +} + +func TestFailedSendClearsTheOldHold(t *testing.T) { + reg := &fakeRegistry{generation: 1, running: true} + h := newHolds(reg) + if _, err := send(t, h, 1.0, time.Hour); err != nil { + t.Fatal(err) + } + reg.mu.Lock() + reg.sendErr = errors.New("device gone") + reg.mu.Unlock() + if _, err := send(t, h, 2.0, time.Hour); err == nil { + t.Fatal("send error hidden") + } + if got := h.Active("heat"); got != nil { + t.Fatalf("failed send left hold %+v", got) + } +} + +func TestExpirySendsTheDefault(t *testing.T) { + reg := &fakeRegistry{generation: 1, running: true} + h := newHolds(reg) + if _, err := send(t, h, 1.0, 20*time.Millisecond); err != nil { + t.Fatal(err) + } + deadline := time.Now().Add(2 * time.Second) + for time.Now().Before(deadline) { + if _, d := reg.counts(); d == 1 { + break + } + time.Sleep(5 * time.Millisecond) + } + if _, d := reg.counts(); d != 1 { + t.Fatalf("defaults after expiry = %d, want 1", d) + } + if got := h.Active("heat"); got != nil { + t.Fatalf("hold survived expiry: %+v", got) + } +} + +// A stopped generation must never default its replacement. +func TestExpiryOfAnOldGenerationSendsNoDefault(t *testing.T) { + reg := &fakeRegistry{generation: 1, running: true} + h := newHolds(reg) + if _, err := send(t, h, 1.0, time.Hour); err != nil { + t.Fatal(err) + } + state := h.Peek("heat") + fired := state.CurrentHold() + reg.mu.Lock() + reg.generation = 2 + reg.mu.Unlock() + if err := h.Expire("heat", state, fired); err != nil { + t.Fatal(err) + } + if _, d := reg.counts(); d != 0 { + t.Fatalf("old generation sent %d defaults", d) + } +} + +// A replaced hold's timer must not default the new one. +func TestExpiryOfAReplacedHoldIsANoOp(t *testing.T) { + reg := &fakeRegistry{generation: 1, running: true} + h := newHolds(reg) + if _, err := send(t, h, 1.0, time.Hour); err != nil { + t.Fatal(err) + } + state := h.Peek("heat") + old := state.CurrentHold() + if _, err := send(t, h, 2.0, time.Hour); err != nil { + t.Fatal(err) + } + if err := h.Expire("heat", state, old); err != nil { + t.Fatal(err) + } + if _, d := reg.counts(); d != 0 { + t.Fatalf("replaced hold sent %d defaults", d) + } + if got := h.Active("heat"); got == nil || got.Value != 2.0 { + t.Fatalf("Active = %+v, want the replacement", got) + } +} + +func TestSendDefaultClearsTheHold(t *testing.T) { + reg := &fakeRegistry{generation: 1, running: true} + h := newHolds(reg) + if _, err := send(t, h, 1.0, time.Hour); err != nil { + t.Fatal(err) + } + if err := h.SendDefault(context.Background(), "heat", nil); err != nil { + t.Fatal(err) + } + if _, d := reg.counts(); d != 1 { + t.Fatalf("defaults = %d, want 1", d) + } + if got := h.Active("heat"); got != nil { + t.Fatalf("hold survived default: %+v", got) + } +} + +func TestSendDefaultForAnUnknownDriverFails(t *testing.T) { + h := newHolds(&fakeRegistry{}) + if err := h.SendDefault(context.Background(), "heat", nil); err == nil { + t.Fatal("default to a driver that is not running succeeded") + } +} + +func TestClearDropsStateAndTimer(t *testing.T) { + reg := &fakeRegistry{generation: 1, running: true} + h := newHolds(reg) + if _, err := send(t, h, 1.0, 20*time.Millisecond); err != nil { + t.Fatal(err) + } + h.Clear("heat") + if h.Len() != 0 || h.Peek("heat") != nil { + t.Fatalf("Clear left state: len=%d", h.Len()) + } + time.Sleep(60 * time.Millisecond) + if _, d := reg.counts(); d != 0 { + t.Fatalf("cleared hold's timer sent %d defaults", d) + } +} + +// Without a registry nothing is sent and nothing is held. +func TestNoRegistry(t *testing.T) { + h := New(func() Registry { return nil }) + if _, err := send(t, h, 1.0, time.Hour); err == nil { + t.Fatal("send without a registry succeeded") + } + if err := h.SendDefault(context.Background(), "heat", nil); err == nil { + t.Fatal("default without a registry succeeded") + } + if h.Active("heat") != nil { + t.Fatal("hold without a registry") + } +} diff --git a/go/internal/drivers/catalog_thermal.go b/go/internal/drivers/catalog_thermal.go index 815afaff6..2487f9d6a 100644 --- a/go/internal/drivers/catalog_thermal.go +++ b/go/internal/drivers/catalog_thermal.go @@ -42,8 +42,7 @@ type CatalogSensor struct { Zone string `json:"zone,omitempty"` } -// The strategies a control may declare. synthetic_price is a signal Core -// would publish, not a lever a driver offers, so it is not one of them. +// The strategies a control may declare, and the input type each needs. var controlStrategyInput = map[thermal.InfluenceStrategy]string{ thermal.InfluenceCurveOffset: "number", thermal.InfluenceSetpoint: "number", diff --git a/go/internal/thermal/model.go b/go/internal/thermal/model.go index b5eb71cde..223e76a75 100644 --- a/go/internal/thermal/model.go +++ b/go/internal/thermal/model.go @@ -1,9 +1,9 @@ -// Package thermal defines the site-level contract for heat pumps, cooling, -// domestic hot water, and thermal buffers. +// Package thermal is the vocabulary heat controls are declared in: which heat +// store an asset is, and which lever a driver offers on it. It holds no +// policy. What a planner proposes and what Core accepts are in +// docs/adr/0012-heat-control.md. package thermal -import "math" - // AssetKind identifies which thermal store an asset represents. type AssetKind string @@ -18,10 +18,9 @@ const ( type InfluenceStrategy string const ( - InfluenceSyntheticPrice InfluenceStrategy = "synthetic_price" - InfluenceCurveOffset InfluenceStrategy = "curve_offset" - InfluenceSetpoint InfluenceStrategy = "setpoint" - InfluenceDiscreteMode InfluenceStrategy = "discrete_mode" + InfluenceCurveOffset InfluenceStrategy = "curve_offset" + InfluenceSetpoint InfluenceStrategy = "setpoint" + InfluenceDiscreteMode InfluenceStrategy = "discrete_mode" // InfluenceOnOff is a device whose only lever is start and stop. InfluenceOnOff InfluenceStrategy = "on_off" ) @@ -34,175 +33,3 @@ func ValidAssetKind(k AssetKind) bool { } return false } - -// IntentKind is planner intent before the driver maps it to vendor commands. -type IntentKind string - -const ( - IntentNeutral IntentKind = "neutral" - IntentPrecondition IntentKind = "precondition" - IntentShed IntentKind = "shed" - IntentProtectComfort IntentKind = "protect_comfort" - IntentBoostDHW IntentKind = "boost_dhw" -) - -// TemperatureBand is a hard min/max envelope with an optional normal target. -type TemperatureBand struct { - MinC float64 - NormalC float64 - MaxC float64 -} - -// Contains reports whether temp is within the hard envelope. -func (b TemperatureBand) Contains(tempC float64) bool { - return finite(tempC) && tempC >= b.MinC && tempC <= b.MaxC -} - -// WarmHeadroomC returns degrees available before hitting the max bound. -func (b TemperatureBand) WarmHeadroomC(tempC float64) float64 { - if !finite(tempC) || tempC >= b.MaxC { - return 0 - } - return b.MaxC - tempC -} - -// ShedHeadroomC returns degrees available before hitting the min bound. -func (b TemperatureBand) ShedHeadroomC(tempC float64) float64 { - if !finite(tempC) || tempC <= b.MinC { - return 0 - } - return tempC - b.MinC -} - -// MarginalPrice is the site-level cost signal for the next thermal kWh. -type MarginalPrice struct { - // ImportOreKWh is consumer cost when the marginal kWh comes from grid import. - ImportOreKWh float64 - // ExportOreKWh is opportunity cost when PV surplus would otherwise export. - ExportOreKWh float64 - // PVSurplusW > 0 means the marginal thermal load can be supplied from PV - // that would otherwise cross the site meter as export. - PVSurplusW float64 - // ImportPressureOreKWh penalizes thermal load during fuse/import pressure. - ImportPressureOreKWh float64 - // BatteryOpportunityOreKWh penalizes spending energy the battery needs later. - BatteryOpportunityOreKWh float64 - // ThermalUrgencyCreditOreKWh rewards filling thermal storage before a - // forecast comfort or DHW shortfall. - ThermalUrgencyCreditOreKWh float64 -} - -// SyntheticOreKWh returns the price to publish to smart native controllers. -func (p MarginalPrice) SyntheticOreKWh() float64 { - base := p.ImportOreKWh - if p.PVSurplusW > 0 && finite(p.ExportOreKWh) { - base = p.ExportOreKWh - } - out := base + p.ImportPressureOreKWh + p.BatteryOpportunityOreKWh - p.ThermalUrgencyCreditOreKWh - if !finite(out) { - return 0 - } - return out -} - -// DecisionInput is the minimum state needed for the first policy controller. -type DecisionInput struct { - Kind AssetKind - - SpaceTempC *float64 - SpaceBand TemperatureBand - - DHWTempC *float64 - DHWBand TemperatureBand - - Price MarginalPrice - - // CheapBelowOreKWh and ExpensiveAboveOreKWh are optional thresholds. - // nil means "unset" — a *float64 so that an explicit 0 öre/kWh - // threshold (precondition/boost only when energy is free or - // negative-priced) is distinguishable from "not configured". Swedish - // spot prices go negative regularly, so 0 is a legitimate threshold. - CheapBelowOreKWh *float64 - ExpensiveAboveOreKWh *float64 - PVPreconditionW float64 - AllowDHWBoost bool -} - -// Intent is a strategy-neutral instruction from planner to driver. -type Intent struct { - Kind IntentKind - SyntheticOreKWh float64 - PreconditionHeadroomC float64 - ShedHeadroomC float64 - DHWWarmHeadroomC float64 - Reason string -} - -// DecideIntent implements the conservative first-pass thermal policy. MPC can -// later replace the decision engine while keeping this same driver contract. -func DecideIntent(in DecisionInput) Intent { - price := in.Price.SyntheticOreKWh() - intent := Intent{ - Kind: IntentNeutral, - SyntheticOreKWh: price, - Reason: "neutral", - } - - if in.SpaceTempC != nil { - temp := *in.SpaceTempC - intent.PreconditionHeadroomC, intent.ShedHeadroomC = spaceHeadroom(in.Kind, in.SpaceBand, temp) - if finite(temp) && temp < in.SpaceBand.MinC { - intent.Kind = IntentProtectComfort - intent.Reason = "space temperature below minimum" - return intent - } - if finite(temp) && temp > in.SpaceBand.MaxC && in.Kind == AssetCooling { - intent.Kind = IntentProtectComfort - intent.Reason = "space temperature above maximum" - return intent - } - } - - if in.DHWTempC != nil { - temp := *in.DHWTempC - intent.DHWWarmHeadroomC = in.DHWBand.WarmHeadroomC(temp) - if finite(temp) && temp < in.DHWBand.MinC { - intent.Kind = IntentProtectComfort - intent.Reason = "dhw temperature below minimum" - return intent - } - } - - cheap := in.CheapBelowOreKWh != nil && price <= *in.CheapBelowOreKWh - expensive := in.ExpensiveAboveOreKWh != nil && price >= *in.ExpensiveAboveOreKWh - pvSurplus := in.PVPreconditionW > 0 && in.Price.PVSurplusW >= in.PVPreconditionW - - if in.AllowDHWBoost && (cheap || pvSurplus) && intent.DHWWarmHeadroomC > 0 { - intent.Kind = IntentBoostDHW - intent.Reason = "dhw headroom and cheap energy" - return intent - } - if (cheap || pvSurplus) && intent.PreconditionHeadroomC > 0 { - intent.Kind = IntentPrecondition - intent.Reason = "thermal headroom and cheap energy" - return intent - } - if expensive && intent.ShedHeadroomC > 0 { - intent.Kind = IntentShed - intent.Reason = "expensive energy and shed headroom" - return intent - } - - return intent -} - -func spaceHeadroom(kind AssetKind, band TemperatureBand, tempC float64) (preconditionC, shedC float64) { - if kind == AssetCooling { - return band.ShedHeadroomC(tempC), band.WarmHeadroomC(tempC) - } - return band.WarmHeadroomC(tempC), band.ShedHeadroomC(tempC) -} - -func finite(v float64) bool { - return !math.IsNaN(v) && !math.IsInf(v, 0) -} diff --git a/go/internal/thermal/model_test.go b/go/internal/thermal/model_test.go index a0d38d610..34293ff3a 100644 --- a/go/internal/thermal/model_test.go +++ b/go/internal/thermal/model_test.go @@ -2,141 +2,15 @@ package thermal import "testing" -func ptr(v float64) *float64 { return &v } - -func TestSyntheticPriceUsesExportValueWhenPVSurplus(t *testing.T) { - price := MarginalPrice{ - ImportOreKWh: 180, - ExportOreKWh: 40, - PVSurplusW: 2500, - ImportPressureOreKWh: 10, - BatteryOpportunityOreKWh: 5, - ThermalUrgencyCreditOreKWh: 20, - } - - got := price.SyntheticOreKWh() - want := 35.0 - if got != want { - t.Fatalf("SyntheticOreKWh() = %.1f, want %.1f", got, want) - } -} - -func TestDecideIntentProtectsComfortBeforePrice(t *testing.T) { - space := 19.4 - intent := DecideIntent(DecisionInput{ - Kind: AssetSpaceHeat, - SpaceTempC: &space, - SpaceBand: TemperatureBand{MinC: 20, NormalC: 21, MaxC: 22.5}, - Price: MarginalPrice{ - ImportOreKWh: 300, - }, - ExpensiveAboveOreKWh: ptr(200), - }) - - if intent.Kind != IntentProtectComfort { - t.Fatalf("intent = %q, want %q", intent.Kind, IntentProtectComfort) - } -} - -func TestDecideIntentBoostsDHWBeforeSpacePreheat(t *testing.T) { - space := 21.0 - dhw := 48.0 - intent := DecideIntent(DecisionInput{ - Kind: AssetSpaceHeat, - SpaceTempC: &space, - SpaceBand: TemperatureBand{MinC: 20, NormalC: 21, MaxC: 22.5}, - DHWTempC: &dhw, - DHWBand: TemperatureBand{MinC: 45, NormalC: 50, MaxC: 58}, - AllowDHWBoost: true, - Price: MarginalPrice{ - ImportOreKWh: 40, - }, - CheapBelowOreKWh: ptr(60), - }) - - if intent.Kind != IntentBoostDHW { - t.Fatalf("intent = %q, want %q", intent.Kind, IntentBoostDHW) - } -} - -func TestDecideIntentShedsWhenExpensiveAndComfortAllows(t *testing.T) { - space := 21.5 - intent := DecideIntent(DecisionInput{ - Kind: AssetSpaceHeat, - SpaceTempC: &space, - SpaceBand: TemperatureBand{MinC: 20, NormalC: 21, MaxC: 22.5}, - Price: MarginalPrice{ - ImportOreKWh: 260, - }, - ExpensiveAboveOreKWh: ptr(200), - }) - - if intent.Kind != IntentShed { - t.Fatalf("intent = %q, want %q", intent.Kind, IntentShed) - } -} - -func TestDecideIntentPreconditionsOnPVSurplus(t *testing.T) { - space := 21.0 - intent := DecideIntent(DecisionInput{ - Kind: AssetSpaceHeat, - SpaceTempC: &space, - SpaceBand: TemperatureBand{MinC: 20, NormalC: 21, MaxC: 22.5}, - PVPreconditionW: 1000, - Price: MarginalPrice{ - ImportOreKWh: 180, - ExportOreKWh: 35, - PVSurplusW: 1600, - }, - }) - - if intent.Kind != IntentPrecondition { - t.Fatalf("intent = %q, want %q", intent.Kind, IntentPrecondition) - } -} - -// A CheapBelowOreKWh threshold of exactly 0 means "precondition only when -// energy is free or negative-priced". Because Swedish spot prices go negative -// regularly, 0 is a legitimate threshold and must not be treated as "unset". -func TestDecideIntentPreconditionsAtZeroThresholdOnNegativePrice(t *testing.T) { - space := 21.0 - intent := DecideIntent(DecisionInput{ - Kind: AssetSpaceHeat, - SpaceTempC: &space, - SpaceBand: TemperatureBand{MinC: 20, NormalC: 21, MaxC: 22.5}, - Price: MarginalPrice{ - ImportOreKWh: -50, - }, - CheapBelowOreKWh: ptr(0), - }) - - if intent.Kind != IntentPrecondition { - t.Fatalf("intent = %q, want %q", intent.Kind, IntentPrecondition) - } -} - -func TestDecideIntentCoolingUsesInverseTemperatureHeadroom(t *testing.T) { - space := 23.0 - intent := DecideIntent(DecisionInput{ - Kind: AssetCooling, - SpaceTempC: &space, - SpaceBand: TemperatureBand{MinC: 21, NormalC: 23, MaxC: 25}, - PVPreconditionW: 1000, - ExpensiveAboveOreKWh: ptr(200), - Price: MarginalPrice{ - ImportOreKWh: 260, - ExportOreKWh: 30, - PVSurplusW: 1200, - }, - }) - - if intent.Kind != IntentPrecondition { - t.Fatalf("intent = %q, want %q", intent.Kind, IntentPrecondition) - } - if intent.PreconditionHeadroomC != 2 { - t.Fatalf("precondition headroom = %.1f, want 2.0", intent.PreconditionHeadroomC) - } - if intent.ShedHeadroomC != 2 { - t.Fatalf("shed headroom = %.1f, want 2.0", intent.ShedHeadroomC) +func TestValidAssetKind(t *testing.T) { + for _, k := range []AssetKind{AssetSpaceHeat, AssetCooling, AssetDHW, AssetBuffer} { + if !ValidAssetKind(k) { + t.Errorf("ValidAssetKind(%q) = false", k) + } + } + for _, k := range []AssetKind{"", "sauna", "Space_Heat"} { + if ValidAssetKind(k) { + t.Errorf("ValidAssetKind(%q) = true", k) + } } }