diff --git a/src/modules/AdminModule.cpp b/src/modules/AdminModule.cpp index 87d423f21..cc2811f8c 100644 --- a/src/modules/AdminModule.cpp +++ b/src/modules/AdminModule.cpp @@ -43,6 +43,7 @@ #if !defined(ARCH_STM32WL) && !MESHTASTIC_EXCLUDE_I2C #include "motion/AccelerometerThread.h" #endif +#include "SerialModule.h" AdminModule *adminModule; bool hasOpenEditTransaction; @@ -807,10 +808,12 @@ void AdminModule::handleSetConfig(const meshtastic_Config &c) bool AdminModule::handleSetModuleConfig(const meshtastic_ModuleConfig &c) { - // If we are in an open transaction or configuring MQTT, defer disabling Bluetooth + // If we are in an open transaction or configuring MQTT or Serial (which have validation), defer disabling Bluetooth // Otherwise, disable Bluetooth to prevent the phone from interfering with the config - if (!hasOpenEditTransaction && c.which_payload_variant != meshtastic_ModuleConfig_mqtt_tag) + if (!hasOpenEditTransaction && + !IS_ONE_OF(c.which_payload_variant, meshtastic_ModuleConfig_mqtt_tag, meshtastic_ModuleConfig_serial_tag)) { disableBluetooth(); + } switch (c.which_payload_variant) { case meshtastic_ModuleConfig_mqtt_tag: @@ -830,6 +833,10 @@ bool AdminModule::handleSetModuleConfig(const meshtastic_ModuleConfig &c) break; case meshtastic_ModuleConfig_serial_tag: LOG_INFO("Set module config: Serial"); + if (!SerialModule::isValidConfig(c.payload_variant.serial)) { + LOG_ERROR("Invalid serial config"); + return false; + } moduleConfig.has_serial = true; moduleConfig.serial = c.payload_variant.serial; break; @@ -985,9 +992,10 @@ void AdminModule::handleGetConfig(const meshtastic_MeshPacket &req, const uint32 // So even if we internally use 0 to represent 'use default' we still need to send the value we are // using to the app (so that even old phone apps work with new device loads). // r.get_radio_response.preferences.ls_secs = getPref_ls_secs(); - // hideSecret(r.get_radio_response.preferences.wifi_ssid); // hmm - leave public for now, because only minimally private - // and useful for users to know current provisioning) hideSecret(r.get_radio_response.preferences.wifi_password); - // r.get_config_response.which_payloadVariant = Config_ModuleConfig_telemetry_tag; + // hideSecret(r.get_radio_response.preferences.wifi_ssid); // hmm - leave public for now, because only minimally + // private and useful for users to know current provisioning) + // hideSecret(r.get_radio_response.preferences.wifi_password); r.get_config_response.which_payloadVariant = + // Config_ModuleConfig_telemetry_tag; res.which_payload_variant = meshtastic_AdminMessage_get_config_response_tag; setPassKey(&res); myReply = allocDataProtobuf(res); @@ -1071,9 +1079,10 @@ void AdminModule::handleGetModuleConfig(const meshtastic_MeshPacket &req, const // So even if we internally use 0 to represent 'use default' we still need to send the value we are // using to the app (so that even old phone apps work with new device loads). // r.get_radio_response.preferences.ls_secs = getPref_ls_secs(); - // hideSecret(r.get_radio_response.preferences.wifi_ssid); // hmm - leave public for now, because only minimally private - // and useful for users to know current provisioning) hideSecret(r.get_radio_response.preferences.wifi_password); - // r.get_config_response.which_payloadVariant = Config_ModuleConfig_telemetry_tag; + // hideSecret(r.get_radio_response.preferences.wifi_ssid); // hmm - leave public for now, because only minimally + // private and useful for users to know current provisioning) + // hideSecret(r.get_radio_response.preferences.wifi_password); r.get_config_response.which_payloadVariant = + // Config_ModuleConfig_telemetry_tag; res.which_payload_variant = meshtastic_AdminMessage_get_module_config_response_tag; setPassKey(&res); myReply = allocDataProtobuf(res); diff --git a/src/modules/SerialModule.cpp b/src/modules/SerialModule.cpp index f3921ef19..fe180ddbd 100644 --- a/src/modules/SerialModule.cpp +++ b/src/modules/SerialModule.cpp @@ -74,6 +74,27 @@ static Print *serialPrint = &Serial2; char serialBytes[512]; size_t serialPayloadSize; +bool SerialModule::isValidConfig(const meshtastic_ModuleConfig_SerialConfig &config) +{ + if (config.override_console_serial_port && !IS_ONE_OF(config.mode, meshtastic_ModuleConfig_SerialConfig_Serial_Mode_NMEA, + meshtastic_ModuleConfig_SerialConfig_Serial_Mode_CALTOPO)) { + const char *warning = + "Invalid Serial config: override console serial port is only supported in NMEA and CalTopo output-only modes."; + LOG_ERROR(warning); +#if !IS_RUNNING_TESTS + meshtastic_ClientNotification *cn = clientNotificationPool.allocZeroed(); + cn->level = meshtastic_LogRecord_Level_ERROR; + cn->time = getValidTime(RTCQualityFromNet); + strncpy(cn->message, warning, sizeof(cn->message) - 1); + cn->message[sizeof(cn->message) - 1] = '\0'; // Ensure null termination + service->sendClientNotification(cn); +#endif + return false; + } + + return true; +} + SerialModuleRadio::SerialModuleRadio() : MeshModule("SerialModuleRadio") { switch (moduleConfig.serial.mode) { diff --git a/src/modules/SerialModule.h b/src/modules/SerialModule.h index fa86db28f..1c74c927c 100644 --- a/src/modules/SerialModule.h +++ b/src/modules/SerialModule.h @@ -20,6 +20,8 @@ class SerialModule : public StreamAPI, private concurrency::OSThread public: SerialModule(); + static bool isValidConfig(const meshtastic_ModuleConfig_SerialConfig &config); + protected: virtual int32_t runOnce() override; diff --git a/test/test_serial/SerialModule.cpp b/test/test_serial/SerialModule.cpp new file mode 100644 index 000000000..6de11c3fe --- /dev/null +++ b/test/test_serial/SerialModule.cpp @@ -0,0 +1,140 @@ +#include "DebugConfiguration.h" +#include "TestUtil.h" +#include + +#ifdef ARCH_PORTDUINO +#include "configuration.h" +#include "modules/SerialModule.h" + +#if defined(UNIT_TEST) +#define IS_RUNNING_TESTS 1 +#else +#define IS_RUNNING_TESTS 0 +#endif + +// Test that empty configuration is valid. +void test_serialConfigEmptyIsValid(void) +{ + meshtastic_ModuleConfig_SerialConfig config = {}; + + TEST_ASSERT_TRUE(SerialModule::isValidConfig(config)); +} + +// Test that basic enabled configuration is valid. +void test_serialConfigEnabledIsValid(void) +{ + meshtastic_ModuleConfig_SerialConfig config = {.enabled = true}; + + TEST_ASSERT_TRUE(SerialModule::isValidConfig(config)); +} + +// Test that configuration with override_console_serial_port and NMEA mode is valid. +void test_serialConfigWithOverrideConsoleNmeaModeIsValid(void) +{ + meshtastic_ModuleConfig_SerialConfig config = { + .enabled = true, .override_console_serial_port = true, .mode = meshtastic_ModuleConfig_SerialConfig_Serial_Mode_NMEA}; + + TEST_ASSERT_TRUE(SerialModule::isValidConfig(config)); +} + +// Test that configuration with override_console_serial_port and CalTopo mode is valid. +void test_serialConfigWithOverrideConsoleCalTopoModeIsValid(void) +{ + meshtastic_ModuleConfig_SerialConfig config = { + .enabled = true, .override_console_serial_port = true, .mode = meshtastic_ModuleConfig_SerialConfig_Serial_Mode_CALTOPO}; + + TEST_ASSERT_TRUE(SerialModule::isValidConfig(config)); +} + +// Test that configuration with override_console_serial_port and DEFAULT mode is invalid. +void test_serialConfigWithOverrideConsoleDefaultModeIsInvalid(void) +{ + meshtastic_ModuleConfig_SerialConfig config = { + .enabled = true, .override_console_serial_port = true, .mode = meshtastic_ModuleConfig_SerialConfig_Serial_Mode_DEFAULT}; + + TEST_ASSERT_FALSE(SerialModule::isValidConfig(config)); +} + +// Test that configuration with override_console_serial_port and SIMPLE mode is invalid. +void test_serialConfigWithOverrideConsoleSimpleModeIsInvalid(void) +{ + meshtastic_ModuleConfig_SerialConfig config = { + .enabled = true, .override_console_serial_port = true, .mode = meshtastic_ModuleConfig_SerialConfig_Serial_Mode_SIMPLE}; + + TEST_ASSERT_FALSE(SerialModule::isValidConfig(config)); +} + +// Test that configuration with override_console_serial_port and TEXTMSG mode is invalid. +void test_serialConfigWithOverrideConsoleTextMsgModeIsInvalid(void) +{ + meshtastic_ModuleConfig_SerialConfig config = { + .enabled = true, .override_console_serial_port = true, .mode = meshtastic_ModuleConfig_SerialConfig_Serial_Mode_TEXTMSG}; + + TEST_ASSERT_FALSE(SerialModule::isValidConfig(config)); +} + +// Test that configuration with override_console_serial_port and PROTO mode is invalid. +void test_serialConfigWithOverrideConsoleProtoModeIsInvalid(void) +{ + meshtastic_ModuleConfig_SerialConfig config = { + .enabled = true, .override_console_serial_port = true, .mode = meshtastic_ModuleConfig_SerialConfig_Serial_Mode_PROTO}; + + TEST_ASSERT_FALSE(SerialModule::isValidConfig(config)); +} + +// Test that various modes work without override_console_serial_port. +void test_serialConfigVariousModesWithoutOverrideAreValid(void) +{ + meshtastic_ModuleConfig_SerialConfig config = {.enabled = true, .override_console_serial_port = false}; + + // Test DEFAULT mode + config.mode = meshtastic_ModuleConfig_SerialConfig_Serial_Mode_DEFAULT; + TEST_ASSERT_TRUE(SerialModule::isValidConfig(config)); + + // Test SIMPLE mode + config.mode = meshtastic_ModuleConfig_SerialConfig_Serial_Mode_SIMPLE; + TEST_ASSERT_TRUE(SerialModule::isValidConfig(config)); + + // Test TEXTMSG mode + config.mode = meshtastic_ModuleConfig_SerialConfig_Serial_Mode_TEXTMSG; + TEST_ASSERT_TRUE(SerialModule::isValidConfig(config)); + + // Test PROTO mode + config.mode = meshtastic_ModuleConfig_SerialConfig_Serial_Mode_PROTO; + TEST_ASSERT_TRUE(SerialModule::isValidConfig(config)); + + // Test NMEA mode + config.mode = meshtastic_ModuleConfig_SerialConfig_Serial_Mode_NMEA; + TEST_ASSERT_TRUE(SerialModule::isValidConfig(config)); + + // Test CALTOPO mode + config.mode = meshtastic_ModuleConfig_SerialConfig_Serial_Mode_CALTOPO; + TEST_ASSERT_TRUE(SerialModule::isValidConfig(config)); +} + +void setup() +{ + initializeTestEnvironment(); + + UNITY_BEGIN(); + RUN_TEST(test_serialConfigEmptyIsValid); + RUN_TEST(test_serialConfigEnabledIsValid); + RUN_TEST(test_serialConfigWithOverrideConsoleNmeaModeIsValid); + RUN_TEST(test_serialConfigWithOverrideConsoleCalTopoModeIsValid); + RUN_TEST(test_serialConfigWithOverrideConsoleDefaultModeIsInvalid); + RUN_TEST(test_serialConfigWithOverrideConsoleSimpleModeIsInvalid); + RUN_TEST(test_serialConfigWithOverrideConsoleTextMsgModeIsInvalid); + RUN_TEST(test_serialConfigWithOverrideConsoleProtoModeIsInvalid); + RUN_TEST(test_serialConfigVariousModesWithoutOverrideAreValid); + exit(UNITY_END()); +} +#else +void setup() +{ + initializeTestEnvironment(); + LOG_WARN("This test requires the ARCH_PORTDUINO variant"); + UNITY_BEGIN(); + UNITY_END(); +} +#endif +void loop() {}