Skip to content

Commit 922746e

Browse files
WaylandYangclaude
andcommitted
example(lamp): add blink intent — non-idempotent + duration param coverage
The lamp manifest so far showed two intent shapes: idempotent write with float/duration params (set_brightness, set_color) and read (read_brightness). blink adds the third interesting shape: non-idempotent write with a duration param that has to round-trip through CBOR as a float. This makes the canonical example exercise every code path in the bridge x firmware boundary worth testing. Changes: - examples/lamp_manifest.yaml: blink(times: int, period: duration ms) with idempotent: false and dry_run: true; ranges [1,20] and [50,2000]ms cap total real-time at 80s worst case - firmware/esp32/examples/lamp/lamp.ino: handle_blink + binding, ~33 LOC. Saves/restores PWM duty so prior brightness survives the blink. Blocking implementation; the comment notes a real product would use a millis() state machine - tools/test_uart_roundtrip.py: 3 blink cases folded into the existing suite — now 13/13 round-trips pass against ESP32-WROOM-32 - README.md, docs/paper/main.tex: 10/10 → 13/13 - docs/ADDING_FEATURES.md: trap report + sync example ranges with the shipped manifest. The trap: CBOR is type-strict, so a `duration` param (encoded as CBOR float on the wire) must be decoded with read_float() not read_int(); a mismatched read wedges the parser and the handler returns STATUS_DENIED for both dry-run and real calls. Worth documenting because the same symptom hits anyone porting a third-party handler that assumed numeric flexibility. The Python Bridge, MCP server wrapper, and GenericSimulator all required zero changes — the manifest is the single source of truth. That's the load-bearing claim of the protocol, and it held up under the add-an-intent test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 89c6d2d commit 922746e

6 files changed

Lines changed: 90 additions & 9 deletions

File tree

README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ device boundary.
7575
As of v0.3 the reference firmware is **measured-validated on an
7676
ESP32-WROOM-32 dev board** over CH340 USB-Serial at 115 200 baud:
7777

78-
- 10/10 round-trip tests pass (`tools/test_uart_roundtrip.py`)
78+
- 13/13 round-trip tests pass (`tools/test_uart_roundtrip.py`)
7979
- 88/88 Python unit & conformance tests pass
8080
- Compiled firmware: 294 KB flash, 22.7 KB globals (Arduino-ESP32 core 3.3.8)
8181
- The pure DCP layer is approximately 14 KB over a baseline empty
@@ -264,7 +264,7 @@ MIT.
264264
- [x] Conformance test suite (golden frames, language-neutral YAML)
265265
- [x] Codegen `--stubs`: emits handler signatures + binding table
266266
- [x] Quickstart video script ([docs/QUICKSTART_VIDEO.md](docs/QUICKSTART_VIDEO.md))
267-
- [x] Real-hardware UART validation (ESP32-WROOM-32, 10/10 round-trips)
267+
- [x] Real-hardware UART validation (ESP32-WROOM-32, 13/13 round-trips)
268268
- [x] Public repo at `device-context-protocol/dcp` (v0.3.0 released)
269269
- [x] PyPI release (`pip install pydcp`)
270270
- [ ] T-Panel S3 + CAN bus demo (firmware ready, awaiting hardware)

