don't send messages the phone sent us back towards the phone

docs/software/TODO.md

@@ -29,13 +29,14 @@ You probably don't care about this section - skip to the next one.
 * DONE release protobufs
 * DONE release to developers
 * DONE fix setch-fast in python tool
+* age out pendingrequests in the python API
+* DONE stress test channel download from python, sometimes it seems like we don't get all replies, bug was due to simultaneous android connection
 * combine acks and responses in a single message if possible (do routing plugin LAST and drop ACK if someone else has already replied)
-* don't send packets we received from the phone BACK TOWARDS THE PHONE (possibly use fromnode 0 for packets the phone sends?)
+* DONE don't send packets we received from the phone BACK TOWARDS THE PHONE (possibly use fromnode 0 for packets the phone sends?)
 * fix 1.1.50 android debug panel display
 * DONE warn in android app about unset regions
 * DONE use set-channel from android
 * DONE add gui in android app for setting region
-* stress test channel download from python, sometimes it seems like we don't get all replies
 * make python tests more exhaustive
 * pick default random admin key
 * exclude admin channels from URL?
@@ -61,6 +62,7 @@ You probably don't care about this section - skip to the next one.
 * confirm we are still calling the plugins for messages inbound from the phone (or generated locally)
 * confirm we are still multi hop routing flood broadcasts
 * confirm we are still doing resends on unicast reliable packets
+* add history to routed packets: https://meshtastic.discourse.group/t/packet-source-tracking/2764/2
 * add support for full DSR unicast delivery
 * DONE move acks into routing
 * DONE make all subpackets different versions of data

src/graphics/Screen.cpp

