Merge branch 'develop' into mesh-pager-x2

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

src/modules/MeshBeaconModule.cpp

@@ -11,6 +11,7 @@
 #include "main.h"
 #include "meshUtils.h"
 #include <Throttle.h>
+#include <algorithm>
 #include <string.h>
 
 // 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<int32_t>(intervalMs);
+    // A save from the nRF52 BLE callback can land mid-run, where run() would overwrite its setIntervalFromNow()
+    if (rescheduleRequested)
+        nextRunMs = std::min<uint32_t>(nextRunMs, MESHMODULE_MIN_BROADCAST_DELAY_MS);
+
+    return static_cast<int32_t>(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);
 }
 
 // ---------------------------------------------------------------------------

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 <atomic>
 
 // 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<meshtastic_MeshBeacon>,
                                   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<bool> rescheduleRequested{false};
 };
 extern MeshBeaconBroadcastModule *meshBeaconBroadcastModule;
 

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

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 <cstdio>
 #include <cstring>
+#include <functional>
 #include <memory>
 #include <pb_decode.h>
 #include <vector>
@@ -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<void()> 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<MeshBeaconBroadcastModule &>(*this).setInterval(ms); }
+    unsigned long threadInterval() const { return static_cast<const MeshBeaconBroadcastModule &>(*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");