Feat/6704 neighbor info on demand (#8523)

* full thing. works

* works

* minimal changes

* roll back previous changes, move to using the alloc() overrride

* clean up comments

* format

* run clang-format manually.
Trunk may be the absolute worst formatter in existance

* format on WSL to fix trunks awfulness

* add a 3 minute cooldown to prevent messages going back and forth

* add ignoring the dummy neighbor.

* fix or.

* fix spelling, increase logging

---------

Co-authored-by: Ben Meadors <benmmeadors@gmail.com>

src/modules/NeighborInfoModule.cpp

@@ -34,7 +34,8 @@ void NeighborInfoModule::printNodeDBNeighbors()
     }
 }
 
-/* Send our initial owner announcement 35 seconds after we start (to give network time to setup) */
+/* Send our initial owner announcement 35 seconds after we start (to give
+ * network time to setup) */
 NeighborInfoModule::NeighborInfoModule()
     : ProtobufModule("neighborinfo", meshtastic_PortNum_NEIGHBORINFO_APP, &meshtastic_NeighborInfo_msg),
       concurrency::OSThread("NeighborInfo")
@@ -53,8 +54,8 @@ NeighborInfoModule::NeighborInfoModule()
 }
 
 /*
-Collect neighbor info from the nodeDB's history, capping at a maximum number of entries and max time
+Collect neighbor info from the nodeDB's history, capping at a maximum number of
-Assumes that the neighborInfo packet has been allocated
+entries and max time Assumes that the neighborInfo packet has been allocated
 @returns the number of entries collected
 */
 uint32_t NeighborInfoModule::collectNeighborInfo(meshtastic_NeighborInfo *neighborInfo)
@@ -71,8 +72,8 @@ uint32_t NeighborInfoModule::collectNeighborInfo(meshtastic_NeighborInfo *neighb
         if ((neighborInfo->neighbors_count < MAX_NUM_NEIGHBORS) && (nbr.node_id != my_node_id)) {
             neighborInfo->neighbors[neighborInfo->neighbors_count].node_id = nbr.node_id;
             neighborInfo->neighbors[neighborInfo->neighbors_count].snr = nbr.snr;
-            // Note: we don't set the last_rx_time and node_broadcast_intervals_secs here, because we don't want to send this over
+            // Note: we don't set the last_rx_time and node_broadcast_intervals_secs
-            // the mesh
+            // here, because we don't want to send this over the mesh
             neighborInfo->neighbors_count++;
         }
     }
