mirror of
https://github.com/meshtastic/firmware.git
synced 2025-06-08 06:02:05 +00:00
Improve ACK logic for responses and repeated packets (#5232)
* Don't send ACKs to responses over multiple hops * Move repeated sending logic to `wasSeenRecently()` * Add exception for simulator for duplicate packets from PhoneAPI * Add short debug message
This commit is contained in:
parent
1bec23b6bb
commit
da7424a604
@ -33,7 +33,7 @@ MeshModule::~MeshModule()
|
||||
}
|
||||
|
||||
meshtastic_MeshPacket *MeshModule::allocAckNak(meshtastic_Routing_Error err, NodeNum to, PacketId idFrom, ChannelIndex chIndex,
|
||||
uint8_t hopStart, uint8_t hopLimit)
|
||||
uint8_t hopLimit)
|
||||
{
|
||||
meshtastic_Routing c = meshtastic_Routing_init_default;
|
||||
|
||||
@ -50,7 +50,7 @@ meshtastic_MeshPacket *MeshModule::allocAckNak(meshtastic_Routing_Error err, Nod
|
||||
|
||||
p->priority = meshtastic_MeshPacket_Priority_ACK;
|
||||
|
||||
p->hop_limit = routingModule->getHopLimitForResponse(hopStart, hopLimit); // Flood ACK back to original sender
|
||||
p->hop_limit = hopLimit; // Flood ACK back to original sender
|
||||
p->to = to;
|
||||
p->decoded.request_id = idFrom;
|
||||
p->channel = chIndex;
|
||||
@ -181,8 +181,8 @@ void MeshModule::callModules(meshtastic_MeshPacket &mp, RxSource src)
|
||||
// SECURITY NOTE! I considered sending back a different error code if we didn't find the psk (i.e. !isDecoded)
|
||||
// but opted NOT TO. Because it is not a good idea to let remote nodes 'probe' to find out which PSKs were "good" vs
|
||||
// bad.
|
||||
routingModule->sendAckNak(meshtastic_Routing_Error_NO_RESPONSE, getFrom(&mp), mp.id, mp.channel, mp.hop_start,
|
||||
mp.hop_limit);
|
||||
routingModule->sendAckNak(meshtastic_Routing_Error_NO_RESPONSE, getFrom(&mp), mp.id, mp.channel,
|
||||
routingModule->getHopLimitForResponse(mp.hop_start, mp.hop_limit));
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -162,7 +162,7 @@ class MeshModule
|
||||
virtual Observable<const UIFrameEvent *> *getUIFrameObservable() { return NULL; }
|
||||
|
||||
meshtastic_MeshPacket *allocAckNak(meshtastic_Routing_Error err, NodeNum to, PacketId idFrom, ChannelIndex chIndex,
|
||||
uint8_t hopStart = 0, uint8_t hopLimit = 0);
|
||||
uint8_t hopLimit = 0);
|
||||
|
||||
/// Send an error response for the specified packet.
|
||||
meshtastic_MeshPacket *allocErrorResponse(meshtastic_Routing_Error err, const meshtastic_MeshPacket *p);
|
||||
|
@ -38,6 +38,13 @@ bool PacketHistory::wasSeenRecently(const meshtastic_MeshPacket *p, bool withUpd
|
||||
seenRecently = false;
|
||||
}
|
||||
|
||||
/* 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");
|
||||
seenRecently = false;
|
||||
}
|
||||
|
||||
if (seenRecently) {
|
||||
LOG_DEBUG("Found existing packet record for fr=0x%x,to=0x%x,id=0x%x", p->from, p->to, p->id);
|
||||
}
|
||||
|
@ -596,10 +596,13 @@ bool PhoneAPI::handleToRadioPacket(meshtastic_MeshPacket &p)
|
||||
{
|
||||
printPacket("PACKET FROM PHONE", &p);
|
||||
|
||||
// For use with the simulator, we should not ignore duplicate packets
|
||||
#if !(defined(ARCH_PORTDUINO) && !HAS_RADIO)
|
||||
if (p.id > 0 && wasSeenRecently(p.id)) {
|
||||
LOG_DEBUG("Ignoring packet from phone, already seen recently");
|
||||
return false;
|
||||
}
|
||||
#endif
|
||||
|
||||
if (p.decoded.portnum == meshtastic_PortNum_TRACEROUTE_APP && lastPortNumToRadio[p.decoded.portnum] &&
|
||||
Throttle::isWithinTimespanMs(lastPortNumToRadio[p.decoded.portnum], THIRTY_SECONDS_MS)) {
|
||||
|
@ -1,10 +1,10 @@
|
||||
#include "ReliableRouter.h"
|
||||
#include "Default.h"
|
||||
#include "MeshModule.h"
|
||||
#include "MeshTypes.h"
|
||||
#include "configuration.h"
|
||||
#include "mesh-pb-constants.h"
|
||||
#include "modules/NodeInfoModule.h"
|
||||
#include "modules/RoutingModule.h"
|
||||
|
||||
// ReliableRouter::ReliableRouter() {}
|
||||
|
||||
@ -73,18 +73,6 @@ bool ReliableRouter::shouldFilterReceived(const meshtastic_MeshPacket *p)
|
||||
i->second.nextTxMsec += iface->getPacketTime(p);
|
||||
}
|
||||
|
||||
/* Resend implicit ACKs for repeated packets (hopStart equals hopLimit);
|
||||
* this way if an implicit ACK is dropped and a packet is resent we'll rebroadcast again.
|
||||
* Resending real ACKs is omitted, as you might receive a packet multiple times due to flooding and
|
||||
* flooding this ACK back to the original sender already adds redundancy. */
|
||||
bool isRepeated = p->hop_start == 0 ? (p->hop_limit == HOP_RELIABLE) : (p->hop_start == p->hop_limit);
|
||||
if (wasSeenRecently(p, false) && isRepeated && !MeshModule::currentReply && !isToUs(p)) {
|
||||
LOG_DEBUG("Resending implicit ack for a repeated floodmsg");
|
||||
meshtastic_MeshPacket *tosend = packetPool.allocCopy(*p);
|
||||
tosend->hop_limit--; // bump down the hop count
|
||||
Router::send(tosend);
|
||||
}
|
||||
|
||||
return FloodingRouter::shouldFilterReceived(p);
|
||||
}
|
||||
|
||||
@ -107,16 +95,22 @@ void ReliableRouter::sniffReceived(const meshtastic_MeshPacket *p, const meshtas
|
||||
if (MeshModule::currentReply) {
|
||||
LOG_DEBUG("Another module replied to this message, no need for 2nd ack");
|
||||
} else if (p->which_payload_variant == meshtastic_MeshPacket_decoded_tag) {
|
||||
sendAckNak(meshtastic_Routing_Error_NONE, getFrom(p), p->id, p->channel, p->hop_start, p->hop_limit);
|
||||
// A response may be set to want_ack for retransmissions, but we don't need to ACK a response if it received an
|
||||
// implicit ACK already. If we received it directly, only ACK with a hop limit of 0
|
||||
if (!p->decoded.request_id)
|
||||
sendAckNak(meshtastic_Routing_Error_NONE, getFrom(p), p->id, p->channel,
|
||||
routingModule->getHopLimitForResponse(p->hop_start, p->hop_limit));
|
||||
else if (p->hop_start > 0 && p->hop_start == p->hop_limit)
|
||||
sendAckNak(meshtastic_Routing_Error_NONE, getFrom(p), p->id, p->channel, 0);
|
||||
} else if (p->which_payload_variant == meshtastic_MeshPacket_encrypted_tag && p->channel == 0 &&
|
||||
(nodeDB->getMeshNode(p->from) == nullptr || nodeDB->getMeshNode(p->from)->user.public_key.size == 0)) {
|
||||
LOG_INFO("PKI packet from unknown node, send PKI_UNKNOWN_PUBKEY");
|
||||
sendAckNak(meshtastic_Routing_Error_PKI_UNKNOWN_PUBKEY, getFrom(p), p->id, channels.getPrimaryIndex(),
|
||||
p->hop_start, p->hop_limit);
|
||||
routingModule->getHopLimitForResponse(p->hop_start, p->hop_limit));
|
||||
} else {
|
||||
// Send a 'NO_CHANNEL' error on the primary channel if want_ack packet destined for us cannot be decoded
|
||||
sendAckNak(meshtastic_Routing_Error_NO_CHANNEL, getFrom(p), p->id, channels.getPrimaryIndex(), p->hop_start,
|
||||
p->hop_limit);
|
||||
sendAckNak(meshtastic_Routing_Error_NO_CHANNEL, getFrom(p), p->id, channels.getPrimaryIndex(),
|
||||
routingModule->getHopLimitForResponse(p->hop_start, p->hop_limit));
|
||||
}
|
||||
}
|
||||
if (p->which_payload_variant == meshtastic_MeshPacket_decoded_tag && c &&
|
||||
|
@ -136,10 +136,9 @@ meshtastic_MeshPacket *Router::allocForSending()
|
||||
/**
|
||||
* Send an ack or a nak packet back towards whoever sent idFrom
|
||||
*/
|
||||
void Router::sendAckNak(meshtastic_Routing_Error err, NodeNum to, PacketId idFrom, ChannelIndex chIndex, uint8_t hopStart,
|
||||
uint8_t hopLimit)
|
||||
void Router::sendAckNak(meshtastic_Routing_Error err, NodeNum to, PacketId idFrom, ChannelIndex chIndex, uint8_t hopLimit)
|
||||
{
|
||||
routingModule->sendAckNak(err, to, idFrom, chIndex, hopStart, hopLimit);
|
||||
routingModule->sendAckNak(err, to, idFrom, chIndex, hopLimit);
|
||||
}
|
||||
|
||||
void Router::abortSendAndNak(meshtastic_Routing_Error err, meshtastic_MeshPacket *p)
|
||||
|
@ -108,8 +108,7 @@ class Router : protected concurrency::OSThread
|
||||
/**
|
||||
* Send an ack or a nak packet back towards whoever sent idFrom
|
||||
*/
|
||||
void sendAckNak(meshtastic_Routing_Error err, NodeNum to, PacketId idFrom, ChannelIndex chIndex, uint8_t hopStart = 0,
|
||||
uint8_t hopLimit = 0);
|
||||
void sendAckNak(meshtastic_Routing_Error err, NodeNum to, PacketId idFrom, ChannelIndex chIndex, uint8_t hopLimit = 0);
|
||||
|
||||
private:
|
||||
/**
|
||||
|
@ -49,10 +49,9 @@ meshtastic_MeshPacket *RoutingModule::allocReply()
|
||||
return NULL;
|
||||
}
|
||||
|
||||
void RoutingModule::sendAckNak(meshtastic_Routing_Error err, NodeNum to, PacketId idFrom, ChannelIndex chIndex, uint8_t hopStart,
|
||||
uint8_t hopLimit)
|
||||
void RoutingModule::sendAckNak(meshtastic_Routing_Error err, NodeNum to, PacketId idFrom, ChannelIndex chIndex, uint8_t hopLimit)
|
||||
{
|
||||
auto p = allocAckNak(err, to, idFrom, chIndex, hopStart, hopLimit);
|
||||
auto p = allocAckNak(err, to, idFrom, chIndex, hopLimit);
|
||||
|
||||
router->sendLocal(p); // we sometimes send directly to the local node
|
||||
}
|
||||
|
@ -13,8 +13,7 @@ class RoutingModule : public ProtobufModule<meshtastic_Routing>
|
||||
*/
|
||||
RoutingModule();
|
||||
|
||||
void sendAckNak(meshtastic_Routing_Error err, NodeNum to, PacketId idFrom, ChannelIndex chIndex, uint8_t hopStart = 0,
|
||||
uint8_t hopLimit = 0);
|
||||
void sendAckNak(meshtastic_Routing_Error err, NodeNum to, PacketId idFrom, ChannelIndex chIndex, uint8_t hopLimit = 0);
|
||||
|
||||
// Given the hopStart and hopLimit upon reception of a request, return the hop limit to use for the response
|
||||
uint8_t getHopLimitForResponse(uint8_t hopStart, uint8_t hopLimit);
|
||||
|
Loading…
Reference in New Issue
Block a user