fix: config parser/storage robustness — sized skipping, 4 KB configs, factory embed

[balloob]

Summary

Brings the nRF54 config subsystem to parity with the reference firmware (OpenDisplay/Firmware). Five related robustness fixes plus the storage/threading changes they require. Stacked on the CI PR (#1, feat/ci) — it targets main but the diff is only the config subsystem; review after #1 merges.

Both PlatformIO CI envs (seeed-xiao-nrf54l15, seeed-xiao-nrf54lm20a) are green.

1. Sized skipping instead of skip-to-CRC (config parser)

The parser's default branch set offset = configLen - 2 on any packet with no explicit case, jumping straight to the trailing CRC and silently dropping every packet after the first such type. Replaced with a static size table keyed by packet-type ID. Known-but-unparsed types advance by their true on-wire size so later packets still parse; only a genuinely unknown ID (not in the table) falls back to skip-to-CRC (the TLV format carries no per-packet length, so there is no safe alternative) — and it is logged. The existing security-packet rescan fallback is unchanged.

0x28 (touch) and 0x29 (buzzer) are in the table even though this branch has no parse case for them — sized skipping is exactly what lets the sibling feat/touch-gt911 / feat/buzzer branches add real parse cases independently.

Size table (on-wire data bytes, excluding the 2-byte [number][type] header)

Cross-checked three ways — reference Firmware/src/structs.h, this port's opendisplay_structs.h, and the py-opendisplay serializer. 13/15 were measured by actually running serialize_*; wifi and security are the serializer's own WifiConfig.SIZE=160 / SecurityConfig.SIZE=64 class constants. All three agree everywhere:

ID	Type	Size	ID	Type	Size
0x01	system_config	22	0x27	security_config	64
0x02	manufacturer_data	22	0x28	touch_controller	32
0x04	power_option	30	0x29	passive_buzzer	32
0x20	display	46	0x2A	nfc_config	32
0x21	led	22	0x2B	flash_config	32
0x23	sensor_data	30	0x2C	data_extended	288
0x24	data_bus	30	0x26	wifi_config	160
0x25	binary_inputs	30

One disagreement, and the wire wins: the old code skipped wifi_config as a hardcoded 162 bytes, but the packet is 160 on the wire (serializer WifiConfig.SIZE = 160; reference struct WifiConfig = ssid[32] + password[32] + encryption_type[1] + reserved[95] = 160). That off-by-2 desynced the offset for every packet after wifi. Corrected to 160.

Offset-walk trace

Hand-packed system(0x01) → touch(0x28, unparsed here) → data_extended(0x2C), walked through the exact parseConfigBytes arithmetic:

blob len=353 (declared 353)

[OLD skip-to-CRC]
  parsed  : ['0x1']            skipped: []        dropped: ['0x28']
  => 0x2C dropped (old bug reproduced).

[NEW sized-skip]
  parsed  : ['0x1', '0x2c']    skipped: ['0x28']  dropped: []
  final offset 351 / body-end 351  -> reached end (all consumed)
  => 0x2C AFTER unparsed 0x28 is parsed. FIX CONFIRMED.

2. wifi_config (0x26): parse and store instead of skip

The radio has no Wi-Fi, but the packet is now parsed into struct WifiConfig (exact reference layout) and stored on GlobalConfig, so a client's Wi-Fi settings survive a config read-back rather than being dropped. server_port — the one big-endian field in the TLV format — lives in the reference's reserved[]; it is kept as raw bytes here since this port does not consume it. The primary win is the corrected 160-byte size (see #1); storing is for round-trip parity.

3. MAX_CONFIG_SIZE 512 → 4096

Matches the reference (config_parser.h) and the factory generator's MAX_PACKET (tools/config_packet.py = 4096), so the generated factory_flash_cfg_t.data[MAX_CONFIG_SIZE] fits the padded blob exactly.

NVS / settings capacity (the load-bearing check)

Storage is Zephyr settings over the NVS backend (CONFIG_SETTINGS_NVS). Both boards' storage_partition is 36 KB on RRAM, whose erase block (= NVS sector) is 4096 B. The record is [magic:4][version:4][crc:4][data_len:4][data:len] = 16 + len bytes, saved as one NVS item.

• Max NVS item ≈ sector − 4×ATE = 4096 − 32 = 4064 B.
• The BLE write paths cap an inbound config: chunked = MAX_CONFIG_CHUNKS(20) × CONFIG_CHUNK_SIZE(200) = 4000 B, single-shot ≤ 200 B. (Reference has the same 4000-B chunked ceiling — parity kept.)
• So any client-writable config records at most 16 + 4000 = 4016 B < 4064 B — fits with headroom.

Chosen value: 4096 (full parity). The 4096 is a buffer bound, not the stored size; the stored record never exceeds 4016 B over BLE. A blob larger than ~4048 B is unreachable via BLE; if one were ever provisioned (e.g. an implausibly large factory embed), settings_save_one returns non-zero and saveConfig fails cleanly — and thanks to fix #4 the cache is not corrupted on that failure. Nothing else assumed 512 (the per-ATT-packet 512-byte buffers in opendisplay_pipe.c are MTU-sized and unrelated).

Stack / RAM implications

saveConfig/loadConfig held a opendisplay_config_storage_t (now ~4112 B) on the stack — that exceeds CONFIG_MAIN_STACK_SIZE (4096) and would overflow. Moved both to static scratch (writes are serialized on the main thread). Net static/BSS growth from the whole change is ~30 KB (three ~4 KB records + the 4 KB read/parse/chunk buffers); the nRF54L15 has 256 KB RAM.

Config read chunk cap

handle_config_read used a hardcoded max_chunks = 10 (≈940 B readable — the MJ-14 hang for larger configs). Now derived (MAX_CONFIG_SIZE + 93) / 94 = 44, matching the reference (communication.cpp:330); 94 is the conservative per-chunk data rate (chunk 0 carries 94 after the 6-byte header, later chunks 96).

Raising the cap surfaced a latent flow-control gap: bt_gatt_notify returns -ENOMEM (never blocks) when the TX pool is momentarily exhausted, so a 44-notification burst could drop chunks with only 12 pooled buffers (prj.conf). Added a bounded retry-with-yield in pipe_send_raw: a single response still succeeds on the first try (no added latency), but under pool pressure it retries while the link is up so buffers free as the RX thread drains completions. This keeps the buffer pool small — no prj.conf buffer-count change needed.

4. saveConfig cache-before-persist bug

The RAM cache (s_cached/s_loaded) was committed before settings_save_one, so a failed write left the cache reporting an unpersisted config until reboot. Now the cache is committed only after the write succeeds.

5. Factory-embed provisioning

Ported the reference mechanism into src/factory_config.{c,h} (C port of factory_config.cpp/.h): magic (0xFAC70A5A) + declared-length + CRC-16/CCITT (length bytes zeroed) validation of an embedded factory_flash_cfg_t, gated on FACTORY_HAS_EMBED. Wired into opendisplay_ble_init where loadGlobalConfig fails — the same shape as the reference (config_parser.cpp:833-835):

config_loaded = loadGlobalConfig(&s_od_global_config);
if (!config_loaded && tryProvisionFactoryEmbed())
    config_loaded = loadGlobalConfig(&s_od_global_config);

Also honors the generator's FACTORY_CLEAR_CONFIG_ON_BOOT one-shot-clear define. The existing generator (scripts/factory_config_gen.py) and stub (src/generated/factory_config_data.c) are unchanged — the firmware now consumes the .c they emit, matching the reference's generated-file shape (const factory_flash_cfg_t g_factory_embed = { magic, len, { …padded to MAX_PACKET… } }).

Expected conflicts

Textual conflicts are expected in src/opendisplay_config_parser.c and src/opendisplay_constants.h with the sibling feat/buzzer (#3) and feat/touch-gt911 branches — they add case CONFIG_PKT_TOUCH / case CONFIG_PKT_PASSIVE_BUZZER and the matching #defines. This PR uses the same constant names (CONFIG_PKT_TOUCH 0x28, CONFIG_PKT_PASSIVE_BUZZER 0x29) to minimize friction. Those two types are already in the size table here, so ordering between the branches doesn't matter for correctness; whichever merges second just resolves to the explicit case overriding the sized-skip default.

🤖 Generated with Claude Code

https://claude.ai/code/session_012g2e8mr132vcizx92WsgiR