web: refuse frame-blast SoCs instead of failing at the wire by widgetii · Pull Request #122 · OpenIPC/defib

widgetii · 2026-08-16T19:37:39Z

Interim fix for #121.

The problem

66 of the 109 chips in the browser dropdown cannot be recovered by this build. Their bootrom needs a frame-blast handshake — an active 0xAA+HEAD blast rather than passively waiting for the 0x20 markers, plus a PRESTEP0 block sent ahead of DDRSTEP0. web/ implements neither, and the inline PROFILES blob has had PRESTEP0 stripped from every entry, so the data isn't even there to send.

Until now those chips were selectable. The user picks one, watches the U-Boot download succeed and verify its SHA-256, hits Start Recovery, and then sits through a handshake that was never going to complete — with nothing on screen suggesting the tool couldn't drive their part.

The affected list is not exotic:

Affected (CLI only)	Working in browser
`hi3516ev300`, `hi3516ev200`, `hi3516cv300`, `hi3516cv500`, `hi3516av200`, `hi3518ev200` + ~60 STB parts	`gk7202v300`, `gk7205v200`, `gk7205v300`, `hi3518ev300`, `hi3516cv200`, `hi3516dv300`, …

hi3516ev300 is one of the most widely deployed parts we support — and we have been actively driving traffic here from OpenIPC/wiki#484 and from replies on firmware#1494/#1532/#1674/#1827. Two of those replies recommended the browser tool for hi3516ev300 specifically; both have been corrected.

The fix

Block them clearly rather than implement frame-blast in JS:

FRAME_BLAST_SOCS + needsFrameBlast() in protocol.js
affected options render as <chip> — CLI only, and the count now reads "43 of 109 supported in browser" instead of "109 supported"
selecting one swaps the download UI for the equivalent CLI command and a link to web: 65 frame-blast (PRESTEP0) chips are selectable but the web build never sends PRESTEP0 #121; Start Recovery stays disabled

Why this drifted unnoticed — and the guard for it

Profile lookup follows alias files, and an alias holds nothing but a filename. hi3516ev300.json contains the single line hi3516ev200.json. Checking hi3516ev300.json for a PRESTEP0 finds none because it has no fields at all, which reads as "plain handshake" unless you resolve the alias first. I made exactly that mistake before catching it, which is why the guard is part of this PR rather than a follow-up.

web/profile-parity.test.js resolves aliases the same way loader.py does, recomputes the frame-blast set from src/defib/profiles/data, and fails if it disagrees with FRAME_BLAST_SOCS in either direction. It also asserts every web chip has a CLI profile behind it, and that PRESTEP0 data never reappears in the web blob without an implementation to use it.

Wired into CI alongside the existing suite.

Verification

$ node --test web/protocol.test.js web/profile-parity.test.js
ℹ tests 86
ℹ pass 86
ℹ fail 0

Confirmed the guard actually catches the regression by deleting hi3516ev300 from FRAME_BLAST_SOCS:

✖ FRAME_BLAST_SOCS matches the chips whose resolved profile has PRESTEP0
  AssertionError: these chips need a frame-blast handshake but are not in
  FRAME_BLAST_SOCS, so the UI will let users start a recovery that cannot
  succeed: hi3516ev300

Inline page JS syntax-checked, and needsFrameBlast confirmed reachable as a global from index.html (protocol.js is loaded via <script src>).

Not hardware-tested — I don't have a frame-blast part in front of me. The claim that these chips can't work is a read of the code path, not an observed failure; if some of them tolerate the plain handshake, this PR is over-strict for those and the list should shrink.

Does not close #121

The real fix is generating PROFILES from the CLI profile data and implementing the frame-blast path in protocol.js. This just stops the silent failure in the meantime.

