feat(web): set a heat pump's curve offset from the Heating view - #749
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99690233ba
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
miravoss26
left a comment
There was a problem hiding this comment.
Adds a heat-pump curve-offset control to the Heating view, rendered entirely from the driver's declaration and wired to the operator control-hold endpoint (POST/DELETE /api/drivers/{name}/control) from #741.
What I checked:
- Every driver-derived string is escaped: escapeHtml on the name, control id, value and aria-label in the rendered HTML, and encodeURIComponent(name) on the fetch path. No XSS or path-injection surface.
- Declaration-driven: nothing keys on a driver name (a test asserts the control section never names heishamon/myuplink/nibe), a pump that declares nothing renders nothing, and bounds/step/unit come from the declaration. It clamps to the declared min/max for the button-disable state; the server (#741) clamps authoritatively.
- "Auto" when there's no hold rather than printing a 0 it doesn't know; held state carried by text and weight, not colour (deuteranopia); a write_ack driver says "does not confirm this setting". All tested (9 tests).
- stopPropagation so commanding the pump doesn't open the detail drill-in, and refreshAfterControl so a press shows its result mid-30s-refresh.
One question, not a blocker: the PR also adds go/state.db.clean (empty), go/state.db.snapshot (binary), and two go/driver-repository/cache/*.json files. Are those intended committed fixtures, or leftovers from a local run? Worth a glance before merge.
Safe to merge from my read. Not in my auto-merge allowlist, so a human merges.
f877ae1 to
ccd65a3
Compare
9ccb928 to
ecabb30
Compare
32f871d to
e911cc6
Compare
e911cc6 to
b7faedf
Compare
A pump whose driver declares a control gets a row on its card: the value in force, when the hold ends, and buttons to move or release it. A pump that declares nothing looks exactly as it did. On the Heating card rather than Settings → Devices, because that is where the pump's own state already is — the offset sits next to the temperatures it moves. Settings is for connecting a device, not running it. Rendered entirely from the declaration; nothing here knows a driver by name. Stepper buttons rather than a slider or number field: the card is re-rendered wholesale every 30 s and a control holding input state would lose a half-typed value on every refresh. Control clicks stop propagating, or the card — which is itself a button into the all-signals view — opens over the thing the operator just pressed. With no hold the row reads "Auto" rather than 0: nothing in the browser knows what offset the pump settled on internally. Held state is carried by text and weight, never colour, since the theme's green/red pair is not separable under deuteranopia. A driver declaring write_ack rather than readback says so in the row. Verified against a running FTW with a probe driver: pressing + drove the driver's own hp_z1_heat_offset to 1, raise disabled at the declared +3, Release returned it to 0 through driver_default_mode, and a control click did not open the detail view while a card click still did. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
b7faedf to
c9dfd43
Compare
Built on merged #741 and #738.
Base: master at 369d120
Head: 5de3525
A pump whose driver declares a control gets a row on its Heating card. A pump that declares nothing keeps the existing telemetry view.
Changes
Verification
All three original review threads are resolved: observed offset start, queued refresh, and generated artifacts. This head also includes the independent rereview fixes for the persistent in-flight control gate and fail-closed telemetry requirement.
A driver-specific controls block remains a follow-up in srcfl/device-drivers.