WIP moving positions to new system

docs/software/TODO.md

@@ -4,9 +4,11 @@ You probably don't care about this section - skip to the next one.
 
 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
+* fix position sending to use new plugin
 * move positions into regular data packets (use new app framework)
 * move user info into regular data packets (use new app framework)
 * test that positions, text messages and user info still work

src/mesh/MeshPlugin.h

@@ -14,8 +14,6 @@
  */
 class MeshPlugin
 {
-    const char *name;
-
     static std::vector<MeshPlugin *> *plugins;
 
   public:
@@ -31,6 +29,8 @@ class MeshPlugin
     static void callPlugins(const MeshPacket &mp);
 
   protected:
+    const char *name;
+
     /**
      * Initialize your plugin.  This setup function is called once after all hardware and mesh protocol layers have
      * been initialized

src/mesh/MeshService.cpp

@@ -14,6 +14,7 @@
 #include "main.h"
 #include "mesh-pb-constants.h"
 #include "power.h"
+#include "plugins/PositionPlugin.h"
 
 /*
 receivedPacketQueue - this is a queue of messages we've received from the mesh, which we are keeping to deliver to the phone.
@@ -254,7 +255,7 @@ 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) {
+    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;
@@ -273,27 +274,12 @@ void MeshService::sendNetworkPing(NodeNum dest, bool wantReplies)
 
     DEBUG_MSG("Sending network ping to 0x%x, with position=%d, wantReplies=%d\n", dest, node->has_position, wantReplies);
     if (node->has_position)
-        sendOurPosition(dest, wantReplies);
+        positionPlugin.sendOurPosition(dest, wantReplies);
     else
         sendOurOwner(dest, wantReplies);
 }
 
-void MeshService::sendOurPosition(NodeNum dest, bool wantReplies)
-{
-    NodeInfo *node = nodeDB.getNode(nodeDB.getNodeNum());
-    assert(node);
-    assert(node->has_position);
 
-    // Update our local node info with our position (even if we don't decide to update anyone else)
-    MeshPacket *p = router->allocForSending();
-    p->to = dest;
-    p->decoded.which_payload = SubPacket_position_tag;
-    p->decoded.position = node->position;
-    p->decoded.want_response = wantReplies;
-    p->decoded.position.time =
-        getValidTime(RTCQualityGPS); // This nodedb timestamp might be stale, so update it if our clock is valid.
-    sendToMesh(p);
-}
 
 int MeshService::onGPSChanged(const meshtastic::GPSStatus *unused)
 {

src/mesh/MeshService.h

@@ -78,15 +78,13 @@ class MeshService
     /// Send our owner info to a particular node
     void sendOurOwner(NodeNum dest = NODENUM_BROADCAST, bool wantReplies = false);
 
-  private:
-    /// Broadcasts our last known position
-    void sendOurPosition(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
     void sendToMesh(MeshPacket *p);
 
+  private:
+
     /// Called when our gps position has changed - updates nodedb and sends Location message out into the mesh
     /// returns 0 to allow futher processing
     int onGPSChanged(const meshtastic::GPSStatus *arg);

src/mesh/NodeDB.cpp

@@ -405,6 +405,19 @@ size_t NodeDB::getNumOnlineNodes()
 
 #include "MeshPlugin.h"
 
+/** Update position info for this node based on received position data
+ */
+void NodeDB::updatePosition(uint32_t nodeId, const Position &p) {
+
+    NodeInfo *info = getOrCreateNode(nodeId);
+
+            // we always trust our local timestamps more
+            info->position = p;
+            info->has_position = true;
+            updateGUIforNode = info;
+            notifyObservers(true); // Force an update whether or not our node counts have changed
+}
+
 /// 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)
@@ -423,48 +436,44 @@ void NodeDB::updateFrom(const MeshPacket &mp)
         info->snr = mp.rx_snr; // keep the most recent SNR we received for this node.
 
         switch (p.which_payload) {
-        case SubPacket_position_tag: {
-            // we always trust our local timestamps more
-            info->position = p.position;
-            if (mp.rx_time)
-                info->position.time = mp.rx_time;
-            info->has_position = true;
-            updateGUIforNode = info;
-            notifyObservers(true); // Force an update whether or not our node counts have changed
-            break;
-        }
-
-        case SubPacket_data_tag: {
-            if(mp.to == NODENUM_BROADCAST || mp.to == nodeDB.getNodeNum())
-                MeshPlugin::callPlugins(mp);
-            break;
-        }
-
-        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();
+            case SubPacket_position_tag: {
+                // handle a legacy position packet 
+                DEBUG_MSG("WARNING: Processing a (deprecated) position packet from %d\n", mp.from);
+                updatePosition(mp.from, p.position);
+                break;
             }
-            break;
-        }
 
-        default: {
-            notifyObservers(); // If the node counts have changed, notify observers
-        }
+            case SubPacket_data_tag: {
+                if(mp.to == NODENUM_BROADCAST || mp.to == nodeDB.getNodeNum())
+                    MeshPlugin::callPlugins(mp);
+                break;
+            }
+
+            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();
+                }
+                break;
+            }
+
+            default: {
+                notifyObservers(); // If the node counts have changed, notify observers
+            }
         }
     }
 }

