fix(promicro): exclude emoji to fit under the warm-store region - #11256
Merged
Conversation
nrf52_promicro_diy_tcxo has been failing the nrf52 warm-region guard on develop: the image ends at 0xEA0C0, 192 bytes past the 12 KB WarmNodeStore record-ring reserved at 0xEA000. This variant compiles four radio driver families (SX126x/LLCC68, SX127x/RF95, LR11x0/LR1121, LR2021) so any module can be soldered on, which makes it the largest nrf52 image we ship. Building it with -D EXCLUDE_EMOJI saves 6,808 bytes and puts the image at 0xE8618, 6.6 KB clear of the warm region. EXCLUDE_EMOJI was not previously usable: graphics::emotes[] becomes empty, but the canned-message emote picker never checked for that. Entering the picker clamped emotePickerIndex to numEmotes - 1 (i.e. -1), and selecting read emotes[-1].label into a String - an out-of-bounds read of whatever precedes the array in flash. Guard both entry points instead: refuse to open the picker when there are no emotes, and bounce back to freetext if the picker state is somehow reached anyway. Under whole-image LTO those guards fold to constants, so the picker draw and input paths dead-strip entirely on builds that set EXCLUDE_EMOJI - which is where the savings come from beyond the bitmap data itself. Received messages containing emoji render as text on this variant, and the emote-list key is a no-op. Verified rak4631 (emoji enabled) is unaffected: 0xE46D8, 22 KB clear.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe emote picker now remains closed when no emotes are available and exits safely if emote picker input is active with an empty list. The TCXO variant enables ChangesEmote picker safety
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Contributor
⚡ Try this PR in the Web Flasher
Warning This is an automated, unreviewed CI test build. Back up your device configuration Supported boards built by this PR (31)
Build artifacts expire on 2026-08-26. Updated for |
jackyes
pushed a commit
to jackyes/meshtastic-firmware
that referenced
this pull request
Jul 29, 2026
Conflict in variants/nrf52840/diy/nrf52_promicro_diy_tcxo/platformio.ini: both sides fixed the same warm-store overflow, differently. d33ceeb excluded canned messages (12360 bytes). Upstream meshtastic#11256 excludes emoji instead (6808 bytes) and additionally fixes an out-of-bounds read in the emote picker that made EXCLUDE_EMOJI unusable before. Kept upstream's flag and dropped ours: it clears the guard while leaving canned messages working on the board, which is the better outcome for the variant. Verified - the image now ends at 0xE8EE8, 4376 bytes below the 0xEA000 region, with the module present again. Only that one target was rebuilt; the rest of the merge is upstream code that their CI covers.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
nrf52_promicro_diy_tcxois failing the nrf52 warm-region guard on develop (example run):It's only 192 bytes over. This variant deliberately compiles four radio driver families (SX126x/LLCC68, SX127x/RF95, LR11x0/LR1121, LR2021) so builders can solder on whatever module they have, which makes it the largest nRF52 image we ship. Measured from the ELF: SX126x/LLCC68 24.5 KB, SX127x/RF95 14.1 KB, LR2021 12.0 KB, LR11x0/LR1121 11.4 KB.
For reference, rak4631 on the same commit ends at
0xE46D8— 22 KB clear.Fix
Build the variant with
-D EXCLUDE_EMOJI: −6,808 bytes, image ends at0xE8618, 6.6 KB clear of the warm region.That flag wasn't usable before this PR. With it set,
graphics::emotes[]is empty andnumEmotes == 0, but the canned-message emote picker never checked:drawEmotePickerScreen()clampsemotePickerIndextonumEmotes - 1, i.e. -1graphics::emotes[-1].labeland constructs aStringfrom it — an out-of-bounds read of whatever precedes the array in flashSo this adds two guards: don't open the picker when there are no emotes, and bounce back to freetext if the picker state is reached anyway. (Same latent bug applies to any board that wants this flag — it's not promicro-specific.)
A nice side effect: under whole-image LTO the guards fold to constants, so the entire picker — draw path and input handler — dead-strips on
EXCLUDE_EMOJIbuilds. That's why the saving is larger than the ~5.7 KB of bitmap data.Trade-off
On this variant only, received messages containing emoji render as their text label instead of a 16px bitmap, and the emote-list key becomes a no-op. Given the alternative is a variant that doesn't build at all, that seems like the right call — but I'm open to dropping
USE_LR2021instead (−13,504 bytes, measured), since LR2021 support landed on this variant only two weeks ago in #10998 and the readme's known-modules table doesn't list an LR2021 module yet.Verification
Both built locally at
1e982fa:nrf52_promicro_diy_tcxobefore0xEA0B0nrf52_promicro_diy_tcxoafter0xE8618rak4631(emoji enabled, unchanged)0xE46D8Summary by CodeRabbit
Bug Fixes
Configuration