From 1cbda7431b7e3e189c0928f717e98a193b024eac Mon Sep 17 00:00:00 2001 From: Jan Unterbrink <19386509+subpathdev@users.noreply.github.com> Date: Sun, 1 Nov 2026 15:44:51 +0100 Subject: [PATCH 1/3] feat!: add airing, and deliver commands by comparing desired and reported state A room can now be aired for 15, 30 or 60 minutes: off is sent, and at the end or on "Heat again" the room gets back its mode and target. End times and held targets are saved to a file, so they survive a restart. The controller now keeps what a room should be doing apart from what its thermostat reports. Run is the only place that publishes or saves, and repeats a command until a report confirms it. This replaces the send lock, the flush goroutine and the rollback code, and closes the gap where a command could be resent after a reconnect without still being wanted. BREAKING CHANGE: POST /api/rooms/{id}/target returns 202 instead of 204, and 502 is gone. Whether a target arrived shows in targetState in the room list. Co-Authored-By: Claude Opus 5.5 --- README.md | 11 +- .../heating-control/templates/deployment.yaml | 6 + charts/heating-control/templates/pvc.yaml | 14 + charts/heating-control/values.yaml | 7 + cmd/heating-control/main.go | 13 +- internal/api/api.gen.go | 382 +++++++- internal/api/openapi.yaml | 142 ++- internal/config/config.go | 16 +- internal/controller/controller.go | 389 ++++---- internal/controller/controller_test.go | 848 +++++++++++++----- internal/controller/room.go | 336 +++++++ internal/metrics/metrics.go | 33 +- internal/metrics/metrics_test.go | 17 +- internal/mqtt/client.go | 15 +- internal/mqtt/client_test.go | 41 +- internal/store/store.go | 64 ++ internal/store/store_test.go | 56 ++ internal/web/static/index.html | 90 +- internal/web/web.go | 67 +- internal/web/web_test.go | 121 ++- 20 files changed, 2187 insertions(+), 481 deletions(-) create mode 100644 charts/heating-control/templates/pvc.yaml create mode 100644 internal/controller/room.go create mode 100644 internal/store/store.go create mode 100644 internal/store/store_test.go diff --git a/README.md b/README.md index dd71f8f..caeefff 100644 --- a/README.md +++ b/README.md @@ -2,11 +2,18 @@ A small web page to adjust the heating per room from a phone on the home network: change a room's target temperature, or mark a window as open so the room doesn't heat. It controls Sonoff TRVZB thermostats through Zigbee2MQTT. -Work in progress. +## Airing + +"Airing 15/30/60" sends `system_mode: off`. When the time is up, or on "Heat again", the room gets back the mode and target from before. The end time is saved, so it survives a restart. If the service is down when the time is up, heating comes back when it starts again. Until then the thermostat's own frost protection still applies. + +- A target changed during airing is applied when it ends. +- A thermostat leaving off during airing gets off once more. If it leaves off again, someone is at the radiator, so airing ends. +- Commands repeat until the thermostat confirms them, every 30 s for about 5 minutes. A target then gives up and shows the thermostat's value. Off and heat-again keep trying every 5 minutes and show a problem on the page, because giving up would mean heating with the window open or a cold room. +- A thermostat that turned itself off is left alone: only rooms this program turned off get restored. ## Deployment -The Helm chart in [`charts/heating-control`](charts/heating-control) is published with every release as `oci://ghcr.io/subpathdev/charts/heating-control`. Set `config.broker` and `config.rooms`; adding a room is one more entry in `config.rooms`. Run one replica only, because the MQTT client ID is fixed. +The Helm chart in [`charts/heating-control`](charts/heating-control) is published with every release as `oci://ghcr.io/subpathdev/charts/heating-control`. Set `config.broker` and `config.rooms`; adding a room is one more entry in `config.rooms`. Run one replica only, and give any other instance its own `config.clientId`: a second client with the same ID disconnects the first. The chart keeps saved state on a small PVC. ## API diff --git a/charts/heating-control/templates/deployment.yaml b/charts/heating-control/templates/deployment.yaml index c170b18..97ffd4b 100644 --- a/charts/heating-control/templates/deployment.yaml +++ b/charts/heating-control/templates/deployment.yaml @@ -25,6 +25,7 @@ spec: runAsNonRoot: true runAsUser: 65532 runAsGroup: 65532 + fsGroup: 65532 seccompProfile: type: RuntimeDefault containers: @@ -55,7 +56,12 @@ spec: - name: config mountPath: /etc/heating-control readOnly: true + - name: state + mountPath: {{ dir .Values.config.stateFile }} volumes: - name: config configMap: name: {{ .Release.Name }} + - name: state + persistentVolumeClaim: + claimName: {{ .Release.Name }} diff --git a/charts/heating-control/templates/pvc.yaml b/charts/heating-control/templates/pvc.yaml new file mode 100644 index 0000000..c8cb35a --- /dev/null +++ b/charts/heating-control/templates/pvc.yaml @@ -0,0 +1,14 @@ +apiVersion: v1 +kind: PersistentVolumeClaim +metadata: + name: {{ .Release.Name }} + labels: + {{- include "heating-control.labels" . | nindent 4 }} +spec: + accessModes: ["ReadWriteOnce"] + {{- with .Values.persistence.storageClass }} + storageClassName: {{ . }} + {{- end }} + resources: + requests: + storage: {{ .Values.persistence.size }} diff --git a/charts/heating-control/values.yaml b/charts/heating-control/values.yaml index d91ec14..6f13464 100644 --- a/charts/heating-control/values.yaml +++ b/charts/heating-control/values.yaml @@ -18,6 +18,13 @@ config: rooms: [] # Host names the page is reached by. IP addresses and localhost always work. allowedHosts: [] + stateFile: /var/lib/heating-control/state.json + +# Keeps airing end times and held targets across restarts. +persistence: + size: 16Mi + # Empty uses the cluster's default storage class. + storageClass: "" service: type: ClusterIP diff --git a/cmd/heating-control/main.go b/cmd/heating-control/main.go index a5fd483..91c2edf 100644 --- a/cmd/heating-control/main.go +++ b/cmd/heating-control/main.go @@ -17,6 +17,7 @@ import ( "github.com/subpathdev/heating-control/internal/health" "github.com/subpathdev/heating-control/internal/metrics" "github.com/subpathdev/heating-control/internal/mqtt" + "github.com/subpathdev/heating-control/internal/store" "github.com/subpathdev/heating-control/internal/web" ) @@ -37,7 +38,7 @@ func main() { client := mqtt.New(cfg) m := metrics.New(version) - ctrl := controller.New(cfg, m.Thermostat(client)) + ctrl := controller.New(cfg, m.Thermostat(client), store.New(cfg.StateFile)) client.Start(ctrl) defer client.Close() @@ -49,13 +50,19 @@ func main() { health.Register(mux, version, checker) mux.Handle("GET /metrics", m.Handler()) // Probes and scrapes use the pod's IP and stay outside the page's checks. - mux.Handle("/", web.Handler(ctrl, cfg.AllowedHosts)) + mux.Handle("/", web.Handler(ctrl, checker, cfg.AllowedHosts)) srv := &http.Server{ Addr: ":8080", Handler: mux, ReadHeaderTimeout: 5 * time.Second, } + controllerDone := make(chan struct{}) + go func() { + defer close(controllerDone) + ctrl.Run(ctx) + }() + shutdownDone := make(chan struct{}) go func() { defer close(shutdownDone) @@ -73,5 +80,5 @@ func main() { } // ListenAndServe returns as soon as Shutdown starts, so let requests finish. <-shutdownDone - ctrl.Wait() + <-controllerDone } diff --git a/internal/api/api.gen.go b/internal/api/api.gen.go index 5f58c35..08647ad 100644 --- a/internal/api/api.gen.go +++ b/internal/api/api.gen.go @@ -11,10 +11,35 @@ import ( "encoding/json" "fmt" "net/http" + "time" "github.com/oapi-codegen/runtime" ) +// Defines values for AiringState. +const ( + Active AiringState = "active" + None AiringState = "none" + Restoring AiringState = "restoring" + Starting AiringState = "starting" +) + +// Valid indicates whether the value is a known member of the AiringState enum. +func (e AiringState) Valid() bool { + switch e { + case Active: + return true + case None: + return true + case Restoring: + return true + case Starting: + return true + default: + return false + } +} + // Defines values for Mode. const ( Auto Mode = "auto" @@ -39,6 +64,54 @@ func (e Mode) Valid() bool { } } +// Defines values for StartAiringRequestMinutes. +const ( + N15 StartAiringRequestMinutes = 15 + N30 StartAiringRequestMinutes = 30 + N60 StartAiringRequestMinutes = 60 +) + +// Valid indicates whether the value is a known member of the StartAiringRequestMinutes enum. +func (e StartAiringRequestMinutes) Valid() bool { + switch e { + case N15: + return true + case N30: + return true + case N60: + return true + default: + return false + } +} + +// Defines values for TargetState. +const ( + Applied TargetState = "applied" + Failed TargetState = "failed" + Held TargetState = "held" + Sending TargetState = "sending" +) + +// Valid indicates whether the value is a known member of the TargetState enum. +func (e TargetState) Valid() bool { + switch e { + case Applied: + return true + case Failed: + return true + case Held: + return true + case Sending: + return true + default: + return false + } +} + +// AiringState none means not airing. starting means airing was accepted and the thermostat hasn't confirmed it's off. active means it's off. restoring means airing has ended and the thermostat hasn't confirmed it's heating again. +type AiringState string + // Error defines model for Error. type Error struct { Message string `json:"message"` @@ -49,23 +122,41 @@ type Mode string // Room defines model for Room. type Room struct { + // AiringState none means not airing. starting means airing was accepted and the thermostat hasn't confirmed it's off. active means it's off. restoring means airing has ended and the thermostat hasn't confirmed it's heating again. + AiringState AiringState `json:"airingState"` + + // AiringUntil When airing ends and heating comes back, or null if not airing. + AiringUntil *time.Time `json:"airingUntil"` + + // CommandFailed Whether the thermostat hasn't confirmed a command to turn off or heat again. The backend keeps trying, more slowly. + CommandFailed bool `json:"commandFailed"` + // Heating Whether the thermostat is calling for heat right now. Heating bool `json:"heating"` Id string `json:"id"` Label string `json:"label"` + // LastSeen When the thermostat last reported, or null if not since the backend started. + LastSeen *time.Time `json:"lastSeen"` + // Mode The thermostat's mode. unknown means it hasn't reported yet. Mode Mode `json:"mode"` // Target The chosen target in °C, or null until it is known. Target *float64 `json:"target"` + // TargetState applied means the thermostat confirmed the target. sending means it's on its way. held means it waits until the thermostat can take it. failed means the thermostat never confirmed the last change, so target shows what it reports instead. + TargetState TargetState `json:"targetState"` + // Temperature The room's temperature in °C, or null until it has been reported. Temperature *float64 `json:"temperature"` } // RoomList defines model for RoomList. type RoomList struct { + // Connected Whether the backend is connected to the broker. While it isn't, nothing can be sent and the rooms' values may be out of date. + Connected bool `json:"connected"` + // MaxTarget The highest target allowed, in °C. MaxTarget float64 `json:"maxTarget"` @@ -80,6 +171,21 @@ type SetTargetRequest struct { Target float64 `json:"target"` } +// StartAiringRequest defines model for StartAiringRequest. +type StartAiringRequest struct { + // Minutes How long to air the room. + Minutes StartAiringRequestMinutes `json:"minutes"` +} + +// StartAiringRequestMinutes How long to air the room. +type StartAiringRequestMinutes int + +// TargetState applied means the thermostat confirmed the target. sending means it's on its way. held means it waits until the thermostat can take it. failed means the thermostat never confirmed the last change, so target shows what it reports instead. +type TargetState string + +// StartRoomAiringJSONRequestBody defines body for StartRoomAiring for application/json ContentType. +type StartRoomAiringJSONRequestBody = StartAiringRequest + // SetRoomTargetJSONRequestBody defines body for SetRoomTarget for application/json ContentType. type SetRoomTargetJSONRequestBody = SetTargetRequest @@ -88,6 +194,12 @@ type ServerInterface interface { // ListRooms List the rooms and the allowed target range. // (GET /api/rooms) ListRooms(w http.ResponseWriter, r *http.Request) + // StopRoomAiring End airing now and heat again. + // (DELETE /api/rooms/{id}/airing) + StopRoomAiring(w http.ResponseWriter, r *http.Request, id string) + // StartRoomAiring Stop heating a room because a window is open. + // (POST /api/rooms/{id}/airing) + StartRoomAiring(w http.ResponseWriter, r *http.Request, id string) // SetRoomTarget Change a room's target temperature. // (POST /api/rooms/{id}/target) SetRoomTarget(w http.ResponseWriter, r *http.Request, id string) @@ -116,6 +228,58 @@ func (siw *ServerInterfaceWrapper) ListRooms(w http.ResponseWriter, r *http.Requ handler.ServeHTTP(w, r) } +// StopRoomAiring operation middleware +func (siw *ServerInterfaceWrapper) StopRoomAiring(w http.ResponseWriter, r *http.Request) { + + var err error + _ = err + + // ------------- Path parameter "id" ------------- + var id string + + err = runtime.BindStyledParameterWithOptions("simple", "id", r.PathValue("id"), &id, runtime.BindStyledParameterOptions{ParamLocation: runtime.ParamLocationPath, Explode: false, Required: true, Type: "string", Format: "", ValueIsUnescaped: true}) + if err != nil { + siw.ErrorHandlerFunc(w, r, &InvalidParamFormatError{ParamName: "id", Err: err}) + return + } + + handler := http.Handler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + siw.Handler.StopRoomAiring(w, r, id) + })) + + for _, middleware := range siw.HandlerMiddlewares { + handler = middleware(handler) + } + + handler.ServeHTTP(w, r) +} + +// StartRoomAiring operation middleware +func (siw *ServerInterfaceWrapper) StartRoomAiring(w http.ResponseWriter, r *http.Request) { + + var err error + _ = err + + // ------------- Path parameter "id" ------------- + var id string + + err = runtime.BindStyledParameterWithOptions("simple", "id", r.PathValue("id"), &id, runtime.BindStyledParameterOptions{ParamLocation: runtime.ParamLocationPath, Explode: false, Required: true, Type: "string", Format: "", ValueIsUnescaped: true}) + if err != nil { + siw.ErrorHandlerFunc(w, r, &InvalidParamFormatError{ParamName: "id", Err: err}) + return + } + + handler := http.Handler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + siw.Handler.StartRoomAiring(w, r, id) + })) + + for _, middleware := range siw.HandlerMiddlewares { + handler = middleware(handler) + } + + handler.ServeHTTP(w, r) +} + // SetRoomTarget operation middleware func (siw *ServerInterfaceWrapper) SetRoomTarget(w http.ResponseWriter, r *http.Request) { @@ -264,6 +428,8 @@ func HandlerWithOptions(si ServerInterface, options StdHTTPServerOptions) http.H m.HandleFunc(http.MethodGet+" "+options.BaseURL+"/api/rooms", wrapper.ListRooms) m.HandleFunc(http.MethodPost+" "+options.BaseURL+"/api/rooms/{id}/target", wrapper.SetRoomTarget) + m.HandleFunc(http.MethodDelete+" "+options.BaseURL+"/api/rooms/{id}/airing", wrapper.StopRoomAiring) + m.HandleFunc(http.MethodPost+" "+options.BaseURL+"/api/rooms/{id}/airing", wrapper.StartRoomAiring) return m } @@ -327,6 +493,137 @@ func (response ListRooms500JSONResponse) VisitListRoomsResponse(w http.ResponseW return err } +type StopRoomAiringRequestObject struct { + Id string `json:"id"` +} + +type StopRoomAiringResponseObject interface { + VisitStopRoomAiringResponse(w http.ResponseWriter) error +} + +type StopRoomAiring202Response struct { +} + +func (response StopRoomAiring202Response) VisitStopRoomAiringResponse(w http.ResponseWriter) error { + w.WriteHeader(202) + return nil +} + +type StopRoomAiring403JSONResponse Error + +func (response StopRoomAiring403JSONResponse) VisitStopRoomAiringResponse(w http.ResponseWriter) error { + + var buf bytes.Buffer + if err := json.NewEncoder(&buf).Encode(response); err != nil { + return err + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(403) + _, err := buf.WriteTo(w) + return err +} + +type StopRoomAiring404JSONResponse Error + +func (response StopRoomAiring404JSONResponse) VisitStopRoomAiringResponse(w http.ResponseWriter) error { + + var buf bytes.Buffer + if err := json.NewEncoder(&buf).Encode(response); err != nil { + return err + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(404) + _, err := buf.WriteTo(w) + return err +} + +type StopRoomAiring500JSONResponse Error + +func (response StopRoomAiring500JSONResponse) VisitStopRoomAiringResponse(w http.ResponseWriter) error { + + var buf bytes.Buffer + if err := json.NewEncoder(&buf).Encode(response); err != nil { + return err + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(500) + _, err := buf.WriteTo(w) + return err +} + +type StartRoomAiringRequestObject struct { + Id string `json:"id"` + Body *StartRoomAiringJSONRequestBody +} + +type StartRoomAiringResponseObject interface { + VisitStartRoomAiringResponse(w http.ResponseWriter) error +} + +type StartRoomAiring202Response struct { +} + +func (response StartRoomAiring202Response) VisitStartRoomAiringResponse(w http.ResponseWriter) error { + w.WriteHeader(202) + return nil +} + +type StartRoomAiring400JSONResponse Error + +func (response StartRoomAiring400JSONResponse) VisitStartRoomAiringResponse(w http.ResponseWriter) error { + + var buf bytes.Buffer + if err := json.NewEncoder(&buf).Encode(response); err != nil { + return err + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(400) + _, err := buf.WriteTo(w) + return err +} + +type StartRoomAiring403JSONResponse Error + +func (response StartRoomAiring403JSONResponse) VisitStartRoomAiringResponse(w http.ResponseWriter) error { + + var buf bytes.Buffer + if err := json.NewEncoder(&buf).Encode(response); err != nil { + return err + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(403) + _, err := buf.WriteTo(w) + return err +} + +type StartRoomAiring404JSONResponse Error + +func (response StartRoomAiring404JSONResponse) VisitStartRoomAiringResponse(w http.ResponseWriter) error { + + var buf bytes.Buffer + if err := json.NewEncoder(&buf).Encode(response); err != nil { + return err + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(404) + _, err := buf.WriteTo(w) + return err +} + +type StartRoomAiring500JSONResponse Error + +func (response StartRoomAiring500JSONResponse) VisitStartRoomAiringResponse(w http.ResponseWriter) error { + + var buf bytes.Buffer + if err := json.NewEncoder(&buf).Encode(response); err != nil { + return err + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(500) + _, err := buf.WriteTo(w) + return err +} + type SetRoomTargetRequestObject struct { Id string `json:"id"` Body *SetRoomTargetJSONRequestBody @@ -336,11 +633,11 @@ type SetRoomTargetResponseObject interface { VisitSetRoomTargetResponse(w http.ResponseWriter) error } -type SetRoomTarget204Response struct { +type SetRoomTarget202Response struct { } -func (response SetRoomTarget204Response) VisitSetRoomTargetResponse(w http.ResponseWriter) error { - w.WriteHeader(204) +func (response SetRoomTarget202Response) VisitSetRoomTargetResponse(w http.ResponseWriter) error { + w.WriteHeader(202) return nil } @@ -400,25 +697,17 @@ func (response SetRoomTarget500JSONResponse) VisitSetRoomTargetResponse(w http.R return err } -type SetRoomTarget502JSONResponse Error - -func (response SetRoomTarget502JSONResponse) VisitSetRoomTargetResponse(w http.ResponseWriter) error { - - var buf bytes.Buffer - if err := json.NewEncoder(&buf).Encode(response); err != nil { - return err - } - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(502) - _, err := buf.WriteTo(w) - return err -} - // StrictServerInterface represents all server handlers. type StrictServerInterface interface { // ListRooms List the rooms and the allowed target range. // (GET /api/rooms) ListRooms(ctx context.Context, request ListRoomsRequestObject) (ListRoomsResponseObject, error) + // StopRoomAiring End airing now and heat again. + // (DELETE /api/rooms/{id}/airing) + StopRoomAiring(ctx context.Context, request StopRoomAiringRequestObject) (StopRoomAiringResponseObject, error) + // StartRoomAiring Stop heating a room because a window is open. + // (POST /api/rooms/{id}/airing) + StartRoomAiring(ctx context.Context, request StartRoomAiringRequestObject) (StartRoomAiringResponseObject, error) // SetRoomTarget Change a room's target temperature. // (POST /api/rooms/{id}/target) SetRoomTarget(ctx context.Context, request SetRoomTargetRequestObject) (SetRoomTargetResponseObject, error) @@ -487,6 +776,65 @@ func (sh *strictHandler) ListRooms(w http.ResponseWriter, r *http.Request) { } } +// StopRoomAiring operation middleware +func (sh *strictHandler) StopRoomAiring(w http.ResponseWriter, r *http.Request, id string) { + var request StopRoomAiringRequestObject + + request.Id = id + + handler := func(ctx context.Context, w http.ResponseWriter, r *http.Request, request interface{}) (interface{}, error) { + return sh.ssi.StopRoomAiring(ctx, request.(StopRoomAiringRequestObject)) + } + for _, middleware := range sh.middlewares { + handler = middleware(handler, "StopRoomAiring") + } + + response, err := handler(r.Context(), w, r, request) + + if err != nil { + sh.options.ResponseErrorHandlerFunc(w, r, err) + } else if validResponse, ok := response.(StopRoomAiringResponseObject); ok { + if err := validResponse.VisitStopRoomAiringResponse(w); err != nil { + sh.options.ResponseErrorHandlerFunc(w, r, err) + } + } else if response != nil { + sh.options.ResponseErrorHandlerFunc(w, r, fmt.Errorf("unexpected response type: %T", response)) + } +} + +// StartRoomAiring operation middleware +func (sh *strictHandler) StartRoomAiring(w http.ResponseWriter, r *http.Request, id string) { + var request StartRoomAiringRequestObject + + request.Id = id + + var body StartRoomAiringJSONRequestBody + if err := runtime.BindJSONBody(r, &body); err != nil { + sh.options.RequestErrorHandlerFunc(w, r, fmt.Errorf("can't decode JSON body: %w", err)) + return + } + request.Body = &body + + handler := func(ctx context.Context, w http.ResponseWriter, r *http.Request, request interface{}) (interface{}, error) { + return sh.ssi.StartRoomAiring(ctx, request.(StartRoomAiringRequestObject)) + } + for _, middleware := range sh.middlewares { + handler = middleware(handler, "StartRoomAiring") + } + + response, err := handler(r.Context(), w, r, request) + + if err != nil { + sh.options.ResponseErrorHandlerFunc(w, r, err) + } else if validResponse, ok := response.(StartRoomAiringResponseObject); ok { + if err := validResponse.VisitStartRoomAiringResponse(w); err != nil { + sh.options.ResponseErrorHandlerFunc(w, r, err) + } + } else if response != nil { + sh.options.ResponseErrorHandlerFunc(w, r, fmt.Errorf("unexpected response type: %T", response)) + } +} + // SetRoomTarget operation middleware func (sh *strictHandler) SetRoomTarget(w http.ResponseWriter, r *http.Request, id string) { var request SetRoomTargetRequestObject diff --git a/internal/api/openapi.yaml b/internal/api/openapi.yaml index 924bc6b..b3d30cc 100644 --- a/internal/api/openapi.yaml +++ b/internal/api/openapi.yaml @@ -5,7 +5,7 @@ info: The contract between the heating-control backend and its page. Go types and the server interface are generated from this file, so change it first and run `go generate ./...`. - version: 0.1.0 + version: 0.2.0 paths: /api/rooms: get: @@ -40,9 +40,10 @@ paths: operationId: setRoomTarget summary: Change a room's target temperature. description: >- - The target is rounded to 0.5 °C. While a thermostat is off, or hasn't - reported yet, the target is held and sent once it heats again, because - writing a setpoint could switch it back on. + The target is rounded to 0.5 °C. A 202 means the target was accepted, not + that it has arrived: follow targetState in the room list. It's held while + the room is airing or the thermostat is off or hasn't reported, because + writing a setpoint would switch a thermostat that is off back on. parameters: - name: id in: path @@ -57,8 +58,8 @@ paths: schema: $ref: "#/components/schemas/SetTargetRequest" responses: - "204": - description: The target was sent, or held until the thermostat can take it. + "202": + description: The target was accepted. "400": description: The body is invalid or the target is outside the allowed range. content: @@ -83,8 +84,82 @@ paths: application/json: schema: $ref: "#/components/schemas/Error" - "502": - description: The thermostat couldn't be reached through the broker. + /api/rooms/{id}/airing: + post: + operationId: startRoomAiring + summary: Stop heating a room because a window is open. + description: >- + When the time is up, heating comes back with the mode and target from + before airing. Calling this again during airing sets a new end time. A 202 + means airing was accepted: follow airingState in the room list. + parameters: + - name: id + in: path + required: true + description: The room's id from the room list. + schema: + type: string + requestBody: + required: true + content: + application/json: + schema: + $ref: "#/components/schemas/StartAiringRequest" + responses: + "202": + description: Airing was accepted. + "400": + description: The body is invalid or minutes isn't one of the allowed values. + content: + application/json: + schema: + $ref: "#/components/schemas/Error" + "403": + description: The request came from another website, or by an unknown host name. + content: + application/json: + schema: + $ref: "#/components/schemas/Error" + "404": + description: The room doesn't exist. + content: + application/json: + schema: + $ref: "#/components/schemas/Error" + "500": + description: The server failed to write the response. + content: + application/json: + schema: + $ref: "#/components/schemas/Error" + delete: + operationId: stopRoomAiring + summary: End airing now and heat again. + description: Does nothing if the room isn't airing. + parameters: + - name: id + in: path + required: true + description: The room's id from the room list. + schema: + type: string + responses: + "202": + description: Accepted. + "403": + description: The request came from another website, or by an unknown host name. + content: + application/json: + schema: + $ref: "#/components/schemas/Error" + "404": + description: The room doesn't exist. + content: + application/json: + schema: + $ref: "#/components/schemas/Error" + "500": + description: The server failed to write the response. content: application/json: schema: @@ -93,8 +168,13 @@ components: schemas: RoomList: type: object - required: [rooms, minTarget, maxTarget] + required: [rooms, connected, minTarget, maxTarget] properties: + connected: + type: boolean + description: >- + Whether the backend is connected to the broker. While it isn't, nothing + can be sent and the rooms' values may be out of date. rooms: type: array items: @@ -109,7 +189,7 @@ components: description: The highest target allowed, in °C. Room: type: object - required: [id, label, temperature, target, mode, heating] + required: [id, label, temperature, target, targetState, mode, heating, airingState, airingUntil, lastSeen, commandFailed] properties: id: type: string @@ -130,10 +210,52 @@ components: heating: type: boolean description: Whether the thermostat is calling for heat right now. + targetState: + $ref: "#/components/schemas/TargetState" + airingState: + $ref: "#/components/schemas/AiringState" + airingUntil: + type: string + format: date-time + nullable: true + description: When airing ends and heating comes back, or null if not airing. + lastSeen: + type: string + format: date-time + nullable: true + description: When the thermostat last reported, or null if not since the backend started. + commandFailed: + type: boolean + description: >- + Whether the thermostat hasn't confirmed a command to turn off or heat + again. The backend keeps trying, more slowly. Mode: type: string enum: [unknown, "off", auto, heat] description: The thermostat's mode. unknown means it hasn't reported yet. + TargetState: + type: string + enum: [applied, sending, held, failed] + description: >- + applied means the thermostat confirmed the target. sending means it's on + its way. held means it waits until the thermostat can take it. failed means + the thermostat never confirmed the last change, so target shows what it + reports instead. + AiringState: + type: string + enum: [none, starting, active, restoring] + description: >- + none means not airing. starting means airing was accepted and the thermostat + hasn't confirmed it's off. active means it's off. restoring means airing has + ended and the thermostat hasn't confirmed it's heating again. + StartAiringRequest: + type: object + required: [minutes] + properties: + minutes: + type: integer + enum: [15, 30, 60] + description: How long to air the room. SetTargetRequest: type: object required: [target] diff --git a/internal/config/config.go b/internal/config/config.go index df06fb4..f1199a7 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -18,7 +18,13 @@ type Room struct { // Config is the program's configuration file. type Config struct { - Broker string `yaml:"broker"` + // StateFile is where airing and held targets are saved. Its folder must be + // writable and survive restarts. + StateFile string `yaml:"stateFile"` + Broker string `yaml:"broker"` + // ClientID must be unique on the broker: a second client with the same ID + // disconnects the first. + ClientID string `yaml:"clientId"` BaseTopic string `yaml:"baseTopic"` MinTarget float64 `yaml:"minTarget"` MaxTarget float64 `yaml:"maxTarget"` @@ -38,7 +44,13 @@ func Load(path string) (*Config, error) { } func parse(data []byte) (*Config, error) { - cfg := Config{BaseTopic: "zigbee2mqtt", MinTarget: 10, MaxTarget: 25} + cfg := Config{ + ClientID: "heating-control", + BaseTopic: "zigbee2mqtt", + MinTarget: 10, + MaxTarget: 25, + StateFile: "/var/lib/heating-control/state.json", + } if err := yaml.Unmarshal(data, &cfg); err != nil { return nil, err } diff --git a/internal/controller/controller.go b/internal/controller/controller.go index db91a67..c516e51 100644 --- a/internal/controller/controller.go +++ b/internal/controller/controller.go @@ -1,19 +1,25 @@ -// Package controller holds each room's state and decides what to send to its -// thermostat. +// Package controller keeps what each room should be doing apart from what its +// thermostat reports, and sends commands until the two match. // -// Three behaviours of the Sonoff TRVZB shape this code. They were measured on -// a real device (firmware 1.3.0): +// HTTP handlers and device reports only change state under mu and wake Run, +// which is the only place that talks to a thermostat or the Store. So nothing +// that delivers a report ever waits for a publish: the MQTT client couldn't read +// the broker's acknowledgement while it does. +// +// Four behaviours of the Sonoff TRVZB shape this code. They were measured on a +// real device (firmware 1.3.0): // - In system_mode off it reports its frost protection temperature as the // setpoint, and briefly keeps reporting it after going back to heat. That // value is never a target the user chose. // - Writing a setpoint while it is off switches it back to heat, so a target -// changed during that time is held until the thermostat heats again. +// changed during that time is held until it can be sent. // - After a write it can still send a few reports with the old setpoint. -// - It can go off on its own and return by itself, so "off" doesn't mean -// this program turned it off. +// - It can go off on its own and return by itself, so "off" doesn't mean this +// program turned it off. Only rooms with a saved record are ever restored. package controller import ( + "context" "errors" "fmt" "log" @@ -46,9 +52,27 @@ func ParseMode(s string) (Mode, error) { } } -// echoGrace is how long a thermostat gets to confirm a write. A real TRVZB took -// about 2 s, including the reports it sent with the old setpoint. -const echoGrace = 15 * time.Second +// TargetState says how far a room's target has got. +type TargetState string + +// The states of a room's target. +const ( + TargetApplied TargetState = "applied" + TargetSending TargetState = "sending" + TargetHeld TargetState = "held" + TargetFailed TargetState = "failed" +) + +// AiringState says where a room is in airing. +type AiringState string + +// The states of airing. +const ( + AiringNone AiringState = "none" + AiringStarting AiringState = "starting" + AiringActive AiringState = "active" + AiringRestoring AiringState = "restoring" +) // ErrUnknownRoom is returned for a room that isn't configured. var ErrUnknownRoom = errors.New("unknown room") @@ -56,9 +80,27 @@ var ErrUnknownRoom = errors.New("unknown room") // ErrTargetOutOfRange is returned for a target outside the allowed range. var ErrTargetOutOfRange = errors.New("target out of range") +// tickInterval is how often Run checks timeouts and retries without being woken. +const tickInterval = 5 * time.Second + // Thermostat sends commands to a physical thermostat. type Thermostat interface { - SetTarget(trv string, target float64) error + Apply(trv string, cmd Command) error +} + +// Store saves what must survive a restart. +type Store interface { + Load() (map[string]RoomRecord, error) + Save(records map[string]RoomRecord) error +} + +// RoomRecord is a room's saved state. +type RoomRecord struct { + Target *float64 + TargetDirty bool + AiringUntil *time.Time + RestoreMode Mode + Restoring bool } // ThermostatState is a thermostat's report. Fields it didn't include are nil. @@ -72,54 +114,134 @@ type ThermostatState struct { // RoomStatus is a room's current state. type RoomStatus struct { - ID string - Label string - Temperature *float64 - Target *float64 - Mode Mode - Heating bool -} - -type room struct { - cfg config.Room - sensorTemp *float64 - localTemp *float64 - target *float64 - pending *float64 - mode Mode - heating bool - frost *float64 - - // expected is a target that was sent, and expectUntil how long reports with - // another setpoint count as older than the write. - expected *float64 - expectUntil time.Time - flushing bool + ID string + Label string + Temperature *float64 + Target *float64 + TargetState TargetState + Mode Mode + Heating bool + AiringState AiringState + AiringUntil *time.Time + LastSeen *time.Time + CommandFailed bool } // Controller is safe for concurrent use. type Controller struct { - // mu guards the rooms' state and is never held during a publish. rooms - // itself and each room's cfg don't change after New. - mu sync.Mutex - // sendMu keeps commands in order. Device reports never take it. - sendMu sync.Mutex - flushes sync.WaitGroup thermostat Thermostat + store Store now func() time.Time min, max float64 - rooms []*room + wake chan struct{} + + // mu guards the rooms' state and is never held during a publish or save. + // rooms itself and each room's cfg don't change after New. + mu sync.Mutex + rooms []*room + changed bool } -// New returns a Controller for the configured rooms. -func New(cfg *config.Config, thermostat Thermostat) *Controller { - c := &Controller{thermostat: thermostat, now: time.Now, min: cfg.MinTarget, max: cfg.MaxTarget} +// New returns a Controller for the configured rooms, with what store has saved. +// A store that can't be read is logged, and the rooms start empty. +func New(cfg *config.Config, thermostat Thermostat, store Store) *Controller { + c := &Controller{ + thermostat: thermostat, + store: store, + now: time.Now, + min: cfg.MinTarget, + max: cfg.MaxTarget, + wake: make(chan struct{}, 1), + } + records, err := store.Load() + if err != nil { + log.Printf("loading saved state: %v", err) + } for _, r := range cfg.Rooms { - c.rooms = append(c.rooms, &room{cfg: r}) + c.rooms = append(c.rooms, newRoom(r, records[r.ID])) } return c } +// Run sends commands and saves state until ctx is done. +func (c *Controller) Run(ctx context.Context) { + ticker := time.NewTicker(tickInterval) + defer ticker.Stop() + for { + c.tick() + select { + case <-ctx.Done(): + c.save() + return + case <-ticker.C: + case <-c.wake: + } + } +} + +// tick moves every room on by one step. +func (c *Controller) tick() { + for _, r := range c.rooms { + c.mu.Lock() + now := c.now() + if r.advance(now) { + c.changed = true + } + cmd, send := r.decide(now) + if send { + r.attempts++ + r.lastSent = now + } + c.mu.Unlock() + + if !send { + continue + } + if err := c.thermostat.Apply(r.cfg.TRV, cmd); err != nil { + log.Printf("%s: %s: %v", r.cfg.TRV, cmd.Kind(), err) + // Never reached the thermostat, so it says nothing about it. + c.mu.Lock() + r.attempts-- + r.publishFailed = true + c.mu.Unlock() + continue + } + c.mu.Lock() + r.publishFailed = false + c.mu.Unlock() + } + c.save() +} + +// save writes the rooms if they changed. A failed save tries again next tick. +func (c *Controller) save() { + c.mu.Lock() + if !c.changed { + c.mu.Unlock() + return + } + records := make(map[string]RoomRecord, len(c.rooms)) + for _, r := range c.rooms { + records[r.cfg.ID] = r.record() + } + c.changed = false + c.mu.Unlock() + + if err := c.store.Save(records); err != nil { + log.Printf("saving state: %v", err) + c.mu.Lock() + c.changed = true + c.mu.Unlock() + } +} + +func (c *Controller) signal() { + select { + case c.wake <- struct{}{}: + default: + } +} + // Limits returns the allowed target range. func (c *Controller) Limits() (min, max float64) { return c.min, c.max @@ -132,31 +254,12 @@ func (c *Controller) Rooms() []RoomStatus { out := make([]RoomStatus, 0, len(c.rooms)) for _, r := range c.rooms { - s := RoomStatus{ - ID: r.cfg.ID, - Label: r.cfg.Label, - Temperature: r.sensorTemp, - Target: r.target, - Mode: r.mode, - Heating: r.heating, - } - if s.Temperature == nil { - s.Temperature = r.localTemp - } - if r.pending != nil { - s.Target = r.pending - } - out = append(out, s) + out = append(out, r.status()) } return out } -// SetTarget changes a room's target. It holds the target while the thermostat -// is off or hasn't reported, because a write could switch it back on. -// -// The room's state is changed first and c.mu released before publishing: a -// device report needs c.mu, and the MQTT client can't read the broker's -// acknowledgement while it is delivering a report. +// SetTarget accepts a new target for a room. Run delivers it when it can. func (c *Controller) SetTarget(id string, target float64) error { r := c.find(id) if r == nil { @@ -168,122 +271,58 @@ func (c *Controller) SetTarget(id string, target float64) error { } target = math.Round(target*2) / 2 - c.sendMu.Lock() - defer c.sendMu.Unlock() - c.mu.Lock() - if !r.canWrite() { - r.pending = &target - c.mu.Unlock() - return nil - } - previous := r.target - r.pending = nil - c.expect(r, target) + r.setTarget(target) + c.changed = true c.mu.Unlock() - - if err := c.thermostat.SetTarget(r.cfg.TRV, target); err != nil { - c.mu.Lock() - defer c.mu.Unlock() - r.target = previous - r.expected = nil - return err - } + c.signal() return nil } -// HandleThermostatState records a thermostat's report. It's called while the -// MQTT client delivers a message, so it must not wait for a publish. -func (c *Controller) HandleThermostatState(trv string, s ThermostatState) { - c.mu.Lock() - defer c.mu.Unlock() - - var r *room - for _, candidate := range c.rooms { - if candidate.cfg.TRV == trv { - r = candidate - break - } - } +// StartAiring stops heating a room for d. Called during airing, it sets a new +// end time. +func (c *Controller) StartAiring(id string, d time.Duration) error { + r := c.find(id) if r == nil { - return - } - - if s.Mode != nil { - r.mode = *s.Mode - } - if s.Heating != nil { - r.heating = *s.Heating - } - if s.LocalTemperature != nil { - r.localTemp = s.LocalTemperature - } - if s.FrostProtection != nil { - r.frost = s.FrostProtection - } - if s.Setpoint == nil || r.mode == ModeOff || r.isFrost(*s.Setpoint) { - return - } - - // Reports sent before a write took effect still carry the old setpoint. - // After the grace period the thermostat wins: someone may have turned the - // knob, or the write was lost. - if r.expected == nil || *s.Setpoint == *r.expected || !c.now().Before(r.expectUntil) { - r.target = s.Setpoint - r.expected = nil - } - - // A thermostat back from off restores its own setpoint, so a held target - // only goes out once that has happened, or the restore would overwrite it. - if r.pending != nil && r.canWrite() && !r.flushing { - r.flushing = true - c.flushes.Add(1) - go c.flush(r) + return fmt.Errorf("%w: %q", ErrUnknownRoom, id) } + c.mu.Lock() + r.startAiring(c.now().Add(d)) + c.changed = true + c.mu.Unlock() + c.signal() + return nil } -// flush sends a held target. If that fails the target stays held, and the -// thermostat's next report tries again. -func (c *Controller) flush(r *room) { - defer c.flushes.Done() - - c.sendMu.Lock() - defer c.sendMu.Unlock() - +// StopAiring ends airing now. It does nothing if the room isn't airing. +func (c *Controller) StopAiring(id string) error { + r := c.find(id) + if r == nil { + return fmt.Errorf("%w: %q", ErrUnknownRoom, id) + } c.mu.Lock() - r.flushing = false - if r.pending == nil || !r.canWrite() { - c.mu.Unlock() - return + if r.airingUntil != nil { + r.endAiring() + c.changed = true } - target := *r.pending - previous := r.target - r.pending = nil - c.expect(r, target) c.mu.Unlock() + c.signal() + return nil +} - if err := c.thermostat.SetTarget(r.cfg.TRV, target); err != nil { - log.Printf("%s: sending held target: %v", r.cfg.TRV, err) - c.mu.Lock() - defer c.mu.Unlock() - if r.pending == nil { - r.pending = &target +// HandleThermostatState records a thermostat's report. +func (c *Controller) HandleThermostatState(trv string, s ThermostatState) { + c.mu.Lock() + for _, r := range c.rooms { + if r.cfg.TRV == trv { + if r.report(s, c.now()) { + c.changed = true + } + break } - r.target = previous - r.expected = nil } -} - -// expect records a target about to be sent. The caller holds c.mu. -func (c *Controller) expect(r *room, target float64) { - r.target = &target - r.expected = &target - r.expectUntil = c.now().Add(echoGrace) -} - -// Wait blocks until held targets being sent have finished. -func (c *Controller) Wait() { - c.flushes.Wait() + c.mu.Unlock() + c.signal() } // HandleRoomTemperature records a room sensor's report. @@ -306,19 +345,3 @@ func (c *Controller) find(id string) *room { } return nil } - -// canWrite reports whether a setpoint write is safe. Writing to a thermostat -// that is off, or may be, can switch it back on. -func (r *room) canWrite() bool { - return r.mode != ModeOff && r.mode != ModeUnknown -} - -// isFrost reports whether a setpoint is the frost protection value. The -// default of 7 °C applies until the thermostat has reported its own. -func (r *room) isFrost(setpoint float64) bool { - frost := 7.0 - if r.frost != nil { - frost = *r.frost - } - return setpoint == frost -} diff --git a/internal/controller/controller_test.go b/internal/controller/controller_test.go index dfeb103..1e93e16 100644 --- a/internal/controller/controller_test.go +++ b/internal/controller/controller_test.go @@ -1,8 +1,10 @@ package controller import ( + "context" "errors" "math" + "sync" "testing" "time" @@ -10,32 +12,67 @@ import ( ) type fakeThermostat struct { - targets []float64 - err error - // If set, SetTarget signals entered and waits for release. - entered, release chan struct{} + sent []Command + err error } -func (f *fakeThermostat) SetTarget(_ string, target float64) error { - if f.entered != nil { - close(f.entered) - <-f.release - } +func (f *fakeThermostat) Apply(_ string, cmd Command) error { if f.err != nil { return f.err } - f.targets = append(f.targets, target) + f.sent = append(f.sent, cmd) return nil } -func newController(t *testing.T) (*Controller, *fakeThermostat) { +func (f *fakeThermostat) kinds() []string { + out := []string{} + for _, c := range f.sent { + out = append(out, c.Kind()) + } + return out +} + +type fakeStore struct { + records map[string]RoomRecord + loadErr error + saveErr error + saves int +} + +func (f *fakeStore) Load() (map[string]RoomRecord, error) { return f.records, f.loadErr } +func (f *fakeStore) Save(r map[string]RoomRecord) error { + if f.saveErr != nil { + return f.saveErr + } + f.records = r + f.saves++ + return nil +} + +// harness drives a Controller by hand: time and ticks only move when told to. +type harness struct { + t *testing.T + c *Controller + f *fakeThermostat + store *fakeStore + now time.Time +} + +func newHarness(t *testing.T) *harness { t.Helper() - f := &fakeThermostat{} - return New(&config.Config{ + return newHarnessWithStore(t, &fakeStore{}) +} + +func newHarnessWithStore(t *testing.T, store *fakeStore) *harness { + t.Helper() + h := &harness{t: t, f: &fakeThermostat{}, store: store, now: time.Unix(1000, 0)} + h.c = New(&config.Config{ MinTarget: 10, MaxTarget: 25, Rooms: []config.Room{{ID: "office", Label: "Office", TRV: "trv-office", Sensor: "sensor-office"}}, - }, f), f + }, h.f, store) + h.c.now = func() time.Time { return h.now } + return h } func ptr[T any](v T) *T { return &v } @@ -44,104 +81,152 @@ func state(mode Mode, setpoint float64) ThermostatState { return ThermostatState{Mode: &mode, Setpoint: &setpoint} } -func report(t *testing.T, c *Controller, s ThermostatState) { - t.Helper() - c.HandleThermostatState("trv-office", s) - c.Wait() +// report delivers a report and lets the controller act on it. +func (h *harness) report(mode Mode, setpoint float64) { + h.t.Helper() + h.c.HandleThermostatState("trv-office", state(mode, setpoint)) + h.c.tick() } -func target(t *testing.T, c *Controller) float64 { - t.Helper() - got := c.Rooms()[0].Target +func (h *harness) advance(d time.Duration) { + h.t.Helper() + h.now = h.now.Add(d) + h.c.tick() +} + +func (h *harness) status() RoomStatus { + h.t.Helper() + return h.c.Rooms()[0] +} + +func (h *harness) target() float64 { + h.t.Helper() + got := h.status().Target if got == nil { - t.Fatal("target is unknown") + h.t.Fatal("target is unknown") } return *got } -func TestSetTargetWhileHeating(t *testing.T) { - c, f := newController(t) - report(t, c, state("heat", 20)) - - if err := c.SetTarget("office", 20.5); err != nil { - t.Fatal(err) +func (h *harness) wantKinds(want ...string) { + h.t.Helper() + got := h.f.kinds() + if len(got) != len(want) { + h.t.Fatalf("sent %v, want %v", got, want) + } + for i := range got { + if got[i] != want[i] { + h.t.Fatalf("sent %v, want %v", got, want) + } } - if len(f.targets) != 1 || f.targets[0] != 20.5 { - t.Fatalf("sent %v, want [20.5]", f.targets) +} + +func (h *harness) setTarget(target float64) { + h.t.Helper() + if err := h.c.SetTarget("office", target); err != nil { + h.t.Fatal(err) } - if got := target(t, c); got != 20.5 { - t.Fatalf("target = %v, want 20.5", got) + h.c.tick() +} + +func (h *harness) startAiring(d time.Duration) { + h.t.Helper() + if err := h.c.StartAiring("office", d); err != nil { + h.t.Fatal(err) } + h.c.tick() } -func TestSetTargetRoundsToHalfDegree(t *testing.T) { - c, f := newController(t) - report(t, c, state("heat", 20)) +func TestSetTargetIsSentAndConfirmed(t *testing.T) { + h := newHarness(t) + h.report("heat", 20) - if err := c.SetTarget("office", 20.3); err != nil { - t.Fatal(err) + h.setTarget(20.5) + h.wantKinds("set_target") + if got := *h.f.sent[0].Target; got != 20.5 { + t.Fatalf("sent %v, want 20.5", got) + } + if got := h.status().TargetState; got != TargetSending { + t.Fatalf("state = %s, want sending", got) } - if f.targets[0] != 20.5 { - t.Fatalf("sent %v, want 20.5", f.targets[0]) + h.report("heat", 20.5) + if got := h.status().TargetState; got != TargetApplied { + t.Fatalf("state = %s, want applied", got) } } -func TestSetTargetRejects(t *testing.T) { - c, f := newController(t) - report(t, c, state("heat", 20)) +func TestSetTargetRoundsToHalfDegree(t *testing.T) { + h := newHarness(t) + h.report("heat", 20) + h.setTarget(20.3) + if got := *h.f.sent[0].Target; got != 20.5 { + t.Fatalf("sent %v, want 20.5", got) + } +} +func TestSetTargetRejects(t *testing.T) { + h := newHarness(t) + h.report("heat", 20) for _, bad := range []float64{9.5, 25.5, math.NaN(), math.Inf(1)} { - if err := c.SetTarget("office", bad); !errors.Is(err, ErrTargetOutOfRange) { + if err := h.c.SetTarget("office", bad); !errors.Is(err, ErrTargetOutOfRange) { t.Errorf("SetTarget(%v) = %v, want ErrTargetOutOfRange", bad, err) } } - if err := c.SetTarget("attic", 20); !errors.Is(err, ErrUnknownRoom) { + if err := h.c.SetTarget("attic", 20); !errors.Is(err, ErrUnknownRoom) { t.Errorf("unknown room: %v", err) } - if len(f.targets) != 0 { - t.Fatalf("sent %v, want nothing", f.targets) - } + h.c.tick() + h.wantKinds() } -func TestSetTargetFailureKeepsOldTarget(t *testing.T) { - c, f := newController(t) - report(t, c, state("heat", 20)) - f.err = errors.New("broker down") - - if err := c.SetTarget("office", 21); err == nil { - t.Fatal("want an error") +func TestUnknownRoomForAiring(t *testing.T) { + h := newHarness(t) + if err := h.c.StartAiring("attic", time.Minute); !errors.Is(err, ErrUnknownRoom) { + t.Errorf("start: %v", err) } - if got := target(t, c); got != 20 { - t.Fatalf("target = %v, want 20", got) + if err := h.c.StopAiring("attic"); !errors.Is(err, ErrUnknownRoom) { + t.Errorf("stop: %v", err) } } // Writing a setpoint to a TRVZB that is off switches it to heat, which would // heat with the window open. -func TestSetTargetWhileOffIsHeld(t *testing.T) { - c, f := newController(t) - report(t, c, state("heat", 20)) - report(t, c, state("off", 7)) +func TestTargetIsHeldWhileOff(t *testing.T) { + h := newHarness(t) + h.report("heat", 20) + h.report("off", 7) - if err := c.SetTarget("office", 22); err != nil { - t.Fatal(err) - } - if len(f.targets) != 0 { - t.Fatalf("sent %v while off", f.targets) + h.setTarget(22) + h.wantKinds() + if got := h.status().TargetState; got != TargetHeld { + t.Fatalf("state = %s, want held", got) } - if got := target(t, c); got != 22 { + if got := h.target(); got != 22 { t.Fatalf("page shows %v, want the held 22", got) } - report(t, c, state("heat", 20)) - if len(f.targets) != 1 || f.targets[0] != 22 { - t.Fatalf("sent %v, want [22] once heating again", f.targets) + // It restores its own setpoint first, which would overwrite an earlier write. + h.report("heat", 7) + h.wantKinds() + h.report("heat", 20) + h.wantKinds("set_target") + if got := *h.f.sent[0].Target; got != 22 { + t.Fatalf("sent %v, want 22", got) + } +} + +func TestTargetIsHeldWhileModeIsUnknown(t *testing.T) { + h := newHarness(t) + h.setTarget(22) + h.wantKinds() + if got := h.status().TargetState; got != TargetHeld { + t.Fatalf("state = %s, want held", got) } } // Recorded sequence from a real TRVZB going off and back to heat. func TestFrostSetpointIsNeverATarget(t *testing.T) { - c, _ := newController(t) + h := newHarness(t) for _, s := range []ThermostatState{ state("heat", 20), state("off", 20), @@ -149,215 +234,562 @@ func TestFrostSetpointIsNeverATarget(t *testing.T) { state("heat", 7), state("heat", 20), } { - report(t, c, s) - if got := target(t, c); got != 20 { + h.c.HandleThermostatState("trv-office", s) + if got := h.target(); got != 20 { t.Fatalf("after %s/%v target = %v, want 20", *s.Mode, *s.Setpoint, got) } } } func TestReportedFrostTemperatureIsUsed(t *testing.T) { - c, _ := newController(t) - report(t, c, state("heat", 20)) - report(t, c, ThermostatState{FrostProtection: ptr(5.0)}) - - report(t, c, state("heat", 5)) - if got := target(t, c); got != 20 { + h := newHarness(t) + h.report("heat", 20) + h.c.HandleThermostatState("trv-office", ThermostatState{FrostProtection: ptr(5.0)}) + h.report("heat", 5) + if got := h.target(); got != 20 { t.Fatalf("target = %v, want 20", got) } - report(t, c, state("heat", 7)) - if got := target(t, c); got != 7 { + h.report("heat", 7) + if got := h.target(); got != 7 { t.Fatalf("target = %v, want 7", got) } } func TestKnobChangeUpdatesTarget(t *testing.T) { - c, _ := newController(t) - report(t, c, state("heat", 20)) - report(t, c, state("heat", 18)) - if got := target(t, c); got != 18 { + h := newHarness(t) + h.report("heat", 20) + h.report("heat", 18) + if got := h.target(); got != 18 { t.Fatalf("target = %v, want 18", got) } + h.wantKinds() } -func TestSensorTemperatureWinsOverLocal(t *testing.T) { - c, _ := newController(t) - report(t, c, ThermostatState{LocalTemperature: ptr(22.0)}) - if got := c.Rooms()[0].Temperature; got == nil || *got != 22 { - t.Fatalf("temperature = %v, want the local 22", got) +func TestStaleReportDoesntOverwriteANewTarget(t *testing.T) { + h := newHarness(t) + h.report("heat", 20) + h.setTarget(21) + h.report("heat", 20) + if got := h.target(); got != 21 { + t.Fatalf("target = %v, want 21", got) + } + // Once confirmed, any change is a real one. + h.report("heat", 21) + h.report("heat", 18) + if got := h.target(); got != 18 { + t.Fatalf("after the knob target = %v, want 18", got) } - c.HandleRoomTemperature("sensor-office", 21) - if got := c.Rooms()[0].Temperature; got == nil || *got != 21 { - t.Fatalf("temperature = %v, want the sensor's 21", got) +} + +func TestLostTargetIsRetriedThenGivenUp(t *testing.T) { + h := newHarness(t) + h.report("heat", 20) + h.setTarget(21) + + h.advance(retryInterval - time.Second) + h.wantKinds("set_target") + h.advance(time.Second) + h.wantKinds("set_target", "set_target") + + for i := 0; i < maxAttempts; i++ { + h.advance(retryInterval) + } + if got := len(h.f.sent); got != maxAttempts { + t.Fatalf("sent %d, want %d", got, maxAttempts) + } + s := h.status() + if s.TargetState != TargetFailed { + t.Fatalf("state = %s, want failed", s.TargetState) + } + if got := h.target(); got != 20 { + t.Fatalf("target = %v, want the thermostat's 20", got) + } + // The next change starts afresh. + h.setTarget(22) + if got := h.status().TargetState; got != TargetSending { + t.Fatalf("state = %s, want sending", got) } } -func TestParseMode(t *testing.T) { - for _, valid := range []Mode{ModeOff, ModeAuto, ModeHeat} { - if got, err := ParseMode(string(valid)); err != nil || got != valid { - t.Errorf("ParseMode(%q) = %q, %v", valid, got, err) - } +func TestTargetDoesntGiveUpWhileOffAndNeverBecomesFrost(t *testing.T) { + h := newHarness(t) + h.report("heat", 20) + h.setTarget(21) + for i := 0; i < maxAttempts-1; i++ { + h.advance(retryInterval) } - for _, invalid := range []string{"", "Off", "cool"} { - if _, err := ParseMode(invalid); err == nil { - t.Errorf("ParseMode(%q): want an error", invalid) - } + h.report("off", 7) + h.advance(time.Hour) + s := h.status() + if s.TargetState != TargetHeld || *s.Target != 21 { + t.Fatalf("status %s/%v, want held 21", s.TargetState, *s.Target) } } -func TestPendingWaitsForAKnownMode(t *testing.T) { - c, f := newController(t) - report(t, c, state("off", 7)) - if err := c.SetTarget("office", 22); err != nil { - t.Fatal(err) +func TestFailedPublishDoesntCountAsAnAttempt(t *testing.T) { + h := newHarness(t) + h.report("heat", 20) + h.f.err = errors.New("not connected") + h.setTarget(21) + for i := 0; i < maxAttempts+2; i++ { + h.advance(retryInterval) } - report(t, c, ThermostatState{LocalTemperature: ptr(20.0)}) - if len(f.targets) != 0 { - t.Fatalf("sent %v while still off", f.targets) + if got := h.status().TargetState; got != TargetSending { + t.Fatalf("state = %s, want still sending during an outage", got) } + h.f.err = nil + h.advance(retryInterval) + h.wantKinds("set_target") } -// The MQTT client can't read the broker's acknowledgement while it delivers a -// report, so a report must never wait for a publish. -func TestReportsDontWaitForAPublish(t *testing.T) { - c, f := newController(t) - report(t, c, state("heat", 20)) - f.entered, f.release = make(chan struct{}), make(chan struct{}) +func TestFailedPublishWaitsBeforeTryingAgain(t *testing.T) { + h := newHarness(t) + h.report("heat", 20) + counting := &countingThermostat{err: errors.New("not connected")} + h.c.thermostat = counting + h.setTarget(21) + h.advance(retryInterval - time.Second) + if counting.calls != 1 { + t.Fatalf("tried %d times, want 1", counting.calls) + } + h.advance(time.Second) + if counting.calls != 2 { + t.Fatalf("tried %d times, want 2", counting.calls) + } +} - done := make(chan error) - go func() { done <- c.SetTarget("office", 21) }() - <-f.entered +type countingThermostat struct { + calls int + err error +} - finished := make(chan struct{}) - go func() { - c.HandleThermostatState("trv-office", state("heat", 20)) - c.Rooms() - close(finished) - }() - select { - case <-finished: - case <-time.After(5 * time.Second): - t.Fatal("a report or read waited for the publish") +func (c *countingThermostat) Apply(string, Command) error { + c.calls++ + return c.err +} + +func TestAiringTurnsOffAndRestores(t *testing.T) { + h := newHarness(t) + h.report("heat", 20) + + h.startAiring(15 * time.Minute) + h.wantKinds("off") + if got := h.status().AiringState; got != AiringStarting { + t.Fatalf("state = %s, want starting", got) + } + h.report("off", 20) + h.report("off", 7) + s := h.status() + if s.AiringState != AiringActive || s.AiringUntil == nil { + t.Fatalf("status = %+v", s) + } + if got := h.target(); got != 20 { + t.Fatalf("target = %v during airing, want 20", got) + } + + h.advance(15*time.Minute - time.Second) + h.wantKinds("off") + h.advance(time.Second) + h.wantKinds("off", "restore") + cmd := h.f.sent[1] + if *cmd.Mode != ModeHeat || *cmd.Target != 20 { + t.Fatalf("restore sent %v/%v, want heat/20", *cmd.Mode, *cmd.Target) + } + if got := h.status().AiringState; got != AiringRestoring { + t.Fatalf("state = %s, want restoring", got) } - close(f.release) - if err := <-done; err != nil { + h.report("heat", 7) + if got := h.status().AiringState; got != AiringRestoring { + t.Fatalf("after the transient report state = %s, want restoring", got) + } + h.report("heat", 20) + if got := h.status().AiringState; got != AiringNone { + t.Fatalf("state = %s, want none", got) + } + h.advance(time.Hour) + h.wantKinds("off", "restore") +} + +func TestStopAiringRestoresNow(t *testing.T) { + h := newHarness(t) + h.report("heat", 20) + h.startAiring(time.Hour) + h.report("off", 7) + + if err := h.c.StopAiring("office"); err != nil { t.Fatal(err) } + h.c.tick() + h.wantKinds("off", "restore") } -func TestSendingAHeldTargetDoesntBlockTheReport(t *testing.T) { - c, f := newController(t) - report(t, c, state("off", 7)) - if err := c.SetTarget("office", 22); err != nil { +func TestStopAiringWithoutAiringDoesNothing(t *testing.T) { + h := newHarness(t) + h.report("heat", 20) + if err := h.c.StopAiring("office"); err != nil { t.Fatal(err) } - f.entered, f.release = make(chan struct{}), make(chan struct{}) + h.c.tick() + h.wantKinds() +} - finished := make(chan struct{}) - go func() { - c.HandleThermostatState("trv-office", state("heat", 20)) - close(finished) - }() - select { - case <-finished: - case <-time.After(5 * time.Second): - t.Fatal("the report waited for the publish") +func TestAiringAgainSetsANewEndTime(t *testing.T) { + h := newHarness(t) + h.report("heat", 20) + h.startAiring(15 * time.Minute) + h.report("off", 7) + + h.startAiring(time.Hour) + h.advance(30 * time.Minute) + h.wantKinds("off") + h.advance(30 * time.Minute) + h.wantKinds("off", "restore") +} + +func TestTargetChangedDuringAiringIsAppliedAtTheEnd(t *testing.T) { + h := newHarness(t) + h.report("heat", 20) + h.startAiring(15 * time.Minute) + h.report("off", 7) + + h.setTarget(22) + h.wantKinds("off") + s := h.status() + if s.TargetState != TargetHeld || *s.Target != 22 { + t.Fatalf("status = %+v", s) + } + + h.advance(15 * time.Minute) + h.wantKinds("off", "restore") + if got := *h.f.sent[1].Target; got != 22 { + t.Fatalf("restored %v, want 22", got) } - <-f.entered - close(f.release) - c.Wait() - if len(f.targets) != 1 || f.targets[0] != 22 { - t.Fatalf("sent %v, want [22]", f.targets) + h.report("heat", 22) + if got := h.status().TargetState; got != TargetApplied { + t.Fatalf("state = %s, want applied", got) } } -func TestHeldTargetWaitsForTheThermostatToRestore(t *testing.T) { - c, f := newController(t) - report(t, c, state("off", 7)) - if err := c.SetTarget("office", 22); err != nil { - t.Fatal(err) +func TestAiringRestoresTheModeFromBefore(t *testing.T) { + h := newHarness(t) + h.report("auto", 20) + h.startAiring(time.Minute) + h.report("off", 7) + h.advance(time.Minute) + if got := *h.f.sent[1].Mode; got != ModeAuto { + t.Fatalf("restored %s, want auto", got) } - report(t, c, state("heat", 7)) - if len(f.targets) != 0 { - t.Fatalf("sent %v before the thermostat restored its setpoint", f.targets) +} + +// The thermostat is already in its own pause, so off isn't the mode to restore. +func TestAiringDuringAThermostatPauseRestoresHeat(t *testing.T) { + h := newHarness(t) + h.report("heat", 20) + h.report("off", 7) + + h.startAiring(time.Hour) + h.wantKinds() + if got := h.status().AiringState; got != AiringActive { + t.Fatalf("state = %s, want active", got) } - report(t, c, state("heat", 20)) - if len(f.targets) != 1 || f.targets[0] != 22 { - t.Fatalf("sent %v, want [22]", f.targets) + + // Its pause ends, and off goes out again. + h.report("heat", 7) + h.wantKinds("off") + h.report("off", 7) + + h.advance(time.Hour) + if got := *h.f.sent[1].Mode; got != ModeHeat { + t.Fatalf("restored %s, want heat", got) } } -func TestHeldTargetIsRetriedAfterAFailure(t *testing.T) { - c, f := newController(t) - report(t, c, state("off", 7)) - if err := c.SetTarget("office", 22); err != nil { - t.Fatal(err) +func TestAiringWithUnknownModeSendsOffAndRestoresHeat(t *testing.T) { + h := newHarness(t) + h.startAiring(time.Minute) + h.wantKinds("off") + h.report("off", 7) + h.advance(time.Minute) + if got := *h.f.sent[1].Mode; got != ModeHeat { + t.Fatalf("restored %s, want heat", got) } - f.err = errors.New("broker down") - report(t, c, state("heat", 20)) - if got := target(t, c); got != 22 { - t.Fatalf("page shows %v, want the held 22", got) + if got := h.f.sent[1].Target; got != nil { + t.Fatalf("restored target %v, want none known", *got) } + h.report("heat", 19) + if got := h.status().AiringState; got != AiringNone { + t.Fatalf("state = %s, want none", got) + } +} - f.err = nil - report(t, c, state("heat", 20)) - if len(f.targets) != 1 || f.targets[0] != 22 { - t.Fatalf("sent %v, want [22] on the retry", f.targets) +func TestSecondFlipDuringAiringEndsIt(t *testing.T) { + h := newHarness(t) + h.report("heat", 20) + h.startAiring(time.Hour) + h.report("off", 7) + + h.report("heat", 20) + h.wantKinds("off", "off") + h.report("off", 7) + + h.report("heat", 21) + h.wantKinds("off", "off") + s := h.status() + if s.AiringState != AiringNone || s.AiringUntil != nil { + t.Fatalf("status = %+v, want airing over", s) } - report(t, c, state("heat", 20)) - if len(f.targets) != 1 { - t.Fatalf("sent %v, want only one", f.targets) + if got := h.target(); got != 21 { + t.Fatalf("target = %v, want the knob's 21", got) } + h.advance(2 * time.Hour) + h.wantKinds("off", "off") } -func TestUnknownModeHoldsTheTarget(t *testing.T) { - c, f := newController(t) - if err := c.SetTarget("office", 22); err != nil { - t.Fatal(err) +func TestReportBeforeOffTookEffectIsNotAFlip(t *testing.T) { + h := newHarness(t) + h.report("heat", 20) + h.startAiring(time.Hour) + h.report("heat", 20) + h.report("heat", 20) + h.wantKinds("off") + if got := h.status().AiringUntil; got == nil { + t.Fatal("airing ended") } - if len(f.targets) != 0 { - t.Fatalf("sent %v before the thermostat reported", f.targets) +} + +func TestLostOffIsRetriedAndKeepsGoing(t *testing.T) { + h := newHarness(t) + h.report("heat", 20) + h.startAiring(24 * time.Hour) + + for i := 0; i < maxAttempts; i++ { + h.advance(retryInterval) } - if got := target(t, c); got != 22 { - t.Fatalf("page shows %v, want the held 22", got) + sent := len(h.f.sent) + if sent != maxAttempts { + t.Fatalf("sent %d, want %d", sent, maxAttempts) + } + if !h.status().CommandFailed { + t.Fatal("the failure isn't visible") + } + h.advance(retryInterval) + if len(h.f.sent) != sent { + t.Fatal("kept retrying quickly") + } + h.advance(slowRetryInterval) + if len(h.f.sent) != sent+1 { + t.Fatal("stopped retrying") + } + h.report("off", 7) + if h.status().CommandFailed { + t.Fatal("still shown as failed once confirmed") } } -func TestStaleReportDoesntOverwriteANewTarget(t *testing.T) { - c, _ := newController(t) - now := time.Unix(1000, 0) - c.now = func() time.Time { return now } - report(t, c, state("heat", 20)) - if err := c.SetTarget("office", 21); err != nil { - t.Fatal(err) +func TestLostRestoreKeepsGoing(t *testing.T) { + h := newHarness(t) + h.report("heat", 20) + h.startAiring(time.Minute) + h.report("off", 7) + h.advance(time.Minute) + + for i := 0; i < maxAttempts; i++ { + h.advance(retryInterval) + } + if !h.status().CommandFailed { + t.Fatal("the failure isn't visible") } + sent := len(h.f.sent) + h.advance(slowRetryInterval) + if len(h.f.sent) != sent+1 { + t.Fatal("stopped retrying") + } + h.report("heat", 20) + s := h.status() + if s.CommandFailed || s.AiringState != AiringNone { + t.Fatalf("status = %+v", s) + } +} - report(t, c, state("heat", 20)) - if got := target(t, c); got != 21 { - t.Fatalf("after a stale report target = %v, want 21", got) +func TestRestartDuringAiringKeepsItsEndTime(t *testing.T) { + store := &fakeStore{} + h := newHarnessWithStore(t, store) + h.report("heat", 20) + h.startAiring(time.Hour) + h.report("off", 7) + + again := newHarnessWithStore(t, store) + again.now = h.now.Add(10 * time.Minute) + again.c.tick() + // The thermostat hasn't reported yet, and off is safe to repeat. + again.wantKinds("off") + again.report("off", 7) + if got := again.status().AiringState; got != AiringActive { + t.Fatalf("state = %s, want active", got) + } + if got := again.target(); got != 20 { + t.Fatalf("target = %v, want the saved 20", got) } - // Once confirmed, any change is a real one, even inside the grace period. - report(t, c, state("heat", 21)) - report(t, c, state("heat", 18)) - if got := target(t, c); got != 18 { - t.Fatalf("after the knob target = %v, want 18", got) + again.advance(50 * time.Minute) + again.wantKinds("off", "restore") + cmd := again.f.sent[1] + if *cmd.Mode != ModeHeat || *cmd.Target != 20 { + t.Fatalf("restored %v/%v", *cmd.Mode, *cmd.Target) + } +} + +func TestRestartAfterTheEndTimeRestoresAtStart(t *testing.T) { + store := &fakeStore{} + h := newHarnessWithStore(t, store) + h.report("heat", 20) + h.startAiring(time.Minute) + h.report("off", 7) + + again := newHarnessWithStore(t, store) + again.now = h.now.Add(time.Hour) + again.c.tick() + again.wantKinds("restore") +} + +func TestRestartKeepsAHeldTarget(t *testing.T) { + store := &fakeStore{} + h := newHarnessWithStore(t, store) + h.report("heat", 20) + h.report("off", 7) + h.setTarget(22) + + again := newHarnessWithStore(t, store) + again.report("off", 7) + if got := again.status().TargetState; got != TargetHeld { + t.Fatalf("state = %s, want held", got) + } + again.report("heat", 20) + again.wantKinds("set_target") +} + +// Only saved records are restored: the thermostat can go off by itself. +func TestRoomOffAtStartWithoutARecordIsLeftAlone(t *testing.T) { + h := newHarness(t) + h.report("off", 7) + h.advance(time.Hour) + h.wantKinds() + if got := h.status().AiringState; got != AiringNone { + t.Fatalf("state = %s, want none", got) + } +} + +func TestUnreadableStoreStartsEmpty(t *testing.T) { + h := newHarnessWithStore(t, &fakeStore{loadErr: errors.New("corrupt")}) + h.report("heat", 20) + if got := h.target(); got != 20 { + t.Fatalf("target = %v, want 20", got) } } -func TestLostWriteIsShownAfterTheGracePeriod(t *testing.T) { - c, _ := newController(t) - now := time.Unix(1000, 0) - c.now = func() time.Time { return now } - report(t, c, state("heat", 20)) +func TestFailedSaveIsRetried(t *testing.T) { + store := &fakeStore{saveErr: errors.New("disk full")} + h := newHarnessWithStore(t, store) + h.report("heat", 20) + h.startAiring(time.Hour) + if store.saves != 0 { + t.Fatal("saved despite the error") + } + store.saveErr = nil + h.advance(time.Second) + if store.saves != 1 || store.records["office"].AiringUntil == nil { + t.Fatalf("saves = %d, records = %+v", store.saves, store.records) + } + h.advance(time.Second) + if store.saves != 1 { + t.Fatalf("saved %d times without a change", store.saves) + } +} + +func TestSensorTemperatureWinsOverLocal(t *testing.T) { + h := newHarness(t) + h.c.HandleThermostatState("trv-office", ThermostatState{LocalTemperature: ptr(22.0)}) + if got := h.status().Temperature; got == nil || *got != 22 { + t.Fatalf("temperature = %v, want the local 22", got) + } + h.c.HandleRoomTemperature("sensor-office", 21) + if got := h.status().Temperature; got == nil || *got != 21 { + t.Fatalf("temperature = %v, want the sensor's 21", got) + } +} + +func TestLastSeenIsSet(t *testing.T) { + h := newHarness(t) + if h.status().LastSeen != nil { + t.Fatal("lastSeen before any report") + } + h.report("heat", 20) + if got := h.status().LastSeen; got == nil || !got.Equal(h.now) { + t.Fatalf("lastSeen = %v", got) + } +} + +func TestParseMode(t *testing.T) { + for _, valid := range []Mode{ModeOff, ModeAuto, ModeHeat} { + if got, err := ParseMode(string(valid)); err != nil || got != valid { + t.Errorf("ParseMode(%q) = %q, %v", valid, got, err) + } + } + for _, invalid := range []string{"", "Off", "cool"} { + if _, err := ParseMode(invalid); err == nil { + t.Errorf("ParseMode(%q): want an error", invalid) + } + } +} + +// Run is the only place that publishes, so reports and reads never wait for it. +func TestRunDoesntBlockReportsDuringAPublish(t *testing.T) { + block := &blockingThermostat{entered: make(chan struct{}), release: make(chan struct{})} + c := New(&config.Config{ + MinTarget: 10, + MaxTarget: 25, + Rooms: []config.Room{{ID: "office", Label: "Office", TRV: "trv-office"}}, + }, block, &fakeStore{}) + c.HandleThermostatState("trv-office", state("heat", 20)) + + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan struct{}) + go func() { + c.Run(ctx) + close(done) + }() if err := c.SetTarget("office", 21); err != nil { t.Fatal(err) } + <-block.entered - now = now.Add(echoGrace) - report(t, c, state("heat", 20)) - if got := target(t, c); got != 20 { - t.Fatalf("target = %v, want the thermostat's 20", got) + finished := make(chan struct{}) + go func() { + c.HandleThermostatState("trv-office", state("heat", 20)) + c.Rooms() + if err := c.SetTarget("office", 22); err != nil { + t.Error(err) + } + close(finished) + }() + select { + case <-finished: + case <-time.After(5 * time.Second): + t.Fatal("waited for the publish") } + close(block.release) + cancel() + <-done +} + +type blockingThermostat struct { + once sync.Once + entered, release chan struct{} +} + +func (b *blockingThermostat) Apply(string, Command) error { + b.once.Do(func() { close(b.entered) }) + <-b.release + return nil } diff --git a/internal/controller/room.go b/internal/controller/room.go new file mode 100644 index 0000000..840b2b1 --- /dev/null +++ b/internal/controller/room.go @@ -0,0 +1,336 @@ +package controller + +import ( + "log" + "time" + + "github.com/subpathdev/heating-control/internal/config" +) + +const ( + // retryInterval is how long a thermostat gets to confirm a command. A real + // TRVZB took about 2 s, including the reports it sent with the old value. + retryInterval = 30 * time.Second + // maxAttempts at retryInterval give about 5 minutes. Then a target gives up. + maxAttempts = 10 + // slowRetryInterval keeps turning a room off or back on after that, because + // giving up would leave it heating with the window open, or cold. + slowRetryInterval = 5 * time.Minute + // maxFlips is how often a thermostat may leave off during airing. The first + // time could be its own pause ending, so off is sent again. The second time + // someone at the radiator wants heat, so airing ends. + maxFlips = 2 + // defaultFrost applies until the thermostat has reported its own. + defaultFrost = 7.0 +) + +// Command is what to send to a thermostat. Nil fields stay as they are. +type Command struct { + Mode *Mode + Target *float64 +} + +// Kind names the command for logs and metrics. +func (c Command) Kind() string { + switch { + case c.Mode != nil && *c.Mode == ModeOff: + return "off" + case c.Mode != nil: + return "restore" + default: + return "set_target" + } +} + +type room struct { + cfg config.Room + + // What the thermostat and sensor report. + mode Mode + setpoint *float64 + heating bool + localTemp *float64 + sensorTemp *float64 + frost *float64 + lastSeen *time.Time + lastOnMode Mode + // settled is set once it reports its own setpoint after being off. Until + // then a write could be overwritten by it restoring that setpoint. + settled bool + + // What the room should be doing. Saved. + target *float64 + targetDirty bool + airingUntil *time.Time + restoreMode Mode + restoring bool + + // How delivery is going. + attempts int + lastSent time.Time + // publishFailed means the last command never reached the broker, so it waits + // for retryInterval without counting as an attempt. + publishFailed bool + offConfirmed bool + flips int + targetFailed bool +} + +func newRoom(cfg config.Room, rec RoomRecord) *room { + return &room{ + cfg: cfg, + target: rec.Target, + targetDirty: rec.TargetDirty, + airingUntil: rec.AiringUntil, + restoreMode: rec.RestoreMode, + restoring: rec.Restoring, + } +} + +func (r *room) record() RoomRecord { + return RoomRecord{ + Target: r.target, + TargetDirty: r.targetDirty, + AiringUntil: r.airingUntil, + RestoreMode: r.restoreMode, + Restoring: r.restoring, + } +} + +// decide returns the command the room needs now, if any. It changes nothing. +func (r *room) decide(now time.Time) (Command, bool) { + var cmd Command + switch { + case r.airingUntil != nil: + if r.mode == ModeOff { + return cmd, false + } + off := ModeOff + cmd.Mode = &off + case r.restoring: + mode := r.restoreMode + cmd.Mode = &mode + cmd.Target = r.target + case r.targetDirty && r.canWrite() && r.settled: + cmd.Target = r.target + default: + return cmd, false + } + return cmd, r.due(now) +} + +func (r *room) due(now time.Time) bool { + if r.attempts == 0 && !r.publishFailed { + return true + } + wait := retryInterval + if r.attempts >= maxAttempts { + wait = slowRetryInterval + } + return !now.Before(r.lastSent.Add(wait)) +} + +// advance applies what time alone changes, and reports whether anything to save +// changed. +func (r *room) advance(now time.Time) bool { + switch { + case r.airingUntil != nil && !now.Before(*r.airingUntil): + r.endAiring() + return true + case r.targetDirty && r.airingUntil == nil && !r.restoring && r.canWrite() && + r.attempts >= maxAttempts && !now.Before(r.lastSent.Add(retryInterval)): + // A knob turn and a lost write look the same, so the thermostat wins and + // the page says so. + log.Printf("%s: target not confirmed after %d attempts", r.cfg.TRV, r.attempts) + r.targetDirty = false + r.targetFailed = true + r.attempts = 0 + if r.setpoint != nil && !r.isFrost(*r.setpoint) { + r.target = r.setpoint + } + return true + } + return false +} + +// report records a thermostat's report and reports whether anything to save +// changed. +func (r *room) report(s ThermostatState, now time.Time) bool { + r.lastSeen = &now + if s.Heating != nil { + r.heating = *s.Heating + } + if s.LocalTemperature != nil { + r.localTemp = s.LocalTemperature + } + if s.FrostProtection != nil { + r.frost = s.FrostProtection + } + if s.Setpoint != nil { + r.setpoint = s.Setpoint + } + + changed := false + if s.Mode != nil { + r.mode = *s.Mode + if r.canWrite() { + r.lastOnMode = r.mode + } + if r.mode == ModeOff { + r.settled = false + } + changed = r.reportAiringMode() + } + if r.setpoint == nil { + return changed + } + own := r.canWrite() && !r.isFrost(*r.setpoint) + if own { + r.settled = true + } + + switch { + case r.airingUntil != nil: + case r.restoring: + if r.mode == r.restoreMode && (r.target == nil || *r.setpoint == *r.target) { + r.restoring = false + r.targetDirty = false + r.attempts = 0 + changed = true + } + case r.targetDirty: + if *r.setpoint == *r.target { + r.targetDirty = false + r.attempts = 0 + changed = true + } + case own && (r.target == nil || *r.setpoint != *r.target): + // Someone turned the knob. + r.target = r.setpoint + r.targetFailed = false + changed = true + } + return changed +} + +// reportAiringMode handles a mode report during airing. +func (r *room) reportAiringMode() bool { + if r.airingUntil == nil { + return false + } + if r.mode == ModeOff { + if !r.offConfirmed { + r.attempts = 0 + } + r.offConfirmed = true + return false + } + if r.mode == ModeUnknown || !r.offConfirmed { + return false + } + r.offConfirmed = false + r.attempts = 0 + r.flips++ + if r.flips < maxFlips { + return false + } + // The thermostat is on because someone wants it on, so nothing to restore. + log.Printf("%s: left off %d times, ending airing", r.cfg.TRV, r.flips) + r.airingUntil = nil + r.restoring = false + r.flips = 0 + return true +} + +func (r *room) setTarget(target float64) { + r.target = &target + r.targetDirty = true + r.targetFailed = false + // During airing this mustn't reset the count for the off command. + if r.airingUntil == nil && !r.restoring { + r.attempts = 0 + r.publishFailed = false + } +} + +func (r *room) startAiring(until time.Time) { + if r.airingUntil == nil && !r.restoring { + r.restoreMode = r.lastOnMode + if r.restoreMode == ModeUnknown { + r.restoreMode = ModeHeat + } + } + if r.airingUntil == nil { + r.attempts = 0 + r.publishFailed = false + r.flips = 0 + r.offConfirmed = r.mode == ModeOff + } + r.restoring = false + r.airingUntil = &until +} + +func (r *room) endAiring() { + r.airingUntil = nil + r.restoring = true + r.attempts = 0 + r.publishFailed = false + r.flips = 0 + r.offConfirmed = false +} + +func (r *room) status() RoomStatus { + s := RoomStatus{ + ID: r.cfg.ID, + Label: r.cfg.Label, + Temperature: r.sensorTemp, + Target: r.target, + TargetState: r.targetState(), + Mode: r.mode, + Heating: r.heating, + AiringState: AiringNone, + AiringUntil: r.airingUntil, + LastSeen: r.lastSeen, + } + if s.Temperature == nil { + s.Temperature = r.localTemp + } + switch { + case r.airingUntil != nil && r.offConfirmed: + s.AiringState = AiringActive + case r.airingUntil != nil: + s.AiringState = AiringStarting + s.CommandFailed = r.attempts >= maxAttempts + case r.restoring: + s.AiringState = AiringRestoring + s.CommandFailed = r.attempts >= maxAttempts + } + return s +} + +func (r *room) targetState() TargetState { + switch { + case r.targetDirty && r.airingUntil == nil && !r.restoring && r.canWrite(): + return TargetSending + case r.targetDirty: + return TargetHeld + case r.targetFailed: + return TargetFailed + default: + return TargetApplied + } +} + +// canWrite reports whether a setpoint write is safe. Writing to a thermostat +// that is off, or may be, can switch it back on. +func (r *room) canWrite() bool { + return r.mode != ModeOff && r.mode != ModeUnknown +} + +func (r *room) isFrost(setpoint float64) bool { + frost := defaultFrost + if r.frost != nil { + frost = *r.frost + } + return setpoint == frost +} diff --git a/internal/metrics/metrics.go b/internal/metrics/metrics.go index 0396ebc..c6ebee7 100644 --- a/internal/metrics/metrics.go +++ b/internal/metrics/metrics.go @@ -87,13 +87,13 @@ type countingThermostat struct { commands *prometheus.CounterVec } -func (t countingThermostat) SetTarget(trv string, target float64) error { - err := t.next.SetTarget(trv, target) +func (t countingThermostat) Apply(trv string, cmd controller.Command) error { + err := t.next.Apply(trv, cmd) result := "ok" if err != nil { result = "error" } - t.commands.WithLabelValues(trv, "set_target", result).Inc() + t.commands.WithLabelValues(trv, cmd.Kind(), result).Inc() return err } @@ -103,16 +103,41 @@ var targetDesc = prometheus.NewDesc( []string{"room"}, nil, ) +var airingDesc = prometheus.NewDesc( + "heating_control_room_airing", + "1 while a room is airing, including until the thermostat confirms it's back on.", + []string{"room"}, nil, +) + +var commandFailedDesc = prometheus.NewDesc( + "heating_control_room_command_failed", + "1 while the thermostat hasn't confirmed a command to turn off or heat again.", + []string{"room"}, nil, +) + type roomCollector struct { rooms Rooms } -func (c roomCollector) Describe(ch chan<- *prometheus.Desc) { ch <- targetDesc } +func (c roomCollector) Describe(ch chan<- *prometheus.Desc) { + ch <- targetDesc + ch <- airingDesc + ch <- commandFailedDesc +} func (c roomCollector) Collect(ch chan<- prometheus.Metric) { for _, r := range c.rooms.Rooms() { if r.Target != nil { ch <- prometheus.MustNewConstMetric(targetDesc, prometheus.GaugeValue, *r.Target, r.ID) } + ch <- prometheus.MustNewConstMetric(airingDesc, prometheus.GaugeValue, boolValue(r.AiringState != controller.AiringNone), r.ID) + ch <- prometheus.MustNewConstMetric(commandFailedDesc, prometheus.GaugeValue, boolValue(r.CommandFailed), r.ID) + } +} + +func boolValue(b bool) float64 { + if b { + return 1 } + return 0 } diff --git a/internal/metrics/metrics_test.go b/internal/metrics/metrics_test.go index 94f8e0c..1ad6c4b 100644 --- a/internal/metrics/metrics_test.go +++ b/internal/metrics/metrics_test.go @@ -18,7 +18,7 @@ func (f fakeChecker) Check(context.Context) error { return f.err } type fakeThermostat struct{ err error } -func (f fakeThermostat) SetTarget(string, float64) error { return f.err } +func (f fakeThermostat) Apply(string, controller.Command) error { return f.err } type fakeRooms []controller.RoomStatus @@ -62,11 +62,12 @@ func TestCheckUp(t *testing.T) { func TestCommandsAreCounted(t *testing.T) { m := New("dev") failed := errors.New("down") + target := 20.0 - if err := m.Thermostat(fakeThermostat{}).SetTarget("trv-office", 20); err != nil { + if err := m.Thermostat(fakeThermostat{}).Apply("trv-office", controller.Command{Target: &target}); err != nil { t.Fatal(err) } - if err := m.Thermostat(fakeThermostat{err: failed}).SetTarget("trv-office", 20); !errors.Is(err, failed) { + if err := m.Thermostat(fakeThermostat{err: failed}).Apply("trv-office", controller.Command{Target: &target}); !errors.Is(err, failed) { t.Fatalf("the error must be passed on, got %v", err) } @@ -78,11 +79,17 @@ func TestCommandsAreCounted(t *testing.T) { func TestRoomTargets(t *testing.T) { target := 20.5 m := New("dev") - m.AddRooms(fakeRooms{{ID: "office", Target: &target}, {ID: "bedroom"}}) + m.AddRooms(fakeRooms{ + {ID: "office", Target: &target, AiringState: controller.AiringActive}, + {ID: "bedroom", AiringState: controller.AiringNone, CommandFailed: true}, + }) body := scrape(t, m) wantLine(t, body, `heating_control_room_target_celsius{room="office"} 20.5`) - if strings.Contains(body, `room="bedroom"`) { + wantLine(t, body, `heating_control_room_airing{room="office"} 1`) + wantLine(t, body, `heating_control_room_airing{room="bedroom"} 0`) + wantLine(t, body, `heating_control_room_command_failed{room="bedroom"} 1`) + if strings.Contains(body, `heating_control_room_target_celsius{room="bedroom"}`) { t.Error("a room with an unknown target must have no series") } } diff --git a/internal/mqtt/client.go b/internal/mqtt/client.go index ac3c763..9d4790b 100644 --- a/internal/mqtt/client.go +++ b/internal/mqtt/client.go @@ -43,7 +43,7 @@ func New(cfg *config.Config) *Client { func (c *Client) Start(sink Sink) { opts := paho.NewClientOptions(). AddBroker(c.cfg.Broker). - SetClientID("heating-control"). + SetClientID(c.cfg.ClientID). SetAutoReconnect(true). SetConnectRetry(true). SetConnectRetryInterval(5 * time.Second). @@ -65,9 +65,16 @@ func (c *Client) Checker() health.Checker { return connectionChecker{conn: c.client} } -// SetTarget implements controller.Thermostat. -func (c *Client) SetTarget(trv string, target float64) error { - return c.publish(trv, "set", map[string]any{"occupied_heating_setpoint": target}) +// Apply implements controller.Thermostat. +func (c *Client) Apply(trv string, cmd controller.Command) error { + payload := map[string]any{} + if cmd.Mode != nil { + payload["system_mode"] = *cmd.Mode + } + if cmd.Target != nil { + payload["occupied_heating_setpoint"] = *cmd.Target + } + return c.publish(trv, "set", payload) } // publish refuses to send while disconnected. During an outage paho would keep diff --git a/internal/mqtt/client_test.go b/internal/mqtt/client_test.go index ea0d2bd..58ffd3f 100644 --- a/internal/mqtt/client_test.go +++ b/internal/mqtt/client_test.go @@ -27,14 +27,16 @@ func TestConnectionChecker(t *testing.T) { // fakePaho embeds the interface, so only what publish uses is implemented. type fakePaho struct { paho.Client - open bool - topics []string + open bool + topics []string + payload []byte } func (f *fakePaho) IsConnectionOpen() bool { return f.open } -func (f *fakePaho) Publish(topic string, _ byte, _ bool, _ any) paho.Token { +func (f *fakePaho) Publish(topic string, _ byte, _ bool, payload any) paho.Token { f.topics = append(f.topics, topic) + f.payload, _ = payload.([]byte) return fakePahoToken{} } @@ -43,11 +45,12 @@ type fakePahoToken struct{ paho.Token } func (fakePahoToken) WaitTimeout(time.Duration) bool { return true } func (fakePahoToken) Error() error { return nil } -func TestSetTargetFailsFastWhileDisconnected(t *testing.T) { +func TestApplyFailsFastWhileDisconnected(t *testing.T) { f := &fakePaho{} c := &Client{base: "zigbee2mqtt", client: f} + target := 20.0 - if err := c.SetTarget("trv-office", 20); !errors.Is(err, errNotConnected) { + if err := c.Apply("trv-office", controller.Command{Target: &target}); !errors.Is(err, errNotConnected) { t.Fatalf("err = %v, want errNotConnected", err) } if len(f.topics) != 0 { @@ -55,7 +58,7 @@ func TestSetTargetFailsFastWhileDisconnected(t *testing.T) { } f.open = true - if err := c.SetTarget("trv-office", 20); err != nil { + if err := c.Apply("trv-office", controller.Command{Target: &target}); err != nil { t.Fatal(err) } if len(f.topics) != 1 || f.topics[0] != "zigbee2mqtt/trv-office/set" { @@ -137,3 +140,29 @@ func TestParseTemperature(t *testing.T) { t.Fatalf("got %v, %v", got, err) } } + +func TestApplyPayload(t *testing.T) { + heat := controller.ModeHeat + target := 20.5 + tests := []struct { + name string + cmd controller.Command + want string + }{ + {"target", controller.Command{Target: &target}, `{"occupied_heating_setpoint":20.5}`}, + {"mode", controller.Command{Mode: &heat}, `{"system_mode":"heat"}`}, + {"both", controller.Command{Mode: &heat, Target: &target}, `{"occupied_heating_setpoint":20.5,"system_mode":"heat"}`}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := &fakePaho{open: true} + c := &Client{base: "zigbee2mqtt", client: f} + if err := c.Apply("trv-office", tt.cmd); err != nil { + t.Fatal(err) + } + if got := string(f.payload); got != tt.want { + t.Fatalf("payload = %s, want %s", got, tt.want) + } + }) + } +} diff --git a/internal/store/store.go b/internal/store/store.go new file mode 100644 index 0000000..c994b05 --- /dev/null +++ b/internal/store/store.go @@ -0,0 +1,64 @@ +// Package store saves the controller's state to a JSON file. +package store + +import ( + "encoding/json" + "errors" + "io/fs" + "os" + "path/filepath" + + "github.com/subpathdev/heating-control/internal/controller" +) + +// File implements controller.Store. +type File struct { + path string +} + +// New returns a File that saves to path. +func New(path string) *File { + return &File{path: path} +} + +// Load returns the saved records. A file that doesn't exist yet means none. +func (f *File) Load() (map[string]controller.RoomRecord, error) { + data, err := os.ReadFile(f.path) + if errors.Is(err, fs.ErrNotExist) { + return nil, nil + } + if err != nil { + return nil, err + } + var records map[string]controller.RoomRecord + if err := json.Unmarshal(data, &records); err != nil { + return nil, err + } + return records, nil +} + +// Save replaces the file in one step, so a crash leaves the old or the new one. +func (f *File) Save(records map[string]controller.RoomRecord) error { + data, err := json.MarshalIndent(records, "", " ") + if err != nil { + return err + } + tmp, err := os.CreateTemp(filepath.Dir(f.path), filepath.Base(f.path)+".*") + if err != nil { + return err + } + defer func() { _ = os.Remove(tmp.Name()) }() + + if _, err := tmp.Write(data); err != nil { + _ = tmp.Close() + return err + } + if err := tmp.Sync(); err != nil { + _ = tmp.Close() + return err + } + if err := tmp.Close(); err != nil { + return err + } + return os.Rename(tmp.Name(), f.path) +} diff --git a/internal/store/store_test.go b/internal/store/store_test.go new file mode 100644 index 0000000..60d710b --- /dev/null +++ b/internal/store/store_test.go @@ -0,0 +1,56 @@ +package store + +import ( + "os" + "path/filepath" + "testing" + "time" + + "github.com/subpathdev/heating-control/internal/controller" +) + +func TestLoadWithoutAFile(t *testing.T) { + records, err := New(filepath.Join(t.TempDir(), "state.json")).Load() + if err != nil || records != nil { + t.Fatalf("got %v, %v", records, err) + } +} + +func TestSaveAndLoad(t *testing.T) { + path := filepath.Join(t.TempDir(), "state.json") + target := 20.5 + until := time.Unix(1000, 0).UTC() + f := New(path) + if err := f.Save(map[string]controller.RoomRecord{ + "office": {Target: &target, TargetDirty: true, AiringUntil: &until, RestoreMode: controller.ModeAuto, Restoring: true}, + }); err != nil { + t.Fatal(err) + } + + records, err := New(path).Load() + if err != nil { + t.Fatal(err) + } + got := records["office"] + if *got.Target != 20.5 || !got.TargetDirty || !got.AiringUntil.Equal(until) || got.RestoreMode != controller.ModeAuto || !got.Restoring { + t.Fatalf("got %+v", got) + } + + files, err := os.ReadDir(filepath.Dir(path)) + if err != nil { + t.Fatal(err) + } + if len(files) != 1 { + t.Fatalf("left %d files behind", len(files)) + } +} + +func TestLoadCorruptFile(t *testing.T) { + path := filepath.Join(t.TempDir(), "state.json") + if err := os.WriteFile(path, []byte("{nope"), 0o600); err != nil { + t.Fatal(err) + } + if _, err := New(path).Load(); err == nil { + t.Fatal("want an error") + } +} diff --git a/internal/web/static/index.html b/internal/web/static/index.html index 42d4641..e8091ad 100644 --- a/internal/web/static/index.html +++ b/internal/web/static/index.html @@ -47,6 +47,12 @@ button:disabled { opacity: .4; cursor: default; } button:focus-visible { outline: 2px solid var(--accent); outline-offset: 2px; } #error { color: var(--error); min-height: 1.4em; margin: 0; } + .note { color: var(--muted); font-size: .9rem; margin-top: 8px; } + .note:empty { display: none; } + .note.problem { color: var(--error); } + .airing { display: grid; grid-template-columns: repeat(3, 1fr); gap: 6px; margin-top: 8px; } + .airing button.stop { grid-column: 1 / -1; background: var(--accent); color: var(--card); font-weight: 600; } + [hidden] { display: none !important; } @@ -72,16 +78,34 @@

