diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index f0c456f4b..0960d45c4 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -677,6 +677,8 @@ Unit tests in `test/` directory. The canonical suite count is in `test/native-su - `test_radio/` - Radio interface - `test_rtc/` - RTC / time handling - `test_serial/` - Serial communication +- `test_tak_config/` - TAK (ATAK) team/role value fidelity through set/save/load/get +- `test_module_config/` - every ModuleConfig submessage survives admin set -> save -> load -> get - `test_traffic_management/` - Traffic management (dedup, rate-limit, hop-trim, role exceptions) - `test_transmit_history/` - Retransmission tracking - `test_type_conversions/` - NodeDB v25 type conversion (bitfield round-trips, NodeInfoLite) diff --git a/src/mesh/NodeDB.cpp b/src/mesh/NodeDB.cpp index f46bbfcce..a8de3cdd9 100644 --- a/src/mesh/NodeDB.cpp +++ b/src/mesh/NodeDB.cpp @@ -3008,6 +3008,7 @@ bool NodeDB::saveToDiskNoRetry(int saveWhat) moduleConfig.has_audio = true; moduleConfig.has_paxcounter = true; moduleConfig.has_statusmessage = true; + moduleConfig.has_tak = true; #if !MESHTASTIC_EXCLUDE_BEACON moduleConfig.has_mesh_beacon = true; #endif diff --git a/src/modules/AdminModule.cpp b/src/modules/AdminModule.cpp index 945802aba..d57fa142e 100644 --- a/src/modules/AdminModule.cpp +++ b/src/modules/AdminModule.cpp @@ -1299,6 +1299,11 @@ bool AdminModule::handleSetModuleConfig(const meshtastic_ModuleConfig &c) moduleConfig.has_traffic_management = true; moduleConfig.traffic_management = c.payload_variant.traffic_management; break; + case meshtastic_ModuleConfig_tak_tag: + LOG_INFO("Set module config: TAK"); + moduleConfig.has_tak = true; + moduleConfig.tak = c.payload_variant.tak; + break; #if !MESHTASTIC_EXCLUDE_BEACON case meshtastic_ModuleConfig_mesh_beacon_tag: { LOG_INFO("Set module config: MeshBeacon"); @@ -1617,6 +1622,11 @@ void AdminModule::handleGetModuleConfig(const meshtastic_MeshPacket &req, const 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; + case meshtastic_AdminMessage_ModuleConfigType_TAK_CONFIG: + configName = "TAK"; + res.get_module_config_response.which_payload_variant = meshtastic_ModuleConfig_tak_tag; + res.get_module_config_response.payload_variant.tak = moduleConfig.tak; + break; #if !MESHTASTIC_EXCLUDE_BEACON case meshtastic_AdminMessage_ModuleConfigType_MESHBEACON_CONFIG: configName = "MeshBeacon"; diff --git a/test/README.md b/test/README.md index 58700abe7..68b271f1e 100644 --- a/test/README.md +++ b/test/README.md @@ -404,6 +404,8 @@ A well-structured test suite follows this pattern: | `test_position_precision` | Position precision helpers | | `test_radio` | Radio interface | | `test_serial` | Serial communication | +| `test_module_config` | AdminModule module config | +| `test_tak_config` | TAK (ATAK) team/role values | | `test_traffic_management` | Traffic management | | `test_transmit_history` | Retransmission tracking | | `test_type_conversions` | NodeDB v25 type conversions | diff --git a/test/native-suite-count b/test/native-suite-count index e522732c7..425151f3a 100644 --- a/test/native-suite-count +++ b/test/native-suite-count @@ -1 +1 @@ -38 +40 diff --git a/test/test_module_config/test_main.cpp b/test/test_module_config/test_main.cpp new file mode 100644 index 000000000..37cc98b2a --- /dev/null +++ b/test/test_module_config/test_main.cpp @@ -0,0 +1,195 @@ +// AdminModule module-config lifecycle coverage: every submessage must survive set -> save -> load -> get. +// +// Motivated by firmware#11182 / Meshtastic-Android#6430 (TAK team/role never persisted): a submessage +// with no case in handleSetModuleConfig() falls through the switch silently - the setter still ACKs +// and reboots - and one missing has_* flag in saveToDisk() makes nanopb drop the struct on the flash +// write. Both defects are invisible to per-module tests until someone hits them in the field, so the +// sweeps below enumerate the generated constants and put each type through the full lifecycle. +// +// The sweeps assert presence and routing, not field semantics - per-module value validation belongs +// to the per-module suites (test_mesh_beacon, test_admin_session_repro, ...). + +#include "MeshTypes.h" // include BEFORE TestUtil.h +#include "TestUtil.h" +#include + +#include "mesh/NodeDB.h" +#include "mesh/mesh-pb-constants.h" +#include "modules/AdminModule.h" +#include "support/AdminModuleTestShim.h" +#include "support/MockMeshService.h" +#include +#include + +static constexpr NodeNum LOCAL_NODE = 0x0A0A0A0A; +static constexpr NodeNum ADMIN_NODE = 0x0B0B0B0B; + +static MockMeshService *mockService = nullptr; +static AdminModuleTestShim *admin = nullptr; + +// Both generated sets are contiguous: ModuleConfigType runs 0.._MAX and the matching ModuleConfig +// submessage tags run 1.._MAX+1, so type + 1 == tag and no lookup table is needed. +static const uint32_t moduleConfigTypeCount = _meshtastic_AdminMessage_ModuleConfigType_MAX + 1; + +// The submessages whose handlers are compiled out on some builds. Everything else must be present +// unconditionally. +static bool moduleConfigIsCompiledOut(pb_size_t tag) +{ + (void)tag; +#if MESHTASTIC_EXCLUDE_MQTT + if (tag == meshtastic_ModuleConfig_mqtt_tag) + return true; +#endif +#if MESHTASTIC_EXCLUDE_BEACON + if (tag == meshtastic_ModuleConfig_mesh_beacon_tag) + return true; +#endif + return false; +} + +// Unity assertion messages are plain strings, so name the failing iteration by number - nanopb keeps +// no field names to print. Cross-reference module_config.pb.h / admin.pb.h. +static const char *describeTag(const char *what, pb_size_t tag) +{ + static char buf[96]; + snprintf(buf, sizeof(buf), "%s: ModuleConfig submessage tag %u", what, (unsigned)tag); + return buf; +} + +static meshtastic_MeshPacket makeGetRequest(NodeNum from) +{ + meshtastic_MeshPacket req = meshtastic_MeshPacket_init_zero; + req.from = from; + req.which_payload_variant = meshtastic_MeshPacket_decoded_tag; + req.decoded.want_response = true; + return req; +} + +// Read back the variant tag of whatever get_module_config response the handler queued. +static bool decodeResponseTagFromReply(meshtastic_MeshPacket *reply, pb_size_t &out) +{ + meshtastic_AdminMessage am = meshtastic_AdminMessage_init_zero; + if (!reply || reply->which_payload_variant != meshtastic_MeshPacket_decoded_tag) + return false; + if (!pb_decode_from_bytes(reply->decoded.payload.bytes, reply->decoded.payload.size, &meshtastic_AdminMessage_msg, &am)) + return false; + if (am.which_payload_variant != meshtastic_AdminMessage_get_module_config_response_tag) + return false; + out = am.get_module_config_response.which_payload_variant; + return true; +} + +// The proto guarantees the two enumerations stay in step; the sweeps below rely on it to pair a +// ModuleConfigType with its submessage tag without a lookup table. +void test_tagRangeMatchesTypeRange(void) +{ + TEST_ASSERT_EQUAL_MESSAGE(moduleConfigTypeCount, meshtastic_ModuleConfig_mesh_beacon_tag, + "ModuleConfigType and ModuleConfig submessage tags are no longer 1:1"); +} + +// The full lifecycle per submessage: admin set -> encode (what saveToDisk writes) -> decode into the +// live global (a reboot's loadFromDisk) -> admin get answers with that submessage. A missing set case +// shows up as a 0-byte encode; a missing get case as an empty or mismatched response. +void test_everyModuleConfig_survivesSetSaveLoadGet(void) +{ + for (pb_size_t tag = 1; tag <= moduleConfigTypeCount; tag++) { + if (moduleConfigIsCompiledOut(tag)) + continue; + moduleConfig = meshtastic_LocalModuleConfig_init_zero; + + // Set: the only content is this one submessage, so the encode below stands or falls with it. + meshtastic_ModuleConfig mc = meshtastic_ModuleConfig_init_zero; + mc.which_payload_variant = tag; + TEST_ASSERT_TRUE_MESSAGE(admin->handleSetModuleConfig(mc), describeTag("setter rejected", tag)); + + // Save: presence flag unset means nanopb drops the struct and the write encodes to nothing. + uint8_t saved[meshtastic_LocalModuleConfig_size]; + size_t savedLen = pb_encode_to_bytes(saved, sizeof(saved), &meshtastic_LocalModuleConfig_msg, &moduleConfig); + TEST_ASSERT_GREATER_THAN_size_t_MESSAGE(0, savedLen, describeTag("dropped on save, presence flag not set", tag)); + + // Load: overwrite the live global, as a reboot would. + moduleConfig = meshtastic_LocalModuleConfig_init_zero; + TEST_ASSERT_TRUE_MESSAGE(pb_decode_from_bytes(saved, savedLen, &meshtastic_LocalModuleConfig_msg, &moduleConfig), + describeTag("failed to decode", tag)); + + // Re-encoding what was loaded must reproduce the saved bytes - presence survived the reload. + uint8_t reloaded[meshtastic_LocalModuleConfig_size]; + size_t reloadedLen = pb_encode_to_bytes(reloaded, sizeof(reloaded), &meshtastic_LocalModuleConfig_msg, &moduleConfig); + TEST_ASSERT_EQUAL_size_t_MESSAGE(savedLen, reloadedLen, describeTag("changed size across reload", tag)); + TEST_ASSERT_EQUAL_MEMORY_MESSAGE(saved, reloaded, savedLen, describeTag("changed content across reload", tag)); + + // Get: remote and local requests take the same branch and must answer with this submessage. + const NodeNum requesters[] = {ADMIN_NODE, 0}; + for (NodeNum from : requesters) { + meshtastic_MeshPacket req = makeGetRequest(from); + admin->handleGetModuleConfig(req, tag - 1); // type pairs 1:1 with tag (guard test above) + + pb_size_t responseTag = 0; + bool decoded = decodeResponseTagFromReply(admin->reply(), responseTag); + admin->drainReply(); + + TEST_ASSERT_TRUE_MESSAGE(decoded, describeTag("no get response", tag)); + TEST_ASSERT_EQUAL_MESSAGE(tag, responseTag, describeTag("get answered with a different submessage", tag)); + } + } +} + +// Each set is a single-submessage edit: writing type N must not erase types 1..N-1. Every additional +// stored submessage grows the encoded config, so a clobbered earlier write shows up as a non-increase. +void test_sequentialSets_preserveEarlierWrites(void) +{ + size_t prevLen = 0; + for (pb_size_t tag = 1; tag <= moduleConfigTypeCount; tag++) { + if (moduleConfigIsCompiledOut(tag)) + continue; + + meshtastic_ModuleConfig mc = meshtastic_ModuleConfig_init_zero; + mc.which_payload_variant = tag; + admin->handleSetModuleConfig(mc); + + uint8_t buf[meshtastic_LocalModuleConfig_size]; + size_t len = pb_encode_to_bytes(buf, sizeof(buf), &meshtastic_LocalModuleConfig_msg, &moduleConfig); + TEST_ASSERT_GREATER_THAN_size_t_MESSAGE(prevLen, len, describeTag("set erased an earlier submessage", tag)); + prevLen = len; + } +} + +void setUp(void) +{ + mockService = new MockMeshService(); + service = mockService; + + admin = new AdminModuleTestShim(); + admin->deferSaves(); // no disk write or reboot when a setter is accepted + + moduleConfig = meshtastic_LocalModuleConfig_init_zero; + config = meshtastic_LocalConfig_init_zero; + myNodeInfo = meshtastic_MyNodeInfo_init_zero; + myNodeInfo.my_node_num = LOCAL_NODE; +} + +void tearDown(void) +{ + admin->drainReply(); + delete admin; + admin = nullptr; + + service = nullptr; + delete mockService; + mockService = nullptr; +} + +void setup() +{ + delay(10); + initializeTestEnvironment(); + UNITY_BEGIN(); + + RUN_TEST(test_tagRangeMatchesTypeRange); + RUN_TEST(test_everyModuleConfig_survivesSetSaveLoadGet); + RUN_TEST(test_sequentialSets_preserveEarlierWrites); + + exit(UNITY_END()); +} + +void loop() {} diff --git a/test/test_tak_config/test_main.cpp b/test/test_tak_config/test_main.cpp new file mode 100644 index 000000000..672237376 --- /dev/null +++ b/test/test_tak_config/test_main.cpp @@ -0,0 +1,154 @@ +// TAK (ATAK) team/role value fidelity: the fields set by the app must come back verbatim. +// +// Companion to test_module_config, which proves every submessage's presence survives the +// set -> save -> load -> get lifecycle but sweeps with zero-valued payloads and so cannot tell a +// copied field from an ignored one. These tests pin the values themselves for the submessage that +// shipped broken (firmware#11182 / Meshtastic-Android#6430: team/role reverted to "unspecified" +// after every save). + +#include "MeshTypes.h" // include BEFORE TestUtil.h +#include "TestUtil.h" +#include + +#include "mesh/NodeDB.h" +#include "mesh/mesh-pb-constants.h" +#include "modules/AdminModule.h" +#include "support/AdminModuleTestShim.h" +#include "support/MockMeshService.h" +#include + +static constexpr NodeNum LOCAL_NODE = 0x0A0A0A0A; +static constexpr NodeNum ADMIN_NODE = 0x0B0B0B0B; + +static MockMeshService *mockService = nullptr; +static AdminModuleTestShim *admin = nullptr; + +// A set_module_config payload carrying the tak submessage, as the phone app sends it. +static meshtastic_ModuleConfig makeTakModuleConfig(meshtastic_Team team, meshtastic_MemberRole role) +{ + meshtastic_ModuleConfig mc = meshtastic_ModuleConfig_init_zero; + mc.which_payload_variant = meshtastic_ModuleConfig_tak_tag; + mc.payload_variant.tak.team = team; + mc.payload_variant.tak.role = role; + return mc; +} + +static meshtastic_MeshPacket makeGetRequest(NodeNum from) +{ + meshtastic_MeshPacket req = meshtastic_MeshPacket_init_zero; + req.from = from; + req.which_payload_variant = meshtastic_MeshPacket_decoded_tag; + req.decoded.want_response = true; + return req; +} + +// Pull the TAKConfig out of the get_module_config response a handler queued in myReply. +static bool decodeTakFromReply(meshtastic_MeshPacket *reply, meshtastic_ModuleConfig_TAKConfig &out) +{ + meshtastic_AdminMessage am = meshtastic_AdminMessage_init_zero; + if (!reply || reply->which_payload_variant != meshtastic_MeshPacket_decoded_tag) + return false; + if (!pb_decode_from_bytes(reply->decoded.payload.bytes, reply->decoded.payload.size, &meshtastic_AdminMessage_msg, &am)) + return false; + if (am.which_payload_variant != meshtastic_AdminMessage_get_module_config_response_tag || + am.get_module_config_response.which_payload_variant != meshtastic_ModuleConfig_tak_tag) + return false; + out = am.get_module_config_response.payload_variant.tak; + return true; +} + +void test_setStoresTeamAndRoleVerbatim(void) +{ + TEST_ASSERT_TRUE(admin->handleSetModuleConfig(makeTakModuleConfig(meshtastic_Team_Cyan, meshtastic_MemberRole_Medic))); + + TEST_ASSERT_TRUE(moduleConfig.has_tak); + TEST_ASSERT_EQUAL(meshtastic_Team_Cyan, moduleConfig.tak.team); + TEST_ASSERT_EQUAL(meshtastic_MemberRole_Medic, moduleConfig.tak.role); +} + +// Clearing back to the zero values is a real edit, not an absent field: has_tak must stay set so the +// cleared state is what gets written, rather than the previous value surviving. +void test_clearingToUnspecifiedIsStored(void) +{ + admin->handleSetModuleConfig(makeTakModuleConfig(meshtastic_Team_Red, meshtastic_MemberRole_TeamLead)); + admin->handleSetModuleConfig(makeTakModuleConfig(meshtastic_Team_Unspecifed_Color, meshtastic_MemberRole_Unspecifed)); + + TEST_ASSERT_TRUE(moduleConfig.has_tak); + TEST_ASSERT_EQUAL(meshtastic_Team_Unspecifed_Color, moduleConfig.tak.team); + TEST_ASSERT_EQUAL(meshtastic_MemberRole_Unspecifed, moduleConfig.tak.role); +} + +// The reported symptom end to end: values set, saved, reloaded (as a reboot would), and read back. +void test_valuesSurviveSaveLoad(void) +{ + admin->handleSetModuleConfig(makeTakModuleConfig(meshtastic_Team_Magenta, meshtastic_MemberRole_Sniper)); + + uint8_t buf[meshtastic_LocalModuleConfig_size]; + size_t len = pb_encode_to_bytes(buf, sizeof(buf), &meshtastic_LocalModuleConfig_msg, &moduleConfig); + + moduleConfig = meshtastic_LocalModuleConfig_init_zero; + TEST_ASSERT_TRUE(pb_decode_from_bytes(buf, len, &meshtastic_LocalModuleConfig_msg, &moduleConfig)); + + TEST_ASSERT_TRUE(moduleConfig.has_tak); + TEST_ASSERT_EQUAL(meshtastic_Team_Magenta, moduleConfig.tak.team); + TEST_ASSERT_EQUAL(meshtastic_MemberRole_Sniper, moduleConfig.tak.role); +} + +// Remote and local admin reads take the same branch; both must carry the stored values unredacted. +void test_getReturnsStoredValues(void) +{ + admin->handleSetModuleConfig(makeTakModuleConfig(meshtastic_Team_Dark_Blue, meshtastic_MemberRole_K9)); + + const NodeNum requesters[] = {ADMIN_NODE, 0}; + for (NodeNum from : requesters) { + meshtastic_MeshPacket req = makeGetRequest(from); + admin->handleGetModuleConfig(req, meshtastic_AdminMessage_ModuleConfigType_TAK_CONFIG); + + meshtastic_ModuleConfig_TAKConfig tak; + TEST_ASSERT_TRUE_MESSAGE(decodeTakFromReply(admin->reply(), tak), "TAK_CONFIG request must produce a tak response"); + TEST_ASSERT_EQUAL(meshtastic_Team_Dark_Blue, tak.team); + TEST_ASSERT_EQUAL(meshtastic_MemberRole_K9, tak.role); + admin->drainReply(); + } +} + +void setUp(void) +{ + mockService = new MockMeshService(); + service = mockService; + + admin = new AdminModuleTestShim(); + admin->deferSaves(); // no disk write or reboot when a setter is accepted + + moduleConfig = meshtastic_LocalModuleConfig_init_zero; + config = meshtastic_LocalConfig_init_zero; + myNodeInfo = meshtastic_MyNodeInfo_init_zero; + myNodeInfo.my_node_num = LOCAL_NODE; +} + +void tearDown(void) +{ + admin->drainReply(); + delete admin; + admin = nullptr; + + service = nullptr; + delete mockService; + mockService = nullptr; +} + +void setup() +{ + delay(10); + initializeTestEnvironment(); + UNITY_BEGIN(); + + RUN_TEST(test_setStoresTeamAndRoleVerbatim); + RUN_TEST(test_clearingToUnspecifiedIsStored); + RUN_TEST(test_valuesSurviveSaveLoad); + RUN_TEST(test_getReturnsStoredValues); + + exit(UNITY_END()); +} + +void loop() {}