Documentation overhaul: correct what was wrong, document what shipped - #36
Merged
Conversation
added 7 commits
August 4, 2026 22:15
The porting guide specified CRC-16/MODBUS and listed "using CRC-16/CCITT" as mistake number one. It is exactly backwards: the pump uses CCITT with polynomial 0x1021, init 0xFFFF and a final XOR of 0xFFFF, over frame[1:-2]. A port built from that guide could not exchange a single valid frame. The vectors here are captured frames the pump accepted, so they are evidence rather than self-consistency. Also removed, each because following it breaks something: - The two-characteristic GATT model (0000fdd1 / 0000fdd2). Neither exists. There is one characteristic, 859cffd1-036e-432a-aa28-1a0085b87ba9, used for both writes and notifications. - Two-chunk MTU splitting. It is correct for every frame up to 40 bytes, so it passes the obvious tests, and silently truncates the 59-byte schedule write whose second "chunk" is 39 bytes. - The hardcoded configuration-commit packet. Byte 4 of its overview is the schedule's enabled flag, and it was 0x00 - so publishing it as a copyable constant published the bug that disabled a live schedule on every setpoint write. Read the overview, modify, write it back. - The 45 65 70 00 setpoint suffix, which is 3671.0, the pump's maximum speed. It appears verbatim in the pump's own limits block; it was never an inert placeholder. Send 7F FF FF FF to keep the stored value. - CLI commands and flags that do not exist: schedule show/set-entry/ export/import, config list-backups, control set-autoadapt*, monitor telemetry, and the --value/--setpoint/--layer/--format/--output flags attached to commands that take positionals. autoadapt_modes.md is rewritten: it had mode 5 as usable-with-a-warning and 13/14/15 as fully supported with invented SubIDs. Mode 5 now raises (the pump has no wire byte for it, and it used to fall through to Constant Speed and report success), and 13/14/15 switch but have no working setpoint path. 04_set_mode.md and 05_schedule_write.md are regenerated from the current code paths - every frame in them is emitted by the library. test_vectors.md carries a warning banner pending regeneration from executed code; it has more wrong assertions than right ones.
…ounds pyproject.toml credited this package to "Grundfos Firmware Team <firmware@grundfos.com>". It is an independent reverse-engineered client published to PyPI under a name that already reads as Grundfos's; the author line made that worse. Corrected, and README now carries the same not-affiliated notice docs/index.md already had. Remove CommandProcessor. It bypassed the services layer entirely, had no caller anywhere in the library or CLI, and was exercised only by a stub test. It was also the sole user of FrameBuilder.build_write_request, and therefore the only reason to believe writes use a CRC without the final XOR. With it gone the answer is unambiguous: there is one convention. Every captured frame the pump accepted - reads, writes, the handshake - needs the final XOR, and a test now asserts that against three of them. Unify the temperature-range bounds. The legacy setter validated 20-60 C while the write layer validated 20-70, so the same request was valid or invalid depending on which entry point a caller reached for. Both now read one pair of constants on ControlService, documented as a client-side guard rather than a measured pump limit - the pump would not advertise for a bench probe, so 60 vs 70 stays unverified and the wider bound wins because it is the one the shipping verified path already used. Also: - Add py.typed and ship it. Two type checkers run in CI and downstream consumers were getting nothing. - Export the 0.7.0 surface. `from alpha_hwr import WriteResult` failed; __all__ still described the pre-0.7 API. - Module docstrings for models.py and constants.py, which had none. - Strip ~480 lines of cross-language porting notes from src/. Nothing lints, type-checks or tests them, and they had rotted accordingly - transport.py and authentication.py carried the largest as bare module-level strings, invisible to every doc tool. Porting material belongs in docs/reimplementation/. - The copilot instructions forbade service-to-service dependencies and claimed Core depends on nothing. Both are false by design now (transport.py imports protocol.matcher; WriteOperationService takes a ControlService), so as written they told contributors to reject the current architecture. - TESTING_STRATEGY.md described a directory tree that was largely fiction: tests/e2e/, tests/fixtures/ and mock_bleak.py do not exist, while tests/unit/services/ and tests/reference/ went unlisted. Its "to add" list was almost entirely implemented. - Delete two committed .bak files.
Grepping every user-facing doc for WriteResult, WriteStatus, is_ready, wait_until_ready, single_event, vacation, RunState, set_setpoint or clamp returned zero hits. Everything documented was the pre-0.7 bool-returning path - the API that could not tell you the pump had clamped your value. Two new guides: - verified_writes.md. The acknowledgement is not the verdict: ask for 600 RPM and the pump acks, then stores 1650. Covers WriteResult, every WriteStatus, why clamping counts as success, the invalid-vs- rejected retry signal, why set_setpoint also switches the mode (the object is fused), the readiness gate, and the supersede rule - which is documented the way the test proves it, first write runs and the middles are superseded, not the way it reads at a glance. - run_state_and_schedules.md. The four run/schedule combinations and the fourth one that can never run while reporting no fault. Single events, slot capacity read from the pump rather than assumed, and vacations. The local-Unix timestamp rule gets its own warning: encoding those timestamps as real UTC round-trips byte-identically, so the write settles as accepted, a readback agrees with itself, and the event opens hours from where it was meant to. Threaded into the existing pages: the README's three broken CLI examples and its setpoint table (speed is 500-4500 not 1000-4500, pressure to 10.0 not 6.0, flow to 10.0 not 3.0), "automatic validation" replaced with what the pump really does, quick_start's control example moved onto the verified path with wait_until_ready first, control_modes' claim that flow setpoints are stored as m3/h (they are SI m3/s - this was the misconception behind issue #28), backup_restore's invented flags plus the warning that restore drives the unverified setters, and data_models' get_limits_display signature, which returns nested tuples rather than the formatted string it advertised. New API pages for the write layer, single events and run state, and the two broken mkdocstrings selectors in api/client.md are fixed - it listed `events` and `clock`, which are `event_log` and `time`, so autodoc had been silently dropping them. Also strips the last ~250 lines of TypeScript and Rust snippets from src/ docstrings, which the earlier pass missed because they sit inside method docstrings rather than under a Cross-Language heading.
The largest correction is the response model. Byte 5 of a *response* is a length field: the top two bits are always 00 and the low six are the payload length exactly, verified across 13 objects and 10 distinct values. The "OpSpec 0x30 for motor state, 0x2B for flow, 0x14 for temperature, 0x09 for alarms" vocabulary was reading payload sizes 48, 43, 20 and 9 as type codes - which is why filtering replies against that set filtered by length, and why the event log had to be exempted from the filter by hand. What a reply is actually matched on is the identifier pair at bytes 6-9, naming the object's type rather than the address requested. wire_format.md now carries that table, measured, along with the three rules that cost something to learn: the identifiers are not placed consistently, a zero in the first field is a real value rather than a wildcard, and an unmeasured object gets a class match instead of a guessed identifier. control.md: opens with three objects rather than one, since which you address decides whether you change the run state, the mode, or all three at once. The setpoint limits section said "3x uint16 (min, max, default)"; the payload is seven big-endian float32, and the captured Constant Speed block reads 2800/1650/3671/-3671/1/1/1. Fields 1 and 2 are not inferred from position - the pump clamps 600 to 1650 and 4400 to 3671, so the block and the behaviour agree exactly. That is also where 45 65 70 00 comes from. Removed read_setpoint_limits() and validate_setpoint(), which do not exist. The statistics read frame used the write opspec and encoded the object twice. schedules.md: the "One-Time Schedule (Vacation Mode)" section documented an object at 0x6702 with a 4D-prefixed payload and a set_one_time_schedule() API. None of it exists. Single events are Object 84 Sub 900+, type 220, and the section is replaced with the real structure - including that the action byte's sense is inverted relative to the weekly schedule's default_action, that slot capacity comes from the pump rather than the sub-id range, and the local-Unix timestamp rule that round-trips byte-identically when wrong. control_modes.md: cycle config is Object 91 Sub 421, not 430 - 430 is the temperature-range object, which this used to read and write by mistake. DHW suffix is 38 C6 76 EF. Mode bytes corrected (Constant Speed is 0x02, not 0x04). The contradiction with autoadapt_modes.md is resolved in favour of what the code does: 5 and 26 raise, 13/14/15 switch but have no setpoint path.
The audit found roughly forty code examples across docs/ that had never run. Every one of them looked plausible, which is why nobody noticed: `client.clock` reads as naturally as `client.time`, and `config list-backups` reads as naturally as a command that exists. Three checks, all in CI: 1. Every documented CLI invocation is compared against the Typer registry. Would have caught config list-backups, schedule show/set-entry/export/ import, monitor telemetry and four control set-autoadapt* commands. 2. Every documented flag is compared against that command's real parameters. Would have caught control set-mode --setpoint and control set-pressure --value (both take positionals), device info --format, monitor live --timeout, schedule clear --all, and six invented config flags. 3. Every `client.<attr>` is compared against the real client. Would have caught client.clock, client.events and client.get_telemetry(). Writing (3) surfaced a whole layer of drift nobody had flagged: the docs still called about twenty methods on the client that moved to services when it became a thin facade - client.get_schedule, client.read_alarms, client.set_constant_pressure and the rest. Those are now written against the services that own them. Where no equivalent exists at all (clear_all_schedules, export_schedule_json, set_schedule_entry) the examples are rewritten to do the thing the real API does: a schedule layer is written whole, so changing one day is read-edit-write. test_vectors.md is now generated by scripts/generate_test_vectors.py, which executes the codec, and a test fails if the committed page has drifted. That page had more wrong assertions than right ones - it specified CRC-16/MODBUS and decoded 0x46E5B000 as 14710.0 when it is 29400.0 - and nothing caught any of it because nothing ran it. Writing the generator caught one more: a Class 3 STOP frame I had transcribed rather than computed, whose CRC was wrong. mkdocs build --strict already covers internal links.
The guide was written as "the Python implementation is the reference". That has not been true for a while: the ESPHome C++ port has far more bench time, and several corrections in this documentation set originated there. It now presents the measured protocol with two implementations to check against, and says plainly where to place trust - the generated test vectors and the captured frames, over the prose, which has been wrong before. Corrections, each of which produced broken code: - The four authentication packets in layer_by_layer.md were fabricated. They contradicted every other file in this set, and none of them works. Replaced with the captured constants, the APDUs they are built from, and the inter-stage timings the pump actually needs. - The SET OpSpec formula omitted the four identifier bytes, so it produced 0x84 where the pump sees 0x90. The worked example also sent a bare 4-byte float through the fused control object, whose payload is twelve bytes carrying a run state and a mode. The frame in it is now generated and asserted byte for byte. - Schedule times were documented as minutes-since-midnight uint16. They are plain hour and minute bytes, and the pump accepts the wrong encoding without complaint. - Obj 0x0600 does not exist; stop is Class 3. - Byte 5 of a response is a length, not a type code - the last of the opspec-as-type vocabulary is gone from this set. The checklist now covers what a port has to get right that is not obvious from the wire: three separate control objects rather than one, reading state from Sub 7 rather than Sub 6, never sending 45 65 70 00, building the commit from the pump's current overview, N-chunk splitting, the stalled run state, and the local-Unix single-event timestamps that round-trip byte-identically when wrong. A new test extracts every complete hex frame from docs/ and verifies its CRC - 65 of them. Writing it found two more errors nobody had noticed: a Class 2 INFO frame in ble_architecture.md with a wrong CRC, and an alarms-response example that was constructed by hand and presented as though captured. The first is corrected; the second is now labelled, since a pump with two simultaneous active alarms was never recorded.
No 0.7.0 release was declared - the newest tag is v0.5.0 - so the version heading was asserting something that had not happened. The section's contents move under [Unreleased], and the three prose references to 0.7.0 and "0.6 and earlier" are reworded to describe the change rather than a version number. The temperature bound is no longer a guess. Probed on hardware: the pump stored every range offered, including -10 to 0 and 100 to 120 C. It does not clamp this object and does not reject anything, which is the opposite of the setpoint objects. Two consequences now documented: - The client's 20-70 C guard is the only guard that exists. It is a judgement about what a hot-water system can mean, not a mirror of firmware behaviour, and a port that omits its own validation has none. - A `clamped` result is impossible for this object, so `accepted` here really does mean the pump holds what was asked for. Setpoint clamping re-confirmed on the same unit while the pump was up: 600 RPM stored 1650, 4400 stored 3671, 2000 accepted - identical to the original measurement and to the pump's own limits block. The schedule was still enabled afterwards, which is the configuration-commit fix holding. Pump restored to its original state (mode 27, 35.0/38.9, autoadapt on, scheduled).
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
tests/test_docs_consistency.py currently excludes the real config CLI group from validation, weakening the new CI drift guard for documented alpha-hwr config … examples.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
Updates the repository’s user-facing documentation and guardrails to match the post-parity protocol and API behavior (CRC convention, single-characteristic GATT model, verified writes/write statuses, run-state vs schedule interactions), while removing obsolete/unused code paths that previously contradicted measured hardware behavior.
Changes:
- Overhauls protocol + CLI/API docs to reflect measured wire format, correct CRC usage, and the verified-write/readback semantics.
- Adds CI tests that validate documented CLI commands/flags,
client.<attr>references, doc code-block syntax, generated pages, and CRCs of documented frames. - Removes unused
CommandProcessor/write-request paths, exports write/run-state types from the public package surface, and shipspy.typed.
File summaries
| File | Description |
|---|---|
| tests/unit/protocol/test_frame_parser.py.bak | Removed stale backup test file. |
| tests/unit/protocol/test_frame_builder.py | Removes tests for removed write-request builder and legacy CRC assumptions. |
| tests/test_wire_protocol.py | Replaces write-request encoding checks with captured-frame CRC convention assertion. |
| tests/test_protocol_expanded.py | Removes write-request builder test now that the API path is removed. |
| tests/test_docs_consistency.py | Adds structural doc drift checks (CLI registry, flags, client attrs, CRCs, generated pages). |
| tests/test_command_processor.py | Removes tests for deleted CommandProcessor. |
| tests/test_cli_mode_old.py.bak | Removed stale backup test file. |
| tests/reference/test_protocol_vectors.py | Removes write-request structure test tied to deleted builder. |
| src/alpha_hwr/utils.py | Clarifies CRC helpers: calc_crc16_read is the sole wire convention; calc_crc16 is a building block only. |
| src/alpha_hwr/services/write_operation.py | Uses ControlService temperature-range bounds for consistent validation + clearer error detail. |
| src/alpha_hwr/services/telemetry.py | Removes cross-language notes from in-code docstrings. |
| src/alpha_hwr/services/schedule.py | Removes cross-language notes from in-code docstrings. |
| src/alpha_hwr/services/device_info.py | Removes cross-language notes from in-code docstrings. |
| src/alpha_hwr/services/control.py | Adds temperature-range bounds constants; aligns validation + messaging to 20–70 °C guard. |
| src/alpha_hwr/services/configuration.py | Removes cross-language notes from in-code docstrings. |
| src/alpha_hwr/py.typed | Adds marker file to ship typing info to downstream users. |
| src/alpha_hwr/protocol/telemetry_decoder.py | Refocuses docstring away from “other languages” notes toward decoding order/intent. |
| src/alpha_hwr/protocol/frame_parser.py | Refocuses docstring away from “other languages” notes toward parsing order/intent. |
| src/alpha_hwr/protocol/frame_builder.py | Removes write-request builder and documents single CRC convention for all frames. |
| src/alpha_hwr/protocol/codec.py | Removes embedded reference implementations; keeps concise cross-language guidance. |
| src/alpha_hwr/models.py | Adds top-level module docstring describing decoded-state models vs write-result models. |
| src/alpha_hwr/core/transport.py | Removes large embedded cross-language reference notes from module tail. |
| src/alpha_hwr/core/authentication.py | Removes embedded cross-language notes and redundant/incorrect CRC discussion. |
| src/alpha_hwr/constants.py | Adds module docstring clarifying constants are measured/captured. |
| src/alpha_hwr/command_processor.py | Deletes unused services-bypassing command processor implementation. |
| src/alpha_hwr/client.py | Updates docstring to point porting guidance to docs/reimplementation/. |
| src/alpha_hwr/init.py | Exports write/run-state/single-event types and additional services on the public surface. |
| README.md | Updates disclaimers and examples to reflect verified writes, readiness gate, and current CLI syntax. |
| pyproject.toml | Fixes authorship metadata and ships py.typed via setuptools package-data. |
| mkdocs.yml | Adds new guides to navigation and includes alarms/warnings packet trace. |
| docs/TESTING_STRATEGY.md | Corrects test layout description; documents bench-testing approach and current mock capabilities. |
| docs/reimplementation/README.md | Rewrites reimplementation guide overview; corrects CRC/GATT model; emphasizes generated vectors + measured frames. |
| docs/reimplementation/checklist.md | Corrects protocol checklist (single characteristic, CRC convention, reply matching rules, schedule write chunking). |
| docs/reimplementation/architecture.md | Updates architecture notes to include verified write layer and corrects auth/ack assumptions. |
| docs/reference/data_models.md | Updates model docs and adds write-result + run-state documentation. |
| docs/protocol/wire_format.md | Corrects active-query response layout (byte 5 length, identifier pairing) and matching rules. |
| docs/protocol/telemetry.md | Updates telemetry header layout and clarifies alarms payload semantics vs byte-5 meaning. |
| docs/protocol/schedules.md | Replaces non-existent “one-time schedule” object with single-events; adds timestamp/run-state interactions. |
| docs/protocol/packet_traces/06_alarms_warnings.md | Corrects response layout and byte-5 meaning; labels constructed examples explicitly. |
| docs/protocol/packet_traces/05_schedule_write.md | Rewrites schedule write trace: 59-byte frame, 3-chunk splitting, commit semantics. |
| docs/protocol/packet_traces/04_set_mode.md | Rewrites control mode/setpoint trace: distinct objects, commit only where relevant, readback requirement. |
| docs/protocol/packet_traces/03_telemetry_stream.md | Updates response explanation to identifier-pair matching and byte-5 semantics. |
| docs/protocol/packet_traces/02_authentication.md | Updates examples for single characteristic UUID and write_gatt_char usage. |
| docs/protocol/packet_traces/01_connection.md | Corrects GATT model to single characteristic; adds bonding behavior notes. |
| docs/protocol/device_info.md | Updates examples to current service API (client.device_info.*). |
| docs/protocol/control_modes.md | Corrects mode/register mapping, support status, units, and documents verified vs primitive setters. |
| docs/protocol/ble_architecture.md | Fixes CRC example and updates schedule API usage in examples. |
| docs/protocol/bench_findings.md | Adds new measured findings: temp-range object validation behavior + reconfirmed clamping. |
| docs/protocol/autoadapt_modes.md | Corrects AutoAdapt support: mode 5/26 raise; 13/14/15 mode-only; points to Mode 27 for HWR. |
| docs/guides/verified_writes.md | New guide explaining write statuses, clamping semantics, readiness, serialization, supersede behavior. |
| docs/guides/understanding_telemetry.md | Updates examples to current telemetry/service APIs. |
| docs/guides/time_management.md | Fixes service handle name (client.time vs client.clock). |
| docs/guides/run_state_and_schedules.md | New guide documenting run-state/schedule interaction, single events/vacations, and timestamp encoding caveat. |
| docs/guides/control_modes.md | Updates ranges/notes and adds clearer unsupported/deprecated mode behavior. |
| docs/guides/cli_guide.md | Corrects CLI commands/flags to match actual Typer registry and current behavior. |
| docs/guides/backup_restore.md | Updates CLI/API restore/backup semantics and warns about unverified restore behavior. |
| docs/getting_started/quick_start.md | Updates examples to use readiness + verified setpoint writes and current service APIs. |
| docs/api/services.md | Updates service handles and adds SingleEvent/WriteOperation/RunState sections. |
| docs/api/models.md | Adds API doc sections for write-result models. |
| docs/api/client.md | Updates client API listing to current handles and readiness/run-state methods. |
| CHANGELOG.md | Removes nonexistent 0.7.0 heading; keeps changes under Unreleased. |
| .github/instructions/python.instructions.md | Fixes architecture/dependency guidance to match current layered design + permitted service composition. |
| .github/copilot-instructions.md | Updates dependency rules to allow deliberate service composition and correct layer dependencies. |
Review details
- Files reviewed: 67/68 changed files
- Comments generated: 1
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Comment on lines
+31
to
+32
| #: Words that follow "alpha-hwr" in prose without being commands. | ||
| _NOT_COMMANDS = {"pip", "backup", "restore", "library", "config"} |
Copilot found a real defect in the new drift guard, and it is the kind this whole PR is about: `_NOT_COMMANDS` listed `config`, so every documented `alpha-hwr config ...` example was silently skipped - including the backup and restore ones this audit had just rewritten. A guard that quietly covers less than it claims is worse than no guard. `config` was excused early, for a false positive that had nothing to do with it: the invocation regex used `\s+`, which spans newlines, so a code block ending in `alpha-hwr` followed by `pip install` on the next line parsed as a command. Matching within a single line removes that class of false positive entirely, and with it the reason to excuse `pip`, `backup` and `restore` as well. Coverage goes from 135 invocations to 153, with all eight real groups now checked. Two tests keep it that way: one asserts `_NOT_COMMANDS` never intersects the real CLI registry, the other pins the specific regression by name. Verified by re-adding `config` and watching both fail. CI also caught two things I had not run locally: - `ruff format --check .` covers the whole repo, and ruff 0.16 formats Python inside Markdown fences. Ten docs needed reformatting. One block in TESTING_STRATEGY.md came out worse than it went in - an illustrative excerpt exploded across six lines - so that fence is now `text`, which is what it always was. - The type-check job runs `basedpyright`, not just mypy. `get_command()` returns a `Command`, and `.commands` exists only on a group, so the registry helper now narrows to `TyperGroup` with an assertion that says what the guard assumes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The parity work changed which GENI objects the library writes, in which units, what a write returns, and when a write is allowed at all. The documentation did not move with it. An audit of all 12,812 lines of
docs/, plus the README, repo meta-files and in-code docstrings, found three separate problems — and fixing them turned up several more.1. Some documentation was actively harmful
Not stale — it instructed readers to do things now known to be wrong.
docs/reimplementation/specified CRC-16/MODBUS and listed "using CRC-16/CCITT" as mistake feat: Add device MAC address configuration storage #1. Exactly backwards. A port built from that guide could not exchange a single valid frame with the pump. It is CCITT0x1021, init0xFFFF, final XOR0xFFFF, overframe[1:-2]— verified against captured frames the pump accepted.0000fdd1/0000fdd2) across ~200 lines. Neither exists; there is one characteristic,859cffd1-…, for both directions. An implementation looking for the second fails at service discovery.clock_program_enabled = 0x00— the exact bug that disabled a live schedule on every setpoint write.45 65 70 00described as an inert "suffix". It is 3671.0, the pump's maximum speed, and appears verbatim in the pump's own limits block.layer_by_layer.md, contradicting every other file in the set.schedule show/set-entry/export/import,config list-backups,monitor telemetry, fourcontrol set-autoadapt*, and--value/--setpoint/--layer/--formaton commands that take positionals.autoadapt_modes.md,packet_traces/04_set_mode.mdand05_schedule_write.mdwere rewritten from the current code paths rather than patched.2. The shipped API was undocumented
Grepping every user-facing doc for
WriteResult,WriteStatus,is_ready,wait_until_ready,single_event,vacation,RunState,set_setpointorclampreturned zero hits. Everything documented was the pre-parity bool-returning path — the API that could not tell you the pump had clamped your value.Two new guides:
verified_writes.md— the acknowledgement is not the verdict. Ask for 600 RPM and the pump acks, then stores 1650. Covers everyWriteStatus, why clamping counts as success, theinvalid-vs-rejectedretry signal, the readiness gate, and the supersede rule.run_state_and_schedules.md— the four run/schedule combinations and the fourth that can never run while reporting no fault. Single events, vacations, and the local-Unix timestamp rule that round-trips byte-identically when wrong.3. Protocol docs disagreed with what was measured
0x30/0x2B/0x14/0x09"type codes" are payload sizes 48/43/20/9 — which is why filtering replies against that set filtered by length, and why the event log had to be exempted by hand. Replaced with the measured identifier table frommatcher.py.0x6702, a4D-prefixed payload, aset_one_time_schedule()API). Replaced with Object 84 Sub 900+.New: hardware measurements
Probed on a real pump while preparing this:
Pump restored to its original state afterwards.
Guards, so this cannot silently rot again
The audit found ~40 code examples that had never run. Every one looked plausible —
client.clockreads as naturally asclient.time. Five checks now run in CI:client.<attr>vs. the real client.docs/— 65 of them.test_vectors.mdis generated byscripts/generate_test_vectors.py, which executes the codec; a test fails if the committed page has drifted.Writing these found drift nobody had flagged: ~20 client methods documented at names that moved to services when the client became a thin facade; a wrong CRC in
ble_architecture.md; a Class 3 STOP frame I had transcribed rather than computed; and an alarms example constructed by hand and presented as captured — now labelled, since a pump with two simultaneous active alarms was never recorded.Code and repo changes
pyproject.tomlcredited this package to "Grundfos Firmware Team firmware@grundfos.com". It is an independent reverse-engineered client on PyPI under a name that already reads as Grundfos's. Corrected, and the README now carries the not-affiliated noticedocs/index.mdalready had.CommandProcessorremoved. It bypassed the services layer, had no caller anywhere, and was exercised only by a stub test. It was also the sole user ofbuild_write_requestand therefore the only reason to believe writes use a CRC without the final XOR. With it gone the answer is unambiguous: there is one convention, and a test now asserts that against three captured frames.py.typedadded and shipped. Two type checkers run in CI; downstream consumers were getting nothing.from alpha_hwr import WriteResultused to fail.src/. Nothing lints, type-checks or tests them, and they had rotted accordingly —transport.pyandauthentication.pycarried the largest as bare module-level strings, invisible to every doc tool.TESTING_STRATEGY.mddescribed a directory tree that was largely fiction —tests/e2e/,tests/fixtures/andmock_bleak.pydo not exist, whiletests/unit/services/andtests/reference/went unlisted.Changelog
The
[0.7.0]heading asserted a release that was never declared — the newest tag isv0.5.0. Its contents move under[Unreleased], and the prose references to version numbers are reworded to describe the change instead.Verification
mkdocs build --strictcleanruff checkandmypyclean