Lora settings expansion and validation logic improvement by NomDeTom · Pull Request #9878 · meshtastic/firmware

NomDeTom · 2026-03-10T13:07:12Z

@thebentern I think this has reached MVP stage. Please proceed to tear it to shreds.

🤝 Attestations

I have tested that my proposed changes behave as described.
I have tested that my proposed changes do not cause any obvious regressions on the following devices:
- Heltec (Lora32) V3
- LilyGo T-Deck
- LilyGo T-Beam
- RAK WisBlock 4631
- Seeed Studio T-1000E tracker card
- Other (please specify below)

… clarity and consistency

…broken at this point

…ementing validateRegionConfig in RadioInterface

…leSetConfig

… method for invalid settings

…or reject invalid settings

Copilot

Pull request overview

This PR expands LoRa region/preset metadata and introduces new validation/clamping helpers intended to improve modem configuration safety and region-specific preset enforcement.

Changes:

Extends RegionInfo with additional regulatory/preset metadata and adds per-region “available presets” lists.
Adds RadioInterface::{validateConfigRegion, validateConfigLora, clampConfigLora} and updates Admin/boot flows to use them.
Updates unit tests to target the new validation path (though the referenced API currently doesn’t exist).

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 11 comments.

Show a summary per file

File	Description
test/test_radio/test_main.cpp	Renames tests to use a new validation helper (currently missing).
src/modules/AdminModule.h	Changes `handleSetConfig` signature to pass `fromOthers`.
src/modules/AdminModule.cpp	Applies new LoRa region/config validation and different behavior for remote vs local config updates.
src/mesh/RadioInterface.h	Deprecates old preset bootstrap helper and declares new validation/clamp APIs.
src/mesh/RadioInterface.cpp	Adds region preset lists, expands region definitions, implements validation/clamping, and modifies channel/frequency selection.
src/mesh/NodeDB.cpp	Switches boot-time preset field coercion from the old helper to `clampConfigLora()`.
src/mesh/MeshRadio.h	Expands `RegionInfo` struct and exposes preset lists.

test/test_radio/test_main.cpp

src/modules/AdminModule.cpp

src/mesh/RadioInterface.cpp

src/mesh/MeshRadio.h

src/mesh/NodeDB.cpp

…tion and clamping methods; add tests for region and preset handling

…utstanding comments.

…igLora

…ult frequency slot check in applyModemConfig

NomDeTom · 2026-03-11T22:21:33Z

I need to think of some better tests, but I'm tired. Maybe tomorrow...

…ault preset retrieval

…and channel calculations

… presets

Copilot

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 8 comments.

src/mesh/RadioInterface.cpp

src/graphics/draw/MenuHandler.cpp

src/mesh/MeshRadio.h

test/test_admin_radio/test_main.cpp

Copilot

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 7 comments.

Comments suppressed due to low confidence (1)

src/modules/AdminModule.h:1

This changes both the signature and the access level of handleSetConfig (public → protected, plus a new fromOthers parameter). If anything outside AdminModule previously invoked handleSetConfig(const meshtastic_Config&), this becomes a breaking change. A safer approach is to keep a public overload with the old signature (forwarding to the new method with an explicit default like fromOthers=false), or keep the method public if it’s part of the module’s expected integration surface.

#pragma once

You can also share your feedback on Copilot code review. Take the survey.

src/mesh/RadioInterface.cpp

src/graphics/draw/MenuHandler.cpp

src/mesh/RadioInterface.cpp

src/mesh/MeshRadio.h

test/test_admin_radio/test_main.cpp

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 10 comments.

Comments suppressed due to low confidence (1)

src/graphics/draw/MenuHandler.cpp:294

After the early-return if (!myRegion) return;, the later if (myRegion) { ... } else { ... } block (and duplicated warning) is unreachable on the else branch. Removing the redundant conditional will simplify control flow and reduce duplicated logging.

        LOG_WARN("Region not set, cannot calculate number of channels");
        return;
    }

src/mesh/RadioInterface.cpp

src/modules/AdminModule.cpp

src/graphics/draw/MenuHandler.cpp

src/mesh/RadioInterface.cpp

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.

src/mesh/RadioInterface.cpp

src/modules/AdminModule.cpp

src/mesh/RadioInterface.cpp

…e LNA mode handling

Copilot

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 12 comments.

Copilot · 2026-03-20T02:17:29Z

src/mesh/RadioInterface.cpp

+    uint32_t channel_num;
+    float freq;


