From 44f60f42ccb97ae1835979369de474f85744292b Mon Sep 17 00:00:00 2001 From: GUVWAF Date: Wed, 20 Mar 2024 21:46:29 +0100 Subject: [PATCH] Revert "Add original hop limit to header to determine hops used (#3321)" This reverts commit 585805c3b96d58224ffd2327fd6b4e1712a84abd. --- src/mesh/MeshModule.cpp | 9 +++------ src/mesh/MeshModule.h | 3 +-- src/mesh/NodeDB.cpp | 6 ------ src/mesh/RadioInterface.cpp | 5 +---- src/mesh/RadioInterface.h | 6 ++---- src/mesh/RadioLibInterface.cpp | 7 +++---- src/mesh/ReliableRouter.cpp | 13 ++++++------- src/mesh/Router.cpp | 9 ++------- src/mesh/Router.h | 3 +-- src/modules/NeighborInfoModule.cpp | 12 +++--------- src/modules/NeighborInfoModule.h | 3 --- src/modules/RoutingModule.cpp | 19 ++----------------- src/modules/RoutingModule.h | 6 +----- 13 files changed, 25 insertions(+), 76 deletions(-) diff --git a/src/mesh/MeshModule.cpp b/src/mesh/MeshModule.cpp index ad0c781088..9c6ca78ee2 100644 --- a/src/mesh/MeshModule.cpp +++ b/src/mesh/MeshModule.cpp @@ -32,8 +32,7 @@ MeshModule::~MeshModule() assert(0); // FIXME - remove from list of modules once someone needs this feature } -meshtastic_MeshPacket *MeshModule::allocAckNak(meshtastic_Routing_Error err, NodeNum to, PacketId idFrom, ChannelIndex chIndex, - uint8_t hopStart, uint8_t hopLimit) +meshtastic_MeshPacket *MeshModule::allocAckNak(meshtastic_Routing_Error err, NodeNum to, PacketId idFrom, ChannelIndex chIndex) { meshtastic_Routing c = meshtastic_Routing_init_default; @@ -50,7 +49,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 = config.lora.hop_limit; // Flood ACK back to original sender p->to = to; p->decoded.request_id = idFrom; p->channel = chIndex; @@ -177,8 +176,7 @@ void MeshModule::callPlugins(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); } } @@ -219,7 +217,6 @@ void setReplyTo(meshtastic_MeshPacket *p, const meshtastic_MeshPacket &to) assert(p->which_payload_variant == meshtastic_MeshPacket_decoded_tag); // Should already be set by now p->to = getFrom(&to); // Make sure that if we are sending to the local node, we use our local node addr, not 0 p->channel = to.channel; // Use the same channel that the request came in on - p->hop_limit = routingModule->getHopLimitForResponse(to.hop_start, to.hop_limit); // No need for an ack if we are just delivering locally (it just generates an ignored ack) p->want_ack = (to.from != 0) ? to.want_ack : false; diff --git a/src/mesh/MeshModule.h b/src/mesh/MeshModule.h index 6c431adb47..ebe3af1a0b 100644 --- a/src/mesh/MeshModule.h +++ b/src/mesh/MeshModule.h @@ -153,8 +153,7 @@ class MeshModule virtual bool wantUIFrame() { return false; } virtual Observable *getUIFrameObservable() { return NULL; } - meshtastic_MeshPacket *allocAckNak(meshtastic_Routing_Error err, NodeNum to, PacketId idFrom, ChannelIndex chIndex, - uint8_t hopStart = 0, uint8_t hopLimit = 0); + meshtastic_MeshPacket *allocAckNak(meshtastic_Routing_Error err, NodeNum to, PacketId idFrom, ChannelIndex chIndex); /// Send an error response for the specified packet. meshtastic_MeshPacket *allocErrorResponse(meshtastic_Routing_Error err, const meshtastic_MeshPacket *p); diff --git a/src/mesh/NodeDB.cpp b/src/mesh/NodeDB.cpp index 9d7647138f..a4b04e71db 100644 --- a/src/mesh/NodeDB.cpp +++ b/src/mesh/NodeDB.cpp @@ -878,12 +878,6 @@ void NodeDB::updateFrom(const meshtastic_MeshPacket &mp) if (mp.rx_snr) info->snr = mp.rx_snr; // keep the most recent SNR we received for this node. - - info->via_mqtt = mp.via_mqtt; // Store if we received this packet via MQTT - - // If hopStart was set and there wasn't someone messing with the limit in the middle, add hopsAway - if (mp.hop_start != 0 && mp.hop_limit <= mp.hop_start) - info->hops_away = mp.hop_start - mp.hop_limit; } } diff --git a/src/mesh/RadioInterface.cpp b/src/mesh/RadioInterface.cpp index 7a27112519..8950519714 100644 --- a/src/mesh/RadioInterface.cpp +++ b/src/mesh/RadioInterface.cpp @@ -304,8 +304,6 @@ void printPacket(const char *prefix, const meshtastic_MeshPacket *p) out += DEBUG_PORT.mt_sprintf(" rxRSSI=%i", p->rx_rssi); if (p->via_mqtt != 0) out += DEBUG_PORT.mt_sprintf(" via MQTT"); - if (p->hop_start != 0) - out += DEBUG_PORT.mt_sprintf(" hopStart=%d", p->hop_start); if (p->priority != 0) out += DEBUG_PORT.mt_sprintf(" priority=%d", p->priority); @@ -569,7 +567,6 @@ size_t RadioInterface::beginSending(meshtastic_MeshPacket *p) p->hop_limit = HOP_RELIABLE; } h->flags = p->hop_limit | (p->want_ack ? PACKET_FLAGS_WANT_ACK_MASK : 0) | (p->via_mqtt ? PACKET_FLAGS_VIA_MQTT_MASK : 0); - h->flags |= (p->hop_start << PACKET_FLAGS_HOP_START_SHIFT) & PACKET_FLAGS_HOP_START_MASK; // if the sender nodenum is zero, that means uninitialized assert(h->from); @@ -578,4 +575,4 @@ size_t RadioInterface::beginSending(meshtastic_MeshPacket *p) sendingPacket = p; return p->encrypted.size + sizeof(PacketHeader); -} \ No newline at end of file +} diff --git a/src/mesh/RadioInterface.h b/src/mesh/RadioInterface.h index ee4726d745..4a01d453b2 100644 --- a/src/mesh/RadioInterface.h +++ b/src/mesh/RadioInterface.h @@ -10,11 +10,9 @@ #define MAX_RHPACKETLEN 256 -#define PACKET_FLAGS_HOP_LIMIT_MASK 0x07 +#define PACKET_FLAGS_HOP_MASK 0x07 #define PACKET_FLAGS_WANT_ACK_MASK 0x08 #define PACKET_FLAGS_VIA_MQTT_MASK 0x10 -#define PACKET_FLAGS_HOP_START_MASK 0xE0 -#define PACKET_FLAGS_HOP_START_SHIFT 5 /** * This structure has to exactly match the wire layout when sent over the radio link. Used to keep compatibility @@ -229,4 +227,4 @@ class RadioInterface }; /// Debug printing for packets -void printPacket(const char *prefix, const meshtastic_MeshPacket *p); \ No newline at end of file +void printPacket(const char *prefix, const meshtastic_MeshPacket *p); diff --git a/src/mesh/RadioLibInterface.cpp b/src/mesh/RadioLibInterface.cpp index 9f42afa6d2..8a2bc53e5a 100644 --- a/src/mesh/RadioLibInterface.cpp +++ b/src/mesh/RadioLibInterface.cpp @@ -359,9 +359,8 @@ void RadioLibInterface::handleReceiveInterrupt() mp->to = h->to; mp->id = h->id; mp->channel = h->channel; - assert(HOP_MAX <= PACKET_FLAGS_HOP_LIMIT_MASK); // If hopmax changes, carefully check this code - mp->hop_limit = h->flags & PACKET_FLAGS_HOP_LIMIT_MASK; - mp->hop_start = (h->flags & PACKET_FLAGS_HOP_START_MASK) >> PACKET_FLAGS_HOP_START_SHIFT; + assert(HOP_MAX <= PACKET_FLAGS_HOP_MASK); // If hopmax changes, carefully check this code + mp->hop_limit = h->flags & PACKET_FLAGS_HOP_MASK; mp->want_ack = !!(h->flags & PACKET_FLAGS_WANT_ACK_MASK); mp->via_mqtt = !!(h->flags & PACKET_FLAGS_VIA_MQTT_MASK); @@ -408,4 +407,4 @@ void RadioLibInterface::startSend(meshtastic_MeshPacket *txp) // bits enableInterrupt(isrTxLevel0); } -} \ No newline at end of file +} diff --git a/src/mesh/ReliableRouter.cpp b/src/mesh/ReliableRouter.cpp index 2327cbfb75..8322015582 100644 --- a/src/mesh/ReliableRouter.cpp +++ b/src/mesh/ReliableRouter.cpp @@ -71,12 +71,12 @@ bool ReliableRouter::shouldFilterReceived(const meshtastic_MeshPacket *p) i->second.nextTxMsec += iface->getPacketTime(p); } - /* Resend implicit ACKs for repeated packets (hopStart equals hopLimit); + /* Resend implicit ACKs for repeated packets (assuming the original packet was sent with HOP_RELIABLE) * 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 && p->to != nodeDB.getNodeNum()) { + if (wasSeenRecently(p, false) && p->hop_limit == HOP_RELIABLE && !MeshModule::currentReply && p->to != nodeDB.getNodeNum()) { + // retransmission on broadcast has hop_limit still equal to HOP_RELIABLE LOG_DEBUG("Resending implicit ack for a repeated floodmsg\n"); meshtastic_MeshPacket *tosend = packetPool.allocCopy(*p); tosend->hop_limit--; // bump down the hop count @@ -107,11 +107,10 @@ void ReliableRouter::sniffReceived(const meshtastic_MeshPacket *p, const meshtas if (MeshModule::currentReply) { LOG_DEBUG("Some other module has replied to this message, no need for a 2nd ack\n"); } 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); + sendAckNak(meshtastic_Routing_Error_NONE, getFrom(p), p->id, p->channel); } 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()); } } @@ -256,4 +255,4 @@ void ReliableRouter::setNextTx(PendingPacket *pending) LOG_DEBUG("Setting next retransmission in %u msecs: ", d); printPacket("", pending->packet); setReceivedMessage(); // Run ASAP, so we can figure out our correct sleep time -} \ No newline at end of file +} diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index 7c739b8f28..6b707e47c3 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -132,10 +132,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) { - routingModule->sendAckNak(err, to, idFrom, chIndex, hopStart, hopLimit); + routingModule->sendAckNak(err, to, idFrom, chIndex); } void Router::abortSendAndNak(meshtastic_Routing_Error err, meshtastic_MeshPacket *p) @@ -241,10 +240,6 @@ ErrorCode Router::send(meshtastic_MeshPacket *p) // the lora we need to make sure we have replaced it with our local address p->from = getFrom(p); - // If we are the original transmitter, set the hop limit with which we start - if (p->from == getNodeNum()) - p->hop_start = p->hop_limit; - // If the packet hasn't yet been encrypted, do so now (it might already be encrypted if we are just forwarding it) assert(p->which_payload_variant == meshtastic_MeshPacket_encrypted_tag || diff --git a/src/mesh/Router.h b/src/mesh/Router.h index 98486745b0..db810e42e7 100644 --- a/src/mesh/Router.h +++ b/src/mesh/Router.h @@ -104,8 +104,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); private: /** diff --git a/src/modules/NeighborInfoModule.cpp b/src/modules/NeighborInfoModule.cpp index 2e0b04afa1..4541958fa6 100644 --- a/src/modules/NeighborInfoModule.cpp +++ b/src/modules/NeighborInfoModule.cpp @@ -95,7 +95,6 @@ NeighborInfoModule::NeighborInfoModule() ourPortNum = meshtastic_PortNum_NEIGHBORINFO_APP; if (moduleConfig.neighbor_info.enabled) { - isPromiscuous = true; // Update neighbors from all packets this->loadProtoForModule(); setIntervalFromNow(35 * 1000); } else { @@ -203,12 +202,9 @@ Pass it to an upper client; do not persist this data on the mesh */ bool NeighborInfoModule::handleReceivedProtobuf(const meshtastic_MeshPacket &mp, meshtastic_NeighborInfo *np) { - if (np) { + if (enabled) { printNeighborInfo("RECEIVED", np); updateNeighbors(mp, np); - } else if (mp.hop_start != 0 && mp.hop_start == mp.hop_limit) { - // If the hopLimit is the same as hopStart, then it is a neighbor - getOrCreateNeighbor(mp.from, mp.from, 0, mp.rx_snr); // Set the broadcast interval to 0, as we don't know it } // Allow others to handle this packet return false; @@ -265,7 +261,7 @@ meshtastic_Neighbor *NeighborInfoModule::getOrCreateNeighbor(NodeNum originalSen nbr->snr = snr; nbr->last_rx_time = getTime(); // Only if this is the original sender, the broadcast interval corresponds to it - if (originalSender == n && node_broadcast_interval_secs != 0) + if (originalSender == n) nbr->node_broadcast_interval_secs = node_broadcast_interval_secs; saveProtoForModule(); // Save the updated neighbor return nbr; @@ -281,10 +277,8 @@ meshtastic_Neighbor *NeighborInfoModule::getOrCreateNeighbor(NodeNum originalSen new_nbr->snr = snr; new_nbr->last_rx_time = getTime(); // Only if this is the original sender, the broadcast interval corresponds to it - if (originalSender == n && node_broadcast_interval_secs != 0) + if (originalSender == n) new_nbr->node_broadcast_interval_secs = node_broadcast_interval_secs; - else // Assume the same broadcast interval as us for the neighbor if we don't know it - new_nbr->node_broadcast_interval_secs = moduleConfig.neighbor_info.update_interval; saveProtoForModule(); // Save the new neighbor return new_nbr; } diff --git a/src/modules/NeighborInfoModule.h b/src/modules/NeighborInfoModule.h index df5c2c9489..0e3ec09ca3 100644 --- a/src/modules/NeighborInfoModule.h +++ b/src/modules/NeighborInfoModule.h @@ -75,9 +75,6 @@ class NeighborInfoModule : public ProtobufModule, priva /* Does our periodic broadcast */ int32_t runOnce() override; - // Override wantPacket to say we want to see all packets when enabled, not just those for our port number - virtual bool wantPacket(const meshtastic_MeshPacket *p) override { return enabled; } - /* These are for debugging only */ void printNeighborInfo(const char *header, const meshtastic_NeighborInfo *np); void printNodeDBNodes(const char *header); diff --git a/src/modules/RoutingModule.cpp b/src/modules/RoutingModule.cpp index 37a7c37551..edeb1fb860 100644 --- a/src/modules/RoutingModule.cpp +++ b/src/modules/RoutingModule.cpp @@ -36,28 +36,13 @@ 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) { - auto p = allocAckNak(err, to, idFrom, chIndex, hopStart, hopLimit); + auto p = allocAckNak(err, to, idFrom, chIndex); router->sendLocal(p); // we sometimes send directly to the local node } -uint8_t RoutingModule::getHopLimitForResponse(uint8_t hopStart, uint8_t hopLimit) -{ - if (hopStart != 0) { - // Hops used by the request. If somebody in between running modified firmware modified it, ignore it - uint8_t hopsUsed = hopStart < hopLimit ? config.lora.hop_limit : hopStart - hopLimit; - if (hopsUsed > config.lora.hop_limit) { - return hopsUsed; // If the request used more hops than the limit, use the same amount of hops - } else if (hopsUsed + 2 < config.lora.hop_limit) { - return hopsUsed + 2; // Use only the amount of hops needed with some margin as the way back may be different - } - } - return config.lora.hop_limit; // Use the default hop limit -} - RoutingModule::RoutingModule() : ProtobufModule("routing", meshtastic_PortNum_ROUTING_APP, &meshtastic_Routing_msg) { isPromiscuous = true; diff --git a/src/modules/RoutingModule.h b/src/modules/RoutingModule.h index f085b307bf..06e76cfb4d 100644 --- a/src/modules/RoutingModule.h +++ b/src/modules/RoutingModule.h @@ -13,11 +13,7 @@ class RoutingModule : public ProtobufModule */ RoutingModule(); - void sendAckNak(meshtastic_Routing_Error err, NodeNum to, PacketId idFrom, ChannelIndex chIndex, uint8_t hopStart = 0, - 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); + void sendAckNak(meshtastic_Routing_Error err, NodeNum to, PacketId idFrom, ChannelIndex chIndex); protected: friend class Router;