docs/ADDING_FEATURES.md

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,9 @@ Edit `examples/lamp_manifest.yaml`. Append to `intents:`:
2222
```yaml
2323
- name: blink
2424
params:
25-
times: { type: int, range: [1, 100], default: 3 }
25+
times: { type: int, range: [1, 20], default: 3 }
2626
period: { type: duration, unit: ms,
27-
range: [50, 5000], default: 200 }
27+
range: [50, 2000], default: 200 }
2828
capability: lamp.write
2929
idempotent: false # calling twice blinks twice
3030
dry_run: true # the LLM can ask "what would this do?"
@@ -73,8 +73,8 @@ static dcp::Status handle_blink(uint8_t kind,
7373
// Belt-and-suspenders: re-check ranges. The Bridge already did this,
7474
// but defending here means a misbehaving Bridge can't drive the LED
7575
// outside the safe envelope.
76-
if (times < 1 || times > 100) return dcp::STATUS_RANGE;
77-
if (period < 50 || period > 5000) return dcp::STATUS_RANGE;
76+
if (times < 1 || times > 20) return dcp::STATUS_RANGE;
77+
if (period < 50 || period > 2000) return dcp::STATUS_RANGE;
7878
7979
// Dry-run: report what we would do, no side effects.
8080
if (kind == dcp::KIND_DRY_RUN) {
@@ -233,12 +233,39 @@ The Bridge fans this out to any LLM session subscribed to `lamp.read`.
233233
| Symptom | Cause | Fix |
234234
|---|---|---|
235235
| `denied` with empty data from device | error reply buffer too small in firmware | bump the `uint8_t buf[N]` in your handler |
236+
| `denied` after Bridge says `ok` for the same call shape on dry-run | **CBOR type mismatch** — handler used `read_int` on a `duration`/`float` param; the reader wedges, next `next_key` returns false | use `read_float` for any param whose manifest type is `float` or `duration`; `read_int` only for `int`. See "CBOR is type-strict" note below |
236237
| `unknown_intent` for an intent you swore is in the manifest | spelling mismatch — `DCP_ID("foo")` vs manifest `name: Foo` | strings are byte-exact; rename one to match |
237238
| LLM keeps sending out-of-range values | you forgot `range:` in the manifest | add it; the Bridge picks it up after restart |
238239
| Long handler causes `busy` from the Bridge | `delay()` exceeded the Bridge timeout (default 2s) | shorten, or run async via FreeRTOS task and ack-then-event |
239240
| `set_color` works but no light changes | you don't have an RGB LED wired up | that's by design — the example saves state and flashes the brightness LED to acknowledge |
240241
| Capability check fails with `capability_required` | the LLM session's token doesn't include that capability | re-issue a token: `dcp token mint --caps lamp.write,lamp.read,...` |
241242

243+
### CBOR is type-strict (the duration-as-float trap)
244+
245+
CBOR distinguishes integer and float at the major-type level. A `42`
246+
and a `42.0` are **not** the same item. The Bridge encodes manifest
247+
parameter values like this:
248+
249+
| manifest `type` | wire encoding |
250+
|---|---|
251+
| `int`, `bool` | CBOR int |
252+
| `string` | CBOR text string |
253+
| `float`, **`duration`** | **CBOR float (double)** |
254+
255+
So a `duration` param like `period: { type: duration, unit: ms }`
256+
arrives on the device as a CBOR float, even if the caller wrote `200`
257+
(no decimal). Your firmware handler **must** decode it with
258+
`params.read_float(&period)`, not `params.read_int(&period)`. A
259+
mismatched read returns false and leaves the parser positioned wrong
260+
for the next field — the next `next_key()` call will then also fail,
261+
and the handler returns `STATUS_DENIED`.
262+
263+
If you see "Bridge accepts the call (status is not `range` or
264+
`unknown_intent`) but the device returns `denied` for both dry-run
265+
and real calls, while a sibling intent with only `int`/`bool` params
266+
works fine," check this first. The fix is one word per affected
267+
param (`read_int` → `read_float`).
268+
242269
---
243270

244271
## What the loop doesn't ask you to touch

docs/paper/main.tex

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -462,9 +462,11 @@ \subsection{ESP32 reference firmware}
462462
expected to shed the bulk of the residual baseline.
463463

464464
The Bridge $\leftrightarrow$ device path was end-to-end validated against
465-
ten round-trip cases (\texttt{tools/test\_uart\_roundtrip.py}) covering
466-
calls, reads, dry-run, range rejection, and unknown-intent error frames.
467-
All ten cases pass against real hardware.
465+
thirteen round-trip cases (\texttt{tools/test\_uart\_roundtrip.py}) covering
466+
calls, reads, dry-run, range rejection, unknown-intent error frames, and a
467+
non-idempotent intent with a \texttt{duration} parameter that exercises
468+
the firmware-side CBOR float decode path. All thirteen cases pass against
469+
real hardware.
468470

469471
\subsection{Conformance suite}
470472

examples/lamp_manifest.yaml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,14 @@ intents:
2626
returns: { type: float, unit: percent }
2727
capability: lamp.read
2828

29+
- name: blink
30+
params:
31+
times: { type: int, range: [1, 20], default: 3 }
32+
period: { type: duration, unit: ms, range: [50, 2000], default: 200 }
33+
capability: lamp.write
34+
idempotent: false # blink(3) twice = 6 blinks, not 3
35+
dry_run: true
36+
2937
events:
3038
- name: motion_detected
3139
payload:

firmware/esp32/examples/lamp/lamp.ino

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,12 +92,51 @@ static dcp::Status handle_set_color(uint8_t kind,
9292
return dcp::STATUS_OK;
9393
}
9494

95+
// Blink the built-in LED `times` times, each cycle = `period`ms on + `period`ms off.
96+
// Note: blocking implementation — the main loop can't service DCP frames during
97+
// the blink. Manifest range caps total time at 20 * 2000 * 2 = 80s worst case;
98+
// a real product would use a millis()-based state machine instead.
99+
static dcp::Status handle_blink(uint8_t kind,
100+
dcp::CborReader& params,
101+
dcp::CborMap& reply,
102+
void* /*user*/) {
103+
int64_t times = 3;
104+
double period_ms = 200.0; // duration is encoded as CBOR float on the wire
105+
while (params.remaining() > 0) {
106+
const char* key = nullptr;
107+
size_t key_len = 0;
108+
if (!params.next_key(&key, &key_len)) return dcp::STATUS_DENIED;
109+
if (key_len == 5 && memcmp(key, "times", 5) == 0) { if (!params.read_int(&times)) return dcp::STATUS_RANGE; }
110+
else if (key_len == 6 && memcmp(key, "period", 6) == 0) { if (!params.read_float(&period_ms)) return dcp::STATUS_RANGE; }
111+
else { params.skip(); }
112+
}
113+
if (times < 1 || times > 20) return dcp::STATUS_RANGE;
114+
if (period_ms < 50.0 || period_ms > 2000.0) return dcp::STATUS_RANGE;
115+
116+
uint32_t period = (uint32_t)period_ms;
117+
if (kind == dcp::KIND_DRY_RUN) {
118+
reply.add_int("would_blink_times", times);
119+
reply.add_int("would_blink_period", (int64_t)period);
120+
return dcp::STATUS_OK;
121+
}
122+
123+
// Save current brightness so we can restore it after the blink.
124+
uint32_t saved = (uint32_t)(g_brightness * 2.55f);
125+
for (int64_t i = 0; i < times; ++i) {
126+
ledcWrite(LED_PIN, 255); delay(period);
127+
ledcWrite(LED_PIN, 0); delay(period);
128+
}
129+
ledcWrite(LED_PIN, saved);
130+
return dcp::STATUS_OK;
131+
}
132+
95133
// Intent IDs are resolved at compile time via DCP_ID().
96134
// Keep these strings in sync with examples/lamp_manifest.yaml.
97135
static dcp::IntentBinding bindings[] = {
98136
{ DCP_ID("set_brightness"), handle_set_brightness, nullptr },
99137
{ DCP_ID("set_color"), handle_set_color, nullptr },
100138
{ DCP_ID("read_brightness"), handle_read_brightness, nullptr },
139+
{ DCP_ID("blink"), handle_blink, nullptr },
101140
};
102141

103142
static dcp::DCP* dcp_instance = nullptr;

tools/test_uart_roundtrip.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
- dry-run (predicted, no side effect)
1010
- out-of-range (Bridge-rejected)
1111
- unknown intent (device-rejected with error frame)
12+
- non-idempotent intent with duration param (blink) — exercises
13+
the firmware-side CBOR float decode path
1214
"""
1315
from __future__ import annotations
1416

@@ -60,6 +62,9 @@ async def main(port: str) -> int:
6062
("dry-run set_color cyan", "set_color", {"r": 0, "g": 255, "b": 255}, True, "ok"),
6163
("set_color out-of-range", "set_color", {"r": 999, "g": 0, "b": 0}, False, "range"),
6264
("turn_into_pumpkin (unknown)", "turn_into_pumpkin", None, False, "unknown_intent"),
65+
("dry-run blink(5, 150)", "blink", {"times": 5, "period": 150}, True, "ok"),
66+
("blink(3, 150) real", "blink", {"times": 3, "period": 150}, False, "ok"),
67+
("blink(999, 100) bad times", "blink", {"times": 999, "period": 100}, False, "range"),
6368
]
6469

6570
passed = failed = 0

0 commit comments

Comments
 (0)