chore: add bounded Tab5 media stage diagnostics - #47
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, 11:50 AM ET / 15:50 UTC (Revision 8). ClawSweeper reviewWhat this changesAdds bounded camera and Talk diagnostics, prevents unsafe SDK logging during Talk setup, and carries the stacked SDIO PSRAM allocation repair with packaging and regression coverage. Merge readiness⛔ Blocked before merge - 2 items remain The diagnostics remain useful and absent from main. The prior logging defect is repaired, and the recorded maintainer disposition accepts the compatibility tradeoffs; no new blocking defect was found. Priority: P2 Review scores
Verification
How this fits togetherThe room-node firmware handles Gateway camera commands and voice calls through board capture, WebRTC signaling, and local audio resources. These changes expose fixed diagnostic stages and control logging before negotiation; the included SDIO repair changes where receive buffers are allocated. flowchart TD
A[Gateway commands and wake events] --> B[Room-node media controller]
B --> C[Camera capture]
B --> D{Safe SDK logging?}
D -->|Yes| E[WebRTC signaling and audio]
D -->|No| F[Local teardown]
C --> G[Bounded diagnostic records]
E --> G
F --> G
H[SDIO receive buffers in PSRAM] --> E
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Retain the bounded diagnostics and accepted privacy guard with the pinned allocation contract, preserve signed history through the requested merge commit, and keep release qualification separate. Do we have a high-confidence way to reproduce the issue? Not applicable to the primary diagnostic change; the underlying physical camera and RTC failures are not claimed as reproduced or repaired here. Source inspection confirms the prior logging defect is addressed. Is this the best way to solve the issue? Yes. Fixed-field instrumentation preserves existing media ownership and callback behavior, and the maintainer explicitly accepts refusal instead of allowing potentially private SDP logging. AGENTS.md: not found in the target repository. Codex review notes: model internal, reasoning medium; reviewed against dc5c8917467a. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (7 earlier review cycles)
|
Problem
Tab5 camera capture currently collapses several BSP/V4L2 failures into one unavailable result, while Talk setup lacks enough stage evidence to distinguish signaling, HTTP, peer startup and teardown. The physical media failure causes remain unproven; this is diagnostic instrumentation, not a camera or RTC repair.
Stacked on #46 (
fix/tab5-sdio-rx-psram). This PR does not duplicate or change that branch's SDK/component patches or allocator policy.Changes
The log-policy refusal is the only intentional runtime behavior change. Public ABI, SDK/configuration, media ownership, cleanup, capture policy, timeouts, retries, callback return handling and existing ignored HTTP setter returns remain unchanged. Records contain fixed stages, numeric results and flags, not SDP, URLs, credentials, identities or remote error text. Room results retain native stage semantics: timer
pdPASS=1is success, and peer enum values are not error codes.Logging Review Follow-up
Head
e0ca61f313316a2d1c6bd6bc16fd1b18a879b364fixes the no-tag setter changing global verbosity and preserves already quieter policies. It also rejects static logging with a WARN default but an INFO compiled maximum: the getter alone is not evidence that INFO cannot be emitted. No SDK or configuration change is required.The actual-owner no-tag regression failed before the repair, and 48 lifecycle cases passed across the initial logging modes. The additional static-mode regression failed on the prior guard and passes with the correction; all four focused tagged, no-tag, static-unsafe and static-quiet cases pass with ASan/UBSan. Existing teardown, no-negotiation refusal and global-level preservation are checked. Fresh scoped and independent delta reviews are clear. New-head CI, attempt 1, passed all five builds. The earlier hosted and physical evidence below remains tied to its original source/image, not a new live test of this repair.
Validation
actionlint, diff whitespace checks and the public-data scrub passed. Test dependencies use the pinned SDK/WebRTC/Unity sources and registry-verified source/header bytes.The Mach-O-only ROM fixture adaptation changes no firmware memory placement. All five jobs passed in CI run 34318348917, attempt 1 for PR head
32697940a996a742390407142828e6397b80fe24. Tab5 firmware was compiled from test merge9ec6242a08f97c6c450ef4d3df48302c77aad7b7; its parents match this head and the PR46 base, and its tree matches this head.The downloaded Tab5 firmware archive matches the GitHub artifact SHA256. All 11 file checksums and four flash-image offsets were verified; SDK/component patch metadata, configuration and the partition table match PR46, and the flash ranges exclude NVS.
Bounded Hardware Proof
9ec6242a08f97c6c450ef4d3df48302c77aad7b7image was flashed to M5Stack Tab5. All four writes and four independent readbacks passed. NVS, PHY, storage, C6 firmware and eFuses were preserved; no full-device erase or force override was used.Independent review accepted the bounded keyless evidence for one diagnostic Talk attempt, not production qualification. The subsequent bounded trial stopped before
talk.startdispatch because the test verifier rejected a connect-diagnostic record. No assertion or SDIO fault was observed, and no RTC or media result was obtained. This rejection does not establish a PR47 firmware defect. No OpenAI/provider request was verified for that trial; its actual provider request count remains unknown. These observations do not establish camera, RTC or audible-output qualification.Maintainer Disposition
On September 12, 2026, accept the intentional fail-closed logging policy and
the reviewed default-profile builds plus actual-owner mode regressions.
Custom no-tag dynamic INFO builds and static builds that compile INFO or
higher must use a safe logging configuration before Talk can start; preserving
potential SDP disclosure is not an acceptable compatibility fallback.
Native fresh-build and upgrade qualification of every custom logging mode
remains a limitation, not a claimed test pass. The accepted source repair
preserves global verbosity and existing teardown.
Land this bounded diagnostics/privacy change with a merge commit and retain
its signed source history. This disposition does not assert successful
end-to-end voice or full media qualification and does not authorize a release.