Skip to content

Commit

Permalink
Fix sending duplicate packets to PhoneAPI/MQTT (meshtastic#5315)
Browse files Browse the repository at this point in the history
* Fix potential duplicate packets to PhoneAPI/MQTT

* Fix include error for STM32

* Fix cppcheck error
  • Loading branch information
GUVWAF authored Nov 11, 2024
1 parent eb8d38a commit 40bc04b
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 14 deletions.
3 changes: 1 addition & 2 deletions src/mesh/FloodingRouter.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
#pragma once

#include "PacketHistory.h"
#include "Router.h"

/**
Expand All @@ -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();
Expand Down
11 changes: 8 additions & 3 deletions src/mesh/PacketHistory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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;
}

Expand All @@ -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
Expand Down
6 changes: 3 additions & 3 deletions src/mesh/PacketHistory.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#pragma once

#include "Router.h"
#include "NodeDB.h"
#include <unordered_set>

/// We clear our old flood record 10 minutes after we see the last of it
Expand Down Expand Up @@ -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);
};
3 changes: 2 additions & 1 deletion src/mesh/PhoneAPI.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
}
12 changes: 10 additions & 2 deletions src/mesh/Router.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
3 changes: 2 additions & 1 deletion src/mesh/Router.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@
#include "MemoryPool.h"
#include "MeshTypes.h"
#include "Observer.h"
#include "PacketHistory.h"
#include "PointerQueue.h"
#include "RadioInterface.h"
#include "concurrency/OSThread.h"

/**
* 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
Expand Down
10 changes: 8 additions & 2 deletions src/modules/RoutingModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 40bc04b

Please sign in to comment.