@@ -229,7 +229,7 @@ static void drawTextMessageFrame(OLEDDisplay *display, OLEDDisplayUiState *state
     displayedNodeNum = 0; // Not currently showing a node pane
 
     MeshPacket &mp = devicestate.rx_text_message;
-    NodeInfo *node = nodeDB.getNode(mp.from);
+    NodeInfo *node = nodeDB.getNode(getFrom(&mp));
     // DEBUG_MSG("drawing text message from 0x%x: %s\n", mp.from,
     // mp.decoded.variant.data.decoded.bytes);
 

src/mesh/DSRRouter.cpp

@@ -69,7 +69,7 @@ void DSRRouter::sniffReceived(const MeshPacket *p, const Routing *c)
     // ignore rebroadcasts.
     // this will also add records for any ACKs we receive for our messages
     if (p->to != NODENUM_BROADCAST || p->hop_limit != HOP_RELIABLE) {
-        addRoute(p->from, p->from, 0); // We are adjacent with zero hops
+        addRoute(getFrom(p), getFrom(p), 0); // We are adjacent with zero hops
     }
 
     if (c)

src/mesh/MeshPacketQueue.cpp

@@ -71,7 +71,7 @@ static PacketId findId;
 
 static bool isMyPacket(MeshPacket *p)
 {
-    return p->id == findId && p->from == findFrom;
+    return p->id == findId && getFrom(p) == findFrom;
 }
 
 /** Attempt to find and remove a packet from this queue.  Returns true the packet which was removed from the queue */

src/mesh/MeshPlugin.cpp

@@ -52,7 +52,7 @@ void MeshPlugin::callPlugins(const MeshPacket &mp)
             // NOTE: we send a reply *even if the (non broadcast) request was from us* which is unfortunate but necessary because currently when the phone 
             // sends things, it sends things using the local node ID as the from address.  A better solution (FIXME) would be to let phones
             // have their own distinct addresses and we 'route' to them like any other node.
-            if (mp.decoded.want_response && toUs && (mp.from != ourNodeNum || mp.to == ourNodeNum)) {
+            if (mp.decoded.want_response && toUs && (getFrom(&mp) != ourNodeNum || mp.to == ourNodeNum)) {
                 pi.sendResponse(mp);
                 DEBUG_MSG("Plugin %s sent a response\n", pi.name);
             }
@@ -94,7 +94,7 @@ void MeshPlugin::sendResponse(const MeshPacket &req) {
 */
 void setReplyTo(MeshPacket *p, const MeshPacket &to) {
     assert(p->which_payloadVariant == MeshPacket_decoded_tag); // Should already be set by now
-    p->to = to.from;
+    p->to = getFrom(&to);
     p->want_ack = to.want_ack;
     p->decoded.request_id = to.id;
 }

src/mesh/MeshService.cpp

@@ -13,8 +13,8 @@
 #include "RTC.h"
 #include "main.h"
 #include "mesh-pb-constants.h"
-#include "plugins/PositionPlugin.h"
 #include "plugins/NodeInfoPlugin.h"
+#include "plugins/PositionPlugin.h"
 #include "power.h"
 
 /*
@@ -51,8 +51,6 @@ MeshService service;
 
 #include "Router.h"
 
-
-
 MeshService::MeshService() : toPhoneQueue(MAX_RX_TOPHONE)
 {
     // assert(MAX_RX_TOPHONE == 32); // FIXME, delete this, just checking my clever macro
@@ -67,11 +65,11 @@ void MeshService::init()
         gpsObserver.observe(&gps->newStatus);
 }
 
-
 int MeshService::handleFromRadio(const MeshPacket *mp)
 {
     powerFSM.trigger(EVENT_RECEIVED_PACKET); // Possibly keep the node from sleeping
 
+    if (mp->from != 0) {
         printPacket("Forwarding to phone", mp);
         nodeDB.updateFrom(*mp); // update our DB state based off sniffing every RX packet from the radio
 
@@ -86,6 +84,10 @@ int MeshService::handleFromRadio(const MeshPacket *mp)
 
         MeshPacket *copied = packetPool.allocCopy(*mp);
         assert(toPhoneQueue.enqueue(copied, 0)); // FIXME, instead of failing for full queue, delete the oldest mssages
+    }
+    else {
+        DEBUG_MSG("Packet originally from phone, no need to send back that way...\n");
+    }
 
     return 0;
 }
@@ -128,8 +130,12 @@ void MeshService::reloadOwner()
  */
 void MeshService::handleToRadio(MeshPacket &p)
 {
-    if (p.from == 0) // If the phone didn't set a sending node ID, use ours
+    if (p.from != 0) { // We don't let phones assign nodenums to their sent messages
-        p.from = nodeDB.getNodeNum();
+        DEBUG_MSG("Warning: phone tried to pick a nodenum, we don't allow that.\n");
+        p.from = 0;
+    } else {
+        // p.from = nodeDB.getNodeNum();
+    }
 
     if (p.id == 0)
         p.id = generatePacketId(); // If the phone didn't supply one, then pick one
@@ -151,7 +157,8 @@ void MeshService::handleToRadio(MeshPacket &p)
 }
 
 /** Attempt to cancel a previously sent packet from this _local_ node.  Returns true if a packet was found we could cancel */
-bool MeshService::cancelSending(PacketId id) {
+bool MeshService::cancelSending(PacketId id)
+{
     return router->cancelSending(nodeDB.getNodeNum(), id);
 }
 
@@ -176,8 +183,8 @@ void MeshService::sendNetworkPing(NodeNum dest, bool wantReplies)
         nodeInfoPlugin->sendOurNodeInfo(dest, wantReplies);
 }
 
-
+NodeInfo *MeshService::refreshMyNodeInfo()
-NodeInfo *MeshService::refreshMyNodeInfo() {
+{
     NodeInfo *node = nodeDB.getNode(nodeDB.getNodeNum());
     assert(node);
 
@@ -190,7 +197,8 @@ NodeInfo *MeshService::refreshMyNodeInfo() {
     Position &position = node->position;
 
     // Update our local node info with our position (even if we don't decide to update anyone else)
-    position.time = getValidTime(RTCQualityFromNet); // This nodedb timestamp might be stale, so update it if our clock is kinda valid
+    position.time =
+        getValidTime(RTCQualityFromNet); // This nodedb timestamp might be stale, so update it if our clock is kinda valid
 
     position.battery_level = powerStatus->getBatteryChargePercent();
     updateBatteryLevel(position.battery_level);
@@ -209,8 +217,7 @@ int MeshService::onGPSChanged(const meshtastic::GPSStatus *unused)
             pos.altitude = gps->altitude;
         pos.latitude_i = gps->latitude;
         pos.longitude_i = gps->longitude;
-    }
+    } else {
-    else {
         // The GPS has lost lock, if we are fixed position we should just keep using
         // the old position
         if (radioConfig.preferences.fixed_position) {

src/mesh/MeshTypes.h

@@ -33,3 +33,9 @@ typedef int ErrorCode;
 
 /// Alloc and free packets to our global, ISR safe pool
 extern Allocator<MeshPacket> &packetPool;
+
+/**
+ * Most (but not always) of the time we want to treat packets 'from' the local phone (where from == 0), as if they originated on the local node.
+ * If from is zero this function returns our node number instead
+ */
+NodeNum getFrom(const MeshPacket *p);

src/mesh/NodeDB.cpp

@@ -66,6 +66,14 @@ NodeNum displayedNodeNum;
 
 NodeDB::NodeDB() : nodes(devicestate.node_db), numNodes(&devicestate.node_db_count) {}
 
+/**
+ * Most (but not always) of the time we want to treat packets 'from' the local phone (where from == 0), as if they originated on the local node.
+ * If from is zero this function returns our node number instead
+ */
+NodeNum getFrom(const MeshPacket *p) {
+    return (p->from == 0) ? nodeDB.getNodeNum() : p->from;
+}
+
 bool NodeDB::resetRadioConfig()
 {
     bool didFactoryReset = false;
@@ -406,7 +414,7 @@ void NodeDB::updateFrom(const MeshPacket &mp)
     if (mp.which_payloadVariant == MeshPacket_decoded_tag) {
         DEBUG_MSG("Update DB node 0x%x, rx_time=%u\n", mp.from, mp.rx_time);
 
-        NodeInfo *info = getOrCreateNode(mp.from);
+        NodeInfo *info = getOrCreateNode(getFrom(&mp));
 
         if (mp.rx_time) {              // if the packet has a valid timestamp use it to update our last_seen
             info->has_position = true; // at least the time is valid

src/mesh/NodeDB.h

@@ -183,3 +183,4 @@ PREF_GET(min_wake_secs, 10)
  * might have changed is incremented.  Allows others to detect they might now be on a new channel.
  */
 extern uint32_t radioGeneration;
+

src/mesh/PacketHistory.cpp

@@ -26,7 +26,7 @@ bool PacketHistory::wasSeenRecently(const MeshPacket *p, bool withUpdate)
             // DEBUG_MSG("Deleting old broadcast record %d\n", i);
             recentPackets.erase(recentPackets.begin() + i); // delete old record
         } else {
-            if (r.id == p->id && r.sender == p->from) {
+            if (r.id == p->id && r.sender == getFrom(p)) {
                 DEBUG_MSG("Found existing packet record for fr=0x%x,to=0x%x,id=%d\n", p->from, p->to, p->id);
 
                 // Update the time on this record to now
@@ -43,7 +43,7 @@ bool PacketHistory::wasSeenRecently(const MeshPacket *p, bool withUpdate)
     if (withUpdate) {
         PacketRecord r;
         r.id = p->id;
-        r.sender = p->from;
+        r.sender = getFrom(p);
         r.rxTimeMsec = now;
         recentPackets.push_back(r);
         printPacket("Adding packet record", p);

src/mesh/ReliableRouter.cpp

@@ -26,15 +26,15 @@ ErrorCode ReliableRouter::send(MeshPacket *p)
 
 bool ReliableRouter::shouldFilterReceived(const MeshPacket *p)
 {
-    if (p->to == NODENUM_BROADCAST && p->from == getNodeNum()) {
+    if (p->to == NODENUM_BROADCAST && getFrom(p) == getNodeNum()) {
         printPacket("Rx someone rebroadcasting for us", p);
 
         // We are seeing someone rebroadcast one of our broadcast attempts.
         // If this is the first time we saw this, cancel any retransmissions we have queued up and generate an internal ack for
         // the original sending process.
-        if (stopRetransmission(p->from, p->id)) {
+        if (stopRetransmission(getFrom(p), p->id)) {
             DEBUG_MSG("Someone is retransmitting for us, generate implicit ack\n");
-            sendAckNak(Routing_Error_NONE, p->from, p->id);
+            sendAckNak(Routing_Error_NONE, getFrom(p), p->id);
         }
     }
 
@@ -60,7 +60,7 @@ void ReliableRouter::sniffReceived(const MeshPacket *p, const Routing *c)
     if (p->to == ourNode) { // ignore ack/nak/want_ack packets that are not address to us (we only handle 0 hop reliability
                             // - not DSR routing)
         if (p->want_ack) {
-            sendAckNak(Routing_Error_NONE, p->from, p->id);
+            sendAckNak(Routing_Error_NONE, getFrom(p), p->id);
         }
 
         // If the payload is valid, look for ack/nak
@@ -131,7 +131,7 @@ PendingPacket *ReliableRouter::startRetransmission(MeshPacket *p)
     auto rec = PendingPacket(p);
 
     setNextTx(&rec);
-    stopRetransmission(p->from, p->id);
+    stopRetransmission(getFrom(p), p->id);
     pending[id] = rec;
 
     return &pending[id];

src/mesh/ReliableRouter.h

@@ -15,7 +15,7 @@ struct GlobalPacketId {
 
     GlobalPacketId(const MeshPacket *p)
     {
-        node = p->from;
+        node = getFrom(p);
         id = p->id;
     }
 

src/mesh/Router.cpp

@@ -111,7 +111,7 @@ void Router::sendAckNak(Routing_Error err, NodeNum to, PacketId idFrom)
 void Router::abortSendAndNak(Routing_Error err, MeshPacket *p)
 {
     DEBUG_MSG("Error=%d, returning NAK and dropping packet.\n", err);
-    sendAckNak(Routing_Error_NO_INTERFACE, p->from, p->id);
+    sendAckNak(Routing_Error_NO_INTERFACE, getFrom(p), p->id);
     packetPool.release(p);
 }
 
@@ -155,6 +155,10 @@ ErrorCode Router::send(MeshPacket *p)
     if (p->to == NODENUM_BROADCAST)
         p->want_ack = false;
 
+    // Up until this point we might have been using 0 for the from address (if it started with the phone), but when we send over
+    // the lora we need to make sure we have replaced it with our local address
+    p->from = getFrom(p);
+
     // 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_payloadVariant == MeshPacket_encrypted_tag ||
@@ -273,7 +277,7 @@ void Router::handleReceived(MeshPacket *p)
 void Router::perhapsHandleReceived(MeshPacket *p)
 {
     assert(radioConfig.has_preferences);
-    bool ignore = is_in_repeated(radioConfig.preferences.ignore_incoming, p->from);
+    bool ignore = is_in_repeated(radioConfig.preferences.ignore_incoming, getFrom(p));
 
     if (ignore)
         DEBUG_MSG("Ignoring incoming message, 0x%x is in our ignore list\n", p->from);

src/plugins/ExternalNotificationPlugin.cpp

@@ -147,7 +147,7 @@ bool ExternalNotificationPluginRadio::handleReceived(const MeshPacket &mp)
 
         auto &p = mp.decoded;
 
-        if (mp.from != nodeDB.getNodeNum()) {
+        if (getFrom(&mp) != nodeDB.getNodeNum()) {
 
             // TODO: This may be a problem if messages are sent in unicide, but I'm not sure if it will.
             //   Need to know if and how this could be a problem.

src/plugins/NodeInfoPlugin.cpp

@@ -12,7 +12,7 @@ bool NodeInfoPlugin::handleReceivedProtobuf(const MeshPacket &mp, const User *pp
 {
     auto p = *pptr;
 
-    nodeDB.updateUser(mp.from, p);
+    nodeDB.updateUser(getFrom(&mp), p);
 
     bool wasBroadcast = mp.to == NODENUM_BROADCAST;
 

src/plugins/PositionPlugin.cpp

@@ -30,7 +30,7 @@ bool PositionPlugin::handleReceivedProtobuf(const MeshPacket &mp, const Position
         perhapsSetRTC(RTCQualityFromNet, &tv);
     }
 
-    nodeDB.updatePosition(mp.from, p);
+    nodeDB.updatePosition(getFrom(&mp), p);
 
     return false; // Let others look at this message also if they want
 }

src/plugins/SerialPlugin.cpp

@@ -160,7 +160,7 @@ bool SerialPluginRadio::handleReceived(const MeshPacket &mp)
         // DEBUG_MSG("Received text msg self=0x%0x, from=0x%0x, to=0x%0x, id=%d, msg=%.*s\n",
         //          nodeDB.getNodeNum(), mp.from, mp.to, mp.id, p.payload.size, p.payload.bytes);
 
-        if (mp.from == nodeDB.getNodeNum()) {
+        if (getFrom(&mp) == nodeDB.getNodeNum()) {
 
             /*
              * If radioConfig.preferences.serialplugin_echo is true, then echo the packets that are sent out back to the TX

src/plugins/esp32/EnvironmentalMeasurementPlugin.cpp

@@ -137,8 +137,9 @@ bool EnvironmentalMeasurementPlugin::wantUIFrame() {
 String GetSenderName(const MeshPacket &mp) {
     String sender;
 
-    if (nodeDB.getNode(mp.from)){
+    auto node = nodeDB.getNode(getFrom(&mp));
-        sender = nodeDB.getNode(mp.from)->user.short_name;
+    if (node){
+        sender = node->user.short_name;
     }
     else {
         sender = "UNK";

src/plugins/esp32/RangeTestPlugin.cpp

@@ -133,7 +133,7 @@ bool RangeTestPluginRadio::handleReceived(const MeshPacket &mp)
         // DEBUG_MSG("Received text msg self=0x%0x, from=0x%0x, to=0x%0x, id=%d, msg=%.*s\n",
         //          nodeDB.getNodeNum(), mp.from, mp.to, mp.id, p.payload.size, p.payload.bytes);
 
-        if (mp.from != nodeDB.getNodeNum()) {
+        if (getFrom(&mp) != nodeDB.getNodeNum()) {
 
             // DEBUG_MSG("* * Message came from the mesh\n");
             // Serial2.println("* * Message came from the mesh");
@@ -144,7 +144,7 @@ bool RangeTestPluginRadio::handleReceived(const MeshPacket &mp)
 
             */
 
-            NodeInfo *n = nodeDB.getNode(mp.from);
+            NodeInfo *n = nodeDB.getNode(getFrom(&mp));
 
             if (radioConfig.preferences.range_test_plugin_save) {
                 appendFile(mp);
@@ -209,7 +209,7 @@ bool RangeTestPluginRadio::appendFile(const MeshPacket &mp)
 {
     auto &p = mp.decoded;
 
-    NodeInfo *n = nodeDB.getNode(mp.from);
+    NodeInfo *n = nodeDB.getNode(getFrom(&mp));
     /*
         DEBUG_MSG("-----------------------------------------\n");
         DEBUG_MSG("p.payload.bytes  \"%s\"\n", p.payload.bytes);
@@ -290,7 +290,7 @@ bool RangeTestPluginRadio::appendFile(const MeshPacket &mp)
         fileToAppend.printf("??:??:??,"); // Time
     }
 
-    fileToAppend.printf("%d,", mp.from);                          // From
+    fileToAppend.printf("%d,", getFrom(&mp));                          // From
     fileToAppend.printf("%s,", n->user.long_name);                // Long Name
     fileToAppend.printf("%f,", n->position.latitude_i * 1e-7);    // Sender Lat
     fileToAppend.printf("%f,", n->position.longitude_i * 1e-7);   // Sender Long

src/plugins/esp32/StoreForwardPlugin.cpp

@@ -240,12 +240,12 @@ bool StoreForwardPluginRadio::handleReceived(const MeshPacket &mp)
 #ifndef NO_ESP32
     if (radioConfig.preferences.store_forward_plugin_enabled) {
 
-        if (mp.from != nodeDB.getNodeNum()) {
+        if (getFrom(&mp) != nodeDB.getNodeNum()) {
             // DEBUG_MSG("Store & Forward Plugin -- Print Start ---------- ---------- ---------- ---------- ----------\n\n\n");
             // DEBUG_MSG("%s (id=0x%08x Fr0x%02x To0x%02x, WantAck%d, HopLim%d", prefix, p->id, p->from & 0xff, p->to & 0xff,
             // p->want_ack, p->hop_limit);
             printPacket("----- PACKET FROM RADIO -----", &mp);
-            uint32_t sawTime = storeForwardPlugin->sawNode(mp.from & 0xffffffff);
+            uint32_t sawTime = storeForwardPlugin->sawNode(getFrom(&mp) & 0xffffffff);
             DEBUG_MSG("We last saw this node (%u), %u sec ago\n", mp.from & 0xffffffff, (millis() - sawTime) / 1000);
 
             if (mp.decoded.portnum == PortNum_UNKNOWN_APP) {
@@ -283,7 +283,7 @@ bool StoreForwardPluginRadio::handleReceived(const MeshPacket &mp)
 
             if ((millis() - sawTime) > STOREFORWARD_SEND_HISTORY_SHORT) {
                 // Node has been away for a while. 
-                storeForwardPlugin->historySend(sawTime, mp.from);
+                storeForwardPlugin->historySend(sawTime, getFrom(&mp));
             }
         }