fix(tab5): prefer PSRAM for larger ordinary allocations - #43
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, 1:46 AM ET / 05:46 UTC (Revision 5). ClawSweeper reviewWhat this changesThe Tab5 example makes ordinary allocations above 1 KiB prefer external PSRAM and documents fallback behavior and qualification limits. Merge readiness⛔ Blocked before merge - 1 item remains The change remains absent from main, has no actionable correctness finding, and has explicit maintainer acceptance of its qualification limits. The previous review’s accepted uncertainty does not require another approval cycle. Priority: P2 Review scores
Verification
How this fits togetherThe Tab5 firmware connects the device to OpenClaw through WebSocket clients. ESP-IDF’s allocator places their buffers and other ordinary allocations in internal RAM or external PSRAM according to the example’s configuration. flowchart TD
A[Tab5 configuration] --> B[ESP-IDF allocator]
C[WebSocket buffers and ordinary allocations] --> B
B --> D{Allocation above 1 KiB?}
D -->|Yes| E[Prefer PSRAM]
D -->|No| F[Prefer internal RAM]
E --> G[Fallback if preferred memory is full]
F --> G
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Retain the accepted Tab5-specific default and existing configuration overrides, while keeping broader memory-failure diagnosis and hardware qualification separate. Do we have a high-confidence way to reproduce the issue? No high-confidence reproduction of the reported allocation failures is established: allocation sizes and contemporaneous heap state were not captured. The preference change itself is directly supported by the SDK source. Is this the best way to solve the issue? Yes, for the accepted allocation-policy adjustment: using the existing board-specific SDK setting is narrow and maintainable, preserves explicit configurations, and avoids claiming an unproven repair. AGENTS.md: not found in the target repository. Codex review notes: model internal, reasoning medium; reviewed against 37dedfbae80e. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (4 earlier review cycles)
|
Problem
Tab5 showed WebSocket task creation failures after successful session persistence. A subsequent boot hit the
esp_hosted1.4.0 streaming receive-buffer allocation assertion. The allocation sizes and contemporaneous heap state were not captured, so a shared underlying cause is not established.The existing generated configuration prefers internal memory for ordinary allocations up to 16 KiB. Each WebSocket client allocates separate 2 KiB receive and transmit buffers with ordinary
malloc.Change
CONFIG_SPIRAM_MALLOC_ALWAYSINTERNAL=1024only in the Tab5 example defaults.Ordinary allocations above 1 KiB will prefer PSRAM. This does not change the 32 KiB internal reserve, task stack sizes or placement, TLS policy, SDIO DMA requirements, memory pools, assertions, NVS behavior, or existing SDK compatibility patch.
This is a bounded allocation-policy qualification candidate, not a claim that 1 KiB is optimal or that the SDIO failure is fixed.
Validation
git diff --checkpassed.10051460342: API SHA256, all 11 file checksums, manifest sizes/hashes, and all four image offsets verified. The partition table is unchanged, and the flash erase ranges exclude NVS.CONFIG_SPIRAM_MALLOC_ALWAYSINTERNAL, from16384to1024; the internal reserve remains32768.fd1a46a189009e4b379c12aa4e27bffec1c00c31, with parents PR42 headc8e63f6b5510156ba1b12dc4e916031823a39056and this PR headd4aae3f2f07f7d2f8a59b9df1b73c63849df6edd. Its tree matches this PR head.362a1776ec212788fda95f75b733bfdde3a0c394, built asv5.5.5-648-g362a1776ec2-dirty, with the same verified SPM compatibility patch. This is not pristine ESP-IDF 5.5.5.Source And Stack
Stacked on #42 (
fix/tab5-spm-flash-compat). Its firmware/symbol packaging and exact SDK-plus-SPM-patch provenance remain unchanged.Source inspected:
Maintainer Disposition
On September 12, 2026, accept this bounded Tab5 default using the reviewed
allocator contract, exact generated-configuration comparison, native builds,
and subsequent bounded campaign evidence. The matched
fd1a46aimage wasflashed with four readbacks and completed a 60-second node/operator saved-session
check without an allocation or SDK fault. Later transport/media failures were
not resolved by this setting; the cutoff is not claimed optimal or a complete
memory repair.
Existing retained sdkconfig and explicit overrides were not comprehensively
requalified by this PR. No configuration is reset during landing, and no new
claim is made that changing example defaults rewrites an existing configuration.
The original source, artifact, and SDK limits above remain applicable.
Land with a merge commit, retain the signed source objects, and keep voice and
broader hardware qualification separate.