Heating

errorEl.textContent = commandError || connectionError; } + const airingMinutes = [15, 30, 60]; + + const clock = iso => new Date(iso).toLocaleTimeString([], { hour: '2-digit', minute: '2-digit' }); const degrees = v => v == null ? '–' : v.toFixed(1) + ' °C'; const signed = s => (s > 0 ? '+' : '−') + Math.abs(s); const inRange = v => v >= data.minTarget && v <= data.maxTarget; function describe(room) { + switch (room.airingState) { + case 'starting': return 'Airing: turning the heating off'; + case 'active': return 'Airing until ' + clock(room.airingUntil) + ', not heating'; + case 'restoring': return 'Airing is over: turning the heating back on'; + } if (room.mode === 'unknown') return 'Waiting for the thermostat'; - if (room.mode === 'off') return 'Not heating: the thermostat is off'; + if (room.mode === 'off') return 'Not heating: the thermostat turned itself off'; return room.heating ? 'Heating' : 'Not heating right now'; } + function note(room) { + if (room.commandFailed) return { text: 'The thermostat isn\'t confirming. Still trying.', problem: true }; + if (room.targetState === 'failed') return { text: 'The thermostat didn\'t take the last change.', problem: true }; + if (room.targetState === 'held') return { text: 'Target applies when heating is back.', problem: false }; + if (room.targetState === 'sending') return { text: 'Sending the target…', problem: false }; + const lastSeen = room.lastSeen ? Date.now() - new Date(room.lastSeen) : 0; + if (lastSeen > 2 * 3600 * 1000) return { text: 'Last report at ' + clock(room.lastSeen) + '.', problem: true }; + return { text: '', problem: false }; + } + function makeButton(id, label, step) { const b = document.createElement('button'); b.textContent = signed(step); @@ -112,9 +136,27 @@

Heating

const buttons = steps.map(step => makeButton(room.id, room.label, step)); target.append(buttons[0].el, buttons[1].el, value, buttons[2].el, buttons[3].el); - el.append(head, state, target); + const airing = document.createElement('div'); + airing.className = 'airing'; + const start = airingMinutes.map(minutes => { + const b = document.createElement('button'); + b.textContent = 'Airing ' + minutes; + b.setAttribute('aria-label', 'Air ' + room.label + ' for ' + minutes + ' minutes'); + b.addEventListener('click', () => startAiring(room.id, minutes)); + airing.append(b); + return b; + }); + const stop = document.createElement('button'); + stop.className = 'stop'; + stop.textContent = 'Heat again'; + stop.addEventListener('click', () => stopAiring(room.id)); + airing.append(stop); + + const problem = document.createElement('div'); + + el.append(head, state, target, airing, problem); roomsEl.append(el); - return { now, state, value, buttons }; + return { now, state, value, buttons, start, stop, problem }; } // Updates the existing elements: replacing a button during a tap loses the tap. @@ -130,6 +172,12 @@