channel_num is declared as uint32_t but is assigned -1 when override_frequency is set, which wraps to 0xFFFFFFFF. This later affects saveChannelNum(channel_num) and logging (channel_num + 1) and can result in invalid persisted state. Use a signed type for channel_num (e.g., int32_t) or represent “no channel” explicitly (e.g., a separate boolean/optional) and avoid writing a wrapped sentinel into storage.

Suggested change

uint32_t channel_num;

float freq;

int32_t channel_num = -1;

float freq = 0.0f;

Copilot · 2026-03-20T02:17:29Z

src/mesh/RadioInterface.cpp

    // override if we have a verbatim frequency
    if (loraConfig.override_frequency) {
        freq = loraConfig.override_frequency;
        channel_num = -1;
+        uses_default_frequency_slot = false;


channel_num is declared as uint32_t but is assigned -1 when override_frequency is set, which wraps to 0xFFFFFFFF. This later affects saveChannelNum(channel_num) and logging (channel_num + 1) and can result in invalid persisted state. Use a signed type for channel_num (e.g., int32_t) or represent “no channel” explicitly (e.g., a separate boolean/optional) and avoid writing a wrapped sentinel into storage.

Copilot · 2026-03-20T02:17:30Z

src/mesh/RadioInterface.cpp

+    float freqSlotWidth = newRegion->profile->spacing + (newRegion->profile->padding * 2) + (check_bw / 1000); // in MHz
+    uint32_t numFreqSlots = round((newRegion->freqEnd - newRegion->freqStart + newRegion->profile->spacing) / freqSlotWidth);


freqSlotWidth can become 0 (e.g., spacing/padding are 0 and check_bw resolves to 0), which causes division by zero when computing numFreqSlots. If numFreqSlots ends up as 0, the subsequent modulo operations (% numFreqSlots) are also undefined behavior. Add an explicit guard: if freqSlotWidth <= 0 or computed numFreqSlots == 0, then return false (validation) or clamp to a safe default (clamp mode) before any division/modulo.

Copilot · 2026-03-20T02:17:30Z

src/mesh/RadioInterface.cpp

+    const char *channelName = channels.getName(channels.getPrimaryIndex());
+    const char *presetNameDisplay =
+        DisplayFormatters::getModemPresetDisplayName(loraConfig.modem_preset, false, loraConfig.use_preset);
+    uint32_t channelNameHashSlot = hash(channelName) % numFreqSlots;
+    uint32_t presetNameHashSlot = hash(presetNameDisplay) % numFreqSlots;


freqSlotWidth can become 0 (e.g., spacing/padding are 0 and check_bw resolves to 0), which causes division by zero when computing numFreqSlots. If numFreqSlots ends up as 0, the subsequent modulo operations (% numFreqSlots) are also undefined behavior. Add an explicit guard: if freqSlotWidth <= 0 or computed numFreqSlots == 0, then return false (validation) or clamp to a safe default (clamp mode) before any division/modulo.

Copilot · 2026-03-20T02:17:30Z

src/mesh/RadioInterface.cpp

 #include <assert.h>
 #include <pb_decode.h>
 #include <pb_encode.h>
+#include <string.h>


round() is used but there is no clear include for it in this translation unit (typically <cmath>). Relying on transitive includes is fragile and can break builds on different toolchains/settings. Include <cmath> and prefer the float-specific variant (roundf) or a cast-safe integer conversion (lroundf) consistent with the desired rounding semantics.

Suggested change

#include <string.h>

#include <string.h>

#include <cmath>

test/test_admin_radio/test_main.cpp

src/modules/AdminModule.cpp

src/mesh/MeshRadio.h

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

NomDeTom · 2026-03-20T02:56:09Z

@thebentern Kind of going in circles here... I think its time.

NomDeTom · 2026-03-20T15:13:56Z

Woohoo!

NomDeTom and others added 9 commits March 6, 2026 13:25

Enhance LoRa configuration with modem presets and validation logic

fe1f5d9

Rename bootstrapLoRaConfigFromPreset tests to validateModemConfig for…

3ff2c3b

… clarity and consistency

Merge branch 'meshtastic:develop' into radio_interface_cherrypick

9f1a824

additional tidy-ups to the validateModemConfig - still fundamentally …

feb5172

…broken at this point

Enhance region validation by adding numPresets to RegionInfo and impl…

e71e6a9

…ementing validateRegionConfig in RadioInterface

Add validation for modem configuration in applyModemConfig

36a8215

Fix region unset handling and improve modem config validation in hand…

0254105

…leSetConfig

Refactor LoRa configuration validation methods and introduce clamping…

5259494

… method for invalid settings

Update handleSetConfig to use fromOthers parameter to either correct …

cf9712e

…or reject invalid settings

github-actions bot added needs-review Needs human review enhancement New feature or request labels Mar 10, 2026

Merge branch 'develop' into radio_interface_cherrypick

eb240d7

thebentern requested a review from Copilot March 10, 2026 13:09

Copilot started reviewing on behalf of thebentern March 10, 2026 13:12 View session

Copilot AI reviewed Mar 10, 2026

View reviewed changes

NomDeTom and others added 7 commits March 10, 2026 22:42

Fix some of the copilot review comments for LoRa configuration valida…

237ac2c

…tion and clamping methods; add tests for region and preset handling

Redid the slot default checking and calculation. Should resolve the o…

0ada155

…utstanding comments.

Add bandwidth calculation for LoRa modem preset fallback in clampConf…

4a5da3e

…igLora

Remove unused preset name variable in validateConfigLora and fix defa…

ef72c40

…ult frequency slot check in applyModemConfig

update tests for region handling

2e84995

Merge branch 'develop' into radio_interface_cherrypick

251b64f

Got the synthetic colleague to add mock service for testing

b6d55af

NomDeTom marked this pull request as ready for review March 11, 2026 22:21

NomDeTom and others added 6 commits March 12, 2026 00:54

Flash savings... hopefully

c73b592

Refactor modem preset handling to use sentinel values and improve def…

ca28017

…ault preset retrieval

Refactor region handling to use profile structures for modem presets …

3c3ddde

…and channel calculations

added comments for clarity on parameters

d768507

Add shadow table tests and validateConfigLora enhancements for region…

a9bc5ce

… presets

Merge branch 'meshtastic:develop' into radio_interface_cherrypick

e14a6a5

Copilot AI reviewed Mar 18, 2026

View reviewed changes

fix build errors

f5e959d

thebentern requested a review from Copilot March 18, 2026 15:32

Copilot AI reviewed Mar 18, 2026

View reviewed changes

NomDeTom added 2 commits March 18, 2026 18:41

Fix the fix, undo the silly suggestions from synthetic reviewer.

b471666

we all float here

eca8218

thebentern requested a review from Copilot March 18, 2026 19:25

Fix include path for AdminModule in test_main.cpp

dbedf27

Copilot AI reviewed Mar 18, 2026

View reviewed changes

NomDeTom requested a review from Copilot March 19, 2026 01:27

Copilot AI reviewed Mar 19, 2026

View reviewed changes

NomDeTom and others added 6 commits March 19, 2026 17:02

Potential fix for pull request finding

c31c904

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

More suggestion fixes

dfc38fc

Merge branch 'develop' into radio_interface_cherrypick

ca74713

admin module merge conflicts

1d9d6bb

admin module fixes from merge hell

78d8802

Merge branch 'develop' into radio_interface_cherrypick

d060e0d

NomDeTom requested a review from Copilot March 19, 2026 18:03

Copilot AI reviewed Mar 19, 2026

View reviewed changes

NomDeTom and others added 3 commits March 20, 2026 00:31

fix: initialize default frequency slot and custom channel name; updat…

107191b

…e LNA mode handling

save some bytes...

f1a56bc

Merge branch 'develop' into radio_interface_cherrypick

7ef568b

NomDeTom requested a review from Copilot March 20, 2026 02:14

Copilot AI reviewed Mar 20, 2026

View reviewed changes

NomDeTom and others added 2 commits March 20, 2026 02:21

fix: simplify error logging for bandwidth checks in LoRa configuration

2ee48b7

Update src/mesh/MeshRadio.h

3cba534

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

thebentern approved these changes Mar 20, 2026

View reviewed changes

thebentern merged commit 22d63fa into meshtastic:develop Mar 20, 2026
77 checks passed

		float freqSlotWidth = newRegion->profile->spacing + (newRegion->profile->padding * 2) + (check_bw / 1000); // in MHz
		uint32_t numFreqSlots = round((newRegion->freqEnd - newRegion->freqStart + newRegion->profile->spacing) / freqSlotWidth);

Uh oh!

Conversation

NomDeTom commented Mar 10, 2026

🤝 Attestations

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

NomDeTom commented Mar 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026