move nodeinfo messages into new plugin system

docs/software/TODO.md

@@ -6,12 +6,16 @@ For app cleanup:
 
 * have python tool check max packet size before sending to device
 * require a recent python api to talk to these new device loads
-* on android for received positions handle either old or new positions
-* on android side send old or new positions as needed
+* DONE fix handleIncomingPosition
+* move want_replies handling into plugins
+* on android for received positions handle either old or new positions / user messages
+* on android side send old or new positions as needed / user messages
+* on python side print error messages if old position/user messages seen
+* on python side handle new position/user messages
 * DONE fix position sending to use new plugin
 * Add SinglePortNumPlugin - as the new most useful baseclass
 * DONE move positions into regular data packets (use new app framework)
-* move user info into regular data packets (use new app framework)
+* DONE move user info into regular data packets (use new app framework)
 * test that positions, text messages and user info still work
 * test that position, text messages and user info work properly with new android app and old device code
 * call the plugin setup functions

proto

@@ -1 +1 @@
-Subproject commit be48f1cbef1f00a4dbe67c81780dc53916ba5335
+Subproject commit 13b69ad55079e3f35774f63e960064867de20235

src/mesh/MeshService.cpp

@@ -13,8 +13,9 @@
 #include "RTC.h"
 #include "main.h"
 #include "mesh-pb-constants.h"
-#include "power.h"
 #include "plugins/PositionPlugin.h"
