From 40bc04b521d889e932bcb0d828e44516514e302a Mon Sep 17 00:00:00 2001 From: GUVWAF <78759985+GUVWAF@users.noreply.github.com> Date: Tue, 12 Nov 2024 00:54:05 +0100 Subject: [PATCH] Fix sending duplicate packets to PhoneAPI/MQTT (#5315) * Fix potential duplicate packets to PhoneAPI/MQTT * Fix include error for STM32 * Fix cppcheck error --- src/mesh/FloodingRouter.h | 3 +-- src/mesh/PacketHistory.cpp | 11 ++++++++--- src/mesh/PacketHistory.h | 6 +++--- src/mesh/PhoneAPI.cpp | 3 ++- src/mesh/Router.cpp | 12 ++++++++++-- src/mesh/Router.h | 3 ++- src/modules/RoutingModule.cpp | 10 ++++++++-- 7 files changed, 34 insertions(+), 14 deletions(-) diff --git a/src/mesh/FloodingRouter.h b/src/mesh/FloodingRouter.h index 0ed2b5582..e398f8d58 100644 --- a/src/mesh/FloodingRouter.h +++ b/src/mesh/FloodingRouter.h @@ -1,6 +1,5 @@ #pragma once -#include "PacketHistory.h" #include "Router.h" /** @@ -26,7 +25,7 @@ Any entries in recentBroadcasts that are older than X seconds (longer than the max time a flood can take) will be discarded. */ -class FloodingRouter : public Router, protected PacketHistory +class FloodingRouter : public Router { private: bool isRebroadcaster(); diff --git a/src/mesh/PacketHistory.cpp b/src/mesh/PacketHistory.cpp index 94337d092..b29d67d66 100644 --- a/src/mesh/PacketHistory.cpp +++ b/src/mesh/PacketHistory.cpp @@ -16,7 +16,7 @@ PacketHistory::PacketHistory() /** * Update recentBroadcasts and return true if we have already seen this packet */ -bool PacketHistory::wasSeenRecently(const meshtastic_MeshPacket *p, bool withUpdate) +bool PacketHistory::wasSeenRecently(const meshtastic_MeshPacket *p, bool withUpdate, bool *isRepeated) { if (p->id == 0) { LOG_DEBUG("Ignore message with zero id"); @@ -41,7 +41,12 @@ bool PacketHistory::wasSeenRecently(const meshtastic_MeshPacket *p, bool withUpd /* If the original transmitter is doing retransmissions (hopStart equals hopLimit) for a reliable transmission, e.g., when the ACK got lost, we will handle the packet again to make sure it gets an ACK/response to its packet. */ if (seenRecently && p->hop_start > 0 && p->hop_start == p->hop_limit) { - LOG_DEBUG("Repeated reliable tx"); + if (withUpdate) + LOG_DEBUG("Repeated reliable tx"); + + if (isRepeated) + *isRepeated = true; + seenRecently = false; } @@ -54,7 +59,7 @@ bool PacketHistory::wasSeenRecently(const meshtastic_MeshPacket *p, bool withUpd recentPackets.erase(found); // as unsorted_set::iterator is const (can't update timestamp - so re-insert..) } recentPackets.insert(r); - printPacket("Add packet record", p); + LOG_DEBUG("Add packet record fr=0x%x, id=0x%x", p->from, p->id); } // Capacity is reerved, so only purge expired packets if recentPackets fills past 90% capacity diff --git a/src/mesh/PacketHistory.h b/src/mesh/PacketHistory.h index 89d237a02..1f29155fd 100644 --- a/src/mesh/PacketHistory.h +++ b/src/mesh/PacketHistory.h @@ -1,6 +1,6 @@ #pragma once -#include "Router.h" +#include "NodeDB.h" #include /// We clear our old flood record 10 minutes after we see the last of it @@ -41,5 +41,5 @@ class PacketHistory * * @param withUpdate if true and not found we add an entry to recentPackets */ - bool wasSeenRecently(const meshtastic_MeshPacket *p, bool withUpdate = true); -}; + bool wasSeenRecently(const meshtastic_MeshPacket *p, bool withUpdate = true, bool *isRepeated = nullptr); +}; \ No newline at end of file diff --git a/src/mesh/PhoneAPI.cpp b/src/mesh/PhoneAPI.cpp index 20421e73e..f42c0b59f 100644 --- a/src/mesh/PhoneAPI.cpp +++ b/src/mesh/PhoneAPI.cpp @@ -12,6 +12,7 @@ #include "PhoneAPI.h" #include "PowerFSM.h" #include "RadioInterface.h" +#include "Router.h" #include "TypeConversions.h" #include "main.h" #include "xmodem.h" @@ -656,4 +657,4 @@ int PhoneAPI::onNotify(uint32_t newValue) } return timeout ? -1 : 0; // If we timed out, MeshService should stop iterating through observers as we just removed one -} +} \ No newline at end of file diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index 0b46ca3b9..d2017c257 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -622,9 +622,17 @@ void Router::handleReceived(meshtastic_MeshPacket *p, RxSource src) // us (because we would be able to decrypt it) if (!decoded && moduleConfig.mqtt.encryption_enabled && p->channel == 0x00 && !isBroadcast(p->to) && !isToUs(p)) p_encrypted->pki_encrypted = true; + // After potentially altering it, publish received message to MQTT if we're not the original transmitter of the packet - if ((decoded || p_encrypted->pki_encrypted) && moduleConfig.mqtt.enabled && !isFromUs(p) && mqtt) - mqtt->onSend(*p_encrypted, *p, p->channel); + if ((decoded || p_encrypted->pki_encrypted) && moduleConfig.mqtt.enabled && !isFromUs(p) && mqtt) { + // Check if it wasn't already seen, then we don't need to handle it again + bool isRepeated = false; + bool *rptr = &isRepeated; + wasSeenRecently(p, false, rptr); + if (!isRepeated) { + mqtt->onSend(*p_encrypted, *p, p->channel); + } + } #endif } diff --git a/src/mesh/Router.h b/src/mesh/Router.h index da44d67df..14ce4810b 100644 --- a/src/mesh/Router.h +++ b/src/mesh/Router.h @@ -4,6 +4,7 @@ #include "MemoryPool.h" #include "MeshTypes.h" #include "Observer.h" +#include "PacketHistory.h" #include "PointerQueue.h" #include "RadioInterface.h" #include "concurrency/OSThread.h" @@ -11,7 +12,7 @@ /** * A mesh aware router that supports multiple interfaces. */ -class Router : protected concurrency::OSThread +class Router : protected concurrency::OSThread, protected PacketHistory { private: /// Packets which have just arrived from the radio, ready to be processed by this service and possibly diff --git a/src/modules/RoutingModule.cpp b/src/modules/RoutingModule.cpp index a501e319b..5423318a4 100644 --- a/src/modules/RoutingModule.cpp +++ b/src/modules/RoutingModule.cpp @@ -28,8 +28,14 @@ bool RoutingModule::handleReceivedProtobuf(const meshtastic_MeshPacket &mp, mesh // FIXME - move this to a non promsicious PhoneAPI module? // Note: we are careful not to send back packets that started with the phone back to the phone if ((isBroadcast(mp.to) || isToUs(&mp)) && (mp.from != 0)) { - printPacket("Delivering rx packet", &mp); - service->handleFromRadio(&mp); + // Check if it wasn't already seen, then we don't need to handle it again + bool isRepeated = false; + bool *rptr = &isRepeated; + router->wasSeenRecently(&mp, false, rptr); + if (!isRepeated) { + printPacket("Delivering rx packet", &mp); + service->handleFromRadio(&mp); + } } return false; // Let others look at this message also if they want