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,11 @@ typedef struct {
 
 } RadioBuffer;
 
+/// On-air ceiling for MeshPacket.encrypted. RadioBuffer holds one byte more, but the PHY caps a whole
+/// frame at MAX_LORA_PAYLOAD_LEN, so the header comes out of the same budget (matches perhapsEncode).
+constexpr size_t MAX_RADIO_PAYLOAD_LEN = MAX_LORA_PAYLOAD_LEN - sizeof(PacketHeader);
+static_assert(MAX_RADIO_PAYLOAD_LEN < sizeof(RadioBuffer::payload), "payload ceiling must fit the buffer");
+
 /**
  * Basic operations all radio chipsets must implement.
  *

src/mesh/RadioLibInterface.cpp

@@ -248,8 +248,12 @@ bool RadioLibInterface::isSending()
 bool RadioLibInterface::cancelSending(NodeNum from, PacketId id)
 {
     auto p = txQueue.remove(from, id);
-    if (p)
+    if (p) {
+#if !MESHTASTIC_EXCLUDE_BEACON
+        abandonBeaconTarget(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);
@@ -450,7 +454,7 @@ void RadioLibInterface::onNotify(uint32_t notification)
                     // transmit it on the current (home) config - and move on to the next queued packet.
                     LOG_DEBUG("Beacon: invalid TX radio config, drop packet 0x%08x", txp->id);
                     meshtastic_MeshPacket *bad = txQueue.dequeue();
-                    MeshBeaconModule::clearTargetRadioSettings(bad);
+                    abandonBeaconTarget(bad);
                     packetPool.release(bad);
                     setTransmitDelay();
                 } else if (MeshBeaconModule::reconfigureForBeaconTX(this, txp)) {
@@ -561,6 +565,9 @@ bool RadioLibInterface::removePendingTXPacket(NodeNum from, PacketId id, uint32_
     meshtastic_MeshPacket *p = txQueue.remove(from, id, true, true, hop_limit_lt);
     if (p) {
         LOG_DEBUG("Drop pending-TX packet 0x%08x, hop limit %d", p->id, p->hop_limit);
+#if !MESHTASTIC_EXCLUDE_BEACON
+        abandonBeaconTarget(p);
+#endif
         packetPool.release(p);
         return true;
     }
@@ -576,6 +583,14 @@ void RadioLibInterface::handleTransmitInterrupt()
     powerMon->clearState(meshtastic_PowerMon_State_Lora_TXOn); // But our transmitter is definitely off now
 }
 
+#if !MESHTASTIC_EXCLUDE_BEACON
+void RadioLibInterface::abandonBeaconTarget(const meshtastic_MeshPacket *p)
+{
+    MeshBeaconModule::clearTargetRadioSettings(p);
+    MeshBeaconModule::reconfigureForBeaconTX(this, nullptr);
+}
+#endif
+
 void RadioLibInterface::completeSending()
 {
     // We are careful to clear sending packet before calling printPacket because
@@ -596,14 +611,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()
@@ -772,29 +788,15 @@ bool RadioLibInterface::startSend(meshtastic_MeshPacket *txp)
     if (disabled || !config.lora.tx_enabled) {
         LOG_WARN("Drop Tx packet: LoRa Tx disabled");
 #if !MESHTASTIC_EXCLUDE_BEACON
-        // This packet may have already triggered a beacon radio switch in TRANSMIT_DELAY_COMPLETED;
-        // since it never reaches completeSending() here, restore the radio so it isn't left on the
-        // beacon config (which would also break RX on the home channel).
-        MeshBeaconModule::clearTargetRadioSettings(txp);
-        MeshBeaconModule::reconfigureForBeaconTX(this, nullptr);
+        // Never reaches completeSending(), so the radio would be left on the beacon config.
+        abandonBeaconTarget(txp);
 #endif
         packetPool.release(txp);
         return false;
     } 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/RadioLibInterface.h

@@ -311,6 +311,12 @@ class RadioLibInterface : public RadioInterface, protected concurrency::Notified
      * If a send was in progress finish it and return the buffer to the pool */
     void completeSending();
 
+#if !MESHTASTIC_EXCLUDE_BEACON
+    /// A queued packet is being abandoned: drop its beacon target, then restore the home config if it
+    /// was the packet we switched for. The restore gate makes this a no-op for any other packet.
+    void abandonBeaconTarget(const meshtastic_MeshPacket *p);
+#endif
+
     /**
      * Add SNR data to received messages
      */

src/mesh/Router.cpp

@@ -602,6 +602,15 @@ ErrorCode Router::send(meshtastic_MeshPacket *p)
     }
 #endif
 
+    // Only already-encrypted frames (relayed, phone-sourced) reach here oversized; perhapsEncode()
+    // bounds everything it encodes. No NAK: p->channel is a wire hash by now, not an index.
+    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);
+        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

@@ -19,6 +19,11 @@ meshtastic_ChannelSettings MeshBeaconModule::originalPrimaryChannel;
 
 static MeshBeaconModule_TargetRadioSettings targetRadioSettings[8];
 
+// Explicit switch state, not inferred: "live config differs from the snapshot" missed name/PSK-only
+// swaps and fired on legitimate channel edits.
+static bool radioSwitched = false;
+static uint32_t switchedForId = 0;
+
 static bool getTargetRadioSettings(const meshtastic_MeshPacket *p, meshtastic_Config_LoRaConfig_ModemPreset *preset,
                                    uint16_t *slot, bool *legacyHopOverride = nullptr,
                                    meshtastic_Config_LoRaConfig_RegionCode *region = nullptr, bool *has_channel = nullptr,
@@ -46,6 +51,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 +89,22 @@ void MeshBeaconModule::setTargetRadioSettings(const meshtastic_MeshPacket *p, me
         if (!target && !entry.inUse)
             target = &entry;
     }
-    if (!target)
-        target = &targetRadioSettings[0];
+    if (!target) {
+        // Table full. Never evict the entry the outstanding switch is gated on: dropping it would
+        // unblock the restore and put the home config back under a beacon that has not keyed up.
+        for (auto &entry : targetRadioSettings) {
+            if (!radioSwitched || entry.id != switchedForId) {
+                target = &entry;
+                break;
+            }
+        }
+        if (!target) {
+            LOG_WARN("Beacon: target table full and every slot is in flight, drop target for 0x%08x", p->id);
+            return;
+        }
+        LOG_WARN("Beacon: target table full (%u slots), evicting packet 0x%08x for 0x%08x",
+                 (unsigned)(sizeof(targetRadioSettings) / sizeof(targetRadioSettings[0])), target->id, p->id);
+    }
     target->inUse = true;
     target->id = p->id;
     target->preset = preset;
@@ -151,13 +180,23 @@ 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.
-    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;
+
+    // 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;
@@ -202,18 +241,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 +265,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;