fix(tab5): bound camera dequeue waits - #53
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 4). ClawSweeper reviewWhat this changesThe branch bounds Tab5 camera frame waits and adds pipeline timing coverage, alongside stacked camera compatibility, image geometry, display-memory, allocation-diagnostic, and firmware-packaging changes. Merge readiness⛔ Blocked before merge - 3 items remain The bounded wait remains absent from main, and no blocking code defect was found. The maintainer’s explicit acceptance resolves the previous cutoff-policy concern; this member-authored contribution remains useful. Priority: P2 Review scores
Verification
How this fits togetherThe Tab5 board adapter handles camera requests from the OpenClaw Gateway, acquires the camera and privacy indicator, captures a frame through ESP Video, and produces a JPEG response. The changed wait sits between starting the camera stream and transforming the selected frame. flowchart TD
A[Gateway camera request] --> B[Acquire camera and indicator]
B --> C[Configure two-second frame wait]
C --> D{Frame ready}
D -->|Yes| E[Transform and encode JPEG]
D -->|Timeout or error| F[Cleanup and release camera]
E --> F
F --> G[JPEG response or existing error]
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Preserve the accepted finite frame wait and existing cleanup semantics in a verified cumulative landing tree, keeping physical qualification and whole-request timing claims separate. Do we have a high-confidence way to reproduce the issue? Yes, at source level: main leaves ESP Video’s ready-buffer timeout infinite, so a missing frame prevents the later delayMs check from running. No physical failing reproduction was executed in this review. Is this the best way to solve the issue? Yes. The existing dependency ioctl is the narrow solution, and failure follows the existing cleanup path; the maintainer has explicitly accepted the cutoff rather than requiring additional configuration. AGENTS.md: not found in the target repository. Codex review notes: model internal, reasoning medium; reviewed against 6b63951f0ac5. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
History |
Problem
Tab5 capture leaves ESP Video's ready-buffer wait at its default
portMAX_DELAY. The existingdelayMsdeadline is checked only after asuccessful dequeue, so it does not bound a missing frame.
A recent camera attempt produced
dqbuf_beginwithout later completionevidence. That observation does not establish where it stalled; this change
addresses the independently verified unbounded ready-buffer wait and adds
stage timing without claiming an incident root cause.
Stacked on #52.
Changes
VIDIOC_S_DQBUF_TIMEOUTioctl. Fail closed through the existing capturecleanup path if configuration fails.
policy, privacy indicator, and camera/Talk ownership. A missing frame fails
without retry, including during warm-up.
capture, transform, encode, cleanup, and release. No per-frame trace or
request identity; a region returning is not proof of success or sensor
power-off.
finite wait and remaining blocking boundaries.
Validation
regressions: the missing-frame wait remains infinite, and timeout setup is
unchecked.
-Wall -Wextra -Werror. Coverage includes setup failure, missing frames,partial cleanup, repeated open, zero/ten-second virtual warm-up, starvation,
bounded stage records, transform/encode failures, and actual-handler release.
Existing transform geometry tests also pass.
git diff --checkpass. No workflow change.actionable P0-P2 findings on the frozen four-file change.
33748b8c5b43db4f1c494677e75313f07c217969:https://github.com/openclaw/esp-openclaw-node/actions/runs/34519194614.
14f511933af6fceaa0923b4a3457283eb4de36f5(app version14f5119).Its parents and tree match the reviewed base/head. Artifact
10169253664matches GitHub's SHA256
16f4d1d1f8e9bd7f8a323fe9c7e2c334470288888c087ad852e4343b94c9dc97;all 11 internal checksums, 10 manifest file entries, and four offset-mapped
images passed verification. The images do not overlap NVS, PHY, or storage.
362a1776ec212788fda95f75b733bfdde3a0c394with the existing trackedpatches, component patch provenance, generated configuration, resolved
dependency lock, and partition table match the previously verified PR52
bundle. The post-Ninja camera-source verifier and strong-symbol link checks
passed in CI. These checks do not establish reproducible builds or new
hardware qualification.
Bounded Physical Observations
14f5119image was flashed with four writes and four independentreadbacks passing. Runtime identity and
device.infoconfirmed this image.First dequeue returned in 41 ms; selected capture was reported at 508 ms,
transform at 119 ms, encode at 482 ms, cleanup at 2 ms, and release at 15 ms.
These records show the instrumented regions returned, not that a valid
JPEG reached the caller or that sensor power-off was verified.
DISCONNECTEDafter 10,816 ms. Same-nodeconnection reconciliation was recorded immediately before the failure;
host cleanup sent SIGTERM afterward. This ordering does not establish the
disconnect's cause. No JPEG was delivered, and Canvas was not dispatched.
No assertion, panic, or additional reset was observed beyond the single
controlled boot.
initial-drain-not-emptyguard, before Gateway startup or any RPC. It didnot reach the camera. This was a harness preflight stop, not an additional
firmware fault or camera qualification result.
Limits
Two seconds is a policy allowance, not a measured maximum first-frame
latency. Setup, post-wait preprocessing, PPA, JPEG, logging, and cleanup can
still block; this is not a whole-command or Gateway timeout guarantee.
No dependency, SDK patch, format, allocator, retry, reset, or Gateway change.
Host fixtures do not prove actual sensor timing, PPA/JPEG output, teardown,
or media quality. The bounded physical observations above do not establish
successful camera delivery, image quality, or a resolved disconnect cause.
Voice remains inconclusive; no successful voice qualification is claimed.
Further physical qualification remains separate from source landing.
Maintainer Disposition
Accept the two-second per-dequeue policy and its existing failure cleanup:
a camera that needs longer for a frame will now return the existing unavailable
result, including during warm-up. Keeping an indefinitely blocked dequeue is
not the selected default. This accepts that compatibility tradeoff; it does
not claim fresh/upgraded-device latency coverage or physical missing-frame
recovery. The actual-owner tests and exact-head native CI support the bounded
implementation, while the limitations above remain open.
Land after #52 with a merge
commit and verified cumulative tree. Preserve accepted predecessor repairs.
No release, whole-request timing guarantee or successful voice qualification
is implied.