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 3dc30148e7fa776a7b73d2fe6fc536baee9e4903 Mon Sep 17 00:00:00 2001 From: hannesb90 <279736760+hannesb90@users.noreply.github.com> Date: Wed, 11 Nov 2026 20:56:53 +0100 Subject: [PATCH 3/3] test(site): watch go/internal/thermal, which drivers now imports TestWatchedPathsCoverImports fails once drivers imports thermal, since a change there would skip the rig. Co-Authored-By: Claude Opus 5.5 --- go/test/site/runner/runner.go | 1 + 1 file changed, 1 insertion(+) diff --git a/go/test/site/runner/runner.go b/go/test/site/runner/runner.go index 955fab08b..821a8f3fc 100644 --- a/go/test/site/runner/runner.go +++ b/go/test/site/runner/runner.go @@ -28,6 +28,7 @@ var watchedPaths = []string{ "go/internal/units/", // charge steps "go/internal/localzone/", // household clock: goal deadlines, import hour "go/internal/cars/", "go/internal/sitepower/", + "go/internal/thermal/", // control heat effects, imported by drivers "go/internal/events/", "go/internal/state/", "drivers/", "optimizer/native/bundle/", // bundled drivers and solver "go/test/site/",