Skip to content

fix(radio): validate 2.4 GHz bandwidths #11317

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
RCGV1 wants to merge 7 commits into
base: develop
Choose a base branch
from
Open

fix(radio): validate 2.4 GHz bandwidths #11317

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e57ebb0
fix(radio): validate 2.4 GHz bandwidths
RCGV1 Aug 1, 2026
e884072
fix(radio): preserve config across radio probes
RCGV1 Aug 1, 2026
74c86dc
fix(radio): keep sx128x safe with unset region
RCGV1 Aug 1, 2026
3d169c9
fix(radio): harden mixed radio probing
RCGV1 Aug 1, 2026
8d478f8
fix(radio): reject failed probe reconfiguration
RCGV1 Aug 1, 2026
b6d2ed2
fix(radio): enforce exact bandwidth capabilities
RCGV1 Aug 1, 2026
88e643e
fix(radio): preserve selected-radio error reporting
RCGV1 Aug 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/mesh/LR11x0Interface.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ template <class T> class LR11x0Interface : public RadioLibInterface
/// \return true if initialisation succeeded.
virtual bool reconfigure() override;

bool supportsLoRaBandwidth(float bandwidthKHz, bool wideBand) override
{
return supportsLr11x0LoRaBandwidth(bandwidthKHz, wideBand);
}

/// Prepare hardware for sleep. Call this _only_ for deep sleep, not needed for light sleep.
virtual bool sleep() override;

Expand Down Expand Up @@ -74,4 +79,4 @@ template <class T> class LR11x0Interface : public RadioLibInterface

uint32_t getPacketTime(uint32_t pl, bool received) override { return computePacketTime(lora, pl, received); }
};
#endif
#endif
5 changes: 5 additions & 0 deletions src/mesh/LR20x0Interface.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ template <class T> class LR20x0Interface : public RadioLibInterface
/// \return true if initialisation succeeded.
virtual bool reconfigure() override;

bool supportsLoRaBandwidth(float bandwidthKHz, bool wideBand) override
{
return supportsLr20x0LoRaBandwidth(bandwidthKHz, wideBand);
}

/// Prepare hardware for sleep. Call this _only_ for deep sleep, not needed for light sleep.
virtual bool sleep() override;

Expand Down
36 changes: 35 additions & 1 deletion src/mesh/MeshRadio.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,40 @@ static inline uint16_t clampBandwidthCode(uint16_t bwCode)
return bwCode;
}

static inline bool supportsSx128xLoRaBandwidth(float bandwidthKHz, bool wideBand)

@thebentern thebentern Aug 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use the IS_ONE_OF macro to clean up these chained comparisons in these 3 methods

{

@NomDeTom NomDeTom Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole section looks like an anti-pattern to the rest of the file, or at least a horrible typo trap, even for an llm

return wideBand && (bandwidthKHz == 203.125f || bandwidthKHz == 406.25f || bandwidthKHz == 812.5f || bandwidthKHz == 1625.0f);
}

static inline bool supportsLr11x0LoRaBandwidth(float bandwidthKHz, bool wideBand)
{
return !wideBand || bandwidthKHz == 203.125f || bandwidthKHz == 406.25f || bandwidthKHz == 812.5f;
}

static inline bool supportsLr20x0LoRaBandwidth(float bandwidthKHz, bool wideBand)
{
if (!wideBand)
return true;

switch (static_cast<int>(bandwidthKHz / 2 + 0.01f)) {
case 15:
case 20:
case 31:
case 41:
case 50:
case 62:
case 101:
case 125:
case 203:
case 250:
case 406:
case 500:
return true;
default:
return false;
}
}

