diff --git a/src/modules/AdminModule.cpp b/src/modules/AdminModule.cpp index 4a8249a2ce..0e2ded0564 100644 --- a/src/modules/AdminModule.cpp +++ b/src/modules/AdminModule.cpp @@ -1394,8 +1394,10 @@ bool AdminModule::handleSetModuleConfig(const meshtastic_ModuleConfig &c) moduleConfig.mesh_beacon = beaconCfg; shouldReboot = false; // Payload content changed - invalidate the broadcaster's cache. - if (meshBeaconBroadcastModule) + if (meshBeaconBroadcastModule) { meshBeaconBroadcastModule->invalidateCache(); + meshBeaconBroadcastModule->reschedule(); + } break; } #endif diff --git a/src/modules/MeshBeaconModule.cpp b/src/modules/MeshBeaconModule.cpp index eda236d85f..a93e7c728d 100644 --- a/src/modules/MeshBeaconModule.cpp +++ b/src/modules/MeshBeaconModule.cpp @@ -11,6 +11,7 @@ #include "main.h" #include "meshUtils.h" #include +#include #include // Static members @@ -821,25 +822,47 @@ void MeshBeaconBroadcastModule::sendBeacon() int32_t MeshBeaconBroadcastModule::runOnce() { + rescheduleRequested = false; const auto &bcfg = moduleConfig.mesh_beacon; const uint32_t intervalMs = beaconIntervalMs(); + uint32_t nextRunMs = intervalMs; - if ((bcfg.flags & MESH_BEACON_FLAG_BROADCAST_ENABLED) && airTime->isTxAllowedAirUtil() && + if ((bcfg.flags & MESH_BEACON_FLAG_BROADCAST_ENABLED) && config.device.role != meshtastic_Config_DeviceConfig_Role_CLIENT_HIDDEN) { // Throttle against the reboot-safe transmit history (mirrors NodeInfoModule): skip if we // broadcast within the interval, even across a reboot. 0 = never sent → send now. uint32_t lastSent = transmitHistory ? transmitHistory->getLastSentToMeshMillis(meshtastic_PortNum_MESH_BEACON_APP) : 0; - if (lastSent == 0 || !Throttle::isWithinTimespanMs(lastSent, intervalMs)) { + const uint32_t remainingMs = lastSent == 0 ? 0 : Throttle::remainingMs(lastSent, intervalMs); + if (remainingMs == 0 && !airTime->isTxAllowedAirUtil()) { + // The TX budget is a rolling hour, so a due beacon rechecks within it rather than a full interval later + airtimeRetryMs = airtimeRetryMs == 0 ? kAirtimeRetryMinMs : std::min(airtimeRetryMs * 2, kAirtimeRetryMaxMs); + nextRunMs = airtimeRetryMs; + } else if (remainingMs == 0) { + airtimeRetryMs = 0; // Record the send BEFORE transmitting: the LoRa TX is a high-current event that can // brown out a marginal supply, and if that reboots us mid-transmit we still want the // "sent" marker persisted so we don't re-broadcast immediately on every boot. if (transmitHistory) transmitHistory->setLastSentToMesh(meshtastic_PortNum_MESH_BEACON_APP); sendBeacon(); + } else { + // Wake when the last send's interval runs out, not a full interval from now + nextRunMs = remainingMs; } } - return static_cast(intervalMs); + // A save from the nRF52 BLE callback can land mid-run, where run() would overwrite its setIntervalFromNow() + if (rescheduleRequested) + nextRunMs = std::min(nextRunMs, MESHMODULE_MIN_BROADCAST_DELAY_MS); + + return static_cast(nextRunMs); +} + +void MeshBeaconBroadcastModule::reschedule() +{ + // runOnce() re-reads the config and the transmit history, so it sends or waits out the remainder + rescheduleRequested = true; + setIntervalFromNow(MESHMODULE_MIN_BROADCAST_DELAY_MS); } // --------------------------------------------------------------------------- diff --git a/src/modules/MeshBeaconModule.h b/src/modules/MeshBeaconModule.h index 113258a75a..e54330cb0e 100644 --- a/src/modules/MeshBeaconModule.h +++ b/src/modules/MeshBeaconModule.h @@ -7,6 +7,7 @@ #include "concurrency/OSThread.h" #include "mesh/generated/meshtastic/mesh_beacon.pb.h" #include "mesh/generated/meshtastic/module_config.pb.h" +#include // Short aliases for the MeshBeaconConfig.flags bitfield (see module_config.proto MeshBeaconConfig.Flags). #define MESH_BEACON_FLAG_LISTEN_ENABLED meshtastic_ModuleConfig_MeshBeaconConfig_Flags_FLAG_LISTEN_ENABLED @@ -153,12 +154,17 @@ class MeshBeaconBroadcastModule : private MeshBeaconModule, public ProtobufModule, private concurrency::OSThread { + friend class MeshBeaconBroadcastModuleTestShim; // test/test_mesh_beacon reads the thread interval + public: MeshBeaconBroadcastModule(); // Mark the cached payload dirty (call after config change). void invalidateCache() { payloadCacheDirty = true; } + // Re-evaluate the schedule shortly, so a saved enable or interval change does not wait out the old period. + void reschedule(); + protected: virtual bool handleReceivedProtobuf(const meshtastic_MeshPacket &, meshtastic_MeshBeacon *) override { return false; } virtual int32_t runOnce() override; @@ -174,6 +180,14 @@ class MeshBeaconBroadcastModule : private MeshBeaconModule, bool payloadCacheDirty = true; uint8_t payloadCache[meshtastic_MeshBeacon_size] = {}; pb_size_t payloadCacheSize = 0; + + // Backoff while a due beacon is held by the air-util limit; 0 when not held. + static constexpr uint32_t kAirtimeRetryMinMs = 5 * 60 * 1000; + static constexpr uint32_t kAirtimeRetryMaxMs = 60 * 60 * 1000; + uint32_t airtimeRetryMs = 0; + + // Set by reschedule(), which may run on another task; runOnce() honors one that lands while it runs. + std::atomic rescheduleRequested{false}; }; extern MeshBeaconBroadcastModule *meshBeaconBroadcastModule; diff --git a/test/state-manifest.tsv b/test/state-manifest.tsv index a8204c55e3..322c97c5ba 100644 --- a/test/state-manifest.tsv +++ b/test/state-manifest.tsv @@ -55,7 +55,7 @@ test_fuzz_decode errors=20000..250000 fuzzes protobuf decode; every rejection lo test_fuzz_packets writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto,warm.dat,Messages_default.msgs errors=5000..60000 drives decode of fuzzed packets through the real NodeDB and message store test_hop_scaling writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto constructs a NodeDB to hold the hop-distance fixtures test_hop_start_policy writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto constructs a NodeDB (isFromUs needs nodeDB->getNodeNum()), whose constructor persists a default set when the prefs directory is empty -test_mesh_beacon writes=module.proto exercises the beacon's module-config save path +test_mesh_beacon writes=module.proto,transmit_history.dat exercises the beacon's module-config save path; the broadcaster's interval tests stamp TransmitHistory, which persists the beacon send time test_mesh_module writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto,warm.dat module framework tests construct a NodeDB test_mqtt writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto errors=1000..12000 constructs a NodeDB for node lookups in the MQTT paths test_muted_source writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto constructs a NodeDB (isToUs needs nodeDB->getNodeNum(), and the DM branch looks the sender up), whose constructor persists a default set when the prefs directory is empty diff --git a/test/test_mesh_beacon/test_main.cpp b/test/test_mesh_beacon/test_main.cpp index 540e594c67..f5ee5a3ce6 100644 --- a/test/test_mesh_beacon/test_main.cpp +++ b/test/test_mesh_beacon/test_main.cpp @@ -25,12 +25,14 @@ #include "RadioInterface.h" #include "UptimeClock.h" #include "airtime.h" +#include "mesh/TransmitHistory.h" #include "modules/AdminModule.h" #include "modules/MeshBeaconModule.h" #include "support/AdminModuleTestShim.h" #include "support/MockMeshService.h" #include #include +#include #include #include #include @@ -71,6 +73,8 @@ class MockRouter : public Router if (channelFile.channels_count > 0) primaryAtSend.push_back(channels.getByIndex(channels.getPrimaryIndex()).settings); sentPackets.push_back(*p); + if (onSend) + onSend(); // Router::send() frees p on every path but ERRNO_SHOULD_RELEASE, which leaves it to the caller if (nextResult != ERRNO_SHOULD_RELEASE) packetPool.release(p); @@ -78,6 +82,8 @@ class MockRouter : public Router } ErrorCode nextResult = ERRNO_OK; + // Runs inside send(), so a test can act while the broadcaster's runOnce() is still on the stack + std::function onSend; // Locally-addressed packets land here instead of send(). Release immediately // rather than queuing into fromRadioQueue (which is never drained in tests). @@ -89,8 +95,11 @@ class MockRouter : public Router // AdminModuleTestShim (test/support) exposes protected handleSetModuleConfig. +} // namespace + // --------------------------------------------------------------------------- -// MeshBeaconBroadcastModuleTestShim - exposes private internals for testing. +// MeshBeaconBroadcastModuleTestShim - exposes private internals for testing. Outside the anonymous +// namespace so it is the class MeshBeaconBroadcastModule names as a friend. // --------------------------------------------------------------------------- class MeshBeaconBroadcastModuleTestShim : public MeshBeaconBroadcastModule { @@ -101,8 +110,14 @@ class MeshBeaconBroadcastModuleTestShim : public MeshBeaconBroadcastModule using MeshBeaconBroadcastModule::rebuildCache; using MeshBeaconBroadcastModule::runOnce; using MeshBeaconBroadcastModule::sendBeacon; + + void setThreadInterval(unsigned long ms) { static_cast(*this).setInterval(ms); } + unsigned long threadInterval() const { return static_cast(*this).interval; } }; +namespace +{ + // --------------------------------------------------------------------------- // MeshBeaconListenerModuleTestShim - exposes handleReceivedProtobuf. // --------------------------------------------------------------------------- @@ -836,6 +851,103 @@ static void test_broadcaster_runOnce_silentWhenDisabled(void) TEST_ASSERT_EQUAL_UINT32(0, mockRouter->sentPackets.size()); } +/** + * Verify runOnce sleeps only the rest of the interval when the history shows a recent send. + * Important because a reboot inside the window otherwise pushes the next beacon to boot + a full interval, + * nearly doubling the gap on a long interval. + */ +static void test_broadcaster_runOnce_withinInterval_waitsRemainder(void) +{ + resetConfig(); + moduleConfig.has_mesh_beacon = true; + moduleConfig.mesh_beacon.flags |= MESH_BEACON_FLAG_BROADCAST_ENABLED; + moduleConfig.mesh_beacon.broadcast_interval_secs = 48 * 3600; + strncpy(moduleConfig.mesh_beacon.broadcast_message, "remainder", sizeof(moduleConfig.mesh_beacon.broadcast_message) - 1); + + const uint32_t intervalMs = 48UL * 3600 * 1000; + const uint32_t elapsedMs = 12UL * 3600 * 1000; + Time::setTestMillis(3600UL * 1000); + TransmitHistory::getInstance()->setLastSentToMesh(meshtastic_PortNum_MESH_BEACON_APP); // tearDown frees it + Time::advanceTestMillis(elapsedMs); + + MeshBeaconBroadcastModuleTestShim bcast; + const int32_t next = bcast.runOnce(); + + TEST_ASSERT_EQUAL_UINT32(0, mockRouter->sentPackets.size()); + TEST_ASSERT_EQUAL_INT32((int32_t)(intervalMs - elapsedMs), next); + + Time::advanceTestMillis((uint32_t)next); + TEST_ASSERT_EQUAL_INT32((int32_t)intervalMs, bcast.runOnce()); + TEST_ASSERT_EQUAL_UINT32(1, mockRouter->sentPackets.size()); +} + +/** + * Verify a due beacon held by the air-util limit retries on a capped doubling backoff, then sends once the limit clears. + * Important because a held beacon otherwise waits a whole interval, and the TX budget it waits on clears within the hour. + */ +static void test_broadcaster_runOnce_airtimeHeld_backsOffThenSends(void) +{ + resetConfig(); + config.lora.override_duty_cycle = false; // EU_868 is 10% duty cycle, so the polite TX limit is 5% + moduleConfig.has_mesh_beacon = true; + moduleConfig.mesh_beacon.flags |= MESH_BEACON_FLAG_BROADCAST_ENABLED; + moduleConfig.mesh_beacon.broadcast_interval_secs = 48 * 3600; + strncpy(moduleConfig.mesh_beacon.broadcast_message, "held", sizeof(moduleConfig.mesh_beacon.broadcast_message) - 1); + testAirTime->logAirtime(TX_LOG, 10 * 60 * 1000); // 10 minutes of TX in the last hour + TEST_ASSERT_FALSE(testAirTime->isTxAllowedAirUtil()); + + MeshBeaconBroadcastModuleTestShim bcast; + const int32_t expected[] = {5, 10, 20, 40, 60, 60}; + for (int32_t minutes : expected) + TEST_ASSERT_EQUAL_INT32(minutes * 60 * 1000, bcast.runOnce()); + TEST_ASSERT_EQUAL_UINT32(0, mockRouter->sentPackets.size()); + + config.lora.override_duty_cycle = true; + TEST_ASSERT_EQUAL_INT32(48 * 3600 * 1000, bcast.runOnce()); + TEST_ASSERT_EQUAL_UINT32(1, mockRouter->sentPackets.size()); +} + +/** + * Verify saving the beacon config pulls the broadcaster's next run in from the old period. + * Important because enabling broadcast or shortening the interval otherwise waits out the period in force. + */ +static void test_broadcaster_setModuleConfig_reschedulesSoon(void) +{ + resetConfig(); + MeshBeaconBroadcastModuleTestShim bcast; + meshBeaconBroadcastModule = &bcast; + bcast.setThreadInterval(48UL * 3600 * 1000); + + meshtastic_ModuleConfig_MeshBeaconConfig bcfg = meshtastic_ModuleConfig_MeshBeaconConfig_init_zero; + bcfg.flags = MESH_BEACON_FLAG_BROADCAST_ENABLED; + bcfg.broadcast_interval_secs = 2 * 3600; + testAdmin->handleSetModuleConfig(makeBeaconModuleConfig(bcfg)); + + TEST_ASSERT_EQUAL_UINT32(MESHMODULE_MIN_BROADCAST_DELAY_MS, bcast.threadInterval()); +} + +/** + * Verify a config save that lands while runOnce() runs still shortens the next run. + * Important because on nRF52 the BLE write callback saves on its own task, and run() applies runOnce()'s return after + * any setIntervalFromNow() made meanwhile, which would leave the new config waiting out the old interval. + */ +static void test_broadcaster_rescheduleDuringRun_isKept(void) +{ + resetConfig(); + moduleConfig.has_mesh_beacon = true; + moduleConfig.mesh_beacon.flags |= MESH_BEACON_FLAG_BROADCAST_ENABLED; + moduleConfig.mesh_beacon.broadcast_interval_secs = 48 * 3600; + strncpy(moduleConfig.mesh_beacon.broadcast_message, "mid-run", sizeof(moduleConfig.mesh_beacon.broadcast_message) - 1); + + MeshBeaconBroadcastModuleTestShim bcast; + mockRouter->onSend = [&bcast]() { bcast.reschedule(); }; + TEST_ASSERT_EQUAL_INT32(MESHMODULE_MIN_BROADCAST_DELAY_MS, bcast.runOnce()); + TEST_ASSERT_EQUAL_UINT32(1, mockRouter->sentPackets.size()); + + mockRouter->onSend = nullptr; + TEST_ASSERT_EQUAL_INT32(48 * 3600 * 1000, bcast.runOnce()); // the request does not outlive the run that saw it +} + // =========================================================================== // Group 4: Listener - offer caching and guards // =========================================================================== @@ -2307,6 +2419,10 @@ void tearDown(void) { meshBeaconBroadcastModule = nullptr; + delete transmitHistory; + transmitHistory = nullptr; + Time::useRealClock(); + delete testAdmin; testAdmin = nullptr; @@ -2377,6 +2493,10 @@ BEACON_TEST_ENTRY void setup() RUN_TEST(test_broadcaster_sendBeacon_offerOnly_isSent); RUN_TEST(test_broadcaster_runOnce_sendsWhenEnabled); RUN_TEST(test_broadcaster_runOnce_silentWhenDisabled); + RUN_TEST(test_broadcaster_runOnce_withinInterval_waitsRemainder); + RUN_TEST(test_broadcaster_runOnce_airtimeHeld_backsOffThenSends); + RUN_TEST(test_broadcaster_setModuleConfig_reschedulesSoon); + RUN_TEST(test_broadcaster_rescheduleDuringRun_isKept); printf("\n=== Listener offer caching ===\n");