@@ -88,8 +89,9 @@ void NeighborInfoModule::cleanUpNeighbors()
     uint32_t now = getTime();
     NodeNum my_node_id = nodeDB->getNodeNum();
     for (auto it = neighbors.rbegin(); it != neighbors.rend();) {
-        // We will remove a neighbor if we haven't heard from them in twice the broadcast interval
+        // We will remove a neighbor if we haven't heard from them in twice the
-        // cannot use isWithinTimespanMs() as it->last_rx_time is seconds since 1970
+        // broadcast interval cannot use isWithinTimespanMs() as it->last_rx_time is
+        // seconds since 1970
         if ((now - it->last_rx_time > it->node_broadcast_interval_secs * 2) && (it->node_id != my_node_id)) {
             LOG_DEBUG("Remove neighbor with node ID 0x%x", it->node_id);
             it = std::vector<meshtastic_Neighbor>::reverse_iterator(
@@ -132,25 +134,55 @@ int32_t NeighborInfoModule::runOnce()
     return Default::getConfiguredOrDefaultMs(moduleConfig.neighbor_info.update_interval, default_neighbor_info_broadcast_secs);
 }
 
+meshtastic_MeshPacket *NeighborInfoModule::allocReply()
+{
+    LOG_INFO("NeighborInfoRequested.");
+    if (lastSentReply && Throttle::isWithinTimespanMs(lastSentReply, 3 * 60 * 1000)) {
+        LOG_DEBUG("Skip Neighbors reply since we sent a reply <3min ago");
+        ignoreRequest = true; // Mark it as ignored for MeshModule
+        return nullptr;
+    }
+
+    meshtastic_NeighborInfo neighborInfo = meshtastic_NeighborInfo_init_zero;
+    collectNeighborInfo(&neighborInfo);
+
+    meshtastic_MeshPacket *reply = allocDataProtobuf(neighborInfo);
+
+    if (reply) {
+        lastSentReply = millis(); // Track when we sent this reply
+    }
+    return reply;
+}
+
 /*
 Collect a received neighbor info packet from another node
 Pass it to an upper client; do not persist this data on the mesh
 */
 bool NeighborInfoModule::handleReceivedProtobuf(const meshtastic_MeshPacket &mp, meshtastic_NeighborInfo *np)
 {
+    LOG_DEBUG("NeighborInfo: handleReceivedProtobuf");
     if (np) {
         printNeighborInfo("RECEIVED", np);
+        // Ignore dummy/interceptable packets: single neighbor with nodeId 0 and snr 0
+        if (np->neighbors_count != 1 || np->neighbors[0].node_id != 0 || np->neighbors[0].snr != 0.0f) {
+            LOG_DEBUG("  Updating neighbours");
             updateNeighbors(mp, np);
+        } else {
+            LOG_DEBUG("  Ignoring dummy neighbor info packet (single neighbor with nodeId 0, snr 0)");
+        }
     } else if (mp.hop_start != 0 && mp.hop_start == mp.hop_limit) {
+        LOG_DEBUG("Get or create neighbor: %u with snr %f", mp.from, mp.rx_snr);
         // 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
+        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;
 }
 
 /*
-Copy the content of a current NeighborInfo packet into a new one and update the last_sent_by_id to our NodeNum
+Copy the content of a current NeighborInfo packet into a new one and update the
+last_sent_by_id to our NodeNum
 */
 void NeighborInfoModule::alterReceivedProtobuf(meshtastic_MeshPacket &p, meshtastic_NeighborInfo *n)
 {
@@ -168,8 +200,10 @@ void NeighborInfoModule::resetNeighbors()
 
 void NeighborInfoModule::updateNeighbors(const meshtastic_MeshPacket &mp, const meshtastic_NeighborInfo *np)
 {
-    // The last sent ID will be 0 if the packet is from the phone, which we don't count as
+    LOG_DEBUG("updateNeighbors");
-    // an edge. So we assume that if it's zero, then this packet is from our node.
+    // The last sent ID will be 0 if the packet is from the phone, which we don't
+    // count as an edge. So we assume that if it's zero, then this packet is from
+    // our node.
     if (mp.which_payload_variant == meshtastic_MeshPacket_decoded_tag && mp.from) {
         getOrCreateNeighbor(mp.from, np->last_sent_by_id, np->node_broadcast_interval_secs, mp.rx_snr);
     }
@@ -188,7 +222,8 @@ meshtastic_Neighbor *NeighborInfoModule::getOrCreateNeighbor(NodeNum originalSen
             // if found, update it
             neighbors[i].snr = snr;
             neighbors[i].last_rx_time = getTime();
-            // Only if this is the original sender, the broadcast interval corresponds to it
+            // Only if this is the original sender, the broadcast interval corresponds
+            // to it
             if (originalSender == n && node_broadcast_interval_secs != 0)
                 neighbors[i].node_broadcast_interval_secs = node_broadcast_interval_secs;
             return &neighbors[i];
@@ -200,10 +235,12 @@ meshtastic_Neighbor *NeighborInfoModule::getOrCreateNeighbor(NodeNum originalSen
     new_nbr.node_id = n;
     new_nbr.snr = snr;
     new_nbr.last_rx_time = getTime();
-    // Only if this is the original sender, the broadcast interval corresponds to it
+    // Only if this is the original sender, the broadcast interval corresponds to
+    // it
     if (originalSender == n && node_broadcast_interval_secs != 0)
         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
+    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;
 
     if (neighbors.size() < MAX_NUM_NEIGHBORS) {

src/modules/NeighborInfoModule.h

@@ -28,6 +28,10 @@ class NeighborInfoModule : public ProtobufModule<meshtastic_NeighborInfo>, priva
      */
     virtual bool handleReceivedProtobuf(const meshtastic_MeshPacket &mp, meshtastic_NeighborInfo *nb) override;
 
+    /* Messages can be received that have the want_response bit set.  If set, this callback will be invoked
+     * so that subclasses can (optionally) send a response back to the original sender.  */
+    virtual meshtastic_MeshPacket *allocReply() override;
+
     /*
      * Collect neighbor info from the nodeDB's history, capping at a maximum number of entries and max time
      * @return the number of entries collected
@@ -66,5 +70,8 @@ class NeighborInfoModule : public ProtobufModule<meshtastic_NeighborInfo>, priva
     /* These are for debugging only */
     void printNeighborInfo(const char *header, const meshtastic_NeighborInfo *np);
     void printNodeDBNeighbors();
+
+  private:
+    uint32_t lastSentReply = 0; // Last time we sent a position reply (used for reply throttling only)
 };
 extern NeighborInfoModule *neighborInfoModule;