static inline void modemPresetToParams(meshtastic_Config_LoRaConfig_ModemPreset preset, bool wideLora, float &bwKHz, uint8_t &sf,
uint8_t &cr)
{
Expand Down Expand Up @@ -294,4 +328,4 @@ static inline float modemPresetToBwKHz(meshtastic_Config_LoRaConfig_ModemPreset
uint8_t cr = 0;
modemPresetToParams(preset, wideLora, bwKHz, sf, cr);
return bwKHz;
}
}
101 changes: 78 additions & 23 deletions src/mesh/RadioInterface.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,7 @@ extern SPIClass SPI1;
std::unique_ptr<RadioInterface> initLoRa()
{
std::unique_ptr<RadioInterface> rIf = nullptr;
const auto loraConfigBeforeProbe = config.lora;

#if ARCH_PORTDUINO
SPISettings loraSpiSettings(portduino_config.spiSpeed, MSBFIRST, SPI_MODE0);
Expand Down Expand Up @@ -581,10 +582,16 @@ std::unique_ptr<RadioInterface> initLoRa()
if (!rIf) {
rIf = std::unique_ptr<LR1120Interface>(
new LR1120Interface(loraHal, LR1120_SPI_NSS_PIN, LR1120_IRQ_PIN, LR1120_NRESET_PIN, LR1120_BUSY_PIN));
rIf->setConfigErrorReporting(false);
if (!rIf->init()) {
LOG_WARN("No LR1120 radio");
config.lora = loraConfigBeforeProbe;
rIf = nullptr;
} else {
config.lora = loraConfigBeforeProbe;
rIf->setConfigErrorReporting(true);
if (!rIf->reconfigure())
LOG_WARN("LR1120 reconfigure failed after probe");
Comment thread
coderabbitai[bot] marked this conversation as resolved.

@NomDeTom NomDeTom Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you using a string for a known variable?

LOG_INFO("LR1120 init success");
radioType = LR1120_RADIO;
}
Expand All @@ -595,10 +602,16 @@ std::unique_ptr<RadioInterface> initLoRa()
if (!rIf) {
rIf = std::unique_ptr<LR1121Interface>(
new LR1121Interface(loraHal, LR1121_SPI_NSS_PIN, LR1121_IRQ_PIN, LR1121_NRESET_PIN, LR1121_BUSY_PIN));
rIf->setConfigErrorReporting(false);
if (!rIf->init()) {
LOG_WARN("No LR1121 radio");
config.lora = loraConfigBeforeProbe;
rIf = nullptr;
} else {
config.lora = loraConfigBeforeProbe;
rIf->setConfigErrorReporting(true);
if (!rIf->reconfigure())
LOG_WARN("LR1121 reconfigure failed after probe");
LOG_INFO("LR1121 init success");
radioType = LR1121_RADIO;
}
Expand All @@ -609,10 +622,16 @@ std::unique_ptr<RadioInterface> initLoRa()
if (!rIf) {
rIf = std::unique_ptr<LR2021Interface>(
new LR2021Interface(loraHal, LR2021_SPI_NSS_PIN, LR2021_IRQ_PIN, LR2021_NRESET_PIN, LR2021_BUSY_PIN));
rIf->setConfigErrorReporting(false);
if (!rIf->init()) {
LOG_WARN("No LR2021 radio");
config.lora = loraConfigBeforeProbe;
rIf = nullptr;
} else {
config.lora = loraConfigBeforeProbe;
rIf->setConfigErrorReporting(true);
if (!rIf->reconfigure())
LOG_WARN("LR2021 reconfigure failed after probe");
LOG_INFO("LR2021 init success");

@NomDeTom NomDeTom Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strings for known variables. Tell sol to think harder.

radioType = LR2021_RADIO;
}
Expand Down Expand Up @@ -1085,7 +1104,8 @@ bool RadioInterface::validateConfigRegion(const meshtastic_Config_LoRaConfig &lo
* When clamp==false, returns false on first error (pure validation).
* When clamp==true, fixes invalid settings in-place and returns true.
*/
bool RadioInterface::checkOrClampConfigLora(meshtastic_Config_LoRaConfig &loraConfig, bool clamp)
bool RadioInterface::checkOrClampConfigLora(meshtastic_Config_LoRaConfig &loraConfig, bool clamp, RadioInterface *radio,
bool reportErrors)
{
char err_string[160];
float check_bw;
Expand All @@ -1108,14 +1128,17 @@ bool RadioInterface::checkOrClampConfigLora(meshtastic_Config_LoRaConfig &loraCo
// Validation must still fail so callers route into the clamp, but quietly:
// the clamp will accept this config by swapping regions, so don't record a
// critical error or alarm the user over a change that is about to succeed.
LOG_INFO("Preset %s implies region swap %s to %s, deferring to clamp", presetName, newRegion->name,
swapRegion->name);
if (reportErrors)
LOG_INFO("Preset %s implies region swap %s to %s, deferring to clamp", presetName, newRegion->name,
swapRegion->name);
return false;
}
snprintf(err_string, sizeof(err_string), "Preset %s swaps region %s to %s", presetName, newRegion->name,
swapRegion->name);
LOG_INFO("%s", err_string);
sendErrorNotification(err_string, meshtastic_LogRecord_Level_INFO);
if (reportErrors) {
LOG_INFO("%s", err_string);
sendErrorNotification(err_string, meshtastic_LogRecord_Level_INFO);
}

loraConfig.region = swapRegion->code;
newRegion = swapRegion;
Expand All @@ -1131,9 +1154,11 @@ bool RadioInterface::checkOrClampConfigLora(meshtastic_Config_LoRaConfig &loraCo
} else {
snprintf(err_string, sizeof(err_string), "Preset %s invalid for %s", presetName, newRegion->name);
}
LOG_ERROR("%s", err_string);
RECORD_CRITICALERROR(meshtastic_CriticalErrorCode_INVALID_RADIO_SETTING);
sendErrorNotification(err_string);
if (reportErrors) {
LOG_ERROR("%s", err_string);
RECORD_CRITICALERROR(meshtastic_CriticalErrorCode_INVALID_RADIO_SETTING);
sendErrorNotification(err_string);
}

if (clamp) {
loraConfig.modem_preset = newRegion->getDefaultPreset();
Expand All @@ -1147,6 +1172,26 @@ bool RadioInterface::checkOrClampConfigLora(meshtastic_Config_LoRaConfig &loraCo
check_bw = clampBandwidthKHz(bwCodeToKHz(loraConfig.bandwidth));
}

RadioInterface *candidateRadio = radio ? radio : RadioLibInterface::instance;
if (candidateRadio && !candidateRadio->supportsLoRaBandwidth(check_bw, newRegion->wideLora)) {
const float defaultBandwidth = modemPresetToBwKHz(newRegion->getDefaultPreset(), newRegion->wideLora);
snprintf(err_string, sizeof(err_string), "Bandwidth %.3fkHz invalid for this radio in %s", check_bw, newRegion->name);
if (reportErrors) {
LOG_ERROR("%s", err_string);
RECORD_CRITICALERROR(meshtastic_CriticalErrorCode_INVALID_RADIO_SETTING);
sendErrorNotification(err_string);
}

if (!clamp)
return false;

if (loraConfig.use_preset)
loraConfig.modem_preset = newRegion->getDefaultPreset();
else
loraConfig.bandwidth = bwKHzToCode(defaultBandwidth);
check_bw = defaultBandwidth;
}

// Calculate width of slots (aka channels) based on bandwidth and any spacing or padding required by the region:
// spacing = gap between slots (0 for continuous spectrum) and at the beginning of the band
// padding = gap at the beginning and end of the slots (0 for no padding)
Expand All @@ -1157,9 +1202,11 @@ bool RadioInterface::checkOrClampConfigLora(meshtastic_Config_LoRaConfig &loraCo
if ((newRegion->freqEnd - newRegion->freqStart) < freqSlotWidth) {
const float regionSpanKHz = (newRegion->freqEnd - newRegion->freqStart) * 1000.0f;
snprintf(err_string, sizeof(err_string), "%s span %.0fkHz < requested %.0fkHz", newRegion->name, regionSpanKHz, check_bw);
LOG_ERROR("%s", err_string);
RECORD_CRITICALERROR(meshtastic_CriticalErrorCode_INVALID_RADIO_SETTING);
sendErrorNotification(err_string);
if (reportErrors) {
LOG_ERROR("%s", err_string);
RECORD_CRITICALERROR(meshtastic_CriticalErrorCode_INVALID_RADIO_SETTING);
sendErrorNotification(err_string);
}

if (clamp) {
loraConfig.bandwidth = bwKHzToCode(modemPresetToBwKHz(newRegion->getDefaultPreset(), newRegion->wideLora));
Expand Down Expand Up @@ -1199,9 +1246,11 @@ bool RadioInterface::checkOrClampConfigLora(meshtastic_Config_LoRaConfig &loraCo
if (loraConfig.channel_num > numFreqSlots) {
snprintf(err_string, sizeof(err_string), "Channel number %u invalid for %s, max is %u", loraConfig.channel_num,
newRegion->name, numFreqSlots);
LOG_ERROR("%s", err_string);
RECORD_CRITICALERROR(meshtastic_CriticalErrorCode_INVALID_RADIO_SETTING);
sendErrorNotification(err_string);
if (reportErrors) {
LOG_ERROR("%s", err_string);
RECORD_CRITICALERROR(meshtastic_CriticalErrorCode_INVALID_RADIO_SETTING);
sendErrorNotification(err_string);
}

if (clamp) {
if (uses_custom_channel_name) { // clamp to channel name hash
Expand Down Expand Up @@ -1231,15 +1280,15 @@ bool RadioInterface::checkOrClampConfigLora(meshtastic_Config_LoRaConfig &loraCo
return true;
}

bool RadioInterface::validateConfigLora(const meshtastic_Config_LoRaConfig &loraConfig)
bool RadioInterface::validateConfigLora(const meshtastic_Config_LoRaConfig &loraConfig, RadioInterface *radio, bool reportErrors)
{
auto copy = loraConfig;
return checkOrClampConfigLora(copy, false);
return checkOrClampConfigLora(copy, false, radio, reportErrors);
}

void RadioInterface::clampConfigLora(meshtastic_Config_LoRaConfig &loraConfig)
void RadioInterface::clampConfigLora(meshtastic_Config_LoRaConfig &loraConfig, RadioInterface *radio, bool reportErrors)
{
checkOrClampConfigLora(loraConfig, true);
checkOrClampConfigLora(loraConfig, true, radio, reportErrors);
}

/**
Expand All @@ -1252,14 +1301,15 @@ void RadioInterface::applyModemConfig()
// No Sync Words in LORA mode
meshtastic_Config_LoRaConfig &loraConfig = config.lora;
const RegionInfo *newRegion = getRegion(loraConfig.region);
const bool useWideModemParameters = newRegion->wideLora || !supportsSubGhz();
myRegion = newRegion;

if (loraConfig.use_preset) {
if (!validateConfigLora(loraConfig)) {
if (!validateConfigLora(loraConfig, this, reportConfigErrors)) {
loraConfig.modem_preset = newRegion->getDefaultPreset();
}
uint8_t newcr;
modemPresetToParams(loraConfig.modem_preset, newRegion->wideLora, bw, sf, newcr);
modemPresetToParams(loraConfig.modem_preset, useWideModemParameters, bw, sf, newcr);
// If custom CR is being used already, check if the new preset is higher
if (loraConfig.coding_rate >= 5 && loraConfig.coding_rate <= 8 && loraConfig.coding_rate < newcr) {
cr = newcr;
Expand All @@ -1274,14 +1324,19 @@ void RadioInterface::applyModemConfig()
}

} else { // if not using preset, then just use the custom settings
if (validateConfigLora(loraConfig)) {
if (validateConfigLora(loraConfig, this, reportConfigErrors)) {
} else {
LOG_WARN("Invalid LoRa config settings, cannot apply requested modem config - falling back to %s defaults",
newRegion->name);
clampConfigLora(loraConfig);
clampConfigLora(loraConfig, this, reportConfigErrors);
}
// Clamp at the source so numFreqSlots below can never be 0 (a bandwidth-0 config may already be persisted)
bw = clampBandwidthKHz(bwCodeToKHz(loraConfig.bandwidth));
if (!supportsLoRaBandwidth(bw, useWideModemParameters)) {
// Keep 2.4 GHz-only radios operational while an incompatible or UNSET region is being corrected.
bw = modemPresetToBwKHz(newRegion->getDefaultPreset(), useWideModemParameters);
loraConfig.bandwidth = bwKHzToCode(bw);
}
sf = loraConfig.spread_factor;
cr = loraConfig.coding_rate;
}
Expand Down Expand Up @@ -1353,7 +1408,7 @@ void RadioInterface::applyModemConfig()
saveChannelNum(channel_num);
saveFreq(freq + loraConfig.frequency_offset);

if (newRegion->wideLora) { // clamp if wide freq range
if (useWideModemParameters) { // clamp if wide freq range
preambleLength = wideLoraPreambleLengthDefault; // 12 is the default for operation above 2GHz
} else {
preambleLength =
Expand Down
16 changes: 13 additions & 3 deletions src/mesh/RadioInterface.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,11 @@ class RadioInterface
/// multiband chips like the LR1121 keep the default.
virtual bool supportsSubGhz() { return true; }

/// Whether this radio accepts the exact bandwidth for the requested band.
virtual bool supportsLoRaBandwidth(float bandwidthKHz, bool wideBand) { return true; }

void setConfigErrorReporting(bool enabled) { reportConfigErrors = enabled; }

/// Prepare hardware for sleep. Call this _only_ for deep sleep, not needed for light sleep.
virtual bool sleep() { return true; }

Expand Down Expand Up @@ -253,7 +258,8 @@ class RadioInterface
// Whether we have a custom channel name
static bool uses_custom_channel_name;

static bool checkOrClampConfigLora(meshtastic_Config_LoRaConfig &loraConfig, bool clamp);
static bool checkOrClampConfigLora(meshtastic_Config_LoRaConfig &loraConfig, bool clamp, RadioInterface *radio = nullptr,
bool reportErrors = true);

// Check if a candidate region is compatible and valid, with no side effects (safe for
// speculative UI checks). prospectiveLicensedOwner is for a UI flow that requires
Expand All @@ -266,10 +272,12 @@ class RadioInterface
static bool validateConfigRegion(const meshtastic_Config_LoRaConfig &loraConfig);

// Check if a candidate radio configuration is valid.
static bool validateConfigLora(const meshtastic_Config_LoRaConfig &loraConfig);
static bool validateConfigLora(const meshtastic_Config_LoRaConfig &loraConfig, RadioInterface *radio = nullptr,
bool reportErrors = true);

// Make a candidate radio configuration valid, even if it isn't.
static void clampConfigLora(meshtastic_Config_LoRaConfig &loraConfig);
static void clampConfigLora(meshtastic_Config_LoRaConfig &loraConfig, RadioInterface *radio = nullptr,
bool reportErrors = true);

// If preset is locked to a sibling of currentRegion among the swappable EU regions
// (EU_868/EU_866/EU_N_868), return the sibling region owning the preset, else nullptr.
Expand Down Expand Up @@ -313,6 +321,8 @@ class RadioInterface
virtual int16_t getCurrentRSSI() { return 0; }

private:
bool reportConfigErrors = true;

/**
* Convert our modemConfig enum into wf, sf, etc...
*
Expand Down
5 changes: 5 additions & 0 deletions src/mesh/SX128xInterface.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ template <class T> class SX128xInterface : public RadioLibInterface
/// SX128x is a 2.4 GHz-only chip; it cannot tune sub-GHz regions
virtual bool supportsSubGhz() override { return false; }

bool supportsLoRaBandwidth(float bandwidthKHz, bool wideBand) override
{
return supportsSx128xLoRaBandwidth(bandwidthKHz, wideBand);
}

/// Apply any radio provisioning changes
/// Make sure the Driver is properly configured before calling init().
/// \return true if initialisation succeeded.
Expand Down
Loading
Loading