Heating

for (const b of card.buttons) { b.el.disabled = room.target == null || !inRange(room.target + b.step); } + const airing = room.airingState === 'starting' || room.airingState === 'active'; + for (const b of card.start) b.hidden = airing; + card.stop.hidden = !airing; + const n = note(room); + card.problem.className = 'note' + (n.problem ? ' problem' : ''); + card.problem.textContent = n.text; } } @@ -142,7 +190,7 @@

Heating

if (sending > 0 || generation !== refreshGeneration) return; data = next; render(); - connectionError = ''; + connectionError = data.connected ? '' : 'No connection to the broker: values may be old and commands wait.'; } catch { if (generation !== refreshGeneration) return; connectionError = 'Can\'t reach the server. Are you on the home Wi-Fi?'; @@ -157,8 +205,27 @@

Heating

render(); const target = room.target; + enqueue('api/rooms/' + encodeURIComponent(id) + '/target', 'POST', { target }); + } + + function startAiring(id, minutes) { + const room = data.rooms.find(r => r.id === id); + room.airingState = 'starting'; + room.airingUntil = new Date(Date.now() + minutes * 60000).toISOString(); + render(); + enqueue('api/rooms/' + encodeURIComponent(id) + '/airing', 'POST', { minutes }); + } + + function stopAiring(id) { + const room = data.rooms.find(r => r.id === id); + room.airingState = 'restoring'; + render(); + enqueue('api/rooms/' + encodeURIComponent(id) + '/airing', 'DELETE'); + } + + function enqueue(url, method, body) { sending++; - sendQueue = sendQueue.then(() => send(id, target)).finally(() => { + sendQueue = sendQueue.then(() => send(url, method, body)).finally(() => { sending--; if (sending === 0) refresh(); }); @@ -172,13 +239,14 @@

Heating

} } - async function send(id, target) { + async function send(url, method, body) { try { - const res = await fetch('api/rooms/' + encodeURIComponent(id) + '/target', { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ target }), - }); + const options = { method }; + if (body) { + options.headers = { 'Content-Type': 'application/json' }; + options.body = JSON.stringify(body); + } + const res = await fetch(url, options); if (!res.ok) throw new Error(await errorMessage(res)); commandError = ''; } catch (e) { diff --git a/internal/web/web.go b/internal/web/web.go index 86ca445..74653cc 100644 --- a/internal/web/web.go +++ b/internal/web/web.go @@ -11,9 +11,11 @@ import ( "net" "net/http" "strings" + "time" "github.com/subpathdev/heating-control/internal/api" "github.com/subpathdev/heating-control/internal/controller" + "github.com/subpathdev/heating-control/internal/health" ) //go:embed static @@ -26,12 +28,14 @@ type Rooms interface { Rooms() []controller.RoomStatus Limits() (min, max float64) SetTarget(id string, target float64) error + StartAiring(id string, d time.Duration) error + StopAiring(id string) error } // Handler returns the page, the API and its OpenAPI document. The page has no // login, so it refuses requests from other websites, and allowedHosts protects // that check against DNS rebinding. -func Handler(rooms Rooms, allowedHosts []string) http.Handler { +func Handler(rooms Rooms, broker health.Checker, allowedHosts []string) http.Handler { mux := http.NewServeMux() content, err := fs.Sub(static, "static") if err != nil { @@ -46,7 +50,7 @@ func Handler(rooms Rooms, allowedHosts []string) http.Handler { } }) - strict := api.NewStrictHandlerWithOptions(server{rooms: rooms}, nil, api.StrictHTTPServerOptions{ + strict := api.NewStrictHandlerWithOptions(server{rooms: rooms, broker: broker}, nil, api.StrictHTTPServerOptions{ RequestErrorHandlerFunc: badRequest, ResponseErrorHandlerFunc: func(w http.ResponseWriter, _ *http.Request, err error) { log.Printf("api: %v", err) @@ -90,26 +94,37 @@ func checkHost(allowedHosts []string, next http.Handler) http.Handler { // server implements api.StrictServerInterface. type server struct { - rooms Rooms + rooms Rooms + broker health.Checker } -func (s server) ListRooms(context.Context, api.ListRoomsRequestObject) (api.ListRoomsResponseObject, error) { +func (s server) ListRooms(ctx context.Context, _ api.ListRoomsRequestObject) (api.ListRoomsResponseObject, error) { statuses := s.rooms.Rooms() rooms := make([]api.Room, 0, len(statuses)) for _, r := range statuses { rooms = append(rooms, api.Room{ - Id: r.ID, - Label: r.Label, - Temperature: r.Temperature, - Target: r.Target, - Mode: toAPIMode(r.Mode), - Heating: r.Heating, + Id: r.ID, + Label: r.Label, + Temperature: r.Temperature, + Target: r.Target, + TargetState: api.TargetState(r.TargetState), + Mode: toAPIMode(r.Mode), + Heating: r.Heating, + AiringState: api.AiringState(r.AiringState), + AiringUntil: r.AiringUntil, + LastSeen: r.LastSeen, + CommandFailed: r.CommandFailed, }) } min, max := s.rooms.Limits() noStore := "no-store" return api.ListRooms200JSONResponse{ - Body: api.RoomList{Rooms: rooms, MinTarget: min, MaxTarget: max}, + Body: api.RoomList{ + Rooms: rooms, + Connected: s.broker.Check(ctx) == nil, + MinTarget: min, + MaxTarget: max, + }, Headers: api.ListRooms200ResponseHeaders{CacheControl: &noStore}, }, nil } @@ -117,14 +132,38 @@ func (s server) ListRooms(context.Context, api.ListRoomsRequestObject) (api.List func (s server) SetRoomTarget(_ context.Context, r api.SetRoomTargetRequestObject) (api.SetRoomTargetResponseObject, error) { switch err := s.rooms.SetTarget(r.Id, r.Body.Target); { case err == nil: - return api.SetRoomTarget204Response{}, nil + return api.SetRoomTarget202Response{}, nil case errors.Is(err, controller.ErrUnknownRoom): return api.SetRoomTarget404JSONResponse{Message: err.Error()}, nil case errors.Is(err, controller.ErrTargetOutOfRange): return api.SetRoomTarget400JSONResponse{Message: err.Error()}, nil default: - log.Printf("set target: %v", err) - return api.SetRoomTarget502JSONResponse{Message: "could not reach the thermostat"}, nil + return nil, err + } +} + +func (s server) StartRoomAiring(_ context.Context, r api.StartRoomAiringRequestObject) (api.StartRoomAiringResponseObject, error) { + if !r.Body.Minutes.Valid() { + return api.StartRoomAiring400JSONResponse{Message: "minutes must be 15, 30 or 60"}, nil + } + switch err := s.rooms.StartAiring(r.Id, time.Duration(r.Body.Minutes)*time.Minute); { + case err == nil: + return api.StartRoomAiring202Response{}, nil + case errors.Is(err, controller.ErrUnknownRoom): + return api.StartRoomAiring404JSONResponse{Message: err.Error()}, nil + default: + return nil, err + } +} + +func (s server) StopRoomAiring(_ context.Context, r api.StopRoomAiringRequestObject) (api.StopRoomAiringResponseObject, error) { + switch err := s.rooms.StopAiring(r.Id); { + case err == nil: + return api.StopRoomAiring202Response{}, nil + case errors.Is(err, controller.ErrUnknownRoom): + return api.StopRoomAiring404JSONResponse{Message: err.Error()}, nil + default: + return nil, err } } diff --git a/internal/web/web_test.go b/internal/web/web_test.go index 0dbe119..b89999e 100644 --- a/internal/web/web_test.go +++ b/internal/web/web_test.go @@ -1,12 +1,14 @@ package web import ( + "context" "encoding/json" "errors" "net/http" "net/http/httptest" "strings" "testing" + "time" "github.com/subpathdev/heating-control/internal/api" "github.com/subpathdev/heating-control/internal/controller" @@ -15,13 +17,19 @@ import ( type fakeRooms struct { id string target float64 + airing time.Duration + stops int err error } func (f *fakeRooms) Rooms() []controller.RoomStatus { + until := time.Unix(1000, 0).UTC() return []controller.RoomStatus{ - {ID: "office", Label: "Office", Mode: controller.ModeOff}, - {ID: "bedroom", Label: "Bedroom"}, + { + ID: "office", Label: "Office", Mode: controller.ModeOff, + TargetState: controller.TargetHeld, AiringState: controller.AiringActive, AiringUntil: &until, + }, + {ID: "bedroom", Label: "Bedroom", TargetState: controller.TargetApplied, AiringState: controller.AiringNone}, } } func (f *fakeRooms) Limits() (float64, float64) { return 10, 25 } @@ -29,11 +37,25 @@ func (f *fakeRooms) SetTarget(id string, target float64) error { f.id, f.target = id, target return f.err } +func (f *fakeRooms) StartAiring(id string, d time.Duration) error { + f.id, f.airing = id, d + return f.err +} +func (f *fakeRooms) StopAiring(id string) error { + f.id = id + f.stops++ + return f.err +} + +type fakeBroker struct{ err error } + +func (fakeBroker) Name() string { return "fake" } +func (f fakeBroker) Check(context.Context) error { return f.err } const testHost = "192.168.178.60:30081" func handler(rooms Rooms) http.Handler { - return Handler(rooms, []string{"heating.example"}) + return Handler(rooms, fakeBroker{}, []string{"heating.example"}) } func get(t *testing.T, path string) *httptest.ResponseRecorder { @@ -87,6 +109,12 @@ func TestListRooms(t *testing.T) { if len(got.Rooms) != 2 || got.Rooms[0].Id != "office" || got.MaxTarget != 25 { t.Fatalf("got %+v", got) } + if !got.Connected { + t.Error("connected = false") + } + if got.Rooms[0].AiringState != api.Active || got.Rooms[0].AiringUntil == nil || got.Rooms[0].TargetState != api.Held { + t.Errorf("office = %+v", got.Rooms[0]) + } if got.Rooms[0].Mode != api.Off { t.Errorf("mode = %q, want off", got.Rooms[0].Mode) } @@ -102,7 +130,7 @@ func TestListRooms(t *testing.T) { func TestSetTarget(t *testing.T) { f := &fakeRooms{} rec := post(t, handler(f), "/api/rooms/office/target", `{"target":20.5}`, nil) - if rec.Code != http.StatusNoContent { + if rec.Code != http.StatusAccepted { t.Fatalf("status = %d, body %s", rec.Code, rec.Body.String()) } if f.id != "office" || f.target != 20.5 { @@ -121,7 +149,7 @@ func TestSetTargetErrors(t *testing.T) { {"too large", `{"target":20,"x":"` + strings.Repeat("a", maxBodyBytes) + `"}`, nil, http.StatusBadRequest}, {"unknown room", `{"target":20}`, controller.ErrUnknownRoom, http.StatusNotFound}, {"out of range", `{"target":99}`, controller.ErrTargetOutOfRange, http.StatusBadRequest}, - {"broker down", `{"target":20}`, errors.New("down"), http.StatusBadGateway}, + {"unexpected", `{"target":20}`, errors.New("bug"), http.StatusInternalServerError}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -137,6 +165,75 @@ func TestSetTargetErrors(t *testing.T) { } } +func TestListRoomsShowsABrokerOutage(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/api/rooms", nil) + req.Host = testHost + rec := httptest.NewRecorder() + Handler(&fakeRooms{}, fakeBroker{err: errors.New("down")}, nil).ServeHTTP(rec, req) + var got api.RoomList + if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil { + t.Fatal(err) + } + if rec.Code != http.StatusOK || got.Connected { + t.Fatalf("status %d, connected %v", rec.Code, got.Connected) + } +} + +func TestStartAiring(t *testing.T) { + f := &fakeRooms{} + rec := post(t, handler(f), "/api/rooms/office/airing", `{"minutes":30}`, nil) + if rec.Code != http.StatusAccepted { + t.Fatalf("status = %d, body %s", rec.Code, rec.Body.String()) + } + if f.id != "office" || f.airing != 30*time.Minute { + t.Fatalf("got %s %v", f.id, f.airing) + } +} + +func TestStartAiringErrors(t *testing.T) { + tests := []struct { + name string + body string + err error + want int + }{ + {"not allowed", `{"minutes":45}`, nil, http.StatusBadRequest}, + {"negative", `{"minutes":-15}`, nil, http.StatusBadRequest}, + {"missing", `{}`, nil, http.StatusBadRequest}, + {"bad json", `nope`, nil, http.StatusBadRequest}, + {"unknown room", `{"minutes":15}`, controller.ErrUnknownRoom, http.StatusNotFound}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := &fakeRooms{err: tt.err} + rec := post(t, handler(f), "/api/rooms/office/airing", tt.body, nil) + if rec.Code != tt.want { + t.Fatalf("status = %d, want %d", rec.Code, tt.want) + } + if tt.want == http.StatusBadRequest && f.airing != 0 { + t.Fatal("airing started") + } + }) + } +} + +func TestStopAiring(t *testing.T) { + do := func(h http.Handler) *httptest.ResponseRecorder { + req := httptest.NewRequest(http.MethodDelete, "/api/rooms/office/airing", nil) + req.Host = testHost + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + return rec + } + f := &fakeRooms{} + if rec := do(handler(f)); rec.Code != http.StatusAccepted || f.stops != 1 { + t.Fatalf("status %d, stops %d", rec.Code, f.stops) + } + if rec := do(handler(&fakeRooms{err: controller.ErrUnknownRoom})); rec.Code != http.StatusNotFound { + t.Fatalf("status %d, want 404", rec.Code) + } +} + func TestOtherWebsitesAreRefused(t *testing.T) { f := &fakeRooms{} rec := post(t, handler(f), "/api/rooms/office/target", `{"target":20}`, map[string]string{"Origin": "https://evil.example"}) @@ -151,8 +248,8 @@ func TestOtherWebsitesAreRefused(t *testing.T) { t.Fatal("the target was changed") } rec = post(t, handler(f), "/api/rooms/office/target", `{"target":20}`, map[string]string{"Origin": "http://" + testHost}) - if rec.Code != http.StatusNoContent { - t.Fatalf("same origin: status = %d, want 204", rec.Code) + if rec.Code != http.StatusAccepted { + t.Fatalf("same origin: status = %d, want 202", rec.Code) } } @@ -164,11 +261,11 @@ func TestDNSRebindingIsRefused(t *testing.T) { }{ {"evil.example", http.StatusForbidden}, {"evil.example:30081", http.StatusForbidden}, - {"heating.example:30081", http.StatusNoContent}, - {"HEATING.example", http.StatusNoContent}, - {"localhost:8080", http.StatusNoContent}, - {"192.168.178.60:30081", http.StatusNoContent}, - {"[fd2a::1]:30081", http.StatusNoContent}, + {"heating.example:30081", http.StatusAccepted}, + {"HEATING.example", http.StatusAccepted}, + {"localhost:8080", http.StatusAccepted}, + {"192.168.178.60:30081", http.StatusAccepted}, + {"[fd2a::1]:30081", http.StatusAccepted}, } for _, tt := range tests { t.Run(tt.host, func(t *testing.T) { From 7ffd5bccbd6ea28b63a2344fc0c94adc667f49d1 Mon Sep 17 00:00:00 2001 From: Jan Unterbrink <19386509+subpathdev@users.noreply.github.com> Date: Sun, 1 Nov 2026 15:52:55 +0100 Subject: [PATCH 2/3] fix: address the review of airing - Commands use QoS 0, because paho resends QoS 1 messages after a reconnect and a late off can undo a restore. The previous commit wrongly said this gap was closed. The controller already repeats commands until they're confirmed. - State is saved before a command goes out, so a crash can't leave a room off with no record. - A report can't confirm a restore while one from before an unconfirmed off could still arrive, and only confirms a target that was sent. - A corrupt state file is moved aside and reported through the health check. - A thermostat stuck at the frost value no longer holds a target forever. - "Heat again" also turns on a thermostat that is off with no record. - The last on mode is saved, and two rooms can't share a thermostat. Co-Authored-By: Claude Opus 5.5 --- README.md | 2 +- cmd/heating-control/main.go | 6 +- internal/api/openapi.yaml | 4 +- internal/config/config.go | 5 + internal/config/config_test.go | 1 + internal/controller/controller.go | 12 ++- internal/controller/controller_test.go | 135 +++++++++++++++++++++++++ internal/controller/room.go | 82 ++++++++++++--- internal/mqtt/client.go | 9 +- internal/mqtt/client_test.go | 7 +- internal/store/store.go | 42 +++++++- internal/store/store_test.go | 32 +++++- internal/web/static/index.html | 3 +- 13 files changed, 308 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index caeefff..c14a06f 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ A small web page to adjust the heating per room from a phone on the home network - A target changed during airing is applied when it ends. - A thermostat leaving off during airing gets off once more. If it leaves off again, someone is at the radiator, so airing ends. - Commands repeat until the thermostat confirms them, every 30 s for about 5 minutes. A target then gives up and shows the thermostat's value. Off and heat-again keep trying every 5 minutes and show a problem on the page, because giving up would mean heating with the window open or a cold room. -- A thermostat that turned itself off is left alone: only rooms this program turned off get restored. +- A thermostat that turned itself off is left alone, because only rooms this program turned off get restored. The page offers "Heat again" for it. ## Deployment diff --git a/cmd/heating-control/main.go b/cmd/heating-control/main.go index 91c2edf..c406184 100644 --- a/cmd/heating-control/main.go +++ b/cmd/heating-control/main.go @@ -38,16 +38,18 @@ func main() { client := mqtt.New(cfg) m := metrics.New(version) - ctrl := controller.New(cfg, m.Thermostat(client), store.New(cfg.StateFile)) + state := store.New(cfg.StateFile) + ctrl := controller.New(cfg, m.Thermostat(client), state) client.Start(ctrl) defer client.Close() checker := client.Checker() m.AddChecker(checker) + m.AddChecker(state) m.AddRooms(ctrl) mux := http.NewServeMux() - health.Register(mux, version, checker) + health.Register(mux, version, checker, state) mux.Handle("GET /metrics", m.Handler()) // Probes and scrapes use the pod's IP and stay outside the page's checks. mux.Handle("/", web.Handler(ctrl, checker, cfg.AllowedHosts)) diff --git a/internal/api/openapi.yaml b/internal/api/openapi.yaml index b3d30cc..5798e46 100644 --- a/internal/api/openapi.yaml +++ b/internal/api/openapi.yaml @@ -135,7 +135,9 @@ paths: delete: operationId: stopRoomAiring summary: End airing now and heat again. - description: Does nothing if the room isn't airing. + description: >- + If the room isn't airing but its thermostat is off, this turns it back on + with its last mode and target. Otherwise it does nothing. parameters: - name: id in: path diff --git a/internal/config/config.go b/internal/config/config.go index f1199a7..1742c55 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -64,6 +64,7 @@ func parse(data []byte) (*Config, error) { return nil, fmt.Errorf("at least one room is required") } seen := map[string]bool{} + seenTRV := map[string]bool{} for _, r := range cfg.Rooms { if r.ID == "" || r.Label == "" || r.TRV == "" { return nil, fmt.Errorf("room %q needs id, label and trv", r.ID) @@ -72,6 +73,10 @@ func parse(data []byte) (*Config, error) { return nil, fmt.Errorf("duplicate room id %q", r.ID) } seen[r.ID] = true + if seenTRV[r.TRV] { + return nil, fmt.Errorf("thermostat %q is in more than one room", r.TRV) + } + seenTRV[r.TRV] = true } return &cfg, nil } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 0f623bb..b64cb45 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -22,6 +22,7 @@ func TestParseRejects(t *testing.T) { "no rooms": `broker: x`, "no trv": `{broker: x, rooms: [{id: a, label: A}]}`, "duplicate": `{broker: x, rooms: [{id: a, label: A, trv: t}, {id: a, label: B, trv: u}]}`, + "same trv": `{broker: x, rooms: [{id: a, label: A, trv: t}, {id: b, label: B, trv: t}]}`, "bad range": `{broker: x, minTarget: 25, maxTarget: 10, rooms: [{id: a, label: A, trv: t}]}`, } for name, in := range tests { diff --git a/internal/controller/controller.go b/internal/controller/controller.go index c516e51..afa1090 100644 --- a/internal/controller/controller.go +++ b/internal/controller/controller.go @@ -101,6 +101,7 @@ type RoomRecord struct { AiringUntil *time.Time RestoreMode Mode Restoring bool + LastOnMode Mode } // ThermostatState is a thermostat's report. Fields it didn't include are nil. @@ -191,18 +192,23 @@ func (c *Controller) tick() { if send { r.attempts++ r.lastSent = now + r.sentTarget = cmd.Target } c.mu.Unlock() if !send { continue } + // Saved first: after a crash a room that is off with no record would stay + // off, because only rooms with a record get restored. + c.save() if err := c.thermostat.Apply(r.cfg.TRV, cmd); err != nil { log.Printf("%s: %s: %v", r.cfg.TRV, cmd.Kind(), err) // Never reached the thermostat, so it says nothing about it. c.mu.Lock() r.attempts-- r.publishFailed = true + r.sentTarget = nil c.mu.Unlock() continue } @@ -294,15 +300,15 @@ func (c *Controller) StartAiring(id string, d time.Duration) error { return nil } -// StopAiring ends airing now. It does nothing if the room isn't airing. +// StopAiring ends airing now. If the room isn't airing but its thermostat is +// off, it turns it back on. Otherwise it does nothing. func (c *Controller) StopAiring(id string) error { r := c.find(id) if r == nil { return fmt.Errorf("%w: %q", ErrUnknownRoom, id) } c.mu.Lock() - if r.airingUntil != nil { - r.endAiring() + if r.heatAgain(c.now()) { c.changed = true } c.mu.Unlock() diff --git a/internal/controller/controller_test.go b/internal/controller/controller_test.go index 1e93e16..6e31f4e 100644 --- a/internal/controller/controller_test.go +++ b/internal/controller/controller_test.go @@ -311,6 +311,35 @@ func TestLostTargetIsRetriedThenGivenUp(t *testing.T) { } } +// Quick +0.5, -0.5: a report of the old value is older than the second command. +func TestEqualReportDoesntConfirmATargetNotYetSent(t *testing.T) { + h := newHarness(t) + h.report("heat", 20) + h.setTarget(20.5) + + if err := h.c.SetTarget("office", 20); err != nil { + t.Fatal(err) + } + h.c.HandleThermostatState("trv-office", state("heat", 20)) + h.c.tick() + h.wantKinds("set_target", "set_target") + if got := *h.f.sent[1].Target; got != 20 { + t.Fatalf("sent %v, want 20", got) + } +} + +// With the knob at the frost value it never reports another setpoint. +func TestHeldTargetGoesOutOnceTheSettleTimeIsOver(t *testing.T) { + h := newHarness(t) + h.report("off", 7) + h.setTarget(22) + h.report("heat", 7) + h.wantKinds() + + h.advance(settleTime) + h.wantKinds("set_target") +} + func TestTargetDoesntGiveUpWhileOffAndNeverBecomesFrost(t *testing.T) { h := newHarness(t) h.report("heat", 20) @@ -424,6 +453,96 @@ func TestStopAiringRestoresNow(t *testing.T) { h.wantKinds("off", "restore") } +func TestStopAiringTurnsAThermostatThatIsOffBackOn(t *testing.T) { + h := newHarness(t) + h.report("auto", 20) + h.report("off", 7) + + if err := h.c.StopAiring("office"); err != nil { + t.Fatal(err) + } + h.c.tick() + h.wantKinds("restore") + cmd := h.f.sent[0] + if *cmd.Mode != ModeAuto || *cmd.Target != 20 { + t.Fatalf("restored %s/%v, want auto/20", *cmd.Mode, *cmd.Target) + } + h.report("auto", 20) + if got := h.status().AiringState; got != AiringNone { + t.Fatalf("state = %s, want none", got) + } +} + +// A report sent before the off took effect mustn't end the retries: if the restore +// was lost and the off arrives, the room would stay off for good. +func TestReportFromBeforeOffDoesntConfirmARestore(t *testing.T) { + h := newHarness(t) + h.report("heat", 20) + h.startAiring(time.Hour) + if err := h.c.StopAiring("office"); err != nil { + t.Fatal(err) + } + h.c.tick() + h.wantKinds("off", "restore") + + h.report("heat", 20) + if got := h.status().AiringState; got != AiringRestoring { + t.Fatalf("state = %s, want still restoring", got) + } + + // The off landed and the restore was lost, so the retry fixes it. + h.report("off", 7) + h.advance(retryInterval) + h.wantKinds("off", "restore", "restore") + h.report("heat", 20) + if got := h.status().AiringState; got != AiringNone { + t.Fatalf("state = %s, want none", got) + } +} + +// The off itself was lost: the thermostat stays on, and the wait runs out. +func TestRestoreIsConfirmedOnceTheWaitRunsOut(t *testing.T) { + h := newHarness(t) + h.report("heat", 20) + h.startAiring(time.Hour) + if err := h.c.StopAiring("office"); err != nil { + t.Fatal(err) + } + h.c.tick() + + h.advance(retryInterval) + h.report("heat", 20) + if got := h.status().AiringState; got != AiringNone { + t.Fatalf("state = %s, want none", got) + } +} + +// Saved first: a crash during the publish must leave a record of the airing. +func TestAiringIsSavedBeforeOffIsSent(t *testing.T) { + store := &fakeStore{} + h := newHarnessWithStore(t, store) + h.report("heat", 20) + h.c.tick() + + var savedAtSend *RoomRecord + h.c.thermostat = thermostatFunc(func(string, Command) error { + rec := store.records["office"] + savedAtSend = &rec + return nil + }) + if err := h.c.StartAiring("office", time.Hour); err != nil { + t.Fatal(err) + } + h.c.tick() + if savedAtSend == nil || savedAtSend.AiringUntil == nil { + t.Fatalf("saved at send = %+v, want the airing", savedAtSend) + } +} + +type thermostatFunc func(string, Command) error + +func (f thermostatFunc) Apply(trv string, cmd Command) error { return f(trv, cmd) } + func TestStopAiringWithoutAiringDoesNothing(t *testing.T) { h := newHarness(t) h.report("heat", 20) @@ -653,6 +772,22 @@ func TestRestartAfterTheEndTimeRestoresAtStart(t *testing.T) { again.wantKinds("restore") } +func TestRestartKeepsTheLastOnMode(t *testing.T) { + store := &fakeStore{} + h := newHarnessWithStore(t, store) + h.report("auto", 20) + h.report("off", 7) + + again := newHarnessWithStore(t, store) + again.report("off", 7) + again.startAiring(time.Minute) + again.advance(time.Minute) + again.wantKinds("restore") + if got := *again.f.sent[0].Mode; got != ModeAuto { + t.Fatalf("restored %s, want auto", got) + } +} + func TestRestartKeepsAHeldTarget(t *testing.T) { store := &fakeStore{} h := newHarnessWithStore(t, store) diff --git a/internal/controller/room.go b/internal/controller/room.go index 840b2b1..b7ec990 100644 --- a/internal/controller/room.go +++ b/internal/controller/room.go @@ -20,6 +20,9 @@ const ( // time could be its own pause ending, so off is sent again. The second time // someone at the radiator wants heat, so airing ends. maxFlips = 2 + // settleTime is how long a thermostat back from off gets to restore its own + // setpoint. After that a setpoint at the frost value is simply what it's set to. + settleTime = retryInterval // defaultFrost applies until the thermostat has reported its own. defaultFrost = 7.0 ) @@ -54,9 +57,11 @@ type room struct { frost *float64 lastSeen *time.Time lastOnMode Mode - // settled is set once it reports its own setpoint after being off. Until - // then a write could be overwritten by it restoring that setpoint. + // settled is set once it reports its own setpoint after being off, or has had + // settleTime to do so. Until then a write could be overwritten by it restoring + // that setpoint. settled bool + onSince time.Time // What the room should be doing. Saved. target *float64 @@ -71,9 +76,16 @@ type room struct { // publishFailed means the last command never reached the broker, so it waits // for retryInterval without counting as an attempt. publishFailed bool - offConfirmed bool - flips int - targetFailed bool + // sentTarget is the target in the last command. A report only confirms a target + // that went out: an equal value from before could be older than a command still + // on its way, and clearing it then would never send the new one. + sentTarget *float64 + // confirmAfter stops a report confirming a restore while a report from before + // an unconfirmed off could still arrive. + confirmAfter time.Time + offConfirmed bool + flips int + targetFailed bool } func newRoom(cfg config.Room, rec RoomRecord) *room { @@ -84,6 +96,7 @@ func newRoom(cfg config.Room, rec RoomRecord) *room { airingUntil: rec.AiringUntil, restoreMode: rec.RestoreMode, restoring: rec.Restoring, + lastOnMode: rec.LastOnMode, } } @@ -94,6 +107,7 @@ func (r *room) record() RoomRecord { AiringUntil: r.airingUntil, RestoreMode: r.restoreMode, Restoring: r.restoring, + LastOnMode: r.lastOnMode, } } @@ -133,9 +147,12 @@ func (r *room) due(now time.Time) bool { // advance applies what time alone changes, and reports whether anything to save // changed. func (r *room) advance(now time.Time) bool { + if r.canWrite() && !r.settled && !now.Before(r.onSince.Add(settleTime)) { + r.settled = true + } switch { case r.airingUntil != nil && !now.Before(*r.airingUntil): - r.endAiring() + r.endAiring(now) return true case r.targetDirty && r.airingUntil == nil && !r.restoring && r.canWrite() && r.attempts >= maxAttempts && !now.Before(r.lastSent.Add(retryInterval)): @@ -172,14 +189,24 @@ func (r *room) report(s ThermostatState, now time.Time) bool { changed := false if s.Mode != nil { + wasOn := r.canWrite() r.mode = *s.Mode if r.canWrite() { - r.lastOnMode = r.mode + if !wasOn { + r.onSince = now + } + if r.lastOnMode != r.mode { + r.lastOnMode = r.mode + changed = true + } } if r.mode == ModeOff { r.settled = false + r.confirmAfter = time.Time{} + } + if r.reportAiringMode() { + changed = true } - changed = r.reportAiringMode() } if r.setpoint == nil { return changed @@ -192,14 +219,14 @@ func (r *room) report(s ThermostatState, now time.Time) bool { switch { case r.airingUntil != nil: case r.restoring: - if r.mode == r.restoreMode && (r.target == nil || *r.setpoint == *r.target) { + if !now.Before(r.confirmAfter) && r.mode == r.restoreMode && (r.target == nil || *r.setpoint == *r.target) { r.restoring = false r.targetDirty = false r.attempts = 0 changed = true } case r.targetDirty: - if *r.setpoint == *r.target { + if r.sentTarget != nil && *r.sentTarget == *r.target && *r.setpoint == *r.target { r.targetDirty = false r.attempts = 0 changed = true @@ -255,10 +282,7 @@ func (r *room) setTarget(target float64) { func (r *room) startAiring(until time.Time) { if r.airingUntil == nil && !r.restoring { - r.restoreMode = r.lastOnMode - if r.restoreMode == ModeUnknown { - r.restoreMode = ModeHeat - } + r.restoreMode = r.onMode() } if r.airingUntil == nil { r.attempts = 0 @@ -270,7 +294,35 @@ func (r *room) startAiring(until time.Time) { r.airingUntil = &until } -func (r *room) endAiring() { +// heatAgain ends airing, or turns a thermostat that is off back on, and reports +// whether that changed anything. +func (r *room) heatAgain(now time.Time) bool { + switch { + case r.airingUntil != nil: + r.endAiring(now) + return true + case r.mode == ModeOff && !r.restoring: + r.restoreMode = r.onMode() + r.endAiring(now) + return true + } + return false +} + +// onMode returns the mode to go back to. +func (r *room) onMode() Mode { + if r.lastOnMode == ModeUnknown { + return ModeHeat + } + return r.lastOnMode +} + +func (r *room) endAiring(now time.Time) { + // The off may still be on its way, so a report saying "on" could be older than + // it. Either a report says off, or the wait runs out. + if r.airingUntil != nil && !r.offConfirmed { + r.confirmAfter = now.Add(retryInterval) + } r.airingUntil = nil r.restoring = true r.attempts = 0 diff --git a/internal/mqtt/client.go b/internal/mqtt/client.go index 9d4790b..d799287 100644 --- a/internal/mqtt/client.go +++ b/internal/mqtt/client.go @@ -77,9 +77,10 @@ func (c *Client) Apply(trv string, cmd controller.Command) error { return c.publish(trv, "set", payload) } -// publish refuses to send while disconnected. During an outage paho would keep -// the message and send it after reconnecting, long after the caller was told it -// failed, and a late setpoint switches a thermostat that is off back to heat. +// publish sends with QoS 0 and refuses to send while disconnected. Paho keeps +// QoS 1 messages and resends them after reconnecting, and a late off or setpoint +// can undo what has been decided since. The controller already repeats a command +// until the thermostat's report confirms it, so this loses nothing. func (c *Client) publish(device, verb string, payload any) error { if !c.client.IsConnectionOpen() { return errNotConnected @@ -88,7 +89,7 @@ func (c *Client) publish(device, verb string, payload any) error { if err != nil { return err } - return waitForToken(c.client.Publish(fmt.Sprintf("%s/%s/%s", c.base, device, verb), 1, false, data), publishTimeout) + return waitForToken(c.client.Publish(fmt.Sprintf("%s/%s/%s", c.base, device, verb), 0, false, data), publishTimeout) } func (c *Client) onConnect(sink Sink, client paho.Client) { diff --git a/internal/mqtt/client_test.go b/internal/mqtt/client_test.go index 58ffd3f..318c423 100644 --- a/internal/mqtt/client_test.go +++ b/internal/mqtt/client_test.go @@ -30,12 +30,14 @@ type fakePaho struct { open bool topics []string payload []byte + qos byte } func (f *fakePaho) IsConnectionOpen() bool { return f.open } -func (f *fakePaho) Publish(topic string, _ byte, _ bool, payload any) paho.Token { +func (f *fakePaho) Publish(topic string, qos byte, _ bool, payload any) paho.Token { f.topics = append(f.topics, topic) + f.qos = qos f.payload, _ = payload.([]byte) return fakePahoToken{} } @@ -163,6 +165,9 @@ func TestApplyPayload(t *testing.T) { if got := string(f.payload); got != tt.want { t.Fatalf("payload = %s, want %s", got, tt.want) } + if f.qos != 0 { + t.Fatalf("qos = %d: paho would resend it after a reconnect", f.qos) + } }) } } diff --git a/internal/store/store.go b/internal/store/store.go index c994b05..68aea4c 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -2,18 +2,25 @@ package store import ( + "context" "encoding/json" "errors" + "fmt" "io/fs" "os" "path/filepath" + "sync" "github.com/subpathdev/heating-control/internal/controller" ) -// File implements controller.Store. +// File implements controller.Store and health.Checker. type File struct { path string + + mu sync.Mutex + corrupt bool + saveErr error } // New returns a File that saves to path. @@ -21,7 +28,22 @@ func New(path string) *File { return &File{path: path} } -// Load returns the saved records. A file that doesn't exist yet means none. +// Name implements health.Checker. +func (f *File) Name() string { return "state" } + +// Check implements health.Checker. A corrupt file stays reported until a restart, +// because a room it had on record may still be off. +func (f *File) Check(context.Context) error { + f.mu.Lock() + defer f.mu.Unlock() + if f.corrupt { + return fmt.Errorf("%s was corrupt and moved aside", f.path) + } + return f.saveErr +} + +// Load returns the saved records. A file that doesn't exist yet means none. A +// corrupt file is moved aside, because the next save would overwrite it. func (f *File) Load() (map[string]controller.RoomRecord, error) { data, err := os.ReadFile(f.path) if errors.Is(err, fs.ErrNotExist) { @@ -32,13 +54,27 @@ func (f *File) Load() (map[string]controller.RoomRecord, error) { } var records map[string]controller.RoomRecord if err := json.Unmarshal(data, &records); err != nil { - return nil, err + f.mu.Lock() + f.corrupt = true + f.mu.Unlock() + if renameErr := os.Rename(f.path, f.path+".corrupt"); renameErr != nil { + return nil, errors.Join(err, renameErr) + } + return nil, fmt.Errorf("moved to %s.corrupt: %w", f.path, err) } return records, nil } // Save replaces the file in one step, so a crash leaves the old or the new one. func (f *File) Save(records map[string]controller.RoomRecord) error { + err := f.write(records) + f.mu.Lock() + f.saveErr = err + f.mu.Unlock() + return err +} + +func (f *File) write(records map[string]controller.RoomRecord) error { data, err := json.MarshalIndent(records, "", " ") if err != nil { return err diff --git a/internal/store/store_test.go b/internal/store/store_test.go index 60d710b..3b45eeb 100644 --- a/internal/store/store_test.go +++ b/internal/store/store_test.go @@ -1,6 +1,7 @@ package store import ( + "context" "os" "path/filepath" "testing" @@ -45,12 +46,41 @@ func TestSaveAndLoad(t *testing.T) { } } +func TestCheckReportsAFailedSave(t *testing.T) { + f := New(filepath.Join(t.TempDir(), "missing", "state.json")) + if err := f.Save(nil); err == nil { + t.Fatal("want an error") + } + if err := f.Check(context.Background()); err == nil { + t.Fatal("the health check doesn't report it") + } +} + func TestLoadCorruptFile(t *testing.T) { path := filepath.Join(t.TempDir(), "state.json") if err := os.WriteFile(path, []byte("{nope"), 0o600); err != nil { t.Fatal(err) } - if _, err := New(path).Load(); err == nil { + f := New(path) + if _, err := f.Load(); err == nil { t.Fatal("want an error") } + if err := f.Check(context.Background()); err == nil { + t.Fatal("the health check doesn't report it") + } + got, err := os.ReadFile(path + ".corrupt") + if err != nil || string(got) != "{nope" { + t.Fatalf("moved file = %q, %v", got, err) + } + + // A later save neither touches it nor clears the report. + if err := f.Save(map[string]controller.RoomRecord{}); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(path + ".corrupt"); err != nil { + t.Fatal(err) + } + if err := f.Check(context.Background()); err == nil { + t.Fatal("the report was cleared") + } } diff --git a/internal/web/static/index.html b/internal/web/static/index.html index e8091ad..882e0b3 100644 --- a/internal/web/static/index.html +++ b/internal/web/static/index.html @@ -173,8 +173,9 @@

Heating

b.el.disabled = room.target == null || !inRange(room.target + b.step); } const airing = room.airingState === 'starting' || room.airingState === 'active'; + const offAlone = room.airingState === 'none' && room.mode === 'off'; for (const b of card.start) b.hidden = airing; - card.stop.hidden = !airing; + card.stop.hidden = !airing && !offAlone; const n = note(room); card.problem.className = 'note' + (n.problem ? ' problem' : ''); card.problem.textContent = n.text; From bb63a068e5fc98de01d74bac485622576ebd1c1e Mon Sep 17 00:00:00 2001 From: Jan Unterbrink <19386509+subpathdev@users.noreply.github.com> Date: Sun, 1 Nov 2026 16:12:28 +0100 Subject: [PATCH 3/3] refactor: remove saved state, so the backend is stateless All state is now in memory: no state file, Store interface or PVC. A restart during airing forgets it, so that room stays off until someone taps "Heat again", and a target waiting to be sent is lost. Co-Authored-By: Claude Opus 5.5 --- README.md | 8 +- .../heating-control/templates/deployment.yaml | 6 - charts/heating-control/templates/pvc.yaml | 14 -- charts/heating-control/values.yaml | 7 - cmd/heating-control/main.go | 7 +- internal/config/config.go | 6 +- internal/controller/controller.go | 92 ++-------- internal/controller/controller_test.go | 167 +----------------- internal/controller/room.go | 73 ++------ internal/store/store.go | 100 ----------- internal/store/store_test.go | 86 --------- internal/web/static/index.html | 2 +- 12 files changed, 51 insertions(+), 517 deletions(-) delete mode 100644 charts/heating-control/templates/pvc.yaml delete mode 100644 internal/store/store.go delete mode 100644 internal/store/store_test.go diff --git a/README.md b/README.md index c14a06f..16964a4 100644 --- a/README.md +++ b/README.md @@ -4,16 +4,18 @@ A small web page to adjust the heating per room from a phone on the home network ## Airing -"Airing 15/30/60" sends `system_mode: off`. When the time is up, or on "Heat again", the room gets back the mode and target from before. The end time is saved, so it survives a restart. If the service is down when the time is up, heating comes back when it starts again. Until then the thermostat's own frost protection still applies. +"Airing 15/30/60" sends `system_mode: off`. When the time is up, or on "Heat again", the room gets back the mode and target from before. + +The service keeps no saved state, so a restart during airing forgets it: **that room stays off until someone taps "Heat again"**. A target waiting to be sent is lost too. The thermostat's own frost protection still applies. - A target changed during airing is applied when it ends. - A thermostat leaving off during airing gets off once more. If it leaves off again, someone is at the radiator, so airing ends. - Commands repeat until the thermostat confirms them, every 30 s for about 5 minutes. A target then gives up and shows the thermostat's value. Off and heat-again keep trying every 5 minutes and show a problem on the page, because giving up would mean heating with the window open or a cold room. -- A thermostat that turned itself off is left alone, because only rooms this program turned off get restored. The page offers "Heat again" for it. +- A thermostat that is off with no airing is left alone, because it can turn itself off and come back. The page offers "Heat again" for it. ## Deployment -The Helm chart in [`charts/heating-control`](charts/heating-control) is published with every release as `oci://ghcr.io/subpathdev/charts/heating-control`. Set `config.broker` and `config.rooms`; adding a room is one more entry in `config.rooms`. Run one replica only, and give any other instance its own `config.clientId`: a second client with the same ID disconnects the first. The chart keeps saved state on a small PVC. +The Helm chart in [`charts/heating-control`](charts/heating-control) is published with every release as `oci://ghcr.io/subpathdev/charts/heating-control`. Set `config.broker` and `config.rooms`; adding a room is one more entry in `config.rooms`. Run one replica only, and give any other instance its own `config.clientId`: a second client with the same ID disconnects the first. ## API diff --git a/charts/heating-control/templates/deployment.yaml b/charts/heating-control/templates/deployment.yaml index 97ffd4b..c170b18 100644 --- a/charts/heating-control/templates/deployment.yaml +++ b/charts/heating-control/templates/deployment.yaml @@ -25,7 +25,6 @@ spec: runAsNonRoot: true runAsUser: 65532 runAsGroup: 65532 - fsGroup: 65532 seccompProfile: type: RuntimeDefault containers: @@ -56,12 +55,7 @@ spec: - name: config mountPath: /etc/heating-control readOnly: true - - name: state - mountPath: {{ dir .Values.config.stateFile }} volumes: - name: config configMap: name: {{ .Release.Name }} - - name: state - persistentVolumeClaim: - claimName: {{ .Release.Name }} diff --git a/charts/heating-control/templates/pvc.yaml b/charts/heating-control/templates/pvc.yaml deleted file mode 100644 index c8cb35a..0000000 --- a/charts/heating-control/templates/pvc.yaml +++ /dev/null @@ -1,14 +0,0 @@ -apiVersion: v1 -kind: PersistentVolumeClaim -metadata: - name: {{ .Release.Name }} - labels: - {{- include "heating-control.labels" . | nindent 4 }} -spec: - accessModes: ["ReadWriteOnce"] - {{- with .Values.persistence.storageClass }} - storageClassName: {{ . }} - {{- end }} - resources: - requests: - storage: {{ .Values.persistence.size }} diff --git a/charts/heating-control/values.yaml b/charts/heating-control/values.yaml index 6f13464..d91ec14 100644 --- a/charts/heating-control/values.yaml +++ b/charts/heating-control/values.yaml @@ -18,13 +18,6 @@ config: rooms: [] # Host names the page is reached by. IP addresses and localhost always work. allowedHosts: [] - stateFile: /var/lib/heating-control/state.json - -# Keeps airing end times and held targets across restarts. -persistence: - size: 16Mi - # Empty uses the cluster's default storage class. - storageClass: "" service: type: ClusterIP diff --git a/cmd/heating-control/main.go b/cmd/heating-control/main.go index c406184..e38efd8 100644 --- a/cmd/heating-control/main.go +++ b/cmd/heating-control/main.go @@ -17,7 +17,6 @@ import ( "github.com/subpathdev/heating-control/internal/health" "github.com/subpathdev/heating-control/internal/metrics" "github.com/subpathdev/heating-control/internal/mqtt" - "github.com/subpathdev/heating-control/internal/store" "github.com/subpathdev/heating-control/internal/web" ) @@ -38,18 +37,16 @@ func main() { client := mqtt.New(cfg) m := metrics.New(version) - state := store.New(cfg.StateFile) - ctrl := controller.New(cfg, m.Thermostat(client), state) + ctrl := controller.New(cfg, m.Thermostat(client)) client.Start(ctrl) defer client.Close() checker := client.Checker() m.AddChecker(checker) - m.AddChecker(state) m.AddRooms(ctrl) mux := http.NewServeMux() - health.Register(mux, version, checker, state) + health.Register(mux, version, checker) mux.Handle("GET /metrics", m.Handler()) // Probes and scrapes use the pod's IP and stay outside the page's checks. mux.Handle("/", web.Handler(ctrl, checker, cfg.AllowedHosts)) diff --git a/internal/config/config.go b/internal/config/config.go index 1742c55..c9be043 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -18,10 +18,7 @@ type Room struct { // Config is the program's configuration file. type Config struct { - // StateFile is where airing and held targets are saved. Its folder must be - // writable and survive restarts. - StateFile string `yaml:"stateFile"` - Broker string `yaml:"broker"` + Broker string `yaml:"broker"` // ClientID must be unique on the broker: a second client with the same ID // disconnects the first. ClientID string `yaml:"clientId"` @@ -49,7 +46,6 @@ func parse(data []byte) (*Config, error) { BaseTopic: "zigbee2mqtt", MinTarget: 10, MaxTarget: 25, - StateFile: "/var/lib/heating-control/state.json", } if err := yaml.Unmarshal(data, &cfg); err != nil { return nil, err diff --git a/internal/controller/controller.go b/internal/controller/controller.go index afa1090..45faf11 100644 --- a/internal/controller/controller.go +++ b/internal/controller/controller.go @@ -2,9 +2,11 @@ // thermostat reports, and sends commands until the two match. // // HTTP handlers and device reports only change state under mu and wake Run, -// which is the only place that talks to a thermostat or the Store. So nothing -// that delivers a report ever waits for a publish: the MQTT client couldn't read -// the broker's acknowledgement while it does. +// which is the only place that talks to a thermostat. So nothing that delivers a +// report ever waits for a publish. +// +// All state is in memory. After a restart a room that was airing stays off until +// someone taps "Heat again" on the page. // // Four behaviours of the Sonoff TRVZB shape this code. They were measured on a // real device (firmware 1.3.0): @@ -15,7 +17,8 @@ // changed during that time is held until it can be sent. // - After a write it can still send a few reports with the old setpoint. // - It can go off on its own and return by itself, so "off" doesn't mean this -// program turned it off. Only rooms with a saved record are ever restored. +// program turned it off. A room is only turned back on if this program turned +// it off, or someone asks for it. package controller import ( @@ -88,22 +91,6 @@ type Thermostat interface { Apply(trv string, cmd Command) error } -// Store saves what must survive a restart. -type Store interface { - Load() (map[string]RoomRecord, error) - Save(records map[string]RoomRecord) error -} - -// RoomRecord is a room's saved state. -type RoomRecord struct { - Target *float64 - TargetDirty bool - AiringUntil *time.Time - RestoreMode Mode - Restoring bool - LastOnMode Mode -} - // ThermostatState is a thermostat's report. Fields it didn't include are nil. type ThermostatState struct { Mode *Mode @@ -131,40 +118,32 @@ type RoomStatus struct { // Controller is safe for concurrent use. type Controller struct { thermostat Thermostat - store Store now func() time.Time min, max float64 wake chan struct{} - // mu guards the rooms' state and is never held during a publish or save. - // rooms itself and each room's cfg don't change after New. - mu sync.Mutex - rooms []*room - changed bool + // mu guards the rooms' state and is never held during a publish. rooms itself + // and each room's cfg don't change after New. + mu sync.Mutex + rooms []*room } -// New returns a Controller for the configured rooms, with what store has saved. -// A store that can't be read is logged, and the rooms start empty. -func New(cfg *config.Config, thermostat Thermostat, store Store) *Controller { +// New returns a Controller for the configured rooms. +func New(cfg *config.Config, thermostat Thermostat) *Controller { c := &Controller{ thermostat: thermostat, - store: store, now: time.Now, min: cfg.MinTarget, max: cfg.MaxTarget, wake: make(chan struct{}, 1), } - records, err := store.Load() - if err != nil { - log.Printf("loading saved state: %v", err) - } for _, r := range cfg.Rooms { - c.rooms = append(c.rooms, newRoom(r, records[r.ID])) + c.rooms = append(c.rooms, &room{cfg: r}) } return c } -// Run sends commands and saves state until ctx is done. +// Run sends commands until ctx is done. func (c *Controller) Run(ctx context.Context) { ticker := time.NewTicker(tickInterval) defer ticker.Stop() @@ -172,7 +151,6 @@ func (c *Controller) Run(ctx context.Context) { c.tick() select { case <-ctx.Done(): - c.save() return case <-ticker.C: case <-c.wake: @@ -185,9 +163,7 @@ func (c *Controller) tick() { for _, r := range c.rooms { c.mu.Lock() now := c.now() - if r.advance(now) { - c.changed = true - } + r.advance(now) cmd, send := r.decide(now) if send { r.attempts++ @@ -199,9 +175,6 @@ func (c *Controller) tick() { if !send { continue } - // Saved first: after a crash a room that is off with no record would stay - // off, because only rooms with a record get restored. - c.save() if err := c.thermostat.Apply(r.cfg.TRV, cmd); err != nil { log.Printf("%s: %s: %v", r.cfg.TRV, cmd.Kind(), err) // Never reached the thermostat, so it says nothing about it. @@ -216,29 +189,6 @@ func (c *Controller) tick() { r.publishFailed = false c.mu.Unlock() } - c.save() -} - -// save writes the rooms if they changed. A failed save tries again next tick. -func (c *Controller) save() { - c.mu.Lock() - if !c.changed { - c.mu.Unlock() - return - } - records := make(map[string]RoomRecord, len(c.rooms)) - for _, r := range c.rooms { - records[r.cfg.ID] = r.record() - } - c.changed = false - c.mu.Unlock() - - if err := c.store.Save(records); err != nil { - log.Printf("saving state: %v", err) - c.mu.Lock() - c.changed = true - c.mu.Unlock() - } } func (c *Controller) signal() { @@ -279,7 +229,6 @@ func (c *Controller) SetTarget(id string, target float64) error { c.mu.Lock() r.setTarget(target) - c.changed = true c.mu.Unlock() c.signal() return nil @@ -294,7 +243,6 @@ func (c *Controller) StartAiring(id string, d time.Duration) error { } c.mu.Lock() r.startAiring(c.now().Add(d)) - c.changed = true c.mu.Unlock() c.signal() return nil @@ -308,9 +256,7 @@ func (c *Controller) StopAiring(id string) error { return fmt.Errorf("%w: %q", ErrUnknownRoom, id) } c.mu.Lock() - if r.heatAgain(c.now()) { - c.changed = true - } + r.heatAgain(c.now()) c.mu.Unlock() c.signal() return nil @@ -321,9 +267,7 @@ func (c *Controller) HandleThermostatState(trv string, s ThermostatState) { c.mu.Lock() for _, r := range c.rooms { if r.cfg.TRV == trv { - if r.report(s, c.now()) { - c.changed = true - } + r.report(s, c.now()) break } } diff --git a/internal/controller/controller_test.go b/internal/controller/controller_test.go index 6e31f4e..692aa95 100644 --- a/internal/controller/controller_test.go +++ b/internal/controller/controller_test.go @@ -32,45 +32,22 @@ func (f *fakeThermostat) kinds() []string { return out } -type fakeStore struct { - records map[string]RoomRecord - loadErr error - saveErr error - saves int -} - -func (f *fakeStore) Load() (map[string]RoomRecord, error) { return f.records, f.loadErr } -func (f *fakeStore) Save(r map[string]RoomRecord) error { - if f.saveErr != nil { - return f.saveErr - } - f.records = r - f.saves++ - return nil -} - // harness drives a Controller by hand: time and ticks only move when told to. type harness struct { - t *testing.T - c *Controller - f *fakeThermostat - store *fakeStore - now time.Time + t *testing.T + c *Controller + f *fakeThermostat + now time.Time } func newHarness(t *testing.T) *harness { t.Helper() - return newHarnessWithStore(t, &fakeStore{}) -} - -func newHarnessWithStore(t *testing.T, store *fakeStore) *harness { - t.Helper() - h := &harness{t: t, f: &fakeThermostat{}, store: store, now: time.Unix(1000, 0)} + h := &harness{t: t, f: &fakeThermostat{}, now: time.Unix(1000, 0)} h.c = New(&config.Config{ MinTarget: 10, MaxTarget: 25, Rooms: []config.Room{{ID: "office", Label: "Office", TRV: "trv-office", Sensor: "sensor-office"}}, - }, h.f, store) + }, h.f) h.c.now = func() time.Time { return h.now } return h } @@ -517,32 +494,6 @@ func TestRestoreIsConfirmedOnceTheWaitRunsOut(t *testing.T) { } } -// Saved first: a crash during the publish must leave a record of the airing. -func TestAiringIsSavedBeforeOffIsSent(t *testing.T) { - store := &fakeStore{} - h := newHarnessWithStore(t, store) - h.report("heat", 20) - h.c.tick() - - var savedAtSend *RoomRecord - h.c.thermostat = thermostatFunc(func(string, Command) error { - rec := store.records["office"] - savedAtSend = &rec - return nil - }) - if err := h.c.StartAiring("office", time.Hour); err != nil { - t.Fatal(err) - } - h.c.tick() - if savedAtSend == nil || savedAtSend.AiringUntil == nil { - t.Fatalf("saved at send = %+v, want the airing", savedAtSend) - } -} - -type thermostatFunc func(string, Command) error - -func (f thermostatFunc) Apply(trv string, cmd Command) error { return f(trv, cmd) } - func TestStopAiringWithoutAiringDoesNothing(t *testing.T) { h := newHarness(t) h.report("heat", 20) @@ -731,81 +682,8 @@ func TestLostRestoreKeepsGoing(t *testing.T) { } } -func TestRestartDuringAiringKeepsItsEndTime(t *testing.T) { - store := &fakeStore{} - h := newHarnessWithStore(t, store) - h.report("heat", 20) - h.startAiring(time.Hour) - h.report("off", 7) - - again := newHarnessWithStore(t, store) - again.now = h.now.Add(10 * time.Minute) - again.c.tick() - // The thermostat hasn't reported yet, and off is safe to repeat. - again.wantKinds("off") - again.report("off", 7) - if got := again.status().AiringState; got != AiringActive { - t.Fatalf("state = %s, want active", got) - } - if got := again.target(); got != 20 { - t.Fatalf("target = %v, want the saved 20", got) - } - - again.advance(50 * time.Minute) - again.wantKinds("off", "restore") - cmd := again.f.sent[1] - if *cmd.Mode != ModeHeat || *cmd.Target != 20 { - t.Fatalf("restored %v/%v", *cmd.Mode, *cmd.Target) - } -} - -func TestRestartAfterTheEndTimeRestoresAtStart(t *testing.T) { - store := &fakeStore{} - h := newHarnessWithStore(t, store) - h.report("heat", 20) - h.startAiring(time.Minute) - h.report("off", 7) - - again := newHarnessWithStore(t, store) - again.now = h.now.Add(time.Hour) - again.c.tick() - again.wantKinds("restore") -} - -func TestRestartKeepsTheLastOnMode(t *testing.T) { - store := &fakeStore{} - h := newHarnessWithStore(t, store) - h.report("auto", 20) - h.report("off", 7) - - again := newHarnessWithStore(t, store) - again.report("off", 7) - again.startAiring(time.Minute) - again.advance(time.Minute) - again.wantKinds("restore") - if got := *again.f.sent[0].Mode; got != ModeAuto { - t.Fatalf("restored %s, want auto", got) - } -} - -func TestRestartKeepsAHeldTarget(t *testing.T) { - store := &fakeStore{} - h := newHarnessWithStore(t, store) - h.report("heat", 20) - h.report("off", 7) - h.setTarget(22) - - again := newHarnessWithStore(t, store) - again.report("off", 7) - if got := again.status().TargetState; got != TargetHeld { - t.Fatalf("state = %s, want held", got) - } - again.report("heat", 20) - again.wantKinds("set_target") -} - -// Only saved records are restored: the thermostat can go off by itself. -func TestRoomOffAtStartWithoutARecordIsLeftAlone(t *testing.T) { +// The thermostat can go off by itself, so only a tap turns it back on. +func TestRoomThatIsOffIsLeftAlone(t *testing.T) { h := newHarness(t) h.report("off", 7) h.advance(time.Hour) @@ -815,33 +693,6 @@ func TestRoomOffAtStartWithoutARecordIsLeftAlone(t *testing.T) { } } -func TestUnreadableStoreStartsEmpty(t *testing.T) { - h := newHarnessWithStore(t, &fakeStore{loadErr: errors.New("corrupt")}) - h.report("heat", 20) - if got := h.target(); got != 20 { - t.Fatalf("target = %v, want 20", got) - } -} - -func TestFailedSaveIsRetried(t *testing.T) { - store := &fakeStore{saveErr: errors.New("disk full")} - h := newHarnessWithStore(t, store) - h.report("heat", 20) - h.startAiring(time.Hour) - if store.saves != 0 { - t.Fatal("saved despite the error") - } - store.saveErr = nil - h.advance(time.Second) - if store.saves != 1 || store.records["office"].AiringUntil == nil { - t.Fatalf("saves = %d, records = %+v", store.saves, store.records) - } - h.advance(time.Second) - if store.saves != 1 { - t.Fatalf("saved %d times without a change", store.saves) - } -} - func TestSensorTemperatureWinsOverLocal(t *testing.T) { h := newHarness(t) h.c.HandleThermostatState("trv-office", ThermostatState{LocalTemperature: ptr(22.0)}) @@ -885,7 +736,7 @@ func TestRunDoesntBlockReportsDuringAPublish(t *testing.T) { MinTarget: 10, MaxTarget: 25, Rooms: []config.Room{{ID: "office", Label: "Office", TRV: "trv-office"}}, - }, block, &fakeStore{}) + }, block) c.HandleThermostatState("trv-office", state("heat", 20)) ctx, cancel := context.WithCancel(context.Background()) diff --git a/internal/controller/room.go b/internal/controller/room.go index b7ec990..3227818 100644 --- a/internal/controller/room.go +++ b/internal/controller/room.go @@ -63,7 +63,7 @@ type room struct { settled bool onSince time.Time - // What the room should be doing. Saved. + // What the room should be doing. target *float64 targetDirty bool airingUntil *time.Time @@ -88,29 +88,6 @@ type room struct { targetFailed bool } -func newRoom(cfg config.Room, rec RoomRecord) *room { - return &room{ - cfg: cfg, - target: rec.Target, - targetDirty: rec.TargetDirty, - airingUntil: rec.AiringUntil, - restoreMode: rec.RestoreMode, - restoring: rec.Restoring, - lastOnMode: rec.LastOnMode, - } -} - -func (r *room) record() RoomRecord { - return RoomRecord{ - Target: r.target, - TargetDirty: r.targetDirty, - AiringUntil: r.airingUntil, - RestoreMode: r.restoreMode, - Restoring: r.restoring, - LastOnMode: r.lastOnMode, - } -} - // decide returns the command the room needs now, if any. It changes nothing. func (r *room) decide(now time.Time) (Command, bool) { var cmd Command @@ -144,16 +121,14 @@ func (r *room) due(now time.Time) bool { return !now.Before(r.lastSent.Add(wait)) } -// advance applies what time alone changes, and reports whether anything to save -// changed. -func (r *room) advance(now time.Time) bool { +// advance applies what time alone changes. +func (r *room) advance(now time.Time) { if r.canWrite() && !r.settled && !now.Before(r.onSince.Add(settleTime)) { r.settled = true } switch { case r.airingUntil != nil && !now.Before(*r.airingUntil): r.endAiring(now) - return true case r.targetDirty && r.airingUntil == nil && !r.restoring && r.canWrite() && r.attempts >= maxAttempts && !now.Before(r.lastSent.Add(retryInterval)): // A knob turn and a lost write look the same, so the thermostat wins and @@ -165,14 +140,11 @@ func (r *room) advance(now time.Time) bool { if r.setpoint != nil && !r.isFrost(*r.setpoint) { r.target = r.setpoint } - return true } - return false } -// report records a thermostat's report and reports whether anything to save -// changed. -func (r *room) report(s ThermostatState, now time.Time) bool { +// report records a thermostat's report. +func (r *room) report(s ThermostatState, now time.Time) { r.lastSeen = &now if s.Heating != nil { r.heating = *s.Heating @@ -187,7 +159,6 @@ func (r *room) report(s ThermostatState, now time.Time) bool { r.setpoint = s.Setpoint } - changed := false if s.Mode != nil { wasOn := r.canWrite() r.mode = *s.Mode @@ -195,21 +166,16 @@ func (r *room) report(s ThermostatState, now time.Time) bool { if !wasOn { r.onSince = now } - if r.lastOnMode != r.mode { - r.lastOnMode = r.mode - changed = true - } + r.lastOnMode = r.mode } if r.mode == ModeOff { r.settled = false r.confirmAfter = time.Time{} } - if r.reportAiringMode() { - changed = true - } + r.reportAiringMode() } if r.setpoint == nil { - return changed + return } own := r.canWrite() && !r.isFrost(*r.setpoint) if own { @@ -223,50 +189,45 @@ func (r *room) report(s ThermostatState, now time.Time) bool { r.restoring = false r.targetDirty = false r.attempts = 0 - changed = true } case r.targetDirty: if r.sentTarget != nil && *r.sentTarget == *r.target && *r.setpoint == *r.target { r.targetDirty = false r.attempts = 0 - changed = true } case own && (r.target == nil || *r.setpoint != *r.target): // Someone turned the knob. r.target = r.setpoint r.targetFailed = false - changed = true } - return changed } // reportAiringMode handles a mode report during airing. -func (r *room) reportAiringMode() bool { +func (r *room) reportAiringMode() { if r.airingUntil == nil { - return false + return } if r.mode == ModeOff { if !r.offConfirmed { r.attempts = 0 } r.offConfirmed = true - return false + return } if r.mode == ModeUnknown || !r.offConfirmed { - return false + return } r.offConfirmed = false r.attempts = 0 r.flips++ if r.flips < maxFlips { - return false + return } // The thermostat is on because someone wants it on, so nothing to restore. log.Printf("%s: left off %d times, ending airing", r.cfg.TRV, r.flips) r.airingUntil = nil r.restoring = false r.flips = 0 - return true } func (r *room) setTarget(target float64) { @@ -294,19 +255,15 @@ func (r *room) startAiring(until time.Time) { r.airingUntil = &until } -// heatAgain ends airing, or turns a thermostat that is off back on, and reports -// whether that changed anything. -func (r *room) heatAgain(now time.Time) bool { +// heatAgain ends airing, or turns a thermostat that is off back on. +func (r *room) heatAgain(now time.Time) { switch { case r.airingUntil != nil: r.endAiring(now) - return true case r.mode == ModeOff && !r.restoring: r.restoreMode = r.onMode() r.endAiring(now) - return true } - return false } // onMode returns the mode to go back to. diff --git a/internal/store/store.go b/internal/store/store.go deleted file mode 100644 index 68aea4c..0000000 --- a/internal/store/store.go +++ /dev/null @@ -1,100 +0,0 @@ -// Package store saves the controller's state to a JSON file. -package store - -import ( - "context" - "encoding/json" - "errors" - "fmt" - "io/fs" - "os" - "path/filepath" - "sync" - - "github.com/subpathdev/heating-control/internal/controller" -) - -// File implements controller.Store and health.Checker. -type File struct { - path string - - mu sync.Mutex - corrupt bool - saveErr error -} - -// New returns a File that saves to path. -func New(path string) *File { - return &File{path: path} -} - -// Name implements health.Checker. -func (f *File) Name() string { return "state" } - -// Check implements health.Checker. A corrupt file stays reported until a restart, -// because a room it had on record may still be off. -func (f *File) Check(context.Context) error { - f.mu.Lock() - defer f.mu.Unlock() - if f.corrupt { - return fmt.Errorf("%s was corrupt and moved aside", f.path) - } - return f.saveErr -} - -// Load returns the saved records. A file that doesn't exist yet means none. A -// corrupt file is moved aside, because the next save would overwrite it. -func (f *File) Load() (map[string]controller.RoomRecord, error) { - data, err := os.ReadFile(f.path) - if errors.Is(err, fs.ErrNotExist) { - return nil, nil - } - if err != nil { - return nil, err - } - var records map[string]controller.RoomRecord - if err := json.Unmarshal(data, &records); err != nil { - f.mu.Lock() - f.corrupt = true - f.mu.Unlock() - if renameErr := os.Rename(f.path, f.path+".corrupt"); renameErr != nil { - return nil, errors.Join(err, renameErr) - } - return nil, fmt.Errorf("moved to %s.corrupt: %w", f.path, err) - } - return records, nil -} - -// Save replaces the file in one step, so a crash leaves the old or the new one. -func (f *File) Save(records map[string]controller.RoomRecord) error { - err := f.write(records) - f.mu.Lock() - f.saveErr = err - f.mu.Unlock() - return err -} - -func (f *File) write(records map[string]controller.RoomRecord) error { - data, err := json.MarshalIndent(records, "", " ") - if err != nil { - return err - } - tmp, err := os.CreateTemp(filepath.Dir(f.path), filepath.Base(f.path)+".*") - if err != nil { - return err - } - defer func() { _ = os.Remove(tmp.Name()) }() - - if _, err := tmp.Write(data); err != nil { - _ = tmp.Close() - return err - } - if err := tmp.Sync(); err != nil { - _ = tmp.Close() - return err - } - if err := tmp.Close(); err != nil { - return err - } - return os.Rename(tmp.Name(), f.path) -} diff --git a/internal/store/store_test.go b/internal/store/store_test.go deleted file mode 100644 index 3b45eeb..0000000 --- a/internal/store/store_test.go +++ /dev/null @@ -1,86 +0,0 @@ -package store - -import ( - "context" - "os" - "path/filepath" - "testing" - "time" - - "github.com/subpathdev/heating-control/internal/controller" -) - -func TestLoadWithoutAFile(t *testing.T) { - records, err := New(filepath.Join(t.TempDir(), "state.json")).Load() - if err != nil || records != nil { - t.Fatalf("got %v, %v", records, err) - } -} - -func TestSaveAndLoad(t *testing.T) { - path := filepath.Join(t.TempDir(), "state.json") - target := 20.5 - until := time.Unix(1000, 0).UTC() - f := New(path) - if err := f.Save(map[string]controller.RoomRecord{ - "office": {Target: &target, TargetDirty: true, AiringUntil: &until, RestoreMode: controller.ModeAuto, Restoring: true}, - }); err != nil { - t.Fatal(err) - } - - records, err := New(path).Load() - if err != nil { - t.Fatal(err) - } - got := records["office"] - if *got.Target != 20.5 || !got.TargetDirty || !got.AiringUntil.Equal(until) || got.RestoreMode != controller.ModeAuto || !got.Restoring { - t.Fatalf("got %+v", got) - } - - files, err := os.ReadDir(filepath.Dir(path)) - if err != nil { - t.Fatal(err) - } - if len(files) != 1 { - t.Fatalf("left %d files behind", len(files)) - } -} - -func TestCheckReportsAFailedSave(t *testing.T) { - f := New(filepath.Join(t.TempDir(), "missing", "state.json")) - if err := f.Save(nil); err == nil { - t.Fatal("want an error") - } - if err := f.Check(context.Background()); err == nil { - t.Fatal("the health check doesn't report it") - } -} - -func TestLoadCorruptFile(t *testing.T) { - path := filepath.Join(t.TempDir(), "state.json") - if err := os.WriteFile(path, []byte("{nope"), 0o600); err != nil { - t.Fatal(err) - } - f := New(path) - if _, err := f.Load(); err == nil { - t.Fatal("want an error") - } - if err := f.Check(context.Background()); err == nil { - t.Fatal("the health check doesn't report it") - } - got, err := os.ReadFile(path + ".corrupt") - if err != nil || string(got) != "{nope" { - t.Fatalf("moved file = %q, %v", got, err) - } - - // A later save neither touches it nor clears the report. - if err := f.Save(map[string]controller.RoomRecord{}); err != nil { - t.Fatal(err) - } - if _, err := os.Stat(path + ".corrupt"); err != nil { - t.Fatal(err) - } - if err := f.Check(context.Background()); err == nil { - t.Fatal("the report was cleared") - } -} diff --git a/internal/web/static/index.html b/internal/web/static/index.html index 882e0b3..9b7f2ed 100644 --- a/internal/web/static/index.html +++ b/internal/web/static/index.html @@ -92,7 +92,7 @@

Heating

case 'restoring': return 'Airing is over: turning the heating back on'; } if (room.mode === 'unknown') return 'Waiting for the thermostat'; - if (room.mode === 'off') return 'Not heating: the thermostat turned itself off'; + if (room.mode === 'off') return 'Not heating: the thermostat is off'; return room.heating ? 'Heating' : 'Not heating right now'; }