fix(beacon): repair the MeshBeacon radio switch/restore regression from #11573

src/mesh/RadioInterface.cpp

@@ -1518,15 +1518,17 @@ size_t RadioInterface::beginSending(meshtastic_MeshPacket *p)
 
     // if the sender nodenum is zero, that means uninitialized
     assert(radioBuffer.header.from);
-    // Runtime packet payload size bounds check against radioBuffer to prevent overflow in memcpy()
-    if (static_cast<size_t>(p->encrypted.size) > sizeof(radioBuffer.payload)) {
-        LOG_ERROR("Packet payload size %u exceeds radioBuffer capacity %u", static_cast<unsigned>(p->encrypted.size),
-                  static_cast<unsigned>(sizeof(radioBuffer.payload)));
-        packetPool.release(p);
-        return 0;
+
+    // Oversize is rejected at the radio queue in Router::send(); clamp rather than fail here so this
+    // stays a call that always succeeds, with no failure return for startSend() to unwind.
+    size_t payloadLen = p->encrypted.size;
+    if (payloadLen > MAX_RADIO_PAYLOAD_LEN) {
+        LOG_ERROR("Payload %u exceeds radioBuffer capacity %u, truncate", (unsigned)payloadLen, (unsigned)MAX_RADIO_PAYLOAD_LEN);
+        payloadLen = MAX_RADIO_PAYLOAD_LEN;
     }
-    memcpy(radioBuffer.payload, p->encrypted.bytes, p->encrypted.size);
+
+    memcpy(radioBuffer.payload, p->encrypted.bytes, payloadLen);
 
     sendingPacket = p;
-    return p->encrypted.size + sizeof(PacketHeader);
+    return payloadLen + sizeof(PacketHeader);
 }

src/mesh/RadioInterface.h

@@ -67,6 +67,9 @@ typedef struct {
 
 } RadioBuffer;
 
