feat(lr2021): enable LR2021 LoRa radio support on native/portduino - #11138
feat(lr2021): enable LR2021 LoRa radio support on native/portduino#11138a-li3n wants to merge 7 commits into
Conversation
Add full LR2021 radio support for meshtasticd on Linux native builds, including USB (CH341) and SPI (GPIO) configurations. Key changes: PortduinoGlue (config plumbing): - Add use_lr2021 to lora_module_enum and loraModules map - Add lr2021_max_power, lr2021_max_power_hf, lr2021_irq_dio_num config fields - Parse LR2021_MAX_POWER, LR2021_MAX_POWER_HF, IRQ_DIO_NUM (and LR2021_IRQ_DIO_NUM alias) from YAML - Use LR2021::MODE_* enums for rfswitch_table when module is lr2021 (LR2021 has MODE_RX_HF at value 4 where LR11x0 has MODE_TX_HP; no MODE_GNSS/MODE_WIFI) - Parse MODE_RX_HF YAML key for LR2021 configs instead of MODE_TX_HP - Branch YAML serialization on use_lr2021 so configs round-trip correctly LR20x0Interface (radio driver): - Set irqDioNum BEFORE lora.begin() so config() programs the correct DIO for IRQ routing. Setting it after begin() is too late - config() has already configured DIO5 as IRQ, then setRfSwitchTable() overrides it to RF_SWITCH, breaking all radio interrupts. This is the fix for the DIO5 IRQ/RF-switch conflict that caused meshtasticd to crash (SIGABRT/segfault) on TX when DIO5 is used for both IRQ and RF switch control. - Add ARCH_PORTDUINO path for reading irqDioNum from config - Add LR20X0_RFSWITCH_NATIVE guard for variant-defined RF switch tables LR11x0Interface (symbol namespacing): - Namespace RF switch symbols as lr11x0_rfswitch_* to prevent collisions when LR11x0 and LR20x0 templates share a translation unit (InterfacesTemplates.cpp). Removes the old #undef hack. RadioInterface: - Add case use_lr2021 to construct LR2021Interface in portduino mode - Guard hardcoded SPI pin fallback with !defined(ARCH_PORTDUINO) ProMicro rfswitch.h: - Add per-table include guards (RFSWITCH_NRF52TCXO_LR11X0_DEFINED, RFSWITCH_NRF52TCXO_LR20X0_DEFINED) to prevent redefinition - Nest #ifdef USE_LR1121 / #ifdef USE_LR2021 inside include guards so tables are only emitted when their radio is built - Add LR20X0_RFSWITCH_NATIVE define so LR20x0Interface uses the variant's native lr20x0_rfswitch_* symbols - Add explanatory comment for DIO7 (in pin array for matrix completeness, never driven HIGH on this variant) Native build config: - Add -D USE_LR2021 to portduino.ini build flags Sample configs: - lora-femtofox_LR2021_TCXO.yaml: SPI/GPIO config for Luckfox Pico Mini with GNiceRF LoRa2021F33-2G4 module (DIO3 TCXO, DIO5-8 RF switch, IRQ_DIO_NUM: 9 to avoid DIO5 conflict) - lora-usb-meshtoad-nicerflora2021f33.yaml: USB/CH341 config for NiceRF LoRa2021F33 module Co-authored-by: jessm33 <112707725+jessm33@users.noreply.github.com> Ref: meshtastic#10567 (portions of LR11x0 symbol namespacing, ProMicro rfswitch.h guards, and USB config YAML adapted from this PR)
|
|
📝 WalkthroughWalkthroughAdds LR2021 radio configurations and integrates LR2021 module selection, YAML parsing and emission, IRQ routing, RF-switch tables, native build flags, and NRF52 RF-switch guards. ChangesLR2021 integration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PortduinoConfig
participant RadioInterface
participant LR20x0Interface
participant RadioLib
PortduinoConfig->>RadioInterface: select use_lr2021
RadioInterface->>LR20x0Interface: construct LR2021Interface
LR20x0Interface->>RadioLib: configure IRQ DIO and RF-switch table
LR20x0Interface->>RadioLib: call lora.begin()
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/mesh/LR20x0Interface.cpp (1)
87-91: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep new comments within the minimal-comment convention.
These explanatory blocks are longer than the required one or two lines. Retain only the non-obvious reason at each site.
src/mesh/LR20x0Interface.cpp#L87-L91: condense the IRQ-ordering explanation to one short comment.variants/nrf52840/diy/nrf52_promicro_diy_tcxo/rfswitch.h#L38-L54: condense the RF-switch topology and alias rationale to short reason-focused comments.As per coding guidelines, comments must be minimal and limited to one or two lines.
🤖 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/LR20x0Interface.cpp` around lines 87 - 91, The comments at src/mesh/LR20x0Interface.cpp#L87-L91 and variants/nrf52840/diy/nrf52_promicro_diy_tcxo/rfswitch.h#L38-L54 exceed the minimal-comment convention. Condense the IRQ-ordering comment near the LR20x0 initialization to one short, reason-focused line, and shorten the RF-switch topology and alias comments in rfswitch.h to one or two lines each while preserving only their non-obvious rationale.Source: Coding guidelines
src/platform/portduino/PortduinoGlue.cpp (1)
902-909: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winNo validation that
IRQ_DIO_NUMis in range or avoids RF-switch pins.The header comment documents
lr2021_irq_dio_numas "set to 5-11 to override", but nothing here rejects out-of-range values or values that collide with DIO pins already claimed byrfswitch_table(e.g. DIO5 in the shipped Femtofox/meshtoad configs). Given this field exists specifically to avoid the DIO5 IRQ/RF-switch conflict this PR addresses, a silent misconfiguration here reintroduces that exact conflict.🛡️ Proposed fix to add a basic range check
if (yamlConfig["Lora"]["IRQ_DIO_NUM"]) portduino_config.lr2021_irq_dio_num = yamlConfig["Lora"]["IRQ_DIO_NUM"].as<int>(0); else if (yamlConfig["Lora"]["LR2021_IRQ_DIO_NUM"]) portduino_config.lr2021_irq_dio_num = yamlConfig["Lora"]["LR2021_IRQ_DIO_NUM"].as<int>(0); + if (portduino_config.lr2021_irq_dio_num != 0 && + (portduino_config.lr2021_irq_dio_num < 5 || portduino_config.lr2021_irq_dio_num > 11)) { + LOG_WARN("IRQ_DIO_NUM %d out of valid range [5, 11]; using chip default", portduino_config.lr2021_irq_dio_num); + portduino_config.lr2021_irq_dio_num = 0; + }🤖 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/platform/portduino/PortduinoGlue.cpp` around lines 902 - 909, Validate the value assigned through the LR2021_IRQ_DIO_NUM/IRQ_DIO_NUM handling in the Portduino configuration loader before storing it in portduino_config.lr2021_irq_dio_num. Accept only the documented override range of DIO5 through DIO11, and reject or fall back for values that conflict with pins already claimed by rfswitch_table, including DIO5 in affected configurations.src/platform/portduino/PortduinoGlue.h (1)
80-83: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winGuard the
use_lr2021module-map registration to match the guarded consumer code.Root cause:
loraModulesunconditionally mapsuse_lr2021 -> "lr2021", while every consumer oflora_module == use_lr2021(mode-table init/value population in the .cpp, numModes/mode-key emission in the .h) is correctly wrapped in#if !RADIOLIB_EXCLUDE_LR2021. On a build that excludes LR2021, a user-suppliedModule: lr2021would still resolve successfully via the map, then silently fall through to the LR11x0 RF-switch branch with mismatchedOpMode_tvalues instead of being rejected.
src/platform/portduino/PortduinoGlue.h#L80-L83: wrap the{use_lr2021, "lr2021"}map entry in#if !RADIOLIB_EXCLUDE_LR2021 ...#endif`` so an excluded build can't resolveModule: lr2021to a valid enum value (it will then hit the existing "Unknown Lora.Module" error path).src/platform/portduino/PortduinoGlue.cpp#L976-L1001: no code change needed here once the map is guarded — kept as the illustrating consumer site; verify the "Unknown Lora.Module" exit path inloadConfig()(cpp) is reached for excluded builds after the map fix.♻️ Proposed guard for the map entry
std::map<lora_module_enum, std::string> loraModules = {{use_simradio, "sim"}, {use_autoconf, "auto"}, {use_rf95, "RF95"}, {use_sx1262, "sx1262"}, {use_sx1268, "sx1268"}, {use_sx1280, "sx1280"}, {use_lr1110, "lr1110"}, {use_lr1120, "lr1120"}, {use_lr1121, "lr1121"}, - {use_llcc68, "LLCC68"}, {use_lr2021, "lr2021"}}; + {use_llcc68, "LLCC68"} +#if !RADIOLIB_EXCLUDE_LR2021 + , {use_lr2021, "lr2021"} +#endif + };🤖 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/platform/portduino/PortduinoGlue.h` around lines 80 - 83, The loraModules map unconditionally registers use_lr2021 despite guarded LR2021 consumers. In src/platform/portduino/PortduinoGlue.h lines 80-83, wrap the use_lr2021 map entry in `#if` !RADIOLIB_EXCLUDE_LR2021/#endif so excluded builds reject Module: lr2021 through the existing Unknown Lora.Module path. In src/platform/portduino/PortduinoGlue.cpp lines 976-1001, make no code change; verify loadConfig() reaches that rejection path when LR2021 is excluded.
🤖 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/mesh/LR20x0Interface.cpp`:
- Around line 92-104: Update the ARCH_PORTDUINO branch that assigns
lora.irqDioNum from portduino_config.lr2021_irq_dio_num to validate the value
against the contract: allow 0 or 5–11, reject or fall back for other values, and
prevent conflicts with DIO5, DIO6, DIO7, DIO8, or DIO10 used by
setRfSwitchTable().
---
Nitpick comments:
In `@src/mesh/LR20x0Interface.cpp`:
- Around line 87-91: The comments at src/mesh/LR20x0Interface.cpp#L87-L91 and
variants/nrf52840/diy/nrf52_promicro_diy_tcxo/rfswitch.h#L38-L54 exceed the
minimal-comment convention. Condense the IRQ-ordering comment near the LR20x0
initialization to one short, reason-focused line, and shorten the RF-switch
topology and alias comments in rfswitch.h to one or two lines each while
preserving only their non-obvious rationale.
In `@src/platform/portduino/PortduinoGlue.cpp`:
- Around line 902-909: Validate the value assigned through the
LR2021_IRQ_DIO_NUM/IRQ_DIO_NUM handling in the Portduino configuration loader
before storing it in portduino_config.lr2021_irq_dio_num. Accept only the
documented override range of DIO5 through DIO11, and reject or fall back for
values that conflict with pins already claimed by rfswitch_table, including DIO5
in affected configurations.
In `@src/platform/portduino/PortduinoGlue.h`:
- Around line 80-83: The loraModules map unconditionally registers use_lr2021
despite guarded LR2021 consumers. In src/platform/portduino/PortduinoGlue.h
lines 80-83, wrap the use_lr2021 map entry in `#if`
!RADIOLIB_EXCLUDE_LR2021/#endif so excluded builds reject Module: lr2021 through
the existing Unknown Lora.Module path. In
src/platform/portduino/PortduinoGlue.cpp lines 976-1001, make no code change;
verify loadConfig() reaches that rejection path when LR2021 is excluded.
🪄 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: 8554a247-6c38-4ad0-9d7a-4d1b372db7a6
📒 Files selected for processing (9)
bin/config.d/lora-femtofox_LR2021_TCXO.yamlbin/config.d/lora-usb-meshtoad-nicerflora2021f33.yamlsrc/mesh/LR11x0Interface.cppsrc/mesh/LR20x0Interface.cppsrc/mesh/RadioInterface.cppsrc/platform/portduino/PortduinoGlue.cppsrc/platform/portduino/PortduinoGlue.hvariants/native/portduino.inivariants/nrf52840/diy/nrf52_promicro_diy_tcxo/rfswitch.h
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Enables LR2021 LoRa radio support for native/portduino builds by wiring LR2021 into portduino config parsing/serialization, constructing the correct radio interface, and fixing RF-switch/IRQ DIO routing interactions.
Changes:
- Add LR2021 as a selectable LoRa module in portduino, with new LR2021-specific config fields and YAML round-tripping.
- Update RF switch table handling to use LR2021 op-mode enums and avoid symbol collisions between LR11x0 and LR20x0 drivers.
- Enable LR2021 in native build flags and add sample USB + SPI YAML configurations.
Reviewed changes
Copilot reviewed 7 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| variants/nrf52840/diy/nrf52_promicro_diy_tcxo/rfswitch.h | Adds per-radio RF switch symbols/guards and LR20x0 native symbol hint for LR2021. |
| variants/native/portduino.ini | Enables LR2021 support in native/portduino build flags. |
| src/platform/portduino/PortduinoGlue.h | Adds LR2021 module enum + config fields and adjusts YAML serialization for LR2021 RF-switch modes. |
| src/platform/portduino/PortduinoGlue.cpp | Parses LR2021 power/IRQ DIO config and loads RF-switch tables using LR2021 vs LR11x0 mode enums. |
| src/mesh/RadioInterface.cpp | Constructs LR2021Interface for portduino and avoids embedded fallback path on portduino builds. |
| src/mesh/LR20x0Interface.cpp | Fixes IRQ DIO routing timing and introduces native/aliased RF-switch symbol handling for LR20x0. |
| src/mesh/LR11x0Interface.cpp | Namespaces LR11x0 RF-switch symbols to avoid collisions. |
| bin/config.d/lora-usb-meshtoad-nicerflora2021f33.yaml | Adds sample USB/CH341 LR2021 configuration. |
| bin/config.d/lora-femtofox_LR2021_TCXO.yaml | Adds sample SPI/GPIO LR2021 configuration with IRQ DIO override and RF-switch table. |
Address bot review comments on PR meshtastic#11138: 1. LR20x0Interface.cpp: Validate portduino_config.lr2021_irq_dio_num against the LR2021 contract (DIO 5-11, 0=default) before assigning to lora.irqDioNum. Warn on out-of-range values instead of silently passing them to RadioLib's setDioFunction(). 2. PortduinoGlue.cpp: Validate IRQ_DIO_NUM/LR2021_IRQ_DIO_NUM at YAML load time (accept 0 or 5-11, reject and warn for others) so bad config is caught before reaching the driver. 3. PortduinoGlue.h: Wrap the use_lr2021 loraModules map entry in #if !RADIOLIB_EXCLUDE_LR2021 so excluded builds reject Module: lr2021 through the existing Unknown Lora.Module path. Nitpick (comment condensing): skipped — the IRQ-ordering and rfswitch topology comments explain non-obvious root causes that cost real debugging time; condensing would lose essential context.
The previous range check (5-11) didn't catch the case where the user sets IRQ_DIO_NUM to a DIO that's already claimed by the RF switch table (DIO5-8). This would cause setRfSwitchTable() to reconfigure the IRQ pin as an RF switch output, silently breaking interrupts. Now compares the requested IRQ DIO number against the configured rfswitch_dio_pins array using the RFSWITCH_PIN_FLAG encoding. Falls back to the chip default with a LOG_WARN if a conflict is detected.
@a-li3n, 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 😄 |
Address Copilot review comments on PR meshtastic#11138: 1. RadioInterface.cpp: Wrap case use_lr2021 in #if defined(USE_LR2021) && RADIOLIB_EXCLUDE_LR2021 != 1 so builds that exclude LR2021 don't fail on the undefined class. 2. LR20x0Interface.cpp: Reorder the irqDioNum #if chain so ARCH_PORTDUINO (runtime YAML config) is checked first, before compile-time LR2021_IRQ_DIO_NUM / IRQ_DIO_NUM macros. This lets users override IRQ routing via config.yaml without rebuilding. 3. PortduinoGlue.cpp: Correct the OpMode_t enum values in the comment — STBY=0 (MODE_IDLE), not STBY=1. The actual enum values start at 0, not 1.
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/platform/portduino/PortduinoGlue.cpp`:
- Around line 987-988: Update the RF-switch mode comment adjacent to the
LR11x0/LR2021 mapping to accurately state LR2021 mode 3 as RX_HF and mode 4 as
TX_HF, while condensing the explanation to no more than two lines. Preserve the
existing mappings and clarify that LR2021 lacks TX_HP, GNSS, and WIFI.
🪄 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: 4267f884-c6db-4d6c-9ec6-1d50316adc36
📒 Files selected for processing (3)
src/mesh/LR20x0Interface.cppsrc/mesh/RadioInterface.cppsrc/platform/portduino/PortduinoGlue.cpp
🚧 Files skipped from review as they are similar to previous changes (2)
- src/mesh/RadioInterface.cpp
- src/mesh/LR20x0Interface.cpp
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 9 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
src/platform/portduino/PortduinoGlue.cpp:1016
- The pin parsing only assigns when the string matches known values; otherwise the existing value in
portduino_config.rfswitch_dio_pins[i]is retained. This can lead to stale/undefined pins when YAML provides fewer entries or contains a typo. Initialize each slot toRADIOLIB_NCat the start of the loop and consider anelsebranch that logs a warning for unrecognized pin strings.
for (int i = 0; i < 5; i++) {
// set up the pin array first
// DIO pin constants are the same for LR11x0 and LR2021 (both use LRXXXX_DIOx)
if (yamlConfig["Lora"]["rfswitch_table"]["pins"][i].as<std::string>("") == "DIO5")
portduino_config.rfswitch_dio_pins[i] = RADIOLIB_LR11X0_DIO5;
if (yamlConfig["Lora"]["rfswitch_table"]["pins"][i].as<std::string>("") == "DIO6")
portduino_config.rfswitch_dio_pins[i] = RADIOLIB_LR11X0_DIO6;
…comment Address Copilot review comments on PR meshtastic#11138: 1. LR20x0Interface.cpp: When lr2021_irq_dio_num == 0 (use default), the default DIO5 may also conflict with the RF switch table. Added a warning check for the default case. Also changed the explicit conflict warning to instruct the user to set IRQ_DIO_NUM instead of silently falling back to a potentially conflicting default. 2. PortduinoGlue.h: Replaced numeric enum values in the comment with slot ordering (slot 3 = RX_HF, slot 4 = TX_HF) to avoid confusion with the PR description's different numbering. Condensed to 2 lines. Skipped: r3627106736 (reset values[i] to LOW before YAML override) — this is pre-existing behavior for all radio modules in loadConfig(), not introduced by our PR. Fixing it would change behavior for LR11x0/SX1262/etc. and is out of scope.
caveman99
left a comment
There was a problem hiding this comment.
The initialization order of LR1x and LR2x chips may not be changed. This is a previous regression where the switch table is ignored if it is set too early. If you need safeguards on LR2x irq line, you need to do it differently or file a report against radiolib.
Also the LF/HF power limits are done intrinsically for LR1x chips - don't introduce a second verbatim mechanism for LR2x.
Did you actually check with a power meter this works?
Per maintainer review on PR meshtastic#11138: 1. Revert irqDioNum conflict detection and reordering rationale: The maintainer states that changing the initialization order of LR1x/LR2x chips is a known regression — setting the switch table too early causes it to be ignored. Reverted the long comment and conflict detection logic. The ARCH_PORTDUINO config path is kept (runtime YAML config for IRQ_DIO_NUM), but the user is responsible for ensuring IRQ_DIO_NUM doesn't conflict with RF switch pins. Range validation (5-11) is retained as a simple guard. 2. Remove LR2021_MAX_POWER / LR2021_MAX_POWER_HF config plumbing: RadioLib's setOutputPower()/checkOutputPower() already handles LF/HF power clamping intrinsically (-9..22 dBm sub-GHz, -19..12 dBm HF). Removed the portduino_config.lr2021_max_power and lr2021_max_power_hf struct fields, YAML parsing, serialization, and the #if ARCH_PORTDUINO defines in LR20x0Interface.cpp that referenced them. The compile-time fallbacks (22/12) remain as defaults matching the chip's intrinsic limits. 3. Removed LR2021_MAX_POWER / LR2021_MAX_POWER_HF from sample YAML configs since they're no longer parsed.
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/mesh/LR20x0Interface.cpp`:
- Around line 31-33: Shorten the RadioLib power-clamping comment at
src/mesh/LR20x0Interface.cpp lines 31-33 to no more than two lines while
retaining its essential meaning. Also condense the IRQ initialization rationale
at src/mesh/LR20x0Interface.cpp lines 81-84 to no more than two lines; no
implementation changes are needed.
🪄 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: 25d15c6c-38c8-49fd-b12a-58c54aeca350
📒 Files selected for processing (5)
bin/config.d/lora-femtofox_LR2021_TCXO.yamlbin/config.d/lora-usb-meshtoad-nicerflora2021f33.yamlsrc/mesh/LR20x0Interface.cppsrc/platform/portduino/PortduinoGlue.cppsrc/platform/portduino/PortduinoGlue.h
💤 Files with no reviewable changes (4)
- bin/config.d/lora-femtofox_LR2021_TCXO.yaml
- bin/config.d/lora-usb-meshtoad-nicerflora2021f33.yaml
- src/platform/portduino/PortduinoGlue.h
- src/platform/portduino/PortduinoGlue.cpp
| // LR2021 power limits are handled intrinsically by RadioLib's setOutputPower()/checkOutputPower(), | ||
| // which clamps to -9..22 dBm (sub-GHz PA) or -19..12 dBm (HF PA) based on the frequency band. | ||
| // No module-level override is needed - the chip enforces its own limits. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Shorten the new comments to two lines or fewer. Both blocks exceed the project’s comment-length limit without adding implementation detail that requires this much prose.
src/mesh/LR20x0Interface.cpp#L31-L33: condense the RadioLib power-clamping explanation to one or two lines.src/mesh/LR20x0Interface.cpp#L81-L84: condense the IRQ initialization rationale to one or two lines.
As per coding guidelines, “Keep code comments minimal—one or two lines maximum.”
📍 Affects 1 file
src/mesh/LR20x0Interface.cpp#L31-L33(this comment)src/mesh/LR20x0Interface.cpp#L81-L84
🤖 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/LR20x0Interface.cpp` around lines 31 - 33, Shorten the RadioLib
power-clamping comment at src/mesh/LR20x0Interface.cpp lines 31-33 to no more
than two lines while retaining its essential meaning. Also condense the IRQ
initialization rationale at src/mesh/LR20x0Interface.cpp lines 81-84 to no more
than two lines; no implementation changes are needed.
Source: Coding guidelines
caveman99
left a comment
There was a problem hiding this comment.
I have ONE request sincerely from the heart. May i talk to a human? Cause there is one on this side for sure. Being addressed as 'the maintainer' in your latest commit message is disrespectful at least.
Don't meddle in the init sequence of things you don't fully understand or hope the bot got right. If a change goes this deeply into the hardware part of things, always validate and test your changes, don't just trust the bot.
Again: did you validate these chages actually work with a portduino connected LR2021?
|
@caveman99 - I meant no disrespect, so I definitely apologize for that. I've been testing every commit, however, the latest one I haven't been able to test yet (only have one LR2021F33 at the moment), but I plan to this morning. Additionally, I had to outsource the power meter testing as I don't own one so I'm waiting to hear back with the results. |
|
With the removal of the config options LR2021_MAX_POWER / LR2021_MAX_POWER_HF what options are there for board that need to limit the power to lower then the lr2021 max? For example to protect a amp or prevent overloading a usb port. If this option is completely unnecessary shouldn’t be removed completely instead just removing one way to set it? There’s still defines for it in code just now it can’t be changed without code changes. What is the purpose of |
|
changing |
… screen from config. Requires meshtastic/device-ui#355 and supersedes #10567 and #11138 Many thanks to the original authors https://github.com/a-li3n and https://github.com/jessm33
… screen from config. Requires meshtastic/device-ui#355 and supersedes #10567 and #11138 Many thanks to the original authors https://github.com/a-li3n and https://github.com/jessm33
Add full LR2021 radio support for meshtasticd on Linux native builds, including USB (CH341) and SPI (GPIO) configurations.
Key changes:
PortduinoGlue (config plumbing):
LR20x0Interface (radio driver):
LR11x0Interface (symbol namespacing):
RadioInterface:
ProMicro rfswitch.h:
Native build config:
Sample configs:
Ref: #10567 (portions of LR11x0 symbol namespacing, ProMicro rfswitch.h guards, and USB config YAML adapted from this PR)
This PR was indirectly co-authored by: @jessm33 - I modified my PR to include their findings as well as the comments from Copilot/other bots on Jess' original PR.
🤝 Attestations
Summary by CodeRabbit