66 of the 109 chips in the browser dropdown cannot be recovered by this build. Their bootrom needs a frame-blast handshake — an active 0xAA+HEAD blast rather than passively waiting for the 0x20 markers, plus a PRESTEP0 block sent ahead of DDRSTEP0 — and web/ implements neither. The inline PROFILES blob has had PRESTEP0 stripped from every entry, so the data isn't even present to send. Until now those chips were selectable. A user picked one, watched the U-Boot download succeed and verify its SHA-256, hit Start Recovery, and then sat through a handshake that was never going to complete, with nothing on screen suggesting the tool couldn't drive their part. The affected list is not exotic: hi3516ev300, hi3516ev200, hi3516cv300, hi3516cv500, hi3516av200 and hi3518ev200 are all common camera SoCs, and hi3516ev300 in particular is one of the most widely deployed parts we support. We have been actively pointing people here from OpenIPC/wiki#484 and from replies on firmware#1494/#1532/#1674/#1827. This is the interim fix from #121 — block them clearly rather than implement frame-blast in JS: - FRAME_BLAST_SOCS + needsFrameBlast() in protocol.js - affected options render as "<chip> — CLI only"; the count now reads "43 of 109 supported in browser" - selecting one replaces the download UI with the equivalent CLI command and a link to #121, and Start Recovery stays disabled Why the drift went unnoticed: profile lookup follows alias files, and an alias holds nothing but a filename — hi3516ev300.json contains the single line "hi3516ev200.json". Checking hi3516ev300.json for a PRESTEP0 finds none because it has no fields at all, which reads as "plain handshake" unless you resolve the alias first. I got this wrong myself before catching it. So the guard resolves aliases the same way loader.py does, recomputes the frame-blast set from src/defib/profiles/data, and fails if it disagrees with FRAME_BLAST_SOCS in either direction. Verified it catches the regression by deleting hi3516ev300 from the list. Also asserts every web chip has a CLI profile behind it, and that PRESTEP0 data never reappears in the web blob without an implementation to use it. Does not close #121 — the real fix is generating PROFILES from the CLI data and implementing frame-blast in protocol.js. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

qodo-free-for-open-source-projects · 2026-08-16T19:38:47Z

PR Summary by Qodo

web: block frame-blast SoCs in browser UI and add profile parity guard

🐞 Bug fix 🧪 Tests ⚙️ Configuration changes 🕐 20-40 Minutes

AI Description

• Prevent starting recoveries for SoCs requiring the unimplemented frame-blast handshake.
• Mark affected chips as "CLI only", show the correct CLI command, and disable Start Recovery.
• Add CI-backed parity tests to keep web PROFILES and frame-blast list in sync.

Diagram

graph TD
  CI[".github/workflows/ci.yml"] --> TEST["web/profile-parity.test.js"] --> DATA[("src/defib/profiles/data")]
  TEST --> UI["web/index.html"] --> DEC{"needsFrameBlast()?"} --> BLOCK["CLI-only notice"]
  DEC --> OK["Enable browser recovery"]
  UI --> PROTO["web/protocol.js"]

  subgraph Legend
    direction LR
    _mod["Module/File"] ~~~ _dec{"Decision"} ~~~ _data[("Data")]
  end

High-Level Assessment

The following are alternative approaches to this PR:

1. Implement frame-blast handshake in web/protocol.js

➕ Restores browser recovery support for the blocked SoCs
➕ Eliminates the need for a growing hardcoded denylist
➖ Non-trivial protocol work in JS/WebSerial; higher risk without hardware coverage
➖ Still requires shipping PRESTEP0 data in the web profiles blob

2. Generate web PROFILES from CLI profiles as a build step

➕ Eliminates manual drift between CLI and web profile data
➕ Can keep PRESTEP0 and other fields consistent automatically
➖ Adds a build pipeline (web no longer purely hand-maintained static files)
➖ Still requires frame-blast implementation before enabling those chips

3. Derive frame-blast requirement dynamically from resolved profile metadata

➕ Avoids maintaining FRAME_BLAST_SOCS by hand
➕ Automatically accounts for alias resolution and new chips
➖ Requires embedding PRESTEP0 presence (or equivalent flag) into the shipped web data
➖ Without implementing frame-blast, would still need to block but via data-driven logic

Recommendation: This PR’s approach (explicitly blocking known-unsupported SoCs and clearly directing users to the CLI) is the safest interim fix because it prevents a guaranteed bad UX without introducing protocol changes. The added alias-aware parity test is a strong guardrail against future drift. Longer-term, the best path is to (1) generate the web PROFILES from the CLI data and (2) implement the frame-blast handshake in web/protocol.js, then remove or significantly reduce the blocklist.

Files changed (4) +177 / -4

Bug fix (2) +72 / -3

