Merge pull request #109 from geeksville/master

my saturday recreational work queue

src/GPS.cpp

@@ -27,6 +27,8 @@ GPS::GPS() : PeriodicTask() {}
 
 void GPS::setup()
 {
+    PeriodicTask::setup();
+    
     readFromRTC(); // read the main CPU RTC at first
 
 #ifdef GPS_RX_PIN
@@ -114,12 +116,6 @@ void GPS::perhapsSetRTC(const struct timeval *tv)
 
 #include <time.h>
 
-// for the time being we need to rapidly read from the serial port to prevent overruns
-void GPS::loop()
-{
-    PeriodicTask::loop();
-}
-
 uint32_t GPS::getTime()
 {
     return ((millis() - timeStartMsec) / 1000) + zeroOffsetSecs;

src/GPS.h

@@ -29,7 +29,6 @@ class GPS : public PeriodicTask, public Observable<void *>
 
     void setup();
 
-    virtual void loop();
 
     virtual void doTask();
 

src/MeshService.cpp

@@ -48,6 +48,15 @@ MeshService service;
 
 #define NUM_PACKET_ID 255 // 0 is consider invalid
 
+static uint32_t sendOwnerCb()
+{
+    service.sendOurOwner();
+
+    return radioConfig.preferences.send_owner_interval * radioConfig.preferences.position_broadcast_secs * 1000;
+}
+
+static Periodic sendOwnerPeriod(sendOwnerCb);
+
 /// Generate a unique packet id
 // FIXME, move this someplace better
 PacketId generatePacketId()
@@ -65,6 +74,7 @@ MeshService::MeshService() : toPhoneQueue(MAX_RX_TOPHONE)
 
 void MeshService::init()
 {
+    sendOwnerPeriod.setup();
     nodeDB.init();
 
     gpsObserver.observe(&gps);
@@ -184,15 +194,6 @@ int MeshService::handleFromRadio(const MeshPacket *mp)
     return 0;
 }
 
-uint32_t sendOwnerCb()
-{
-    service.sendOurOwner();
-
-    return radioConfig.preferences.send_owner_interval * radioConfig.preferences.position_broadcast_secs * 1000;
-}
-
-Periodic sendOwnerPeriod(sendOwnerCb);
-
 /// Do idle processing (mostly processing messages which have been queued from the radio)
 void MeshService::loop()
 {
@@ -200,9 +201,6 @@ void MeshService::loop()
         fromNumChanged.notifyObservers(fromNum);
         oldFromNum = fromNum;
     }
-
-    // occasionally send our owner info
-    sendOwnerPeriod.loop();
 }
 
 /// The radioConfig object just changed, call this to force the hw to change to the new settings
@@ -216,7 +214,8 @@ void MeshService::reloadConfig()
 
 /**
  *  Given a ToRadio buffer parse it and properly handle it (setup radio, owner or send packet into the mesh)
- * Called by PhoneAPI.handleToRadio.  Note: p is a scratch buffer, this function is allowed to write to it but it can not keep a reference
+ * Called by PhoneAPI.handleToRadio.  Note: p is a scratch buffer, this function is allowed to write to it but it can not keep a
+ * reference
  */
 void MeshService::handleToRadio(MeshPacket &p)
 {

src/PeriodicTask.cpp

@@ -1,21 +1,39 @@
 #include "PeriodicTask.h"
 #include "Periodic.h"
+PeriodicScheduler periodicScheduler;
 
 PeriodicTask::PeriodicTask(uint32_t initialPeriod) : period(initialPeriod) {}
 
-/// call this from loop
-void PeriodicTask::loop()
+void PeriodicTask::setup()
 {
-    {
-        meshtastic::LockGuard lg(&lock);
-        uint32_t now = millis();
-        if (!period || (now - lastMsec) < period) {
-            return;
+    periodicScheduler.schedule(this);
+}
+
+/// call this from loop
+void PeriodicScheduler::loop()
+{
+    meshtastic::LockGuard lg(&lock);
+
+    uint32_t now = millis();
+    for (auto t : tasks) {
+        if (t->period && (now - t->lastMsec) >= t->period) {
+
+            t->doTask();
+            t->lastMsec = now;
         }
-        lastMsec = now;
     }
-    // Release the lock in case the task wants to change the period.
-    doTask();
+}
+
+void PeriodicScheduler::schedule(PeriodicTask *t)
+{
+    meshtastic::LockGuard lg(&lock);
+    tasks.insert(t);
+}
+
+void PeriodicScheduler::unschedule(PeriodicTask *t)
+{
+    meshtastic::LockGuard lg(&lock);
+    tasks.erase(t);
 }
 
 void Periodic::doTask()

src/PeriodicTask.h

@@ -1,8 +1,40 @@
 #pragma once
 
-#include <cstdint>
-
 #include "lock.h"
+#include <cstdint>
+#include <unordered_set>
+
+class PeriodicTask;
+
+/**
+ * Runs all PeriodicTasks in the system.
+ *
+ * Currently called from main loop() but eventually should be its own thread blocked on a freertos timer.
+ */
+class PeriodicScheduler
+{
+    friend class PeriodicTask;
+
+    /**
+     * This really should be some form of heap, and when the period gets changed on a task it should get
+     * rescheduled in that heap.  Currently it is just a dumb array and everytime we run loop() we check
+     * _every_ tasks.  If it was a heap we'd only have to check the first task.
+     */
+    std::unordered_set<PeriodicTask *> tasks;
+
+    // Protects the above variables.
+    meshtastic::Lock lock;
+
+  public:
+    /// Run any next tasks which are due for execution
+    void loop();
+
+  private:
+    void schedule(PeriodicTask *t);
+    void unschedule(PeriodicTask *t);
+};
+
+extern PeriodicScheduler periodicScheduler;
 
 /**
  * A base class for tasks that want their doTask() method invoked periodically
@@ -13,26 +45,33 @@
  */
 class PeriodicTask
 {
+    friend class PeriodicScheduler;
+
     uint32_t lastMsec = 0;
     uint32_t period = 1; // call soon after creation
 
-    // Protects the above variables.
-    meshtastic::Lock lock;
-
   public:
-    virtual ~PeriodicTask() {}
+    virtual ~PeriodicTask() { periodicScheduler.unschedule(this); }
 
+    /**
+     * Constructor (will schedule with the global PeriodicScheduler)
+     */
     PeriodicTask(uint32_t initialPeriod = 1);
 
-    /// call this from loop
-    virtual void loop();
+    /** MUST be be called once at startup (but after threading is running - i.e. not from a constructor)
+     */
+    void setup();
 
-    /// Set a new period in msecs (can be called from doTask or elsewhere and the scheduler will cope)
-    void setPeriod(uint32_t p)
-    {
-        meshtastic::LockGuard lg(&lock);
-        period = p;
-    }
+    /**
+     * Set a new period in msecs (can be called from doTask or elsewhere and the scheduler will cope)
+     * While zero this task is disabled and will not run
+     */
+    void setPeriod(uint32_t p) { period = p; }
+
+    /**
+     * Syntatic sugar for suspending tasks
+     */
+    void disable() { setPeriod(0); }
 
   protected:
     virtual void doTask() = 0;

src/main.cpp

@@ -27,6 +27,7 @@
 #include "NodeDB.h"
 #include "Periodic.h"
 #include "PowerFSM.h"
+#include "Router.h"
 #include "configuration.h"
 #include "error.h"
 #include "power.h"
@@ -225,7 +226,18 @@ const char *getDeviceName()
 
 static MeshRadio *radio = NULL;
 
-#include "Router.h"
+static uint32_t ledBlinker()
+{
+    static bool ledOn;
+    ledOn ^= 1;
+
+    setLed(ledOn);
+
+    // have a very sparse duty cycle of LED being on, unless charging, then blink 0.5Hz square wave rate to indicate that
+    return powerStatus.charging ? 1000 : (ledOn ? 2 : 1000);
+}
+
+Periodic ledPeriodic(ledBlinker);
 
 void setup()
 {
@@ -261,6 +273,8 @@ void setup()
     digitalWrite(LED_PIN, 1 ^ LED_INVERTED); // turn on for now
 #endif
 
+    ledPeriodic.setup();
+
     // Hello
     DEBUG_MSG("Meshtastic swver=%s, hwver=%s\n", xstr(APP_VERSION), xstr(HW_VERSION));
 
@@ -283,6 +297,8 @@ void setup()
 
     service.init();
 
+    realRouter.setup(); // required for our periodic task (kinda skanky FIXME)
+
 #ifndef NO_ESP32
     // MUST BE AFTER service.init, so we have our radio config settings (from nodedb init)
     radio = new MeshRadio();
@@ -299,19 +315,6 @@ void setup()
     setCPUFast(false); // 80MHz is fine for our slow peripherals
 }
 
-uint32_t ledBlinker()
-{
-    static bool ledOn;
-    ledOn ^= 1;
-
-    setLed(ledOn);
-
-    // have a very sparse duty cycle of LED being on, unless charging, then blink 0.5Hz square wave rate to indicate that
-    return powerStatus.charging ? 1000 : (ledOn ? 2 : 1000);
-}
-
-Periodic ledPeriodic(ledBlinker);
-
 #if 0
 // Turn off for now
 
@@ -330,18 +333,18 @@ uint32_t axpDebugRead()
 }
 
 Periodic axpDebugOutput(axpDebugRead);
+axpDebugOutput.setup();
 #endif
 
 void loop()
 {
     uint32_t msecstosleep = 1000 * 30; // How long can we sleep before we again need to service the main loop?
 
-    gps.loop();
     router.loop();
     powerFSM.run_machine();
     service.loop();
 
-    ledPeriodic.loop();
+    periodicScheduler.loop();
     // axpDebugOutput.loop();
 
 #ifndef NO_ESP32
@@ -419,7 +422,6 @@ void loop()
     screen.debug()->setPowerStatus(powerStatus);
     // TODO(#4): use something based on hdop to show GPS "signal" strength.
     screen.debug()->setGPSStatus(gps.hasLock() ? "ok" : ":(");
-    screen.loop();
 
     // No GPS lock yet, let the OS put the main CPU in low power mode for 100ms (or until another interrupt comes in)
     // i.e. don't just keep spinning in loop as fast as we can.

src/rf95/FloodingRouter.cpp

@@ -5,9 +5,10 @@
 /// We clear our old flood record five minute after we see the last of it
 #define FLOOD_EXPIRE_TIME (5 * 60 * 1000L)
 
-FloodingRouter::FloodingRouter()
+FloodingRouter::FloodingRouter() : toResend(MAX_NUM_NODES)
 {
     recentBroadcasts.reserve(MAX_NUM_NODES); // Prealloc the worst case # of records - to prevent heap fragmentation
+                                             // setup our periodic task
 }
 
 /**
@@ -23,6 +24,12 @@ ErrorCode FloodingRouter::send(MeshPacket *p)
     return Router::send(p);
 }
 
+// Return a delay in msec before sending the next packet
+uint32_t getRandomDelay()
+{
+    return random(200, 10 * 1000L); // between 200ms and 10s
+}
+
 /**
  * Called from loop()
  * Handle any packet that is received by an interface on this node.
@@ -38,12 +45,14 @@ void FloodingRouter::handleReceived(MeshPacket *p)
     } else {
         if (p->to == NODENUM_BROADCAST) {
             if (p->id != 0) {
-                DEBUG_MSG("Rebroadcasting received floodmsg to neighbors fr=0x%x,to=0x%x,id=%d\n", p->from, p->to, p->id);
-                // FIXME, wait a random delay
+                uint32_t delay = getRandomDelay();
+
+                DEBUG_MSG("Rebroadcasting received floodmsg to neighbors in %u msec, fr=0x%x,to=0x%x,id=%d\n", delay, p->from,
+                          p->to, p->id);
 
                 MeshPacket *tosend = packetPool.allocCopy(*p);
-                // Note: we are careful to resend using the original senders node id
-                Router::send(tosend); // We are careful not to call our hooked version of send()
+                toResend.enqueue(tosend);
+                setPeriod(delay); // This will work even if we were already waiting a random delay
             } else {
                 DEBUG_MSG("Ignoring a simple (0 hop) broadcast\n");
             }
@@ -54,6 +63,24 @@ void FloodingRouter::handleReceived(MeshPacket *p)
     }
 }
 
+void FloodingRouter::doTask()
+{
+    MeshPacket *p = toResend.dequeuePtr(0);
+
+    if (p) {
+        DEBUG_MSG("Sending delayed message!\n");
+        // Note: we are careful to resend using the original senders node id
+        // We are careful not to call our hooked version of send() - because we don't want to check this again
+        Router::send(p);
+    }
+
+    if (toResend.isEmpty())
+        disable(); // no more work right now
+    else {
+        setPeriod(getRandomDelay());
+    }
+}
+
 /**
  * Update recentBroadcasts and return true if we have already seen this packet
  */

src/rf95/FloodingRouter.h

@@ -1,5 +1,6 @@
 #pragma once
 
+#include "PeriodicTask.h"
 #include "Router.h"
 #include <vector>
 
@@ -35,11 +36,19 @@ struct BroadcastRecord {
   Any entries in recentBroadcasts that are older than X seconds (longer than the
   max time a flood can take) will be discarded.
  */
-class FloodingRouter : public Router
+class FloodingRouter : public Router, public PeriodicTask
 {
   private:
+    /** FIXME: really should be a std::unordered_set with the key being sender,id.
+     * This would make checking packets in wasSeenRecently faster.
+     */
     std::vector<BroadcastRecord> recentBroadcasts;
 
+    /**
+     * Packets we've received that we need to resend after a short delay
+     */
+    PointerQueue<MeshPacket> toResend;
+
   public:
     /**
      * Constructor
@@ -64,6 +73,8 @@ class FloodingRouter : public Router
      */
     virtual void handleReceived(MeshPacket *p);
 
+    virtual void doTask();
+
   private:
     /**
      * Update recentBroadcasts and return true if we have already seen this packet

src/screen.cpp

@@ -384,7 +384,6 @@ void _screen_header()
     if (!disp)
         return;
 
-
     // Message count
     //snprintf(buffer, sizeof(buffer), "#%03d", ttn_get_count() % 1000);
     //display->setTextAlignment(TEXT_ALIGN_LEFT);
@@ -423,6 +422,8 @@ void Screen::handleSetOn(bool on)
 
 void Screen::setup()
 {
+    PeriodicTask::setup();
+
     // We don't set useDisplay until setup() is called, because some boards have a declaration of this object but the device
     // is never found when probing i2c and therefore we don't call setup and never want to do (invalid) accesses to this device.
     useDisplay = true;

src/sleep.cpp

@@ -3,7 +3,6 @@
 #include "MeshRadio.h"
 #include "MeshService.h"
 #include "NodeDB.h"
-#include "Periodic.h"
 #include "configuration.h"
 #include "error.h"