+/// On-air ceiling for MeshPacket.encrypted - what a single RadioBuffer can actually carry.
+constexpr size_t MAX_RADIO_PAYLOAD_LEN = sizeof(RadioBuffer::payload);
+
 /**
  * Basic operations all radio chipsets must implement.
  *

src/mesh/RadioLibInterface.cpp

@@ -248,8 +248,14 @@ bool RadioLibInterface::isSending()
 bool RadioLibInterface::cancelSending(NodeNum from, PacketId id)
 {
     auto p = txQueue.remove(from, id);
-    if (p)
+    if (p) {
+#if !MESHTASTIC_EXCLUDE_BEACON
+        // Every path that abandons a queued packet clears its beacon target first; the restore is
+        // gated on no target being live, so a leftover entry would pin the radio on the beacon config.
+        MeshBeaconModule::clearTargetRadioSettings(p);
+#endif
         packetPool.release(p); // free the packet we just removed
+    }
 
     bool result = (p != NULL);
     LOG_DEBUG("cancelSending id=0x%08x, removed=%d", id, result);
@@ -596,14 +602,15 @@ void RadioLibInterface::completeSending()
             txRelay++;
         printPacket("Completed sending", p);
 #if !MESHTASTIC_EXCLUDE_BEACON
+        // Clear first, and keep both inside `if (p)`: completeSending() also runs on every
+        // setStandby(), where a restore undoes the switch on the pre-TX scan and then recurses.
         MeshBeaconModule::clearTargetRadioSettings(p);
+        MeshBeaconModule::reconfigureForBeaconTX(this, nullptr);
 #endif
+
         // We are done sending that packet, release it
         packetPool.release(p);
     }
-#if !MESHTASTIC_EXCLUDE_BEACON
-    MeshBeaconModule::reconfigureForBeaconTX(this, nullptr);
-#endif
 }
 
 void RadioLibInterface::handleReceiveInterrupt()
@@ -783,18 +790,7 @@ bool RadioLibInterface::startSend(meshtastic_MeshPacket *txp)
     } else {
         configHardwareForSend(); // must be after setStandby
 
-#if !MESHTASTIC_EXCLUDE_BEACON
-        MeshBeaconModule::clearTargetRadioSettings(txp);
-#endif
         size_t numbytes = beginSending(txp);
-        if (numbytes == 0) {
-            if (!sendingPacket) {
-                completeSending();
-                powerMon->clearState(meshtastic_PowerMon_State_Lora_TXOn);
-                startReceive();
-            }
-            return false;
-        }
 
         int res = iface->startTransmit((uint8_t *)&radioBuffer, numbytes);
         if (res != RADIOLIB_ERR_NONE) {

src/mesh/Router.cpp

@@ -602,6 +602,18 @@ ErrorCode Router::send(meshtastic_MeshPacket *p)
     }
 #endif
 
+    // Relayed and phone-sourced frames arrive already encrypted, so perhapsEncode's TOO_LARGE check
+    // never sees them - this is the last gate before the radio queue.
+    if (p->encrypted.size > MAX_RADIO_PAYLOAD_LEN) {
+        LOG_WARN("Drop 0x%08x: payload %u exceeds radio capacity %u", p->id, (unsigned)p->encrypted.size,
+                 (unsigned)MAX_RADIO_PAYLOAD_LEN);
+        if (isFromUs(p))
+            abortSendAndNak(meshtastic_Routing_Error_TOO_LARGE, p);
+        else
+            packetPool.release(p);
+        return meshtastic_Routing_Error_TOO_LARGE;
+    }
+
     assert(iface); // This should have been detected already in sendLocal (or we just received a packet from outside)
     return iface->send(p);
 }

src/modules/MeshBeaconModule.cpp

@@ -46,6 +46,16 @@ static bool getTargetRadioSettings(const meshtastic_MeshPacket *p, meshtastic_Co
     return false;
 }
 
+// Is a target entry still live for this packet id? Unlike sendingPacket or the radio's standby
+// state, this is our own bookkeeping - it answers "has that beacon finished" without asking the radio.
+static bool targetRadioSettingsLive(uint32_t id)
+{
+    for (const auto &entry : targetRadioSettings)
+        if (entry.inUse && entry.id == id)
+            return true;
+    return false;
+}
+
 // ---------------------------------------------------------------------------
 // MeshBeaconModule base
 // ---------------------------------------------------------------------------
@@ -74,8 +84,13 @@ void MeshBeaconModule::setTargetRadioSettings(const meshtastic_MeshPacket *p, me
         if (!target && !entry.inUse)
             target = &entry;
     }
-    if (!target)
+    if (!target) {
+        // All slots live: another beacon's target is about to be overwritten and that packet will key
+        // up on whatever config is running instead of its own.
+        LOG_WARN("Beacon: target table full (%u slots), evicting packet 0x%08x for 0x%08x",
+                 (unsigned)(sizeof(targetRadioSettings) / sizeof(targetRadioSettings[0])), targetRadioSettings[0].id, p->id);
         target = &targetRadioSettings[0];
+    }
     target->inUse = true;
     target->id = p->id;
     target->preset = preset;
@@ -151,14 +166,32 @@ meshtastic_ChannelSettings MeshBeaconModule::beaconChannelSettings(const meshtas
 
 bool MeshBeaconModule::reconfigureForBeaconTX(RadioInterface *iface, meshtastic_MeshPacket *p)
 {
-    // True while a beacon radio switch is in effect and still needs undoing. We track the switch
-    // explicitly rather than inferring it from "live config differs from the snapshot", because that
-    // heuristic both missed cases (a channel name/PSK swap that left preset/slot/region unchanged would
-    // never be restored) and fired falsely (a legitimate non-beacon channel edit would be reverted on
-    // the next TX). With the flag the restore fires for ANY field we changed and only when we changed
-    // it - including on TX-failure paths, which route through this same restore call.
+    // The four statics below hold the switch state explicitly. Inferring it from "live config differs
+    // from the snapshot" instead missed name/PSK-only swaps and fired on legitimate channel edits.
     static bool radioSwitched = false;
 
+    // Consecutive switches with no restore between them, so a multi-target run can be read off the log
+    // and the held home snapshot is attributable to a specific switch.
+    static uint8_t switchDepth = 0;
+
+    // The packet that armed the outstanding switch. Every caller that abandons or completes a beacon
+    // clears its target settings first, so a live entry here means that TX has not finished yet.
+    static uint32_t switchedForId = 0;
+
+    // Both branches end in iface->reconfigure(), whose setStandby() runs completeSending() and calls
+    // straight back in here. Ignore that re-entry: the outer call owns the config it is applying.
+    static bool applying = false;
+    if (applying) {
+        // Expected once per switch and once per restore. A burst of these means something new re-enters.
+        LOG_DEBUG("Beacon: ignore re-entrant reconfigure while a radio config is being applied");
+        return false;
+    }
+    struct ApplyingScope {
+        bool &flag;
+        explicit ApplyingScope(bool &f) : flag(f) { flag = true; }
+        ~ApplyingScope() { flag = false; }
+    } applyingScope(applying);
+
     meshtastic_ChannelSettings *primaryCh = &channels.getByIndex(channels.getPrimaryIndex()).settings;
     meshtastic_Config_LoRaConfig_ModemPreset targetPreset;
     uint16_t targetSlot;
@@ -202,18 +235,22 @@ bool MeshBeaconModule::reconfigureForBeaconTX(RadioInterface *iface, meshtastic_
             return false;
         }
 
-        // Snapshot current (non-beacon) settings so we restore to the latest config. Skip while a
-        // switch is already active, so a second switch before the restore can't capture the beacon
-        // config as the "home" we later restore to.
+        // Snapshot the live (non-beacon) config as "home". Skipped while a switch is already active,
+        // so a second switch before the restore cannot capture the beacon config instead.
         if (!radioSwitched) {
             originalModemPreset = config.lora.modem_preset;
             originalLoraChannel = config.lora.channel_num;
             originalRegion = config.lora.region;
             originalPrimaryChannel = *primaryCh;
+            switchDepth = 0;
         }
+        switchDepth++;
 
-        LOG_INFO("Beacon: switch radio for packet 0x%08x to preset=%d slot=%u region=%d", p->id, targetPreset, targetSlot,
-                 targetRegion);
+        LOG_INFO("Beacon: switch #%u radio for packet 0x%08x to preset=%d slot=%u region=%d", switchDepth, p->id, targetPreset,
+                 targetSlot, targetRegion);
+        if (switchDepth > 1)
+            LOG_WARN("Beacon: switching again with no restore between; home preset=%d slot=%u region=%d still held",
+                     originalModemPreset, originalLoraChannel, originalRegion);
         config.lora.modem_preset = targetPreset;
         config.lora.channel_num = targetSlot;
         if (targetRegion != config.lora.region)
@@ -222,22 +259,33 @@ bool MeshBeaconModule::reconfigureForBeaconTX(RadioInterface *iface, meshtastic_
 
         channels.fixupChannel(channels.getPrimaryIndex());
         p->channel = channels.getHash(channels.getPrimaryIndex());
+        radioSwitched = true; // set before reconfigure(), so the flag never lags the radio it describes
+        switchedForId = p->id;
         iface->reconfigure();
-        radioSwitched = true;
         return true;
 
     } else if ((!p || !getTargetRadioSettings(p, nullptr, nullptr)) && radioSwitched) {
 
-        LOG_INFO("Beacon: restore radio config after TX");
+        // Only restore once the beacon that armed the switch has finished; a caller arriving here on
+        // a radio state change would put the home config back under a beacon that has not keyed up.
+        if (targetRadioSettingsLive(switchedForId)) {
+            LOG_DEBUG("Beacon: skip restore, packet 0x%08x has not finished sending", switchedForId);
+            return false;
+        }
+
+        LOG_INFO("Beacon: restore radio config after TX, undoing %u switch(es) -> preset=%d slot=%u region=%d", switchDepth,
+                 originalModemPreset, originalLoraChannel, originalRegion);
         config.lora.modem_preset = originalModemPreset;
         config.lora.channel_num = originalLoraChannel;
         config.lora.region = originalRegion;
         *primaryCh = originalPrimaryChannel;
         primaryCh->name[sizeof(primaryCh->name) - 1] = '\0';
 
         channels.fixupChannel(channels.getPrimaryIndex());
+        radioSwitched = false; // cleared before reconfigure(), so the flag never lags the radio it describes
+        switchDepth = 0;
+        switchedForId = 0;
         iface->reconfigure();
-        radioSwitched = false;
         return true;
     }
     return false;