From 64f6c0f5c035cac5ae7379481b58ba13300221b4 Mon Sep 17 00:00:00 2001 From: geeksville Date: Sat, 25 Apr 2020 10:59:40 -0700 Subject: [PATCH 1/3] clean up PeriodicTask so I can eventually use it with a scheduler --- src/GPS.cpp | 8 ++---- src/GPS.h | 1 - src/MeshService.cpp | 25 ++++++++--------- src/PeriodicTask.cpp | 38 ++++++++++++++++++------- src/PeriodicTask.h | 67 +++++++++++++++++++++++++++++++++++--------- src/main.cpp | 34 +++++++++++----------- src/screen.cpp | 3 +- src/sleep.cpp | 1 - 8 files changed, 114 insertions(+), 63 deletions(-) diff --git a/src/GPS.cpp b/src/GPS.cpp index 60dedf429..471130b34 100644 --- a/src/GPS.cpp +++ b/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 -// 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; diff --git a/src/GPS.h b/src/GPS.h index caf3fc249..912356c42 100644 --- a/src/GPS.h +++ b/src/GPS.h @@ -29,7 +29,6 @@ class GPS : public PeriodicTask, public Observable void setup(); - virtual void loop(); virtual void doTask(); diff --git a/src/MeshService.cpp b/src/MeshService.cpp index d82f46b55..82d6921e1 100644 --- a/src/MeshService.cpp +++ b/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) { diff --git a/src/PeriodicTask.cpp b/src/PeriodicTask.cpp index 99115faf9..5a5d3621c 100644 --- a/src/PeriodicTask.cpp +++ b/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() diff --git a/src/PeriodicTask.h b/src/PeriodicTask.h index f4a35a2c5..9d2a06b6d 100644 --- a/src/PeriodicTask.h +++ b/src/PeriodicTask.h @@ -1,8 +1,40 @@ #pragma once -#include - #include "lock.h" +#include +#include + +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 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; diff --git a/src/main.cpp b/src/main.cpp index b39e61fc9..da3ee40a0 100644 --- a/src/main.cpp +++ b/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)); @@ -299,19 +313,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 +331,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 +420,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. diff --git a/src/screen.cpp b/src/screen.cpp index 583b67b8b..3acf2c52e 100644 --- a/src/screen.cpp +++ b/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; diff --git a/src/sleep.cpp b/src/sleep.cpp index 69b80e91b..39710ad4c 100644 --- a/src/sleep.cpp +++ b/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" From 3f3a1a11dfce688d40c1e060f05eabcd92c0a599 Mon Sep 17 00:00:00 2001 From: geeksville Date: Sat, 25 Apr 2020 11:43:28 -0700 Subject: [PATCH 2/3] when flooding, randomly delay sent packets to decrease chances of... stomping on other senders that we can't even hear. --- src/main.cpp | 2 ++ src/rf95/FloodingRouter.cpp | 37 ++++++++++++++++++++++++++++++++----- src/rf95/FloodingRouter.h | 13 ++++++++++++- 3 files changed, 46 insertions(+), 6 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index da3ee40a0..1cb40f966 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -297,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(); diff --git a/src/rf95/FloodingRouter.cpp b/src/rf95/FloodingRouter.cpp index 817870131..9f496cb80 100644 --- a/src/rf95/FloodingRouter.cpp +++ b/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); + + DEBUG_MSG("Sending delayed message!\n"); + if (p) { + // 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 */ diff --git a/src/rf95/FloodingRouter.h b/src/rf95/FloodingRouter.h index 7ce7541a7..4ca759ecc 100644 --- a/src/rf95/FloodingRouter.h +++ b/src/rf95/FloodingRouter.h @@ -1,5 +1,6 @@ #pragma once +#include "PeriodicTask.h" #include "Router.h" #include @@ -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 recentBroadcasts; + /** + * Packets we've received that we need to resend after a short delay + */ + PointerQueue 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 From 8f1c1a9049b6b6f5570ad1f539ca82d41afa3240 Mon Sep 17 00:00:00 2001 From: geeksville Date: Sat, 25 Apr 2020 11:46:46 -0700 Subject: [PATCH 3/3] move debug msg --- src/rf95/FloodingRouter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rf95/FloodingRouter.cpp b/src/rf95/FloodingRouter.cpp index 9f496cb80..3965dc049 100644 --- a/src/rf95/FloodingRouter.cpp +++ b/src/rf95/FloodingRouter.cpp @@ -67,8 +67,8 @@ void FloodingRouter::doTask() { MeshPacket *p = toResend.dequeuePtr(0); - DEBUG_MSG("Sending delayed message!\n"); 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);