Wire in canonical opendisplay_structs.h: config-struct de-dup + message-payload adoption by davelee98 · Pull Request #110 · OpenDisplay/Firmware

davelee98 · 2026-07-19T05:03:25Z

Summary

Two related pieces of protocol wiring:

Makes the Firmware consume the canonical opendisplay_structs.h wire-payload contract instead of hand-rolling the same structs/enums/macros a second time, then selectively adopts the message-payload structs where they remove real byte-juggling. No wire-format change — every layout is byte-for-byte identical to the previous code (sizes fixed by OD_STATIC_ASSERT).
Splits CMD_POWER_OFF (0x0052) out of CMD_DEEP_SLEEP (0x0053) per the re-vendored opendisplay_protocol.h (the 2.1 opcode swap).

Stacked on #108 (davelee98:debug/v2.2-audit). The first 2 commits in this PR belong to #108 and will drop out of the diff once it merges. Review only the 5 commits below.

Power-off / deep-sleep wire codes (0x0052 ↔ 0x0053)

The 2.1 swap moved deep-sleep to a higher opcode and gave power-off the lower one. Explicitly:

Opcode	Name	Dir	Request	Success resp	NACK codes
`0x0052`	`CMD_POWER_OFF`	host→device	`[0x00][0x52]` (bare; trailing bytes reserved)	`[0x00][0x52]` (`RESP_POWER_OFF`), then D-FF-latch rail-cut	`[0xFF][0x52][0x00][0x00]` `OD_ERR_POWER_OFF_UNSUPPORTED` (no latch on target)
`0x0053`	`CMD_DEEP_SLEEP`	host→device	`[0x00][0x53]` or `[0x00][0x53][seconds:2 BE]`	`[0x00][0x53]` (`RESP_DEEP_SLEEP`)	`[0xFF][0x53][err][0x00]`: `0x00` UNSUPPORTED (nRF), `0x01` DISABLED (config), `0x02` NOT_BATTERY (mains)

Breaking vs shipped firmware: CMD_DEEP_SLEEP was 0x0052 in PR feat: wake on button press from deep sleep + 0x0052 duration payload #97; a peer still on the old opcode now hits CMD_POWER_OFF. Do not conflate — a device that refuses 0x52 may still accept 0x53.
Deep-sleep duration override rides 0x0053 as [seconds:2 BE]; 0x0000 = no override (use config). Host overrides are clamped to a 60 s floor. Example — sleep 90 s: 00 53 00 5A.
Power-off is capability-gated on a D-FF latch; every non-latch/mains target NACKs unsupported. A future "deep sleep, no wake timer" fallback for non-latch battery targets is marked with an ANCHOR() in handlePowerOffCommand.

Commits to review

fa355c9 chore(protocol): vendor opendisplay_structs.h into include/ (byte-identical to the canonical opendisplay-protocol copy).
ef27584 refactor(config): src/structs.h becomes a thin shim #include-ing the canonical header; delete all 14 duplicated config structs + duplicated flag macros; migrate ~106 macro sites to OD_-prefixed canonical names; rename promoted fields (PassiveBuzzerConfig→BuzzerConfig, tag_type→legacy_tag_type, reserved_pin_2→cs_pin_2, BinaryInputs reserved_pin_1..8→input_pin_1..8, input_flags→pins_used, WifiConfig reserved→server_host/server_port).
ff05d90 refactor(msg): adopt canonical structs where they help — MsdAdvertisement build, PipeStartRequest/PipePartialExt parse, LedFlashPattern overlay-read; PartialWriteStartHeader gets a sizeof guard but stays hand-parsed (it is all big-endian). Trivial one-integer payloads (PipeSack, PipeStartResponse) and the auth memcpys are left as explicit bytes on purpose.
7b67e17 docs(boot): flag OD_COLOR_SCHEME_SEVEN_COLOR (value 7, formerly the bogus GRAY8) as an unimplemented stub — the boot screen labels it "7 color" but renders it as 1bpp mono.
15b9004 feat(power): re-vendor opendisplay_protocol.h for the 0x0052↔0x0053 swap and split the old combined handler into handlePowerOffCommand (0x0052) and handleDeepSleepCommand (0x0053) — see the wire-codes table above.

Verification

Builds clean on nrf52840custom, esp32-s3-N16R8, and esp32-N4 (the tight-DRAM env exercising the PIPE_SMALL_DRAM_WINDOW reorder-window). No OD_STATIC_ASSERT fires → confirms byte-size parity. The power-off split was rebuilt on nrf52840custom (no-op stub latch path) and esp32-s3-N16R8 (real latch path).
Independent byte-offset/endianness review of every renamed field and adopted struct: byte-exact on the wire, no correctness issues.

