docs: add rtl_433 implementation spec + skill for humans and AI agents

.claude/skills/rtl433-uponor-clean1/SKILL.md

@@ -0,0 +1,104 @@
+---
+name: rtl433-uponor-clean1
+description: Use when implementing, extending, reviewing, or debugging an rtl_433 decoder (or any TX/RX implementation) for the Uponor Clean 1's 868.2 MHz radio link — covers where the protocol facts live, the reference decoder, capture/test-vector workflow, and the review process this project uses. Trigger on requests to add/fix a field in the decoder, build a TX proof-of-concept, capture a new alarm type, or port the protocol to another language/platform.
+---
+
+# Implementing rtl_433 support for the Uponor Clean 1
+
+This project reverse-engineered the radio link from firmware, not by guessing
+at captures — every protocol fact is traceable to a disassembled CPU function
+or a specific live capture. Preserve that: don't add a field or behavior you
+can't cite, and don't remove a caveat because it's inconvenient.
+
+## Start here, always
+
+Read **[`docs/rtl433-implementation-spec.md`](../../docs/rtl433-implementation-spec.md)**
+first. It's self-contained — physical layer, frame format, byte-exact payload
+map, test vectors, and the one recipe for the still-uncaptured `device_fault`
+alarm type. Don't re-derive the protocol from the narrative research docs
+(`docs/ghidra/`, `docs/radio-capture-log.md`, etc.) — those are provenance for
+the spec, not a second source of truth. If you find the spec and a research
+doc disagree, that's a bug in one of them — flag it, don't silently pick one.
+
+## Where the code lives
+
+The decoder is **not in this repo**. It's in a fork:
+[andy778/rtl_433, branch `add-uponor-clean1`](https://github.com/andy778/rtl_433/blob/add-uponor-clean1/src/devices/uponor_clean1.c),
+a fork of upstream [merbanan/rtl_433](https://github.com/merbanan/rtl_433) —
+not yet upstreamed (spec §9 covers why: one alarm type still unconfirmed on
+air). Clone/work in that repo for decoder changes; this repo (`UClean1`) is
+where the protocol research and spec live.
+
+## Workflow for a decoder change
+
+1. **Branch off `add-uponor-clean1`**, not `master` — that branch is the
+   accumulated, CONTRIBUTING.md-compliant decoder state.
+2. **Cross-check against the spec** before writing code — most bugs in this
+   decoder's history came from not doing this (e.g. an early version reported
+   the constant frame address as `id` instead of the per-device `node_src`;
+   see spec §5).
+3. **Build and verify against real captures**, not just a clean compile:
+   ```bash
+   cd build && make -j4 rtl_433
+   ./src/rtl_433 -r <capture>.cu8 -Y minmax -F json | grep -i uponor
+   ```
+   Use the test vectors in spec §6 if you don't have a live capture — replay
+   them through the CRC/parsing logic directly, or construct a synthetic
+   `.cu8` if you need to exercise the demodulator too.
+4. **`clang-format -i`** before committing — this decoder targets upstream
+   merbanan/rtl_433 conventions (see that repo's `docs/CONTRIBUTING.md`),
+   not this project's own style.
+5. **Get an independent review pass** before opening the PR. This project's
+   practice: dispatch a second review with a *different* model than the one
+   that wrote the change — one pass for correctness (bounds, buffer safety,
+   state leaks across the decode loop), one for upstream convention
+   compliance (field naming, `output_fields[]` matching `data_make()`, return
+   value idiom). Both passes should read the code directly and cite line
+   numbers, not speculate.
+6. **Open the PR against `add-uponor-clean1`** (not upstream — see "Where the
+   code lives"), with a test plan showing what was actually run, not just
+   "should work."
+7. If the change affects protocol understanding (new field, corrected
+   semantics, a newly confirmed value), **update the spec in this repo in the
+   same work session** — a decoder fix that isn't reflected in the spec will
+   just get re-broken by the next person who trusts the spec.
+
+## Common pitfalls (all previously hit in this project)
+
+- **`id` must be `node_src`, never the frame address.** The address
+  (`EA EA EA EA`) is a firmware-hardcoded constant, identical on every unit
+  ever shipped — using it as `id` collides all devices into one.
+- **Stale tail bytes (`payload[11+N..31]`) are not data.** They vary
+  run-to-run for identical logical messages. Never decode them, never use
+  them as a freshness/nonce signal.
+- **Validate `payload[0] == 12 + N`, not just the CRC.** CRC passing only
+  proves the bytes weren't corrupted in transit — it says nothing about
+  whether your address-sync landed on the right offset.
+- **Don't implement software Manchester encoding for TX.** The on-air
+  Manchester coding is the nRF905 hardware's own behavior; the firmware
+  writes plain bytes. A TX implementation using a real nRF905 chip and the
+  register bytes in spec §2 should reproduce the correct on-air waveform
+  automatically.
+- **A `bitbuffer_invert()` call must be undone on every exit path**, not just
+  the success path, if you're working in the rtl_433 decode loop — an
+  inverted buffer leaks into the next row's parse otherwise. Trace every
+  `continue`/`return` in the loop when touching this logic.
+
+## If you're building something other than the rtl_433 decoder
+
+The spec (§1–§6) is decoder-agnostic — same frame format and test vectors
+apply whether you're writing a Python/GNU Radio flowgraph, an ESP32 TX using
+a real nRF905 module, or a from-scratch RX. Spec §9 flags what's genuinely
+unverified (no third-party TX has ever been tried against the real panel) —
+if you get a TX proof-of-concept working, that result is worth documenting
+back into this repo's `docs/`, since it upgrades an assumption to a
+confirmed fact for everyone after you.
+
+## Definition of done for a decoder change
+
+- [ ] Builds clean, `clang-format` applied
+- [ ] Verified against a real capture or the spec's test vectors, not just
+      "compiles"
+- [ ] Reviewed by a second pass (correctness + convention), findings resolved
+- [ ] PR opened against `add-uponor-clean1` with an honest test plan
+- [ ] Spec updated in this repo if protocol understanding changed

README.md

@@ -56,6 +56,7 @@ accepts it).
 | --- | --- |
 | [docs/u2-serial-protocol.md](docs/u2-serial-protocol.md) | Serial/GSM telemetry — the working Home Assistant route |
 | [docs/rtl433-decoder.md](docs/rtl433-decoder.md) | Radio frame spec, capture recipes, rtl_433 decoder |
+| [docs/rtl433-implementation-spec.md](docs/rtl433-implementation-spec.md) | Self-contained implementation spec + test vectors — start here to build a decoder or TX in any language |
 | [docs/ghidra/README.md](docs/ghidra/README.md) | Payload byte map, who writes/sends it |
 | [docs/eeprom-map.md](docs/eeprom-map.md) | EEPROM layout, alarm/E-code table, actuator + sensor firmware traces |
 | [docs/ghidra/codes.md](docs/ghidra/codes.md) | The 3 separate numbering spaces (E-code/S-code/radio type), kept apart |

docs/rtl433-decoder.md

@@ -8,6 +8,12 @@ Decoder lives in the fork, not this repo:
 It is **enabled**, CRC-gated, and tested end-to-end against a real capture
 (zero false positives on an unrelated 433 MHz capture).
 
+Building a decoder, a TX implementation, or extending this one? Start with
+**[docs/rtl433-implementation-spec.md](rtl433-implementation-spec.md)** — a
+self-contained spec with test vectors, meant to be implementable without
+reading the rest of this page first. This page is the narrative/provenance;
+that one is the reference.
+
 ## Capture
 
 ```bash

docs/rtl433-implementation-spec.md

@@ -0,0 +1,207 @@
+# Uponor Clean 1 radio protocol — implementation spec
+
+A self-contained spec for implementing a decoder for this link, in any
+language or framework. Written so a human or an AI agent can build a correct
+decoder from this document alone, without first reading the reverse-engineering
+narrative in the rest of `docs/`. Every fact here is either firmware-confirmed
+(from disassembling the CPU flash) or confirmed live on air — provenance
+pointers are given, not repeated, so this file doesn't drift out of sync with
+the research docs. If this spec and another doc ever disagree, the research
+doc with the firmware/capture citation wins; file an issue.
+
+## 1. Physical layer
+
+| Parameter | Value | Source |
+| --- | --- | --- |
+| Carrier | 868.2 MHz (channel 117) | [`rtl433-decoder.md`](rtl433-decoder.md) — `CH_NO`/`HFREQ_PLL` config bytes, matches measured carrier |
+| Chip | Nordic nRF9E5 (nRF905 radio + 8051 MCU), on U1 | [`ghidra/README.md`](ghidra/README.md) |
+| Line rate | 100 kbps | firmware config (`W_RF_CONFIG`) |
+| Line coding | Manchester on air | **PHY effect only** — neither the 8051 nor the MC9S08 firmware does Manchester in software. Do not implement software Manchester encoding on the TX side; the nRF905 hardware does it. |
+| Modulation | FSK (nRF905 ShockBurst) | nRF9E5 datasheet |
+
+## 2. Frame format
+
+```
+[preamble] [address: EA EA EA EA, 4 bytes] [payload: 32 bytes] [CRC-16: 2 bytes]
+```
+
+- **Address**: constant `EA EA EA EA` on every unit — it's a literal in the
+  8051 firmware image, not a per-unit value. Use it to anchor/sync on the
+  frame, never as a device identifier (see §5).
+- **CRC-16**: CCITT-FALSE — poly `0x1021`, init `0xFFFF`, no reflection, no
+  xorout, big-endian trailer, computed over `address + payload` (36 bytes).
+  This is the nRF905's own hardware CRC (`CRC_MODE=1`, `CRC_EN=1` in the
+  radio config), not a firmware computation. **Gate every decode on this —
+  reject anything that doesn't pass.**
+
+### nRF905 register configuration (for a TX implementation)
+
+Confirmed from the firmware's own `W_RF_CONFIG` write (not just inferred from
+a capture):
+
+```
+CR0..CR9 = 75 06 44 20 20 EA EA EA EA D4
+```
+
+| Byte | Value | Meaning |
+| --- | --- | --- |
+| CR0 | `0x75` | `CH_NO` low 8 bits |
+| CR1 | `0x06` | `CH_NO` bit 8 + `HFREQ_PLL=1` + `PA_PWR=01` (−2 dBm) |
+| CR2 | `0x44` | `RX_AFW=4`, `TX_AFW=4` (4-byte address width) |
+| CR3 | `0x20` | `RX_PW=32` |
+| CR4 | `0x20` | `TX_PW=32` |
+| CR5–CR8 | `EA EA EA EA` | `RX_ADDRESS` |
+| CR9 | `0xD4` | `CRC_MODE=1`, `CRC_EN=1`, `XOF=010` (16 MHz crystal) |
+
+A separate `W_TX_ADDRESS` SPI write repeats `EA EA EA EA` as `TX_ADDRESS`.
+Full trace: [`ghidra/register-map.md`](ghidra/register-map.md).
+
+## 3. Payload structure (32 bytes)
+
+```
+offset  len  field         meaning
+0       1    hdr_len       12 + N (N = body length below) — restates the frame length
+1..2    2    tag           constant CC 6E
+3       1    dir           0x40 = poll (outer unit -> panel), 0x80 = response (panel ACK)
+4..6    3    node_src      sender's node ID
+7..9    3    node_dst      recipient's node ID (echoed from the other side's node_src)
+10      1    N             message-body length (0, 1, or 2 in all observed traffic)
+11..10+N     body          see §4
+11+N..31     stale         leftover TX buffer contents, NOT data — see below
+```
+
+**Validation**: `payload[0] == 12 + N` must hold. A frame that passes CRC but
+fails this check is misaligned or corrupt — reject it, don't just trust CRC
+alone (CRC passing only proves the 36 bytes weren't corrupted in transit, not
+that your address/length parsing landed correctly).
+
+**Stale tail bytes are not data.** `U2` (the MC9S08) fills only `11+N` bytes
+of the 32-byte payload buffer and radios the rest of whatever was already
+sitting in that buffer from a previous message. Two consequences for an
+implementer: (a) never try to interpret bytes past `11+N`, even though they
+CRC-validate and look structured; (b) two frames with identical logical
+content will NOT have byte-identical payloads (only tag/dir/node/N/body match)
+because the stale tail varies run-to-run.
+
+### Node IDs
+
+3-byte values, stable per device, observed identical across captures days
+apart:
+
+| Device | node_src (when sending) |
+| --- | --- |
+| Outer (tank) unit | `80 0D 6E` |
+| Indoor panel | `C0 23 4B` |
+
+These are the correct per-device identifiers for a decoder's `id` field —
+**not** the constant frame address (`EA EA EA EA`), which is identical on
+every unit that has ever shipped and is useless for distinguishing devices.
+
+## 4. Message body (`payload[11 .. 10+N]`)
+
+| N | Body | Meaning |
+| --- | --- | --- |
+| 0 | *(none)* | ACK — used on `response` frames |
+| 1 | `24` | heartbeat (not an alarm; `0x24` is a poll opcode) |
+| 2 | `[type, state]` | alarm/status. `state`: `0`=clear, `1`=active |
+
+### Alarm/status `type` byte (5 values, = the 5 physical panel symbols)
+
+| Type | Name | On-air confirmation |
+| --- | --- | --- |
+| `0x20` | status / OK (green LED) | confirmed, `state=0` |
+| `0x21` | chemical_low | confirmed, `state=0` |
+| `0x22` | high_water | confirmed, `state=0` |
+| `0x23` | sludge_reminder | confirmed, `state=0` |
+| `0x26` | device_fault | **firmware-confirmed only — never captured on air.** See §7 for how to force it. |
+
+No `state=1` (active-fault) frame of any type has ever been captured. Treat
+the `state` field's exact bit semantics as firmware-derived, not
+empirically verified, until §7's recipe is run.
+
+Cross-reference to the EEPROM/display E-code numbering (a related but
+*different* numbering space — do not conflate) is in
+[`eeprom-map.md`](eeprom-map.md) and [`ghidra/codes.md`](ghidra/codes.md).
+
+## 5. What a correct `id` field looks like
+
+This is the one place a naive implementation gets it wrong (an earlier
+version of the reference decoder did too — see the git history of
+`uponor_clean1.c`). The constant frame address collides every unit on the
+product line into one identifier. Use `node_src` for `id`; emit the constant
+address as a separate field if you want it for debugging/framing purposes.
+
+## 6. Test vectors
+
+Real captured frames (boot-burst capture, 2026-07-06, power-cycling the outer
+unit — full context in [`radio-capture-log.md`](rtl433-decoder.md)). Bytes
+below are `address + payload`, i.e. what a decoder sees after address-sync,
+before the CRC trailer:
+
+```
+heartbeat:  eaeaeaea 0dcc6e4f800d6ec0234b0124b9c635bd3ff2f4b8abdc5effd177bff72fff6fef
+status:     eaeaeaea 0ecc6e4f800d6ec0234b0220006566bd3ff2f4baabde5fbfd17fbff72fff6fef
+chem_low:   eaeaeaea 0ecc6e4f800d6ec0234b0221005657bd3ff2f4baabde5fbfd17fbff72fff6fef
+high_water: eaeaeaea 0ecc6e4f800d6ec0234b0222000304bd3ff2f4baabde5fbfd17fbff72fff6fef
+```
+
+Decode of the `status` frame, byte by byte:
+
+```
+0e cc 6e 4f 80 0d 6e c0 23 4b 02 20 00 65 66 bd ...
+|  \___/  |  \______/  \______/  |  \___/  \_______________________/
+|    |    |     |         |      |    |             `-- stale, not data
+|    |    |     |         |      |    `-- body: type=0x20, state=0x00
+|    |    |     |         |      `-- N=2
+|    |    |     |         `-- node_dst = C0 23 4B (panel)
+|    |    |     `-- node_src = 80 0D 6E (outer unit)
+|    |    `-- dir=0x40 -> poll
+|    `-- tag CC 6E
+`-- hdr_len = 12+N = 0x0e
+```
+
+Expected decode for all four: `id` = the frame's `node_src` hex
+(`800d6e` for all four above, since the outer unit sent all of them),
+`msg_type`/`msg_name` per §4, CRC valid.
+
+A reference implementation's exact output shape (JSON field names) is in the
+rtl_433 decoder itself — see §8.
+
+## 7. Recipe: capturing `device_fault` (`0x26`) and a `state=1` frame
+
+Never captured live (§4). Rather than waiting for a real failure, force it —
+the actuator self-test (`FUN_a138`/`FUN_c9e1` in the firmware, see
+[`eeprom-map.md`](eeprom-map.md#how-the-firmware-addresses-the-7-output-channels--traced-from-fun_a138))
+flags any output channel whose readback doesn't clear:
+
+1. Power off the outer unit.
+2. Disconnect **one** actuator from the output row's screw terminals (e.g.
+   `MV5`, aeration — see [`README.md`](../README.md) for the terminal map).
+3. Start capture first: `rtl_433 -R 321 -f 868.2M -Y minmax -F json:fault.json`
+4. Power on — the self-test runs at startup and should broadcast
+   `msg_type=0x26`, `msg_state=1`.
+5. Reconnect, power-cycle again, confirm a `state=0` clear frame.
+
+Full recipe with rationale: [`rtl433-decoder.md`](rtl433-decoder.md).
+
+## 8. Reference implementation
+
+A C decoder for [rtl_433](https://github.com/merbanan/rtl_433) exists:
+[andy778/rtl_433, branch `add-uponor-clean1`](https://github.com/andy778/rtl_433/blob/add-uponor-clean1/src/devices/uponor_clean1.c).
+It implements every rule in this spec (address-sync via `bitbuffer_search`,
+CRC gate, the `hdr_len == 12+N` validation, `id` = `node_src`) and is the
+easiest way to see a complete, working parser. Use `FSK_PULSE_MANCHESTER_ZEROBIT`
+at 10 µs bit time if implementing against rtl_433's demodulator; see the
+decoder's own file header for the flex-decoder equivalent (no C code needed)
+and capture recipes.
+
+## 9. Known gaps (don't treat as spec — these are open questions, not rules)
+
+- The RF service/handheld command protocol (a superset beyond §3–4) exists
+  but has never been captured — see [`ghidra/codes.md`](ghidra/codes.md) §4.
+- `msg_state=1` has never been observed (§4, §7).
+- No TX has ever been attempted from a third-party implementation against
+  the real panel — the config in §2 is correct per the firmware's own write,
+  but "the panel accepts a frame built by someone else's TX chain" is
+  unverified. If you build a TX implementation, that first successful
+  exchange is worth documenting back into this repo.