[AI-assisted] prefs: fix serializer key "clk12" — a digit in a key silently truncates every /prefs.json - #3

⚠️ Disclosure: this PR was developed with AI

The code here was written by an AI agent (Claude Code), driven by me. Stating that up
front rather than burying it. I found and verified the bug on real hardware (CrowPanel
Advance 2.8"), and the diagnosis below is reproducible independently of how the patch was
authored.

I did this for personal reasons — I was getting a CrowPanel 2.8" working and could not
make any setting persist across a reboot. If you'd rather not take an AI-developed
change, please just close it — genuinely no hard feelings. In that case I'd still
encourage fixing this some other way, because I think it affects your own boards too.

Relationship to #2 — merge order does not matter

This is the standalone version of one commit that also appears in #2. #2 is opened from
my fork's main branch, and this fix landed there too, so #2 currently carries it as its
last commit. That is bookkeeping on my side, not a dependency.

What matters for you:

• Want only this fix? Merge this PR. It branches from this repo's main and touches
  only examples/companion_radio/{NodePrefs.h,DataStore.cpp} — nothing CrowPanel-specific.
• Want both? Merge in either order. The change is identical in each, so whichever lands
  second contributes nothing new (the other PR's file list simply overlaps on these two
  files). There is no ordering requirement.
• I can rebase [AI-assisted] CrowPanel Advance 2.4/2.8: fix touch (FT6336U, not GT911) + catch the SPI tier up to the 3.5 #2 to drop this commit if the duplication is annoying — just say so.

I split it out because, unlike #2, this is not CrowPanel-specific and I did not want a
fix to your own boards gated behind AI-developed variant work you may not want.

It affects every companion build that writes prefs, including the 3.5 and T-Deck.

The bug: no fork setting survives a reboot

ConfigSerializer's tokenizer accepts only [A-Za-z_] in keys:

static bool is_key_char(char c) {
  return (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || c == '_';
}

…while the writer emits whatever key it is handed. UiPrefs defines clk12. So every
/prefs.json written is a file that cannot be read back: loadSerial() reaches the 1,
returns TOK_ERROR, and abandons the remainder of the file. Everything after that point
silently stays at its default — the rest of ui, and the whole of wifi, mqtt, batt,
ota and audio.

cfgver is defined last at root, so it is always inside the truncated region and always
reads back as 0. rescueAppendedPrefs() then concludes the file is foreign (upstream
firmware, an older fork build, or corrupt) and, with no /new_prefs to recover from, calls
applyAppendedPrefsDefaults() and re-applies defaults — on every boot.

Here is the actual /prefs.json off my device, dumped at boot. Note it is well-formed and
ends in cfgver:1; the reader simply never gets there:

{name:"SV01",lat:42.426043,lon:-76.806494,radio:{freq:910.5250,...},gps:{...},repeat:{...},
 comp:{...},ui:{bright:0,rot:0,c_order:255,c_filter:255,tz_min:0,clk12:0,hist:255,...},
 wifi:{en:0,ssid:"",pass:"",...},mqtt:{...},batt:{...},ota:{...},audio:{...},cfgver:1}

And the corresponding read-back, instrumented in loadPrefs():

[PREFS] load: prefs.json=1 new_prefs=0 node_prefs=0
[PREFS] read: cfgver=0 rot=0 bright=0 freq=910.525 sane=1

freq=910.525 loads correctly (radio precedes ui), which is what makes this so
confusing to diagnose — the file is clearly being read, just not all of it.

Two consequences worth calling out

• Setting screen orientation appears to do nothing. The value is written correctly and
  parsed back correctly — rot precedes clk12 — and is then discarded by the defaults
  reset. That was my original symptom.
• Stored WiFi credentials never load. wifi{ssid,pass} sit after clk12, so WiFi could
  never connect using saved settings. I had assumed that was a separate problem.

Why this may look new

Pre-0.5.0 it was partially masked. The tail was still lost, but the older value-sniffing
migration did not wipe everything, so the fields before clk12 (brightness, rotation,
contact order/filter, tz) did persist. Stamping the file with cfgver in f01acf7 turned a
partial failure into a total one, because cfgver lands in the unreadable tail by
construction.

The fix

Kept entirely at the example boundary — ConfigSerializer is untouched, since the
offending key is the fork's, not upstream's:

• def("clk12", ...) → def("clk_ampm", ...).
• A prominent warning above UiPrefs documenting the key charset and this exact failure
  mode, so the next key containing a digit does not silently do this again.
• loadPrefs() now checks loadSerial()'s return value and logs a warning. It was being
  discarded, which is precisely why the truncation was invisible.

I audited all 126 def() keys in the tree; clk12 was the only one outside
[A-Za-z_].

You may also want to fix the tokenizer itself

I did not touch src/helpers/ConfigSerializer.cpp — that is your upstream-tracked code and
not mine to change. But the underlying hazard remains: the writer accepts keys the reader
cannot parse, and fails silently and destructively. Worth considering, entirely your call:

• allow digits in is_key_char (they are valid JSON keys and the writer already emits them), and/or
• make a tokenizer error louder, since loadSerial()'s false return is easy to drop — as
  it was here.

Either would turn this class of bug from silent data loss into something immediately obvious.

Migration

One further reset on the first boot after upgrading, since the old file still contains
clk12; the rewritten file then parses fully and settings persist. Users lose nothing they
actually had, because none of it was persisting.

Testing

Verified on a CrowPanel Advance 2.8": cfgver reads 1 and no parse warning across repeated
boots; settings survive a reboot. Builds: elecrow_crowpanel_advance_35_companion_radio_lvgl
and LilyGo_TDeck_companion_radio_usb, both from this branch's base rather than my
development branch.

I have not tested on a 3.5 or T-Deck — I don't own either — though the change is
board-independent.

🤖 Generated with Claude Code