Follow-ups (not in this PR)

Implement the non-latch power-off fallback (ANCHOR(power-off-no-latch-fallback)) and, if wanted, the nRF 0x0053 unsupported-NACK.
Adopt the remaining low-value message structs / add real RESP_* mirrors for the pipe opcodes upstream.
Re-sync the Python tools/device_cli.py + tools/config_packet.py to the canonical field names.
Extend opendisplay-protocol/tools/sync_protocol_header.py to also track opendisplay_structs.h drift.

🤖 Generated with Claude Code

Pull the new canonical opendisplay_structs.h (config/message payload layout contract, OD_STRUCTS_VERSION 2.0) from opendisplay-protocol into include/, alongside the already-in-sync opendisplay_protocol.h it #includes. sync_protocol_header.py does not yet track structs.h. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

… copies Make src/structs.h a thin shim over the vendored canonical header instead of hand-rolling the config-packet structs, enums, and flag macros a second time. The canonical definitions are the reconciled superset (ble_proto v1.4) and are byte-for-byte size-identical to the old local copies (enforced by OD_STATIC_ASSERT), so this is a pure de-dup with NO wire-format change. - src/structs.h: #include "opendisplay_structs.h"; delete all 14 duplicated config structs and every duplicated macro block; keep only firmware-only runtime types (ImageData, PipeReorderSlot, PipeWriteState, GlobalConfig, ButtonState) and PIPE reorder-window constants. - Migrate ~106 config macro sites to the OD_-prefixed canonical names (COLOR_SCHEME_*, TRANSMISSION_MODE_*, SENSOR_TYPE_*, TOUCH_*, SECURITY_FLAG_*, CHARGER_FLAG_*, SLEEP_FLAG_*, BATTERY_SENSE_FLAG_*, BUZZER_FLAG_*, PANEL_IC_*, FLASH_FLAG_*), including the GRAY8->SEVEN_COLOR (both value 7) and ZIPXL->STREAMING_DECOMPRESSION remaps. - Rename references to promoted/renamed fields: PassiveBuzzerConfig->BuzzerConfig, tag_type->legacy_tag_type, DisplayConfig.reserved_pin_2->cs_pin_2, BinaryInputs.reserved_pin_1..8->input_pin_1..8, input_flags->pins_used. - WifiConfig (ESP32): read the promoted server_host/server_port fields instead of the former reserved[] bytes; server_port stays big-endian. Config-struct scope only; message-payload structs (PIPE/partial/auth/LED/MSD) are now available but their hand-parse sites are unchanged. Builds verified: esp32-s3-N16R8, esp32-N4 (small-DRAM), nrf52840custom all compile clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Selectively replace hand-rolled byte arithmetic with the canonical packed structs from opendisplay_structs.h at the sites where they collapse real byte-juggling; leave trivial one-integer payloads as explicit shifts. All structs are little-endian and byte-identical to the prior code on the LE targets (sizes fixed by OD_STATIC_ASSERT), so no wire-format change. - display_service.cpp updatemsdata(): build the 16-byte advertisement via a local struct MsdAdvertisement + OD_MSD_STATUS_* status bits, then memcpy into the existing msd_payload[16] the BLE adv APIs consume. - display_service.cpp handlePipeWriteStart(): parse the 10-byte START header and 12-byte partial extension via struct PipeStartRequest / PipePartialExt (memcpy-in), replacing ~12 lines of shift arithmetic; length guards now use sizeof. - display_service.cpp handlePartialWriteStart(): documented-only — the 0x76 header is all BIG-endian, so keep the hand byte-swap parse; just swap the magic 17 for sizeof(struct PartialWriteStartHeader) and add a comment. - device_control.cpp led_load_config()/handleLedActivate(): overlay-read the persisted 12-byte pattern as struct LedFlashPattern to name the ledcfg[] indices; the nibble packing stays hand-decoded. Copy uses sizeof. Left as explicit bytes (struct would be a wash): PipeSack, PipeStartResponse (single-integer payloads) and the auth challenge/proof memcpys (already clear; not worth adding an include to the crypto path). Builds verified clean: nrf52840custom, esp32-s3-N16R8, esp32-N4. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Value 7 stays OD_COLOR_SCHEME_SEVEN_COLOR (aligned with the canonical enum, formerly the bogus GRAY8), but the firmware does not actually render 7 colors: the boot screen only prints the "7 color" label while treating scheme 7 as a 1bpp mono stub (kSchemeWhiteValue[7] = 0xFF; footer swatches lumped in with MONO). Add prominent warning comments at all three sites so no one assumes a device reporting scheme 7 renders 7 colors. Comment-only; no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Re-vendor opendisplay_protocol.h (byte-identical to canonical) for the 2.1 opcode swap: CMD_DEEP_SLEEP moves 0x0052 -> 0x0053, and a new CMD_POWER_OFF takes 0x0052, with RESP_POWER_OFF (0x52) and the scoped OD_ERR_POWER_OFF_* / OD_ERR_DEEP_SLEEP_* NACK namespaces. Then reconcile the firmware source: - Dispatch (communication.cpp): route CMD_POWER_OFF -> handlePowerOffCommand, CMD_DEEP_SLEEP -> handleDeepSleepCommand. Auth-gating is inherited from the pre-dispatch check. - handlePowerOffCommand (0x0052): bare request (no payload); D-FF-latch HW does the fire-and-forget hard rail-cut with RESP_POWER_OFF; every non-latch target NACKs OD_ERR_POWER_OFF_UNSUPPORTED. No #ifdef needed - powerLatch* are no-op stubs off ESP32 (powerLatchDffConfigured() returns false -> falls to the NACK). Carries an ANCHOR() for a future "deep sleep, no wake timer" fallback. - handleDeepSleepCommand (0x0053): timed-deep-sleep path only (latch hard-off branch removed); named OD_ERR_DEEP_SLEEP_* NACK codes; log strings derive from the CMD_ constants; documented (behavior-preserving) nRF #else. - Deep-sleep override: enforce a 60 s floor on host-supplied wake timers (0 still means "no override -> use config"); correct the 0x0052 -> 0x0053 doc references in main.cpp/main.h for the override payload. Builds verified clean on nrf52840custom (stub latch path) and esp32-s3-N16R8 (real latch path). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The CLI's BLOCKS dict hand-mirrors the config struct layout and isn't auto-generated; the last drift (pre-OpenDisplay#110 field names) silently dropped zero-valued pins from decoded YAML, so call out the invariant explicitly.

