fix(stm32wl): smaller MAX_RX_TOPHONE and PACKETHISTORY_MAX on stm32wl - #11400
Conversation
⚡ Try this PR in the Web FlasherNote Building this pull request… the flash button, badges and supported-board |
📝 WalkthroughWalkthroughSTM32WL builds now use ChangesSTM32WL resource limits
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
@caveman99 |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/mesh/mesh-pb-constants.h (1)
32-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShorten the
MAX_RX_TOPHONErationale comment.Keep this comment to one or two lines. Retain only the non-obvious RAM rationale.
As per coding guidelines, comments must normally be one or two lines and must explain only non-obvious rationale.
Proposed comment reduction
-// Each slot is a ~340 B MeshPacket in the static pool (Router.cpp MAX_PACKETS_STATIC), so 32 slots -// cost ~11 KB of .bss on the RAM-tightest platform (2.8.0 field reports: 99% heap). 16 still doubles -// the 8 classic ESP32 has shipped with for years; drops start when a stalled phone/serial client has -// 16 packets queued. +// Keep 16 slots on RAM-tight NRF52840 boards to limit static MeshPacket memory. +// A stalled phone/serial client can fill the queue and cause drops.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/mesh/mesh-pb-constants.h` around lines 32 - 35, Shorten the rationale comment above MAX_RX_TOPHONE to one or two lines, retaining only the non-obvious RAM usage rationale for the slot count and removing historical, platform, and operational details.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/mesh/mesh-pb-constants.h`:
- Around line 32-35: Shorten the rationale comment above MAX_RX_TOPHONE to one
or two lines, retaining only the non-obvious RAM usage rationale for the slot
count and removing historical, platform, and operational details.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 6ce1869f-0d65-4988-a935-4f85a5589440
📒 Files selected for processing (1)
src/mesh/mesh-pb-constants.h
9199e6b
Reducing MAX_RX_TOPHONE and PACKETHISTORY_MAX for stm32wl platform, guarded by ARCH_STM32WL,
to save/maintain some free RAM/heap while ensuring client connection via its serial working.
for MAX_RX_TO_PHONE it's fixed to 8 and it's open question that number is enough or not.
while PACKETHISTORY_MAX is tied to MAX_NUM_NODES which is currently set to 10 for stm32wl.
related PRs(to be added):
🤝 Attestations
Summary by CodeRabbit