index.htmlBlock frame-blast chips in UI with CLI-only guidance +39/-3

Block frame-blast chips in UI with CLI-only guidance

• Adds styling and a hidden warning container to display a CLI-only message for frame-blast SoCs. Updates chip selector rendering to suffix blocked chips with "— CLI only", adjusts the supported count to reflect browser-capable chips, and disables Start Recovery when a blocked chip is selected.

web/index.html

protocol.jsAdd FRAME_BLAST_SOCS and needsFrameBlast() capability gate +33/-0

Add FRAME_BLAST_SOCS and needsFrameBlast() capability gate

• Defines a FRAME_BLAST_SOCS set and a needsFrameBlast(chip) helper to identify SoCs requiring the unimplemented frame-blast handshake. Exports both for Node tests and makes them available to the browser UI via the shared script.

web/protocol.js

Tests (1) +104 / -0

profile-parity.test.jsAdd alias-aware parity tests for web PROFILES vs CLI profiles +104/-0

Add alias-aware parity tests for web PROFILES vs CLI profiles

• Introduces Node.js tests that resolve CLI profile aliases, verify every web dropdown chip maps to a CLI profile, and assert FRAME_BLAST_SOCS exactly matches chips whose resolved profiles include PRESTEP0. Also asserts PRESTEP0 does not reappear in the web PROFILES blob unless a frame-blast implementation exists.

web/profile-parity.test.js

Other (1) +1 / -1

ci.ymlRun new web profile parity test in CI +1/-1

Run new web profile parity test in CI

• Extends the existing JS test step to include web/profile-parity.test.js alongside protocol.test.js, ensuring the new guard runs on every CI execution.

.github/workflows/ci.yml

qodo-free-for-open-source-projects · 2026-08-16T19:40:42Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. ~~Local firmware state desync~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

In web/index.html, the frame-blast early-return path sets firmwareData = null but does not
clear/reload the selected file input, so switching back to a supported chip can leave firmwareData
null even though a file is still selected, keeping Start disabled until the user re-selects the
file.

Code

web/index.html[R625-629]

+    autoDiv.style.display = 'none';
+    hint.textContent = '';
+    firmwareData = null;
+    updateStartButton();
+    return;

Evidence

The CLI-only branch clears firmwareData and returns, but does not clear the file input. Later in
the normal path, firmware is only cleared when the file input is empty, and the only place
firmwareData is populated from a local file is the file-input change handler—so after toggling
through a CLI-only chip, firmwareData can remain null while a file is still selected, leaving
Start disabled.

web/index.html[603-631]
web/index.html[646-692]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Selecting a frame-blast (CLI-only) chip clears `firmwareData` and returns early, but the `<input type="file">` selection remains. When switching back to a supported chip, the code assumes a selected file implies `firmwareData` is already populated, so it doesn't repopulate it and Start stays disabled.
## Issue Context
`firmwareData` is only (re)loaded from the file input in the file input `change` handler; `onChipChanged()` only clears firmware when *no* file is selected.
## Fix Focus Areas
- Prefer: do **not** clear `firmwareData` in the frame-blast early-return branch (Start is already blocked by `needsFrameBlast(chip)`), preserving the user’s local-file selection/state.
- Alternative: if you want to clear bytes for CLI-only chips, also clear the file input (`fileInput.value = ''`) so UI and state remain consistent.
### References
- web/index.html[603-631]
- web/index.html[646-692]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~Brittle PROFILES extraction~~ ✓ Resolved 🐞 Bug ☼ Reliability

Description

web/profile-parity.test.js locates the PROFILES blob with a regex that requires an immediate \n
after };, so line-ending normalization (CRLF) or minor formatting changes can break CI even when
the page remains valid.

Code

web/profile-parity.test.js[R53-56]

+  const html = fs.readFileSync(path.join(__dirname, 'index.html'), 'utf8');
+  const m = html.match(/const PROFILES = (\{[\s\S]*?\});\n/);
+  assert.ok(m, 'could not locate the PROFILES blob in web/index.html');
+  return JSON.parse(m[1]);

Evidence

The regex hard-requires a trailing \n after the }; terminator, which is not a semantic
requirement for JavaScript validity and can cause the test to fail on purely formatting/line-ending
changes.