The CLI's config field tables still used pre-OpenDisplay#110 src/structs.h names. Rename displays.tag_type/reserved_pin_2 and binary_inputs' reserved_pin_*/input_flags to their canonical names: legacy_tag_type, cs_pin_2, input_pin_*, pins_used. Stale "reserved_*" names caused real zero-valued pins to be silently dropped from decoded YAML. Also expose manufacturer_data's simple_config_* fields. Expose wifi_config's server_host/server_port instead of an opaque reserved blob. server_port is big-endian, so add a new u16be kind. Add the missing 0x2A nfc_configs block. Extend VALID_ENUMS for the new color_scheme/capacity_estimator values.

* docs: Update CLAUDE.md that tools/od-device-cli.py must stay in sync The CLI's BLOCKS dict hand-mirrors the config struct layout and isn't auto-generated; the last drift (pre-#110 field names) silently dropped zero-valued pins from decoded YAML, so call out the invariant explicitly. * tools: sync od-device-cli.py with canonical opendisplay_structs.h The CLI's config field tables still used pre-#110 src/structs.h names. Rename displays.tag_type/reserved_pin_2 and binary_inputs' reserved_pin_*/input_flags to their canonical names: legacy_tag_type, cs_pin_2, input_pin_*, pins_used. Stale "reserved_*" names caused real zero-valued pins to be silently dropped from decoded YAML. Also expose manufacturer_data's simple_config_* fields. Expose wifi_config's server_host/server_port instead of an opaque reserved blob. server_port is big-endian, so add a new u16be kind. Add the missing 0x2A nfc_configs block. Extend VALID_ENUMS for the new color_scheme/capacity_estimator values.

davelee98 requested a review from jonasniesner as a code owner July 19, 2026 05:03

davelee98 and others added 5 commits July 19, 2026 12:37

davelee98 force-pushed the feat/incorporate-structs2 branch from 15b9004 to 56019e7 Compare July 19, 2026 16:38

jonasniesner merged commit 60946f0 into OpenDisplay:main Jul 19, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Wire in canonical opendisplay_structs.h: config-struct de-dup + message-payload adoption#110

Wire in canonical opendisplay_structs.h: config-struct de-dup + message-payload adoption#110
jonasniesner merged 5 commits into
OpenDisplay:mainfrom
davelee98:feat/incorporate-structs2

davelee98 commented Jul 19, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

davelee98 commented Jul 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Power-off / deep-sleep wire codes (0x0052 ↔ 0x0053)

Commits to review

Verification

Follow-ups (not in this PR)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

davelee98 commented Jul 19, 2026 •

edited

Loading