src/mesh/NodeDB.h

@@ -57,6 +57,10 @@ class NodeDB
     /// we updateGUI and updateGUIforNode if we think our this change is big enough for a redraw
     void updateFrom(const MeshPacket &p);
 
+    /** Update position info for this node based on received position data
+     */
+    void updatePosition(uint32_t nodeId, const Position &p);
+
     /// @return our node number
     NodeNum getNodeNum() { return myNodeInfo.my_node_num; }
 

src/mesh/ProtobufPlugin.cpp

@@ -0,0 +1,3 @@
+#include "ProtobufPlugin.h"
+
+

src/mesh/ProtobufPlugin.h

@@ -0,0 +1,72 @@
+#include "MeshPlugin.h"
+#include "Router.h"
+
+/**
+ * A base class for mesh plugins that assume that they are sending/receiving one particular protobuf based
+ * payload.  Using one particular app ID.
+ *
+ * If you are using protobufs to encode your packets (recommended) you can use this as a baseclass for your plugin
+ * and avoid a bunch of boilerplate code.
+ */
+template <class T> class ProtobufPlugin : private MeshPlugin
+{
+    const pb_msgdesc_t *fields;
+    PortNum ourPortNum;
+
+  public:
+    /** Constructor
+     * name is for debugging output
+     */
+    ProtobufPlugin(const char *_name, PortNum _ourPortNum, const pb_msgdesc_t *_fields)
+        : MeshPlugin(_name), fields(_fields), ourPortNum(_ourPortNum)
+    {
+    }
+
+  protected:
+    /**
+     * @return true if you want to receive the specified portnum
+     */
+    virtual bool wantPortnum(PortNum p) { return p == ourPortNum; }
+
+    /**
+     * Handle a received message, the data field in the message is already decoded and is provided
+     */
+    virtual bool handleReceivedProtobuf(const MeshPacket &mp, const T &decoded) = 0;
+
+    /**
+     * Return a mesh packet which has been preinited with a particular protobuf data payload and port number.
+     * You can then send this packet (after customizing any of the payload fields you might need) with
+     * service.sendToMesh()
+     */
+    MeshPacket *allocForSending(const T &payload)
+    {
+        // Update our local node info with our position (even if we don't decide to update anyone else)
+        MeshPacket *p = router->allocForSending();
+        p->decoded.which_payload = SubPacket_data_tag;
+        p->decoded.data.portnum = ourPortNum;
+
+        p->decoded.data.payload.size =
+            pb_encode_to_bytes(p->decoded.data.payload.bytes, sizeof(p->decoded.data.payload.bytes), fields, &payload);
+        return p;
+    }
+
+  private:
+    /** 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 handleReceived(const MeshPacket &mp)
+    {
+        // 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.
+
+        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);
+
+        T scratch;
+        if (pb_decode_from_bytes(p.payload.bytes, p.payload.size, fields, &scratch))
+            handleReceivedProtobuf(mp, scratch);
+
+        return false; // Let others look at this message also if they want
+    }
+};

src/plugins/PositionPlugin.cpp

@@ -1,13 +1,35 @@
-#include "configuration.h"
 #include "PositionPlugin.h"
+#include "MeshService.h"
 #include "NodeDB.h"
+#include "RTC.h"
+#include "Router.h"
+#include "configuration.h"
 
 PositionPlugin positionPlugin;
 
-bool PositionPlugin::handleReceived(const MeshPacket &mp)
+bool PositionPlugin::handleReceivedProtobuf(const MeshPacket &mp, const Position &p)
 {
-    auto &p = mp.decoded.data;
-    DEBUG_MSG("Received position from=0x%0x, id=%d, msg=%.*s\n", mp.from, mp.id, p.payload.size, p.payload.bytes);
+    // 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.
+
+    nodeDB.updatePosition(mp.from, p);
 
     return false; // Let others look at this message also if they want
 }
+
+void PositionPlugin::sendOurPosition(NodeNum dest, bool wantReplies)
+{
+    NodeInfo *node = nodeDB.getNode(nodeDB.getNodeNum());
+    assert(node);
+    assert(node->has_position);
+
+    // Update our local node info with our position (even if we don't decide to update anyone else)
+    auto position = node->position;
+    position.time = getValidTime(RTCQualityGPS); // This nodedb timestamp might be stale, so update it if our clock is valid.
+
+    MeshPacket *p = allocForSending(position);
+    p->to = dest;
+    p->decoded.want_response = wantReplies;
+
+    service.sendToMesh(p);
+}

src/plugins/PositionPlugin.h

@@ -0,0 +1,28 @@
+#include "ProtobufPlugin.h"
+
+/**
+ * Position plugin for sending/receiving positions into the mesh
+ */
+class PositionPlugin : public ProtobufPlugin<Position>
+{
+  public:
+    /** Constructor
+     * name is for debugging output
+     */
+    PositionPlugin() : ProtobufPlugin("position", PortNum_POSITION_APP, Position_fields) {}
+
+    /**
+     * Send our position into the mesh
+     */
+    void sendOurPosition(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 Position &p);
+};
+
+extern PositionPlugin positionPlugin;