Add #92: write-operation layer, programmatic services, and settle events#105
Conversation
Structural refactor of the write path per the issue #92 design discussion. - New services::WriteOperationService owns every pump write: operations queue and run strictly one at a time, build their wire frames from the caller's arguments (never from a possibly-stale cache), confirm themselves with a readback of what the pump actually stored, and end in exactly one terminal result (accepted/clamped/rejected/timeout/ superseded). Watchdogs, validation rejects, and disconnect termination make the one-terminal-event guarantee structural. - New api_bridge (CustomAPIDevice): pump_set_enabled, pump_set_mode (unfused 0x0A01 per #98), pump_set_setpoint, pump_set_temperature_range, pump_set_cycle_times; fires esphome.alpha_hwr_write_settled with the settled values and caller-supplied op_id. Requires custom_services + homeassistant_services on api: (set in all packages/examples). - One write path: entity facade methods submit to the operation layer (op_id ""); ControlService keeps the cache, the #91 guards, and the wire primitives, and loses its legacy multi-step setters. - Schedule services migrated from alpha_hwr_schedule_editor.yaml into the bridge with unchanged names/data formats plus op_id, and are now verified: overview precondition, mandatory fresh layer read before entry writes, post-commit readbacks, honest slot auto-selection. Previously every schedule write reported success unconditionally. - Cycle-time writes gain the Object 91 verify readback the legacy fire-and-forget setter never had (#94-safe short-payload handling). - New tests/test_write_operations.cpp: pump simulator driving every terminal status (126 cases; full suite 342 green). ESP32-C3 firmware builds clean against the local component. Docs: new docs/programmatic-interface.md (contract), plus architecture, configuration, schedule-management, AGENTS.md, README, CHANGELOG updates.
aioesphomeapi-based CLI used for the hardware verification of the write interface: lists services, snapshots states, calls a service and waits for its op_id's write_settled event, and fires rapid bursts to exercise the supersede path. Verifies the one-terminal-event contract on every call. Connection details come from flags or ALPHA_HWR_HOST/ALPHA_HWR_API_KEY (or --secrets pointing at a secrets.yaml).
The header uses NAN and std::isnan; Apple clang picked up <cmath> transitively but GCC on the CI runner does not. Exposed by test_write_operations, the first host test to compile this header.
Command ids and ACK shapes bench-contributed by jfriend00 on the issue (START 0x06 / STOP 0x05 as Class 3 SET; success is the clean [03 00] ACK, rejection the [03 01 xx] descriptor reply, matching the #46 remote-mode shapes). The on/off operation now sends this instead of the fused 0x0601 control object: it carries no mode and no setpoint, so the RESOLVING mode pre-read is gone and the last fused write leaves the on/off path. Class 3 produces no unsolicited notification, so the existing confirm readback remains the authoritative verdict; an explicit descriptor nack settles the operation rejected, and a closed ACK window falls through to the readback (the pump may still have applied the command). Service and event contract unchanged. Host suite: 350 tests green (new cases: unfused wire shape, descriptor nack -> rejected, ACK-window-closed -> readback decides).
jfriend00 proved the Sub 430 block is byte-invariant across confirmed cycle-time changes. The GENI profile (52/7) explains why: Sub 430 is type 1012 TemperatureRangeControlUserSettings, whose trailing bytes are min/max on/off-time LIMITS. The live values are Sub 421, dhw_on_off_control_configuration_obj (type 985: [setpoint f32][on u8] [off u8]), ReadWrite -- and a GO-app capture shows the exact write frame (OpSpec 0x8F, obj-first addressing, no configuration commit) with the value persisting and reading back. - read_dhw_config/write_dhw_config target Sub 421 with the captured frame shape; the write is read-modify-write, echoing the stored flow setpoint bytes verbatim (refuses to write blind). - The set_cycle_times operation fresh-reads Sub 421, writes with ACK, drops the commit, and verifies via a Sub 421 readback. - Transport learns the Sub 421 OpSpec 0x0D reply shape and the write's short-ACK pattern. - Cycle Time ON/OFF entities are wired to the pump's real cached values (previously optimistic hardcoded 5/15 with no getter); the DHW config joins every cache sync. - Adjacent fix: temperature-range writes previously sent constants over the Sub 430 limit tail bytes, zeroing the pump's limits; they now echo the bytes from the last read. Host suite: 363 tests green (new: Sub 421 wire shape + setpoint preservation, pump-clamp -> clamped, unreadable config -> rejected before any write, temperature write preserves the limit tail).
- Supersede keys for setpoint writes are strictly per-mode (review catch): the pump stores an independent setpoint per mode, so a queued constant_speed setpoint must not be dropped by a subsequent constant_flow setpoint. The shared "mode" key is gone; serialization alone yields the correct final mode. - New terminal status: invalid. Malformed or out-of-range requests (bad values, unknown modes, unparsable data strings) settle invalid, decided pre-wire and deterministic; rejected now strictly means the pump or its state refused. - Settle event gains origin (service|entity, so an API call with an empty op_id is still identifiable as an API call), seq (submission- order number: superseded events fire at submit time and can precede earlier operations' terminals), and requested_* echoes of the original arguments so the event is self-contained for logging and retries. - Docs: set_setpoint's mode-switching semantics made explicit, GO-app installer-limit clamping noted, event schema and status taxonomy updated. Host suite: 376 tests green (new: different-mode setpoints both run, origin/seq/requested echoes, invalid-status assertions).
There was a problem hiding this comment.
Pull request overview
This PR implements the #92 “write-and-verify” programmatic control interface by introducing a single serialized write-operation layer, surfacing new Home Assistant services, and emitting one terminal esphome.alpha_hwr_write_settled event per write. It also fixes Cycle Time Control by moving cycle-time reads/writes to the correct GENI object (Obj 91 Sub 421) and wiring the entities to live cached values.
Changes:
- Add
services::WriteOperationServiceto serialize all pump writes (service + entity), enforce one-terminal-result semantics, and verify outcomes via readbacks. - Add
api_bridgeto register HA services (including migrated schedule services) and fire thewrite_settledevent with requested + settled fields. - Fix Cycle Time Control by switching from Obj 91 Sub 430 to Obj 91 Sub 421, updating transport matching and control/config parsing accordingly, and adding host tests + bench tooling.
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/write_bench.py | Adds an aioesphomeapi bench client to call services and validate the one-terminal-event contract. |
| tests/Makefile | Builds and runs the new test_write_operations suite. |
| tests/.gitignore | Ignores the new test_write_operations binary (and an additional test artifact entry). |
| README.md | Documents the programmatic services and write_settled event and clarifies schedule service ownership. |
| packages/alpha_hwr_schedule_editor.yaml | Removes YAML-defined services; keeps helper entities; enables required api: flags. |
| packages/alpha_hwr_pairing.yaml | Enables api.custom_services + api.homeassistant_services for programmatic interface. |
| packages/alpha_hwr_controls.yaml | Wires Cycle Time ON/OFF entities to live cached values and routes writes through the new layer. |
| packages/alpha_hwr_base.yaml | Enables api.custom_services + api.homeassistant_services for programmatic interface. |
| hwr-pump-schedule-example.yaml | Enables required api: flags for services/events. |
| hwr-pump-example.yaml | Enables required api: flags for services/events. |
| hwr-pairing-example.yaml | Enables required api: flags for services/events. |
| docs/schedule-management.md | Updates documentation to reflect schedule services registered in C++ and verified settle semantics. |
| docs/programmatic-interface.md | Updates the public contract (Class 3 start/stop, invalid, origin, seq, requested_*, supersede semantics). |
| docs/configuration.md | Documents required api: flags for programmatic services + write_settled event. |
| docs/architecture.md | Documents api_bridge and the single write-path architecture via WriteOperationService. |
| components/alpha_hwr/write_operation_service.h | Defines write commands/status/result schema and the write-operation service interface. |
| components/alpha_hwr/write_operation_service.cpp | Implements operation queueing/supersede, per-command sequencing, confirm readbacks, and terminal results. |
| components/alpha_hwr/transport.cpp | Extends response matching/workarounds to cover Obj 91 Sub 421 and new DHW config write opspec. |
| components/alpha_hwr/schedule_service.h | Adds async polling/state-write APIs and per-slot single-event read helpers for verified writes. |
| components/alpha_hwr/schedule_service.cpp | Implements new async schedule APIs used by the operation layer and bridge. |
| components/alpha_hwr/control_service.h | Refactors control into cache + wire primitives, adds mode string mapping, and DHW config read/write APIs. |
| components/alpha_hwr/control_service.cpp | Implements Obj 91 Sub 430 tail preservation, Obj 91 Sub 421 DHW config read/write, and Class 3 START/STOP primitive. |
| components/alpha_hwr/api_bridge.h | Adds HA bridge (compiled conditionally) for service registration and settle-event emission. |
| components/alpha_hwr/api_bridge.cpp | Registers services, parses arguments (including schedule data strings), and fires write_settled events. |
| components/alpha_hwr/alpha_hwr.h | Integrates the operation layer + API bridge and routes entity writes through the single write path. |
| components/alpha_hwr/alpha_hwr.cpp | Wires scheduling/ready gates, disconnect termination, central result hook, and service registration. |
| CHANGELOG.md | Adds detailed Unreleased notes for the new interface, migration, and cycle-time fix. |
| AGENTS.md | Updates agent/dev guidance to reflect the operation layer, test styles, and write-path rules. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for (uint32_t seq : superseded) { | ||
| finish_(seq, WriteStatus::SUPERSEDED, | ||
| op.op_id.empty() ? "superseded by entity write" | ||
| : format_detail("superseded by %s", op.op_id.c_str())); | ||
| } |
There was a problem hiding this comment.
Fixed in 2fd182a: the label now comes from op.origin, so an empty-op_id service call reads "superseded by service write" and only entity-originated writes read "superseded by entity write". Regression test: test_supersede_detail_uses_origin.
| } else { | ||
| finish_(seq, WriteStatus::REJECTED, | ||
| format_detail("pump still reports %s", control_.pump_enabled_ ? "running" : "stopped")); | ||
| } |
There was a problem hiding this comment.
Fixed in 2fd182a: the rejected path now updates mode, value, and enabled from the readback before finishing, so the event carries the pump's actual state; the original request survives in the requested_* echoes. Regression test: test_set_enabled_rejected_reports_readback.
| void AlphaHwrApiBridge::on_set_cycle_times(float on_minutes, float off_minutes, std::string op_id) { | ||
| // Minutes arrive as float only because int-typed service variables hit the | ||
| // ESP32-C3 linker bug; the operation layer validates the 1-60 range. | ||
| if (on_minutes < 0 || on_minutes > 255 || off_minutes < 0 || off_minutes > 255) { | ||
| reject_(WriteCommand::SET_CYCLE_TIMES, op_id, "cycle times must be 1-60 minutes"); | ||
| return; | ||
| } | ||
| component_->submit_set_cycle_times(static_cast<uint8_t>(on_minutes), | ||
| static_cast<uint8_t>(off_minutes), op_id); | ||
| } |
There was a problem hiding this comment.
Fixed in 2fd182a: fractional minutes now settle invalid at the bridge ("cycle times must be whole minutes, 1-60") instead of being silently truncated, and the whole-minute rule is documented in docs/programmatic-interface.md.
| - **Cycle-time write verification** — `set_cycle_times` gains the Object 91 | ||
| readback the legacy fire-and-forget setter never had (degrades gracefully to | ||
| accepted-with-detail on short-payload firmwares, #94). |
- Supersede detail labels the superseder by origin, not by empty op_id (a service call omitting op_id was mislabeled as an entity write) - Rejected run-state results carry the pump's readback state in enabled, matching the contract; the request stays in the requested_* echoes - pump_set_cycle_times settles invalid on fractional minutes instead of silently truncating (pump takes whole minutes) - CHANGELOG: describe set_cycle_times failure behavior accurately (rejected on unreadable precondition, timeout on failed readback); the #94 accepted-with-detail degrade was from the pre-#106 Sub 430 design
|
That's a big deal you landed all this in v0.12.0. I'll check it out over the next couple days. I'll have to stabilize my build over the next week before some extensive travel so my involvement in the project will have some pauses once I'm travelling. |
Fixes #92
Implements the write-and-verify interface from the #92 design discussion. The full public contract is documented in
docs/programmatic-interface.mdand was posted for review in this comment; PR #103 carries that document alone, and this branch includes it, so merging #103 first shrinks this diff to code.What this does
One write path. A new
services::WriteOperationServiceowns every pump write, entity- or service-originated. Operations queue and run strictly one at a time, build their wire frames from the caller's arguments (never from a possibly-stale cache), confirm themselves by reading the settled value back from the pump, and end in exactly one terminal result:accepted,clamped,rejected,timeout, orsuperseded. Validation rejects fire before any wire write, a per-operation watchdog converts stuck operations intotimeout, and a BLE disconnect terminal-events everything pending, so a client waiting on a result can never hang.Home Assistant surface. A new
api_bridge(CustomAPIDevice) registers five control services (pump_set_enabled,pump_set_modeon the unfused 86/sub-id-10 object from #98,pump_set_setpoint,pump_set_temperature_range,pump_set_cycle_times) and fires oneesphome.alpha_hwr_write_settledevent per write with the settled values and a caller-suppliedop_id. Requirescustom_services: trueandhomeassistant_services: trueonapi:; all packages and examples set both, and the bridge compiles out cleanly if a config omits them.Cycle-time fix (#106, added after the report). Cycle-time reads and writes targeted Object 91 Sub 430, which the GENI profile identifies as the temperature-range user settings; its trailing bytes are on/off-time LIMITS, invariant to the live configuration, which is exactly the byte-identical block #106 captured. The live values are Object 91 Sub 421 (
dhw_on_off_control_configuration_obj, type 985: flow setpoint + on/off periods), confirmed against GO-app captures showing the write frame and its persistence.set_cycle_timesnow fresh-reads Sub 421, writes with the captured frame shape echoing the stored flow setpoint verbatim, sends no configuration commit (capture-verified as unnecessary), and verifies via a Sub 421 readback. The Cycle Time entities are wired to the pump's real cached values, and — an adjacent consequence of the same misidentified struct — temperature-range writes now echo the pump's limit bytes instead of zeroing them.Contract-review round (jfriend00's #92 review, applied). Supersede keys for setpoint writes are now strictly per-mode: the pump stores an independent setpoint per mode, so a queued
constant_speedsetpoint is no longer dropped by a subsequentconstant_flowsetpoint (review catch; only same-mode pairs supersede). Newinvalidterminal status for malformed/out-of-range requests, decided pre-wire and deterministic;rejectednow strictly means the pump or its state refused. The settle event gainsorigin(service/entity, so an API call with an emptyop_idis still identifiable as an API call),seq(submission-order number; superseded events fire at submit time and can precede earlier operations' terminals), andrequested_*fields echoing the original arguments so the event is self-contained for logging and retries. Docs makepump_set_setpoint's mode-switching semantics explicit and note that clamping can reflect GO-app installer limits (pipe size / max flow). A Copilot review round is applied in 2fd182a: supersede details label the superseder by origin rather than by empty op_id, rejected run-state results carry the readback state, fractional cycle-time minutes settleinvalidinstead of truncating, and the CHANGELOG describes the cycle-time failure paths accurately.Unfused on/off (added after review input on the issue).
pump_set_enablednow sends the pump's dedicated Class 3 run-state commands (START0x06/ STOP0x05as SET) instead of the fused 0x0601 control object; the command ids and ACK shapes were bench-contributed by jfriend00 in the issue thread. On/off therefore carries no mode and no setpoint at all, which removes the last fused write from the on/off path and the mode pre-read that guarded it. A pump-rejected command (the[03 01]descriptor reply) settles the operationrejected; Class 3 produces no unsolicited notification, so the existing confirm readback stays the authoritative verdict. Service and event contract unchanged.Schedule services migrated and verified. The seven schedule services moved from
alpha_hwr_schedule_editor.yamlinto the bridge with unchanged names anddata-string formats plus the optionalop_id. They previously reported success unconditionally (the layer-write ACK callback firedtrueeven on timeout;set_schedule_enabledhad no completion signal and could blind-RMW hardcoded defaults over the overview). Now: overview precondition before any schedule write, a mandatory fresh layer read before entry writes so a stale layer image cannot clobber out-of-band edits of other days, post-commit verify readbacks, and slot auto-selection that reads the slots before choosing.Entity flip. The dashboard number/switch/select lambdas route through the operation layer with no YAML changes to the controls package; their bool callbacks now report the terminal result rather than fire-and-forget success.
ControlServicekeeps the pump-state cache, the #91 commanded-but-unconfirmed guards, and the wire primitives, and loses its legacy multi-step setters.Bench tooling.
tools/write_bench.pyis the aioesphomeapi client used for hardware verification: list services, snapshot states, call-and-wait on anop_id, and rapid bursts for the supersede path. It checks the one-terminal-event contract on every call.Behavior changes to be aware of
rejectedortimeout; those failures were already happening and are now visible. Malformeddatastrings settlerejectedinstead of failing silently.api:needs the two flags above for the services and event to exist.Verification
Host suite (
cd tests && make test): 385 tests green across 7 suites, including the newtests/test_write_operations.cpp(169 cases driving every terminal status against a pump simulator: supersede races, watchdog/readback finish races, the #45 abort path, an interleaved #54 poll, and the exact #92 setpoint-then-off collision).esphome compileclean for the ESP32-C3 target from the local component tree, which also confirms the string-argument services link on the RISC-V toolchain.Bench-verified end to end on real hardware (ESP32-C3 + Alpha HWR), 18 scenarios, every one settling with exactly one event:
origin=service,seq,requested_value=1500, settledvalue=1650all in the eventwarp_driveinvalidwith reason,seq=0, zero wire writesThe Class 3 row closes the one mode jfriend00 had not been able to check (his bench covered Constant Speed, both pressure modes, Constant Flow, and Cycle Time): Temperature Control starts and stops cleanly via Class 3, with the pump's mode and temperature configuration bit-for-bit unchanged across the round trip.
The 1500 ms schedule settle delay proved sufficient on real hardware (verify readbacks matched on the first attempt throughout). Compare the 1.7 s settle times against the ~7-11 s of fixed sleeps the old interface required.
Not yet exercised: a physical power-off mid-operation (the disconnect path is host-tested).
Implements the direction agreed in #92.