Fixed pin state setup when SX126X_ANT_SW is defined#10947
Conversation
|
|
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis change reorders GPIO setup for the optional SX126x antenna switch pins in ChangesSX126x Init Fix
Estimated code review effort: 1 (Trivial) | ~3 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/mesh/SX126xInterface.cpp (1)
72-78: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winSame ordering bug present in the portduino ANT_SW block.
This PR fixes the digitalWrite-before-pinMode ordering for
SX126X_ANT_SW(lines 49-50), but the identical issue remains here forportduino_config.lora_sx126x_ant_sw_pin.pin:digitalWriteis still called beforepinMode. For consistency and correctness, this should be reordered too.♻️ Proposed fix
if (portduino_config.lora_sx126x_ant_sw_pin.pin != RADIOLIB_NC) { - digitalWrite(portduino_config.lora_sx126x_ant_sw_pin.pin, HIGH); - pinMode(portduino_config.lora_sx126x_ant_sw_pin.pin, OUTPUT); + pinMode(portduino_config.lora_sx126x_ant_sw_pin.pin, OUTPUT); + digitalWrite(portduino_config.lora_sx126x_ant_sw_pin.pin, HIGH); }🤖 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/SX126xInterface.cpp` around lines 72 - 78, The portduino ANT_SW setup in SX126xInterface still calls digitalWrite before configuring the pin as OUTPUT. Update the conditional block that uses portduino_config.lora_sx126x_ant_sw_pin.pin so pinMode is set before digitalWrite, matching the corrected SX126X_ANT_SW handling and keeping the initialization order consistent.
🤖 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/SX126xInterface.cpp`:
- Around line 72-78: The portduino ANT_SW setup in SX126xInterface still calls
digitalWrite before configuring the pin as OUTPUT. Update the conditional block
that uses portduino_config.lora_sx126x_ant_sw_pin.pin so pinMode is set before
digitalWrite, matching the corrected SX126X_ANT_SW handling and keeping the
initialization order consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f15ded72-6647-471a-bdc0-1319017cc5e1
📒 Files selected for processing (1)
src/mesh/SX126xInterface.cpp
@AlexandrZloy, Welcome to Meshtastic!Thanks for opening your first pull request. We really appreciate it. We discuss work as a team in discord, please join us in the #firmware channel. Welcome to the team 😄 |
|
Agreed with coderabbitai that |
Fixed the order of pinMode and digitalWrite calls if SX126X_ANT_SW is defined.
Fixed the order of pinMode and digitalWrite calls if SX126X_ANT_SW is defined.
🤝 Attestations
Summary by CodeRabbit