chore(tab5): trace transport-start allocation failures - #50
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 12, 2026, 12:13 PM ET / 16:13 UTC (Revision 6). ClawSweeper reviewWhat this changesThe branch adds Tab5 transport-start allocation diagnostics, together with stacked camera format and geometry repairs, firmware provenance checks, and host tests. Regression provenancePossible regression — suspected (reviewed change). No predecessor PR is attributed. Merge readiness⛔ Blocked before merge - 3 items remain The diagnostics remain useful and have bounded hardware evidence. The cumulative branch still contains a packaging compatibility defect already repaired in its predecessor; it is not ready to land as currently checked out. Priority: P2 Review scores
Verification
How this fits togetherTab5 firmware connects separate node and operator sessions to the OpenClaw Gateway. The new observation hooks capture allocation failures during WebSocket startup and emit numeric diagnostics after the SDK returns; stacked camera changes affect captured JPEGs and firmware packaging. flowchart TD
A[Node or operator session] --> B[WebSocket startup]
B --> C[Task-scoped allocation capture]
C --> D[Post-return diagnostic log]
B --> E[Existing success or cleanup path]
F[Camera frame] --> G[Format and geometry repair]
G --> H[JPEG output]
Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Retain the bounded diagnostics while ordinary firmware packaging follows its resolved dependency lock and only explicit camera-repair builds enforce fixed repair pins. Do we have a high-confidence way to reproduce the issue? Yes, by source inspection of the pinned PR: ordinary Tab5 packaging with a valid non-2.4.1 esp_video resolution reaches unconditional 2.4.1 checks even when camera compatibility is disabled. This review did not execute that path. Is this the best way to solve the issue? The allocation instrumentation is appropriately bounded, but the cumulative packaging path needs the already-accepted predecessor correction rather than a new implementation. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: not found in the target repository. Codex review notes: model internal, reasoning medium; reviewed against 6b63951f0ac5. LabelsLabel changes:
Label 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
HistoryReview history (5 earlier review cycles)
|
Problem
The existing transport-start failure reports preserve the SDK error but cannot
establish whether a matching allocation failed, which capabilities were requested,
or how much matching-capability memory remains after the SDK returns. A generic
start error alone is not evidence of task-stack allocation failure.
Stacked on #49
(
b05b470fabf54fbc9aac687488eafb76e5fc2814).Changes
client_start; preserve its original result and existing cleanup ordering.fixed internal-memory slots capture the first matching node/operator calling
task failure. ISR and unrelated-task failures are ignored.
the SDK function pointer. No allocation, logging, heap walk, name dereference,
formatting, or waiting occurs there.
allowlisted allocator enum afterward; raw pointers and names are never logged.
phase=after_sdk_return_before_owner_cleanup. Heap values are explicitlypost-return samples, not fault-time availability or proof that an aligned
allocation would fit. Allocator identity remains distinct from unknown caller
origin.
captured=0; also preservea captured allocation followed by SDK success. Uncaptured success is silent.
final Tab5 ELF strong-hook checks to existing CI.
No stack sizes, PSRAM policy, SDK/component patches, dependencies, retry policy,
or storage behavior change. Other boards retain the no-op defaults. The SDK
registration API has no ownership getter; this profile requires no other
failed-allocation callback registration.
Validation
owner separately from the weak defaults and execute the actual transport-start
function against bounded SDK stubs.
rearming, post-return observation, exact error/cleanup behavior, callback
restrictions, absent capture, captured success, registration ordering, and
nonfatal registration failure.
nmverified strong Tab5 definitions and weak standalone defaults.callback-context assertion and passed after lookup moved outside the callback.
actionlintandgit diff --checkpassed.reviews completed without findings on the frozen eight-file diff. The subsequent
one-line CI repair also passed scoped review. Those reviews do not by
themselves authorize hardware testing.
passed the host test steps but failed all five Build steps before compilation:
an inline awk expression broke the IDF action's single-quoted command wrapper.
The one-line repair keeps the strong-symbol gate using quote-safe grep, without
early pipe termination. Offline wrapper proof is red before/green after for
all five matrix entries; strong symbols pass and weak/missing symbols fail.
including the final Tab5 strong-symbol gate. PR head is
04cea90c98c14a357b74fba331a1198242e388f4; the compiled test merge is6be43e8ae825d94db4d35d2555f99816a7838cff, with the expected base/head parentsand a tree matching the PR head.
10136593429only:2,898,645 bytes, API/archive SHA256
f37b903d4f3e6d99a25deb4c802273c6ac782c9d41d6b66ae0b173805a3bf0cf.All 11 internal checksums, 10 manifest file records, and four offset-mapped
images passed; protected data partitions do not overlap those image writes.
362a1776ec212788fda95f75b733bfdde3a0c394,v5.5.5-648-g362a1776ec2-dirty, with the existing tracked patches. SDK,component-patch, BSP, and tool records, generated configuration, and the full
parsed dependency lock match the verified
2e75363baseline. No new dependencyor configuration drift was found.
The native strong-hook result is the enforced CI gate, not a claimed local
inspection of those symbol files.
Bounded Hardware Proof
6be43e8ae825d94db4d35d2555f99816a7838cffwas flashed to M5Stack Tab5. All four writes and independent readbacks passed;
protected data was preserved.
operator roles reconnected. Observation windows were A 60.044 seconds and
B 50.104 seconds. The B budget was 60 seconds including startup, not a second
full 60-second observation window.
record was observed. Fault capture was not exercised in that run.
statusanddiagnostics status, produced oneheap sample. No node RPC, Talk, or provider request was started. No provider
key was used and no media call was made.
reset was performed; connection state after host release was not probed.
Owned host/hardware processes and serial/port handles were released.
Subsequent Allocation Capture
A later preflight on the same compiled image captured a node-role allocation
failure around WebSocket SDK startup:
requested=8192,caps=0x804,sdk_err=-1, andorigin=0(unknown). The matching-capability heap sample wasfree=45187,largest=7680, explicitly inphase=after_sdk_return_before_owner_cleanup, not at the allocation-failureinstant. The diagnostic callback therefore now has live failure-capture evidence.
The accompanying SDK task-creation error and pinned startup path are consistent
with a WebSocket task-stack allocation failure; the callback itself records an
allocator identity, not the C caller, and its origin remains unknown. This does
not identify the allocation history that exhausted a suitable contiguous block.
The trial stopped before allowance claim, provider-key loading, node RPC, Talk,
or provider execution. It supplies no audio or voice proof.
This remains instrumentation, not a memory or reconnect root-cause fix. The
bounded boot/reconnect result does not establish voice, camera, audiovisual,
or production qualification.
Landing
Accept the bounded diagnostics and observed failure capture without claiming
a memory repair. Land after #49
using the verified cumulative tree and a merge commit that retains the signed
source history and accepted predecessor repairs. Historical image proof is not
new live qualification of the cumulative tree. No release is authorized.