+#include "plugins/NodeInfoPlugin.h"
+#include "power.h"
 
 /*
 receivedPacketQueue - this is a queue of messages we've received from the mesh, which we are keeping to deliver to the phone.
@@ -52,7 +53,7 @@ MeshService service;
 
 static int32_t sendOwnerCb()
 {
-    service.sendOurOwner();
+    nodeInfoPlugin.sendOurNodeInfo();
 
     return getPref_send_owner_interval() * getPref_position_broadcast_secs() * 1000;
 }
@@ -75,94 +76,11 @@ void MeshService::init()
     packetReceivedObserver.observe(&router->notifyPacketReceived);
 }
 
-void MeshService::sendOurOwner(NodeNum dest, bool wantReplies)
-{
-    MeshPacket *p = router->allocForSending();
-    p->to = dest;
-    p->decoded.want_response = wantReplies;
-    p->decoded.which_payload = SubPacket_user_tag;
-    User &u = p->decoded.user;
-    u = owner;
-    DEBUG_MSG("sending owner %s/%s/%s\n", u.id, u.long_name, u.short_name);
-
-    sendToMesh(p);
-}
-
-/// handle a user packet that just arrived on the radio, return NULL if we should not process this packet at all
-const MeshPacket *MeshService::handleFromRadioUser(const MeshPacket *mp)
-{
-    bool wasBroadcast = mp->to == NODENUM_BROADCAST;
-
-    // Disable this collision testing if we use 32 bit nodenums
-    // (We do this always now, because we don't use 8 bit nodenums since 0.6 ish)
-    bool isCollision = false; // (sizeof(NodeNum) == 1) && (mp->from == myNodeInfo.my_node_num);
-
-    if (isCollision) {
-        // we win if we have a lower macaddr
-        bool weWin = memcmp(&owner.macaddr, &mp->decoded.user.macaddr, sizeof(owner.macaddr)) < 0;
-
-        if (weWin) {
-            DEBUG_MSG("NOTE! Received a nodenum collision and we are vetoing\n");
-
-            mp = NULL;
-
-            sendOurOwner(); // send our owner as a _broadcast_ because that other guy is mistakenly using our nodenum
-        } else {
-            // we lost, we need to try for a new nodenum!
-            DEBUG_MSG("NOTE! Received a nodenum collision we lost, so picking a new nodenum\n");
-            nodeDB.updateFrom(
-                *mp); // update the DB early - before trying to repick (so we don't select the same node number again)
-            nodeDB.pickNewNodeNum();
-            sendOurOwner(); // broadcast our new attempt at a node number
-        }
-    } else if (wasBroadcast) {
-        // If we haven't yet abandoned the packet and it was a broadcast, reply (just to them) with our User record so they can
-        // build their DB
-
-        // Someone just sent us a User, reply with our Owner
-        DEBUG_MSG("Received broadcast Owner from 0x%x, replying with our owner\n", mp->from);
-
-        sendOurOwner(mp->from);
-
-        String lcd = String("Joined: ") + mp->decoded.user.long_name + "\n";
-        screen->print(lcd.c_str());
-    }
-
-    return mp;
-}
-
-void MeshService::handleIncomingPosition(const MeshPacket *mp)
-{
-    if (mp->which_payload == MeshPacket_decoded_tag && mp->decoded.which_payload == SubPacket_position_tag) {
-        DEBUG_MSG("handled incoming position time=%u\n", mp->decoded.position.time);
-
-        if (mp->decoded.position.time) {
-            struct timeval tv;
-            uint32_t secs = mp->decoded.position.time;
-
-            tv.tv_sec = secs;
-            tv.tv_usec = 0;
-
-            perhapsSetRTC(RTCQualityFromNet, &tv);
-        }
-    } else {
-        DEBUG_MSG("Ignoring incoming packet - not a position\n");
-    }
-}
 
 int MeshService::handleFromRadio(const MeshPacket *mp)
 {
     powerFSM.trigger(EVENT_RECEIVED_PACKET); // Possibly keep the node from sleeping
 
-    // If it is a position packet, perhaps set our clock - this must be before nodeDB.updateFrom
-    handleIncomingPosition(mp);
-
-    if (mp->which_payload == MeshPacket_decoded_tag && mp->decoded.which_payload == SubPacket_user_tag) {
-        mp = handleFromRadioUser(mp);
-    }
-
-    // If we veto a received User packet, we don't put it into the DB or forward it to the phone (to prevent confusing it)
-    if (mp) {
     printPacket("Forwarding to phone", mp);
     nodeDB.updateFrom(*mp); // update our DB state based off sniffing every RX packet from the radio
 
@@ -180,9 +98,6 @@ int MeshService::handleFromRadio(const MeshPacket *mp)
 
     if (mp->decoded.want_response)
         sendNetworkPing(mp->from);
-    } else {
-        DEBUG_MSG("Not delivering vetoed User message\n");
-    }
 
     return 0;
 }
@@ -213,7 +128,7 @@ bool MeshService::reloadConfig()
 /// The owner User record just got updated, update our node DB and broadcast the info into the mesh
 void MeshService::reloadOwner()
 {
-    sendOurOwner();
+    nodeInfoPlugin.sendOurNodeInfo();
     nodeDB.saveToDisk();
 }
 
@@ -224,8 +139,6 @@ void MeshService::reloadOwner()
  */
 void MeshService::handleToRadio(MeshPacket &p)
 {
-    handleIncomingPosition(&p); // If it is a position packet, perhaps set our clock
-
     if (p.from == 0) // If the phone didn't set a sending node ID, use ours
         p.from = nodeDB.getNodeNum();
 
@@ -255,7 +168,8 @@ void MeshService::sendToMesh(MeshPacket *p)
     // Strip out any time information before sending packets to other  nodes - to keep the wire size small (and because other
     // nodes shouldn't trust it anyways) Note: we allow a device with a local GPS to include the time, so that gpsless
     // devices can get time.
-    if (p->which_payload == MeshPacket_decoded_tag && p->decoded.which_payload == SubPacket_position_tag && p->decoded.position.time) {
+    if (p->which_payload == MeshPacket_decoded_tag && p->decoded.which_payload == SubPacket_position_tag &&
+        p->decoded.position.time) {
         if (getRTCQuality() < RTCQualityGPS) {
             DEBUG_MSG("Stripping time %u from position send\n", p->decoded.position.time);
             p->decoded.position.time = 0;
@@ -276,11 +190,9 @@ void MeshService::sendNetworkPing(NodeNum dest, bool wantReplies)
     if (node->has_position)
         positionPlugin.sendOurPosition(dest, wantReplies);
     else
-        sendOurOwner(dest, wantReplies);
+        nodeInfoPlugin.sendOurNodeInfo(dest, wantReplies);
 }
 
-
-
 int MeshService::onGPSChanged(const meshtastic::GPSStatus *unused)
 {
     // Update our local node info with our position (even if we don't decide to update anyone else)

src/mesh/MeshService.h

@@ -75,9 +75,6 @@ class MeshService
     /// sends our owner
     void sendNetworkPing(NodeNum dest, bool wantReplies = false);
 
-    /// Send our owner info to a particular node
-    void sendOurOwner(NodeNum dest = NODENUM_BROADCAST, bool wantReplies = false);
-
     /// Send a packet into the mesh - note p must have been allocated from packetPool.  We will return it to that pool after
     /// sending. This is the ONLY function you should use for sending messages into the mesh, because it also updates the nodedb
     /// cache
@@ -92,12 +89,6 @@ class MeshService
     /// Handle a packet that just arrived from the radio.  This method does _not_ free the provided packet.  If it needs
     /// to keep the packet around it makes a copy
     int handleFromRadio(const MeshPacket *p);
-
-    /// handle a user packet that just arrived on the radio, return NULL if we should not process this packet at all
-    const MeshPacket *handleFromRadioUser(const MeshPacket *mp);
-
-    /// look at inbound packets and if they contain a position with time, possibly set our clock
-    void handleIncomingPosition(const MeshPacket *mp);
 };
 
 extern MeshService service;

src/mesh/NodeDB.cpp

@@ -407,8 +407,8 @@ size_t NodeDB::getNumOnlineNodes()
 
 /** Update position info for this node based on received position data
  */
-void NodeDB::updatePosition(uint32_t nodeId, const Position &p) {
-
+void NodeDB::updatePosition(uint32_t nodeId, const Position &p)
+{
     NodeInfo *info = getOrCreateNode(nodeId);
 
     // we always trust our local timestamps more
@@ -418,6 +418,32 @@ void NodeDB::updatePosition(uint32_t nodeId, const Position &p) {
     notifyObservers(true); // Force an update whether or not our node counts have changed
 }
 
+/** Update user info for this node based on received user data
+ */
+void NodeDB::updateUser(uint32_t nodeId, const User &p)
+{
+    NodeInfo *info = getOrCreateNode(nodeId);
+
+    DEBUG_MSG("old user %s/%s/%s\n", info->user.id, info->user.long_name, info->user.short_name);
+
+    bool changed = memcmp(&info->user, &p,
+                          sizeof(info->user)); // Both of these blocks start as filled with zero so I think this is okay
+
+    info->user = p;
+    DEBUG_MSG("updating changed=%d user %s/%s/%s\n", changed, info->user.id, info->user.long_name, info->user.short_name);
+    info->has_user = true;
+
+    if (changed) {
+        updateGUIforNode = info;
+        powerFSM.trigger(EVENT_NODEDB_UPDATED);
+        notifyObservers(true); // Force an update whether or not our node counts have changed
+
+        // Not really needed - we will save anyways when we go to sleep
+        // We just changed something important about the user, store our DB
+        // saveToDisk();
+    }
+}
+
 /// given a subpacket sniffed from the network, update our DB state
 /// we updateGUI and updateGUIforNode if we think our this change is big enough for a redraw
 void NodeDB::updateFrom(const MeshPacket &mp)
@@ -450,24 +476,8 @@ void NodeDB::updateFrom(const MeshPacket &mp)
         }
 
         case SubPacket_user_tag: {
-                DEBUG_MSG("old user %s/%s/%s\n", info->user.id, info->user.long_name, info->user.short_name);
-
-                bool changed = memcmp(&info->user, &p.user,
-                                    sizeof(info->user)); // Both of these blocks start as filled with zero so I think this is okay
-
-                info->user = p.user;
-                DEBUG_MSG("updating changed=%d user %s/%s/%s\n", changed, info->user.id, info->user.long_name, info->user.short_name);
-                info->has_user = true;
-
-                if (changed) {
-                    updateGUIforNode = info;
-                    powerFSM.trigger(EVENT_NODEDB_UPDATED);
-                    notifyObservers(true); // Force an update whether or not our node counts have changed
-
-                    // Not really needed - we will save anyways when we go to sleep
-                    // We just changed something important about the user, store our DB
-                    // saveToDisk();
-                }
+            DEBUG_MSG("WARNING: Processing a (deprecated) user packet from %d\n", mp.from);
+            updateUser(mp.from, p.user);
             break;
         }
 

src/mesh/NodeDB.h

@@ -61,6 +61,10 @@ class NodeDB
      */
     void updatePosition(uint32_t nodeId, const Position &p);
 
+    /** Update user info for this node based on received user data
+     */
+    void updateUser(uint32_t nodeId, const User &p);
+
     /// @return our node number
     NodeNum getNodeNum() { return myNodeInfo.my_node_num; }
 

src/mesh/ProtobufPlugin.h

@@ -1,3 +1,4 @@
+#pragma once
 #include "MeshPlugin.h"
 #include "Router.h"
 
@@ -47,6 +48,7 @@ template <class T> class ProtobufPlugin : private MeshPlugin
 
         p->decoded.data.payload.size =
             pb_encode_to_bytes(p->decoded.data.payload.bytes, sizeof(p->decoded.data.payload.bytes), fields, &payload);
+        // DEBUG_MSG("did encode\n");
         return p;
     }
 
@@ -61,7 +63,7 @@ template <class T> class ProtobufPlugin : private MeshPlugin
         // it would be better to update even if the message was destined to others.
 
         auto &p = mp.decoded.data;
-        DEBUG_MSG("Received %s from=0x%0x, id=%d, msg=%.*s\n", name, mp.from, mp.id, p.payload.size, p.payload.bytes);
+        DEBUG_MSG("Received %s from=0x%0x, id=%d, payloadlen=%d\n", name, mp.from, mp.id, p.payload.size);
 
         T scratch;
         if (pb_decode_from_bytes(p.payload.bytes, p.payload.size, fields, &scratch))

src/mesh/portnums.pb.h

@@ -17,9 +17,9 @@ extern "C" {
 typedef enum _PortNum {
     PortNum_UNKNOWN_APP = 0,
     PortNum_TEXT_MESSAGE_APP = 1,
-    PortNum_POSITION_APP = 3,
     PortNum_GPIO_APP = 2,
-    PortNum_MESH_USERINFO_APP = 4,
+    PortNum_POSITION_APP = 3,
+    PortNum_NODEINFO_APP = 4,
     PortNum_PRIVATE_APP = 256,
     PortNum_IP_TUNNEL_APP = 1024
 } PortNum;

src/plugins/NodeInfoPlugin.cpp

@@ -0,0 +1,39 @@
+#include "NodeInfoPlugin.h"
+#include "MeshService.h"
+#include "NodeDB.h"
+#include "RTC.h"
+#include "Router.h"
+#include "configuration.h"
+#include "main.h"
+
+NodeInfoPlugin nodeInfoPlugin;
+
+bool NodeInfoPlugin::handleReceivedProtobuf(const MeshPacket &mp, const User &p)
+{
+    // FIXME - we currently update NodeInfo data in the DB only if the message was a broadcast or destined to us
+    // it would be better to update even if the message was destined to others.
+
+    nodeDB.updateUser(mp.from, p);
+
+    bool wasBroadcast = mp.to == NODENUM_BROADCAST;
+
+    // Show new nodes on LCD screen
+    if (wasBroadcast) {
+        String lcd = String("Joined: ") + p.long_name + "\n";
+        screen->print(lcd.c_str());
+    }
+
+    return false; // Let others look at this message also if they want
+}
+
+void NodeInfoPlugin::sendOurNodeInfo(NodeNum dest, bool wantReplies)
+{
+    User &u = owner;
+
+    DEBUG_MSG("sending owner %s/%s/%s\n", u.id, u.long_name, u.short_name);
+    MeshPacket *p = allocForSending(u);
+    p->to = dest;
+    p->decoded.want_response = wantReplies;
+
+    service.sendToMesh(p);
+}

src/plugins/NodeInfoPlugin.h

@@ -0,0 +1,29 @@
+#pragma once
+#include "ProtobufPlugin.h"
+
+/**
+ * NodeInfo plugin for sending/receiving NodeInfos into the mesh
+ */
+class NodeInfoPlugin : public ProtobufPlugin<User>
+{
+  public:
+    /** Constructor
+     * name is for debugging output
+     */
+    NodeInfoPlugin() : ProtobufPlugin("nodeinfo", PortNum_NODEINFO_APP, User_fields) {}
+
+    /**
+     * Send our NodeInfo into the mesh
+     */
+    void sendOurNodeInfo(NodeNum dest = NODENUM_BROADCAST, bool wantReplies = false);
+
+  protected:
+
+    /** Called to handle a particular incoming message
+
+    @return true if you've guaranteed you've handled this message and no other handlers should be considered for it
+    */
+    virtual bool handleReceivedProtobuf(const MeshPacket &mp, const User &p);
+};
+
+extern NodeInfoPlugin nodeInfoPlugin;

src/plugins/PositionPlugin.cpp

@@ -12,6 +12,17 @@ bool PositionPlugin::handleReceivedProtobuf(const MeshPacket &mp, const Position
     // FIXME - we currently update position data in the DB only if the message was a broadcast or destined to us
     // it would be better to update even if the message was destined to others.
 
+    DEBUG_MSG("handled incoming position time=%u\n", p.time);
+    if (p.time) {
+        struct timeval tv;
+        uint32_t secs = p.time;
+
+        tv.tv_sec = secs;
+        tv.tv_usec = 0;
+
+        perhapsSetRTC(RTCQualityFromNet, &tv);
+    }
+
     nodeDB.updatePosition(mp.from, p);
 
     return false; // Let others look at this message also if they want

src/plugins/PositionPlugin.h

@@ -1,3 +1,4 @@
+#pragma once
 #include "ProtobufPlugin.h"
 
 /**

src/plugins/TextMessagePlugin.h

@@ -1,3 +1,4 @@
+#pragma once
 #include "MeshPlugin.h"
 #include "Observer.h"