web/profile-parity.test.js[51-57]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The test uses `/const PROFILES = (\{[\s\S]*?\});\n/`, which is unnecessarily strict about a literal LF immediately after the semicolon.
## Issue Context
This is a CI-only guard. Making it tolerant prevents spurious failures when `web/index.html` is reformatted or line endings change.
## Fix Focus Areas
- web/profile-parity.test.js[51-57]
Suggested change: allow optional whitespace and optional CRLF, e.g.
- `/const\s+PROFILES\s*=\s*(\{[\s\S]*?\});/`
- or `/const\s+PROFILES\s*=\s*(\{[\s\S]*?\});\r?\n/`

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Tip of the day

💡 Did you know, you can add REVIEW.md to your repo root and Qodo follows it on every PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Both raised by Qodo on #122, both reproduced before fixing. Stranded local file selection. The frame-blast branch cleared firmwareData on the way out, but the file input keeps its selection, so the normal path won't reload it — it only clears when no file is selected — and the change handler doesn't re-fire. Selecting a local image, browsing to a CLI-only chip and back left Start disabled with a file still visibly selected. Drop the clear: Start is already gated by needsFrameBlast(), and moving between two supported chips still invalidates a downloaded image via the existing path. CRLF-brittle PROFILES extraction. The parity test anchored on `};\n`, which doesn't match `};\r\n`, so a checkout with core.autocrlf=true failed on a line ending rather than on real drift. Verified: the old pattern finds no match against CRLF input, the new one parses all 109 profiles under both endings. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

widgetii · 2026-08-16T20:00:14Z

Both findings were real — reproduced each before fixing, pushed as 4577ae2.

1. Local firmware state desync — valid, fixed. Confirmed the path: firmwareData is only populated from a local file in the file-input change handler, and the normal path at onChipChanged() only clears it when no file is selected. So selecting a local image → switching to a CLI-only chip → switching back left firmwareData null with a file still visibly selected, and Start disabled until the user re-picked it.

Took the suggested preferred fix: drop the firmwareData = null from the frame-blast branch entirely rather than also clearing the file input. Start is already gated by needsFrameBlast(chip) in updateStartButton(), so retaining the bytes is harmless, and moving between two supported chips still invalidates a previously downloaded image through the existing path. Clearing the file input instead would have thrown away the user's selection for no benefit.

2. Brittle PROFILES extraction — valid, fixed. Verified the failure rather than assuming it:

OLD regex on CRLF: NO MATCH

A checkout with core.autocrlf=true would have failed CI on a line ending rather than on real drift — exactly the kind of false alarm that gets a guard test deleted. Now anchored with [ \\t]*\\r?$ under the m flag; confirmed it parses all 109 profiles under both LF and CRLF.

On the three alternative approaches in the summary: agreed, and that's the split between this PR and #121. Generating PROFILES from the CLI data plus implementing frame-blast in protocol.js is the right end state — deriving the requirement from profile metadata rather than a hand-maintained set falls out of that work for free. This PR stays deliberately interim because we're driving users at the page right now and the silent failure needed stopping today. #121 stays open for the real fix.

86 tests pass locally.

widgetii mentioned this pull request Aug 16, 2026

web: 65 frame-blast (PRESTEP0) chips are selectable but the web build never sends PRESTEP0 #121

Closed

qodo-free-for-open-source-projects Bot reviewed Aug 16, 2026

View reviewed changes

Comment thread web/index.html

Comment thread web/profile-parity.test.js

widgetii merged commit 4952ceb into master Aug 16, 2026
13 checks passed

widgetii deleted the fix/web-frame-blast-guard branch August 16, 2026 20:05

Navigation Menu

Uh oh!

web: refuse frame-blast SoCs instead of failing at the wire - #122

web: refuse frame-blast SoCs instead of failing at the wire#122
widgetii merged 2 commits into
masterfrom
fix/web-frame-blast-guard

widgetii commented Aug 16, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 16, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 16, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

widgetii commented Aug 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

widgetii commented Aug 16, 2026

The problem

The fix

Why this drifted unnoticed — and the guard for it

Verification

Does not close #121

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 16, 2026

PR Summary by Qodo

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

Uh oh!

widgetii commented Aug 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

qodo-free-for-open-source-projects Bot commented Aug 16, 2026 •

edited

Loading