mirror/firmware
1
0
Fork 0
mirror of https://github.com/meshtastic/firmware.git synced 2026-05-30 19:55:48 -04:00
Code Issues Packages Projects Releases Wiki Activity

Lora settings expansion and validation logic improvement (#9878)

Browse Source 
* Enhance LoRa configuration with modem presets and validation logic

* Rename bootstrapLoRaConfigFromPreset tests to validateModemConfig for clarity and consistency

* additional tidy-ups to the validateModemConfig - still fundamentally broken at this point

* Enhance region validation by adding numPresets to RegionInfo and implementing validateRegionConfig in RadioInterface

* Add validation for modem configuration in applyModemConfig

* Fix region unset handling and improve modem config validation in handleSetConfig

* Refactor LoRa configuration validation methods and introduce clamping method for invalid settings

* Update handleSetConfig to use fromOthers parameter to either correct or reject invalid settings

* Fix some of the copilot review comments for LoRa configuration validation and clamping methods; add tests for region and preset handling

* Redid the slot default checking and calculation. Should resolve the outstanding comments.

* Add bandwidth calculation for LoRa modem preset fallback in clampConfigLora

* Remove unused preset name variable in validateConfigLora and fix default frequency slot check in applyModemConfig

* update tests for region handling

* Got the synthetic colleague to add mock service for testing

* Flash savings... hopefully

* Refactor modem preset handling to use sentinel values and improve default preset retrieval

* Refactor region handling to use profile structures for modem presets and channel calculations

* added comments for clarity on parameters

* Add shadow table tests and validateConfigLora enhancements for region presets

* Add isFromUs tests for preset validation in AdminModule

* Respond to copilot github review

* address copilot comments

* address null poointers

* fix build errors

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

* we all float here

* Fix include path for AdminModule in test_main.cpp

* Potential fix for pull request finding

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

* More suggestion fixes

* admin module merge conflicts

* admin module fixes from merge hell

* fix: initialize default frequency slot and custom channel name; update LNA mode handling

* save some bytes...

* fix: simplify error logging for bandwidth checks in LoRa configuration

* Update src/mesh/MeshRadio.h

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

---------

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
Tom
2026-03-20 12:43:47 +00:00
committed by GitHub
parent fc030d2d19
commit 22d63fa69c
11 changed files with 1355 additions and 252 deletions
Download Patch File Download Diff File Expand all files Collapse all files

184
src/modules/AdminModule.cpp
View File

@@ -24,6 +24,7 @@
#include "Default.h"
#include "MeshRadio.h"
#include "RadioInterface.h"
#include "TypeConversions.h"
#if !MESHTASTIC_EXCLUDE_MQTT
@@ -199,7 +200,7 @@ bool AdminModule::handleReceivedProtobuf(const meshtastic_MeshPacket &mp, meshta
case meshtastic_AdminMessage_set_config_tag:
LOG_DEBUG("Client set config");
handleSetConfig(r->set_config);
handleSetConfig(r->set_config, fromOthers);
break;
case meshtastic_AdminMessage_set_module_config_tag:
@@ -626,7 +627,7 @@ void AdminModule::handleSetOwner(const meshtastic_User &o)
}
}
void AdminModule::handleSetConfig(const meshtastic_Config &c)
void AdminModule::handleSetConfig(const meshtastic_Config &c, bool fromOthers)
{
auto changes = SEGMENT_CONFIG;
auto existingRole = config.device.role;
@@ -768,6 +769,69 @@ void AdminModule::handleSetConfig(const meshtastic_Config &c)
validatedLora.spread_factor = LORA_SF_DEFAULT;
}
// If we're setting a new region, check the region is valid and then init the region or discard the change
if (validatedLora.region != myRegion->code) {
// Region has changed so check whether it is valid for e.g. licensing conditions and if the lora config is valid
if (RadioInterface::validateConfigRegion(validatedLora) && RadioInterface::validateConfigLora(validatedLora)) {
// If we're setting region for the first time, init the region and regenerate the keys
if (isRegionUnset && validatedLora.region > meshtastic_Config_LoRaConfig_RegionCode_UNSET) {
#if !(MESHTASTIC_EXCLUDE_PKI_KEYGEN || MESHTASTIC_EXCLUDE_PKI)
if (!owner.is_licensed) {
bool keygenSuccess = false;
if (config.security.private_key.size == 32) {
if (crypto->regeneratePublicKey(config.security.public_key.bytes,
config.security.private_key.bytes)) {
keygenSuccess = true;
}
} else {
LOG_INFO("Generate new PKI keys");
crypto->generateKeyPair(config.security.public_key.bytes, config.security.private_key.bytes);
keygenSuccess = true;
}
if (keygenSuccess) {
config.security.public_key.size = 32;
config.security.private_key.size = 32;
owner.public_key.size = 32;
memcpy(owner.public_key.bytes, config.security.public_key.bytes, 32);
}
}
#endif
// new region is valid and we're coming from an unset region, so enable tx
validatedLora.tx_enabled = true;
}
// If we're unsetting the region for some reason, disable tx
if (!isRegionUnset && validatedLora.region == meshtastic_Config_LoRaConfig_RegionCode_UNSET) {
validatedLora.tx_enabled = false;
}
// Ensure initRegion() uses the newly validated region
config.lora.region = validatedLora.region;
initRegion();
if (myRegion->dutyCycle < 100) {
validatedLora.ignore_mqtt = true; // Ignore MQTT by default if region has a duty cycle limit
}
if (strncmp(moduleConfig.mqtt.root, default_mqtt_root, strlen(default_mqtt_root)) == 0) {
// Default root is in use, so subscribe to the appropriate MQTT topic for this region
sprintf(moduleConfig.mqtt.root, "%s/%s", default_mqtt_root, myRegion->name);
}
changes = SEGMENT_CONFIG | SEGMENT_MODULECONFIG;
} else {
// Region validation has failed, so just copy all of the old config over the new config
validatedLora = oldLoraConfig;
}
} // end of new region handling
if (!RadioInterface::validateConfigLora(validatedLora)) {
if (fromOthers) {
LOG_WARN("Invalid LoRa config received from another node, rejecting changes");
// modem_preset set to use the old setting if the check fails
validatedLora.modem_preset = oldLoraConfig.modem_preset;
} else {
LOG_WARN("Invalid LoRa config received from client, using corrected values");
RadioInterface::clampConfigLora(validatedLora);
}
// use_preset and bandwidth are coerced into valid values by the check.
}
// If no lora radio parameters change, don't need to reboot
if (oldLoraConfig.use_preset == validatedLora.use_preset && oldLoraConfig.region == validatedLora.region &&
oldLoraConfig.modem_preset == validatedLora.modem_preset && oldLoraConfig.bandwidth == validatedLora.bandwidth &&
@@ -795,63 +859,21 @@ void AdminModule::handleSetConfig(const meshtastic_Config &c)
digitalWrite(RF95_FAN_EN, HIGH ^ 0);
}
#endif
config.lora = validatedLora;
#if HAS_LORA_FEM
// Apply FEM LNA mode from config (only meaningful on hardware that supports it)
// Note that a rejected lora config will revert this as well.
if (loraFEMInterface.isLnaCanControl()) {
loraFEMInterface.setLNAEnable(config.lora.fem_lna_mode != meshtastic_Config_LoRaConfig_FEM_LNA_Mode_DISABLED);
} else if (config.lora.fem_lna_mode != meshtastic_Config_LoRaConfig_FEM_LNA_Mode_NOT_PRESENT) {
loraFEMInterface.setLNAEnable(validatedLora.fem_lna_mode != meshtastic_Config_LoRaConfig_FEM_LNA_Mode_DISABLED);
} else if (validatedLora.fem_lna_mode != meshtastic_Config_LoRaConfig_FEM_LNA_Mode_NOT_PRESENT) {
// Hardware FEM does not support LNA control; normalize stored config to match actual capability
LOG_WARN("FEM LNA mode configured but current FEM does not support LNA control; normalizing to NOT_PRESENT");
config.lora.fem_lna_mode = meshtastic_Config_LoRaConfig_FEM_LNA_Mode_NOT_PRESENT;
validatedLora.fem_lna_mode = meshtastic_Config_LoRaConfig_FEM_LNA_Mode_NOT_PRESENT;
}
#endif
// If we're setting region for the first time, init the region and regenerate the keys
if (isRegionUnset && config.lora.region > meshtastic_Config_LoRaConfig_RegionCode_UNSET) {
#if !(MESHTASTIC_EXCLUDE_PKI_KEYGEN || MESHTASTIC_EXCLUDE_PKI)
if (!owner.is_licensed) {
bool keygenSuccess = false;
if (config.security.private_key.size == 32) {
if (crypto->regeneratePublicKey(config.security.public_key.bytes, config.security.private_key.bytes)) {
keygenSuccess = true;
}
} else {
LOG_INFO("Generate new PKI keys");
crypto->generateKeyPair(config.security.public_key.bytes, config.security.private_key.bytes);
keygenSuccess = true;
}
if (keygenSuccess) {
config.security.public_key.size = 32;
config.security.private_key.size = 32;
owner.public_key.size = 32;
memcpy(owner.public_key.bytes, config.security.public_key.bytes, 32);
}
}
#endif
config.lora.tx_enabled = true;
initRegion();
if (myRegion->dutyCycle < 100) {
config.lora.ignore_mqtt = true; // Ignore MQTT by default if region has a duty cycle limit
}
// Compare the entire string, we are sure of the length as a topic has never been set
if (strcmp(moduleConfig.mqtt.root, default_mqtt_root) == 0) {
sprintf(moduleConfig.mqtt.root, "%s/%s", default_mqtt_root, myRegion->name);
changes = SEGMENT_CONFIG | SEGMENT_MODULECONFIG;
}
}
if (config.lora.region != myRegion->code) {
// Region has changed so check whether there is a regulatory one we should be using instead.
// Additionally as a side-effect, assume a new value under myRegion
initRegion();
if (strncmp(moduleConfig.mqtt.root, default_mqtt_root, strlen(default_mqtt_root)) == 0) {
// Default root is in use, so subscribe to the appropriate MQTT topic for this region
sprintf(moduleConfig.mqtt.root, "%s/%s", default_mqtt_root, myRegion->name);
}
config.lora = validatedLora; // Finally, return the validated config back to the main config
changes = SEGMENT_CONFIG | SEGMENT_MODULECONFIG;
}
break;
}
case meshtastic_Config_bluetooth_tag:
@@ -899,10 +921,10 @@ void AdminModule::handleSetConfig(const meshtastic_Config &c)
}
if (requiresReboot && !hasOpenEditTransaction) {
disableBluetooth();
}
} // end of switch case which_payload_variant
saveChanges(changes, requiresReboot);
}
} // end of handleSetConfig
bool AdminModule::handleSetModuleConfig(const meshtastic_ModuleConfig &c)
{
@@ -1127,83 +1149,85 @@ void AdminModule::handleGetModuleConfig(const meshtastic_MeshPacket &req, const
meshtastic_AdminMessage res = meshtastic_AdminMessage_init_default;
if (req.decoded.want_response) {
const char *configName = "?";
switch (configType) {
case meshtastic_AdminMessage_ModuleConfigType_MQTT_CONFIG:
LOG_INFO("Get module config: MQTT");
configName = "MQTT";
res.get_module_config_response.which_payload_variant = meshtastic_ModuleConfig_mqtt_tag;
res.get_module_config_response.payload_variant.mqtt = moduleConfig.mqtt;
break;
case meshtastic_AdminMessage_ModuleConfigType_SERIAL_CONFIG:
LOG_INFO("Get module config: Serial");
configName = "Serial";
res.get_module_config_response.which_payload_variant = meshtastic_ModuleConfig_serial_tag;
res.get_module_config_response.payload_variant.serial = moduleConfig.serial;
break;
case meshtastic_AdminMessage_ModuleConfigType_EXTNOTIF_CONFIG:
LOG_INFO("Get module config: External Notification");
configName = "External Notification";
res.get_module_config_response.which_payload_variant = meshtastic_ModuleConfig_external_notification_tag;
res.get_module_config_response.payload_variant.external_notification = moduleConfig.external_notification;
break;
case meshtastic_AdminMessage_ModuleConfigType_STOREFORWARD_CONFIG:
LOG_INFO("Get module config: Store & Forward");
configName = "Store & Forward";
res.get_module_config_response.which_payload_variant = meshtastic_ModuleConfig_store_forward_tag;
res.get_module_config_response.payload_variant.store_forward = moduleConfig.store_forward;
break;
case meshtastic_AdminMessage_ModuleConfigType_RANGETEST_CONFIG:
LOG_INFO("Get module config: Range Test");
configName = "Range Test";
res.get_module_config_response.which_payload_variant = meshtastic_ModuleConfig_range_test_tag;
res.get_module_config_response.payload_variant.range_test = moduleConfig.range_test;
break;
case meshtastic_AdminMessage_ModuleConfigType_TELEMETRY_CONFIG:
LOG_INFO("Get module config: Telemetry");
configName = "Telemetry";
res.get_module_config_response.which_payload_variant = meshtastic_ModuleConfig_telemetry_tag;
res.get_module_config_response.payload_variant.telemetry = moduleConfig.telemetry;
break;
case meshtastic_AdminMessage_ModuleConfigType_CANNEDMSG_CONFIG:
LOG_INFO("Get module config: Canned Message");
configName = "Canned Message";
res.get_module_config_response.which_payload_variant = meshtastic_ModuleConfig_canned_message_tag;
res.get_module_config_response.payload_variant.canned_message = moduleConfig.canned_message;
break;
case meshtastic_AdminMessage_ModuleConfigType_AUDIO_CONFIG:
LOG_INFO("Get module config: Audio");
configName = "Audio";
res.get_module_config_response.which_payload_variant = meshtastic_ModuleConfig_audio_tag;
res.get_module_config_response.payload_variant.audio = moduleConfig.audio;
break;
case meshtastic_AdminMessage_ModuleConfigType_REMOTEHARDWARE_CONFIG:
LOG_INFO("Get module config: Remote Hardware");
configName = "Remote Hardware";
res.get_module_config_response.which_payload_variant = meshtastic_ModuleConfig_remote_hardware_tag;
res.get_module_config_response.payload_variant.remote_hardware = moduleConfig.remote_hardware;
break;
case meshtastic_AdminMessage_ModuleConfigType_NEIGHBORINFO_CONFIG:
LOG_INFO("Get module config: Neighbor Info");
configName = "Neighbor Info";
res.get_module_config_response.which_payload_variant = meshtastic_ModuleConfig_neighbor_info_tag;
res.get_module_config_response.payload_variant.neighbor_info = moduleConfig.neighbor_info;
break;
case meshtastic_AdminMessage_ModuleConfigType_DETECTIONSENSOR_CONFIG:
LOG_INFO("Get module config: Detection Sensor");
configName = "Detection Sensor";
res.get_module_config_response.which_payload_variant = meshtastic_ModuleConfig_detection_sensor_tag;
res.get_module_config_response.payload_variant.detection_sensor = moduleConfig.detection_sensor;
break;
case meshtastic_AdminMessage_ModuleConfigType_AMBIENTLIGHTING_CONFIG:
LOG_INFO("Get module config: Ambient Lighting");
configName = "Ambient Lighting";
res.get_module_config_response.which_payload_variant = meshtastic_ModuleConfig_ambient_lighting_tag;
res.get_module_config_response.payload_variant.ambient_lighting = moduleConfig.ambient_lighting;
break;
case meshtastic_AdminMessage_ModuleConfigType_PAXCOUNTER_CONFIG:
LOG_INFO("Get module config: Paxcounter");
configName = "Paxcounter";
res.get_module_config_response.which_payload_variant = meshtastic_ModuleConfig_paxcounter_tag;
res.get_module_config_response.payload_variant.paxcounter = moduleConfig.paxcounter;
break;
case meshtastic_AdminMessage_ModuleConfigType_STATUSMESSAGE_CONFIG:
LOG_INFO("Get module config: StatusMessage");
configName = "StatusMessage";
res.get_module_config_response.which_payload_variant = meshtastic_ModuleConfig_statusmessage_tag;
res.get_module_config_response.payload_variant.statusmessage = moduleConfig.statusmessage;
break;
case meshtastic_AdminMessage_ModuleConfigType_TRAFFICMANAGEMENT_CONFIG:
LOG_INFO("Get module config: Traffic Management");
configName = "Traffic Management";
res.get_module_config_response.which_payload_variant = meshtastic_ModuleConfig_traffic_management_tag;
res.get_module_config_response.payload_variant.traffic_management = moduleConfig.traffic_management;
break;
}
LOG_INFO("Get module config: %s", configName);
// NOTE: The phone app needs to know the ls_secsvalue so it can properly expect sleep behavior.
// So even if we internally use 0 to represent 'use default' we still need to send the value we are
@@ -1386,23 +1410,17 @@ void AdminModule::handleStoreDeviceUIConfig(const meshtastic_DeviceUIConfig &uic
void AdminModule::handleSetHamMode(const meshtastic_HamParameters &p)
{
// Validate ham parameters before setting since this would bypass validation in the owner struct
if (*p.call_sign) {
const char *start = p.call_sign;
// Skip all whitespace
while (*start && isspace((unsigned char)*start))
start++;
if (*start == '\0') {
LOG_WARN("Rejected ham call_sign: must contain at least 1 non-whitespace character");
return;
}
}
if (*p.short_name) {
const char *start = p.short_name;
while (*start && isspace((unsigned char)*start))
start++;
if (*start == '\0') {
LOG_WARN("Rejected ham short_name: must contain at least 1 non-whitespace character");
return;
const char *fieldsToCheck[] = {p.call_sign, p.short_name};
const char *fieldNames[] = {"call_sign", "short_name"};
for (int i = 0; i < 2; i++) {
if (*fieldsToCheck[i]) {
const char *start = fieldsToCheck[i];
while (*start && isspace((unsigned char)*start))
start++;
if (*start == '\0') {
LOG_WARN("Rejected ham %s: must contain at least 1 non-whitespace character", fieldNames[i]);
return;
}
}
}