fix(tab5): align camera RGB order with JPEG encoder - #56
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
|
Codex review: blocked before merge. Reviewed September 13, 2026, 7:32 AM ET / 11:32 UTC (Revision 4). ClawSweeper reviewWhat this changesThe Tab5 camera swaps red and blue before JPEG encoding, adds four color-vector regression checks, and documents the correction’s limits. Merge readiness⛔ Blocked before merge - 3 items remain The correction remains absent from current main, and no actionable patch defect was found. Keep this maintainer-authored PR open while its explicit physical-qualification hold remains in force. Priority: P2 Review scores
Verification
How this fits togetherThe Tab5 camera handles snapshot requests from OpenClaw by capturing sensor frames, transforming them with Espressif’s pixel-processing accelerator, and encoding JPEG images. This change aligns the accelerator’s output bytes with the encoder’s input format. flowchart LR
A[Snapshot request] --> B[Capture indicator and camera lease]
B --> C[RGB565 sensor frame]
C --> D[Scale and rotate with channel swap]
D --> E[RGB888 JPEG encoder]
E --> F[Snapshot response]
Decision needed
Why: The author explicitly retains a physical-qualification hold; source review cannot release that owner-imposed landing condition. Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Retain the narrow channel-order correction and resolve the owner’s qualification criteria without expanding its claims to exposure, white balance, or full camera qualification. Do we have a high-confidence way to reproduce the issue? Yes, at the source-contract boundary: main passes B,G,R accelerator output to an encoder requiring R,G,B. The contributor reports a failing-before/passing-after fixture; physical capture was not reproduced. Is this the best way to solve the issue? Yes. Enabling the accelerator’s existing channel-swap option repairs the byte-order mismatch without adding a conversion pass or changing camera policy. AGENTS.md: not found in the target repository. Codex review notes: model internal, reasoning medium; reviewed against f3f7719a9d2d. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
History |
* commit 'f3f7719a9d2d75763c1645dcd758b61189c8e872': fix(ci): pin Tab5 to qualified ESP-IDF image fix(tab5): retain DMA buffers when flash encryption is active feat(tab5): add a static room-node home (#39) fix(talk): enforce safe SDK logging without global changes fix(ci): verify unpatched camera components against their lock fix(ui): retain setup guidance on the node home ci: publish traceable example firmware bundles (#37) ci: require explicit verified component releases (#38) docs: clarify source setup and hardware qualification
Problem
The Tab5 camera passes PPA RGB888 output directly to
esp_new_jpeg. The pinnedPPA pixel layout is B,G,R in memory, while the encoder's RGB888 input contract
is R,G,B. Without a channel swap, red and blue reach the encoder reversed.
Changes
quality policy, SDK and dependencies unchanged in this PR's unique delta.
camera_snap,transform_camera_frame, and the JPEG process boundary with independentred, green, blue and gray vectors.
cast or physical color accuracy.
The pinned SDK contracts are in
color_types.h
and
ppa.h.
The reviewed encoder contract is
espressif/esp_new_jpeg1.0.2.Validation
specifically on reversed red and blue at JPEG ingress; green and gray passed.
integration, with ASan/UBSan and warnings-as-errors. Coverage includes the
existing geometry, warm-up, lifetime and failure-cleanup cases.
review. The ancestry-only integration preserves that exact five-file delta.
https://github.com/openclaw/esp-openclaw-node/actions/runs/34754017538.
https://github.com/openclaw/esp-openclaw-node/actions/runs/34754016230.
No extra workflow was dispatched.
GitHub SHA256 and byte count. All 11 checksum records, ten manifest file
records and four offset-mapped images passed verification. The image ranges
do not overlap NVS, PHY initialization or storage partitions.
Current-Main Integration
Originally stacked on #55,
this PR now targets
main. Signed merge commite517b141cdd80ec3f979aefcfe4ca02a129d2f25preserves the original signed head4f87df7d5b381b4d9f3b1a79364caff2c86dca11and incorporates mainf3f7719a9d2d75763c1645dcd758b61189c8e872.The candidate tree
a369f7a33734ca2d59e60205753b7d597fedcb13exactly matchescurrent main plus the unchanged reviewed patch. GitHub's synthetic merge
8f63e61a4f9fb0adf7f6dfac83dd44d5b9df79c6has the same tree and the expectedmain/head parents.
The archive identifies compiled source
8f63e61a4f9fb0adf7f6dfac83dd44d5b9df79c6, app version8f63e61, and thequalified ESP-IDF image with the existing tracked patches. Firmware artifact
10316388299is 2,899,804 bytes, SHA256815a8f1ed89d6381a69a4c548c8577d793cd0e3d26fd6d01f1832cacf3999848.Symbols are retained in the same run but were not downloaded.
Qualification
On September 13, 2026, candidate
8f63e61a4f9fb0adf7f6dfac83dd44d5b9df79c6was flashed to an M5Stack Tab5:all four programmed images were verified, and a clean ESP32-P4 v1.3 boot
reached
app_main. Both node and operator roles reconnected using savedsessions.
device.infoconfirmed the Tab5 and app version8f63e61; onecamera.snapRPC succeeded and returned a JPEG that decoded successfully.The bounded run ended with its owned resources closed.
The inspected image was dark and nearly uniform, without an identifiable
scene. This proves a working capture/encode/RPC path, not optical RGB
calibration or usable scene quality. Physical color accuracy, exposure,
white balance and orientation remain unqualified. The cause of the dark
frame and previously observed color cast remains unknown. Voice qualification
is still pending; no successful voice exchange is claimed.
The owner has explicitly approved landing this narrow, source-contract-backed
channel-order correction with those optical limitations, releasing the draft
hold. The production delta remains one option and its explanatory comment;
the PPA/JPEG boundaries in the color fixtures are modeled, not physical color
measurements. No orientation, exposure or white-balance repair is included.
Landing is by merge commit to preserve the original signed series. No release,
OTA, secure-provisioning or other-device work is included.