Skip to content

fix(autoresearch/lpc): move pinToggleRx + ws2812SctTest edge buffers off the stack (closes #3125) - #3133

Merged
zackees merged 1 commit into
masterfrom
feat/3125-pin-toggle-stack-bomb
Jun 16, 2026
Merged

fix(autoresearch/lpc): move pinToggleRx + ws2812SctTest edge buffers off the stack (closes #3125)#3133
zackees merged 1 commit into
masterfrom
feat/3125-pin-toggle-stack-bomb

Conversation

@zackees

@zackeeszackees commented Jun 16, 2026

Copy link
Copy Markdown
Member

Summary

Closes#3125. Moves the multi-KB edge buffers from stack to right-sized static storage so the LPC845-BRK LowMemory build doesn't stack-overflow the moment the RPC handler is invoked.

What was wrong

Both LowMemory RPC handlers in examples/AutoResearch/AutoResearchLowMemory.h declared multi-KB stack buffers inside the lambda body:

// line 187 (pinToggleRx)
fl::EdgeTime edges_buf[2048]; // 8 KB on the stack// line 286 (ws2812SctTest)
fl::EdgeTime probe[1024]; // 4 KB on the stack

On LPC845-BRK (16 KB SRAM) .bss already sits at ~8.5 KB after the LowMemory build's fl::Remote + watchdog + fl::json instances. The first allocation alone pushed total SRAM demand over the 16 KB ceiling — the device crashed the moment a pinToggleRx request landed.

Verified today on hardware:

$ bash autoresearch lpc845brk --pin-toggle-rx --upload-port COM10
[lpc-pin-toggle-rx] opening COM10 @ 115200
[lpc-pin-toggle-rx] echo ok # device boots; echo RPC works
--- 1.1 / 1 kHz (1000 Hz / 200 ms) ---
FAIL: no response or RPC error: None
--- 1.2 / 10 kHz ---
FAIL: no response or RPC error: None
--- 1.3 / 100 kHz ---
FAIL: no response or RPC error: None

Echo round-trips cleanly (handler doesn't allocate). pinToggleRx silently never replies (handler allocates 8 KB → stack overflow → trap before any reply).

The fix

Two changes to examples/AutoResearch/AutoResearchLowMemory.h:

  1. pinToggleRx — buffer right-sized from 2048 to 512 entries and moved to static storage. Cap clamp updated to match. The bench cases at 1 / 10 / 100 kHz all converge sigma well before 256 period pairs, so 512 entries is comfortable.
  2. ws2812SctTest — probe right-sized from 1024 to 512 entries, also static. Diagnostic-only buffer; decoder accuracy is controlled by cfg.buffer_size, not by this output probe.

Net RAM budget change:

SiteBeforeAfter
pinToggleRx::edges_buf8 KB stack at every call2 KB permanent .bss
ws2812SctTest::probe4 KB stack at every call2 KB permanent .bss
Totalup to 12 KB stack pressure4 KB .bss

Handlers are invoked serially over JSON-RPC, so sharing the static storage across calls is safe.

Verification

Refs

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

  • Refactor
    • Optimized memory usage in low-memory configurations, reducing buffer allocations and improving efficiency on memory-constrained systems.

…off the stack
Closes#3125.
Both LowMemory RPC handlers in AutoResearchLowMemory.h had multi-KB
stack-allocated edge buffers that overflowed the LPC845-BRK's 16 KB
SRAM when invoked:
* pinToggleRx: fl::EdgeTime edges_buf[2048] -- 8 KB on the stack
* ws2812SctTest: fl::EdgeTime probe[1024] -- 4 KB on the stack
.bss for the unified LowMemory build on LPC845-BRK already sits at
~8.5 KB, so the first allocation alone pushed total SRAM demand over
the 16 KB ceiling. Symptom: echo RPC round-trips cleanly, but the
moment a pinToggleRx request lands the device goes silent and never
replies. Verified today on hardware via bash autoresearch lpc845brk
--pin-toggle-rx --upload-port COM10.
Fix shape (from the issue body's recommended path):
* Right-size the pinToggleRx buffer from 2048 to 512 entries
(constexpr kLowMemEdgeBufSize) and move it to `static` storage. The
bench cases at 1 / 10 / 100 kHz all converge sigma well before 256
period pairs, so 512 entries -- 256 pairs -- is comfortably enough.
Cap clamp updated to match.
* Right-size the ws2812SctTest probe buffer from 1024 to 512 entries
(constexpr kLowMemWs2812ProbeSize), also `static`. Diagnostic-only;
decoder accuracy is controlled by cfg.buffer_size not by this
output probe.
Result on RAM budget: ~12 KB stack pressure replaced with 4 KB
permanent .bss (split across the two handler-local statics). Net SRAM
ceiling stays under 16 KB even with the handler running.
The handlers are invoked serially over JSON-RPC, so sharing the
storage across calls is safe -- no concurrency concern.
Doesn't affect the macro retirement tracked under #3128; that's a
separate ws2812SctTest gate cleanup, not the stack-frame fix this PR
addresses.
Verification:
* Build still fits 64 KB flash (no regression on the #3076 budget).
* bash lint -- clean.
* On-hardware re-test of bash autoresearch lpc845brk --pin-toggle-rx
is the final acceptance gate; expected outcome is non-None RPC
replies with the 1 kHz case showing success=1.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Jun 16, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e5fb2771-31cf-4892-a991-6370bd33540b

📥 Commits

Reviewing files that changed from the base of the PR and between ebfda30 and b3414a5.

📒 Files selected for processing (1)
  • examples/AutoResearch/AutoResearchLowMemory.h

📝 Walkthrough

Walkthrough

AutoResearchLowMemory.h introduces kLowMemEdgeBufSize = 512 and kLowMemWs2812ProbeSize = 512 constants. The pinToggleRx edge capture count is clamped to 512, and its edges_buf is moved from a 2048-entry stack array to static storage. The ws2812SctTest probe buffer is similarly moved to static storage at 512 entries, down from 1024.

Changes

LPC handler buffer reduction

Layer / File(s)Summary
pinToggleRx capture sizing and static buffer
examples/AutoResearch/AutoResearchLowMemory.h
Introduces kLowMemEdgeBufSize = 512, clamps the computed edge capture count to that value (minimum 256), and changes edges_buf from a 2048-entry stack-allocated array to a static array sized to kLowMemEdgeBufSize.
ws2812SctTest static probe buffer reduction
examples/AutoResearch/AutoResearchLowMemory.h
Introduces kLowMemWs2812ProbeSize = 512, changes the RX diagnostic probe from a 1024-entry stack array to a static 512-entry array, and updates the getRawEdgeTimes span accordingly.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related issues

Possibly related PRs

  • FastLED/FastLED#3041: Introduced AutoResearchLowMemory.h and the original pinToggleRx/ws2812SctTest handlers whose oversized stack buffers are being reduced in this PR.

Poem

🐇 Hoppin' through the RAM so tight,
Two thousand slots? A wasteful sight!
Five twelve is plenty for each edge,
Static storage — that's my pledge.
No more stack crash in the night! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title concisely describes the main change: moving edge buffers from stack to static storage for the pinToggleRx and ws2812SctTest handlers, and references the closed issue #3125.
Linked Issues check✅ PassedThe PR implementation fully addresses all coding requirements from #3125: buffers moved to static storage, right-sized from 2048/1024 to 512 entries, and handler caps updated accordingly.
Out of Scope Changes check✅ PassedAll changes are directly scoped to addressing the stack overflow issue in #3125; no unrelated modifications are present in the changeset.
Docstring Coverage✅ PassedDocstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/3125-pin-toggle-stack-bomb

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@zackees
zackees merged commit fbd663b into masterJun 16, 2026
7 of 8 checks passed
@fastled-project-syncfastled-project-syncBot moved this from Triage to Done in FastLED TrackerJun 16, 2026
@zackees
zackees deleted the feat/3125-pin-toggle-stack-bomb branch August 21, 2026 19:15
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

lpc845: pinToggleRx + ws2812SctTest handlers stack-overflow at runtime (8 KB stack buffer on 16 KB SRAM)

1 participant

@zackees