fix: dynamically assign button pin to prevent false interrupts#10742
Open
LN4CY wants to merge 13 commits into
Open
fix: dynamically assign button pin to prevent false interrupts#10742LN4CY wants to merge 13 commits into
LN4CY wants to merge 13 commits into
Conversation
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 (30)
Build artifacts expire on 2026-08-18. Updated for |
Contributor
📝 WalkthroughWalkthroughButton GPIO selection now uses a shared resolver with compile-time and device-configuration sources, validates ESP32 pins, and applies the result across input initialization, sleep-wake configuration, and idle wake-event detection. ChangesButton GPIO handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant InputBroker
participant SleepManager
participant ButtonHelper
participant ESP32Sleep
participant PowerFSM
InputBroker->>ButtonHelper: resolve and validate button GPIO
InputBroker->>InputBroker: initialize valid button input
SleepManager->>ButtonHelper: resolve button GPIO
SleepManager->>ESP32Sleep: configure sleep wake sources
ESP32Sleep-->>PowerFSM: return from wake
PowerFSM->>ButtonHelper: resolve button GPIO
PowerFSM->>PowerFSM: emit EVENT_PRESS or EVENT_WAKE_TIMER
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@src/input/InputBroker.cpp`:
- Around line 170-178: Duplicate button GPIO resolution should be centralized in
an inline helper such as getResolvedButtonPin(), preserving the
USERPREFS_BUTTON_PIN, BUTTON_PIN, and config.device.button_gpio precedence. Add
the helper in the shared configuration or utility location, then replace the
duplicated blocks in src/input/InputBroker.cpp lines 170-178 and 329-337,
src/sleep.cpp lines 313-321, 440-448, and 485-493,
src/platform/esp32/main-esp32.cpp lines 355-363, and src/PowerFSM.cpp lines
180-188 with calls to it.
In `@src/platform/esp32/main-esp32.cpp`:
- Around line 367-369: Validate the resolved button GPIO before use, preferably
in the shared pin-resolution helper, using the platform’s GPIO validity check or
an equivalent bounds check. Apply this to the wake-mask shift in
main-esp32.cpp#L367-L369, the digitalRead path in PowerFSM.cpp#L189, and
OneButton instantiation in InputBroker.cpp#L348-L349; invalid pins must be
excluded so they cannot cause undefined shifts or phantom button events.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 32fa657f-7831-4f92-ba75-b84721911335
📒 Files selected for processing (4)
src/PowerFSM.cppsrc/input/InputBroker.cppsrc/platform/esp32/main-esp32.cppsrc/sleep.cpp
This addresses the CodeRabbit PR comments by moving the duplicate GPIO resolution and validation logic into ButtonHelper.h and updating the call sites.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@src/input/ButtonHelper.h`:
- Around line 17-19: Update the button GPIO assignment logic around
config.device.button_gpio so GPIO 0 can be distinguished from an unset value,
using the protobuf presence flag such as has_button_gpio when available.
Preserve the hardware-defined BUTTON_PIN fallback when no dynamic GPIO is
configured, while allowing an explicitly configured GPIO 0 to be assigned.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1ac485af-d1ad-42cc-b566-800a557c4d71
📒 Files selected for processing (5)
src/PowerFSM.cppsrc/input/ButtonHelper.hsrc/input/InputBroker.cppsrc/platform/esp32/main-esp32.cppsrc/sleep.cpp
🚧 Files skipped from review as they are similar to previous changes (4)
- src/sleep.cpp
- src/input/InputBroker.cpp
- src/PowerFSM.cpp
- src/platform/esp32/main-esp32.cpp
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.
This PR resolves the false interrupt and random wake issues caused by floating button pins on variants where the button is not physically wired (such as
rak3401_1watt), while fully restoring button functionality when a GPIO is explicitly assigned via the Meshtastic configuration.Changes Made:
#ifdef BUTTON_PINguards inInputBroker.cpp,PowerFSM.cpp,sleep.cpp, andmain-esp32.cppin favor of#if HAS_BUTTON._btnPinusing the following priority:config.device.button_gpio(if explicitly assigned at runtime)USERPREFS_BUTTON_PIN(if defined in userprefs)BUTTON_PIN(if defined in the variant header)0xFF), the firmware skips configuring the button pull-ups, skips initializingButtonThread, and ignores digital reads during sleep, avoiding any false positives on floating pins.?? Attestations
Fixes #10574
Summary by CodeRabbit