Merge pull request #630 from geeksville/dev

Dev

docs/software/plugin-api.md

@@ -46,6 +46,7 @@ The easiest way to get started is:
 
 * [Build and install](build-instructions.md) the standard codebase from github.
 * Copy [src/plugins/ReplyPlugin.*](/src/plugins/ReplyPlugin.cpp) into src/plugins/YourPlugin.*.  Then change the port number from *PortNum_REPLY_APP* to *PortNum_PRIVATE_APP*.
+* Edit plugins/Plugins.cpp:setupPlugins() to add a call to create an instance of your plugin (see comment at head of that function)
 * Rebuild with your new messaging goodness and install on the device
 * Use the [meshtastic commandline tool](https://github.com/meshtastic/Meshtastic-python) to send a packet to your board, for example "*meshtastic --dest 1234 --sendping*", where *1234* is another mesh node to send the ping to.
 
@@ -54,6 +55,10 @@ The easiest way to get started is:
 It is very common that you would like your plugin to be invoked periodically.
 We use a crude/basic cooperative threading system to allow this on any of our supported platforms.  Simply inherit from OSThread and implement runOnce().  See the OSThread [documentation](/src/concurrency/OSThread.h) for more details.  For an example consumer of this API see RemoteHardwarePlugin::runOnce.
 
+## Sending messages
+
+If you would like to proactively send messages (rather than just responding to them), just call service.sendToMesh().  For an example of this see [NodeInfoPlugin::sendOurNodeInfo(...)](/src/plugins/NodeInfoPlugin.cpp).
+
 ## Picking a port number
 
 For any new 'apps' that run on the device or via sister apps on phones/PCs they should pick and use a unique 'portnum' for their application.

proto

@@ -1 +1 @@
-Subproject commit 75078afe43934f4ce15ef86ebc6950658a170145
+Subproject commit dfe7bc1217a00c23eecb9dfcf1d56fe95ebddc3b

src/concurrency/OSThread.cpp

@@ -28,6 +28,8 @@ void OSThread::setup()
 OSThread::OSThread(const char *_name, uint32_t period, ThreadController *_controller)
     : Thread(NULL, period), controller(_controller)
 {
+    assertIsSetup();
+
     ThreadName = _name;
 
     if (controller)
@@ -81,4 +83,36 @@ void OSThread::run()
     currentThread = NULL;
 }
 
+/**
+ * This flag is set **only** when setup() starts, to provide a way for us to check for sloppy static constructor calls.
+ * Call assertIsSetup() to force a crash if someone tries to create an instance too early.
+ *
+ * it is super important to never allocate those object statically.  instead, you should explicitly
+ *  new them at a point where you are guaranteed that other objects that this instance
+ * depends on have already been created.
+ *
+ * in particular, for OSThread that means "all instances must be declared via new() in setup() or later" -
+ * this makes it guaranteed that the global mainController is fully constructed first.
+ */
+bool hasBeenSetup;
+
+void assertIsSetup()
+{
+
+    /**
+     * Dear developer comrade - If this assert fails() that means you need to fix the following:
+     *
+     * This flag is set **only** when setup() starts, to provide a way for us to check for sloppy static constructor calls.
+     * Call assertIsSetup() to force a crash if someone tries to create an instance too early.
+     *
+     * it is super important to never allocate those object statically.  instead, you should explicitly
+     *  new them at a point where you are guaranteed that other objects that this instance
+     * depends on have already been created.
+     *
+     * in particular, for OSThread that means "all instances must be declared via new() in setup() or later" -
+     * this makes it guaranteed that the global mainController is fully constructed first.
+     */
+    assert(hasBeenSetup);
+}
+
 } // namespace concurrency

src/concurrency/OSThread.h

@@ -17,14 +17,14 @@ extern InterruptableDelay mainDelay;
 
 /**
  * @brief Base threading
- * 
+ *
  * This is a pseudo threading layer that is super easy to port, well suited to our slow network and very ram & power efficient.
  *
  * TODO FIXME @geeksville
  *
  * move more things into OSThreads
  * remove lock/lockguard
- * 
+ *
  * move typedQueue into concurrency
  * remove freertos from typedqueue
  */
@@ -42,7 +42,6 @@ class OSThread : public Thread
     static bool showWaiting;
 
   public:
-
     /// For debug printing only (might be null)
     static const OSThread *currentThread;
 
@@ -71,4 +70,19 @@ class OSThread : public Thread
     virtual void run();
 };
 
+/**
+ * This flag is set **only** when setup() starts, to provide a way for us to check for sloppy static constructor calls.
+ * Call assertIsSetup() to force a crash if someone tries to create an instance too early.
+ *
+ * it is super important to never allocate those object statically.  instead, you should explicitly
+ *  new them at a point where you are guaranteed that other objects that this instance
+ * depends on have already been created.
+ *
+ * in particular, for OSThread that means "all instances must be declared via new() in setup() or later" -
+ * this makes it guaranteed that the global mainController is fully constructed first.
+ */
+extern bool hasBeenSetup;
+
+void assertIsSetup();
+
 } // namespace concurrency

src/graphics/Screen.cpp

@@ -746,7 +746,7 @@ void Screen::setup()
     powerStatusObserver.observe(&powerStatus->onNewStatus);
     gpsStatusObserver.observe(&gpsStatus->onNewStatus);
     nodeStatusObserver.observe(&nodeStatus->onNewStatus);
-    textMessageObserver.observe(&textMessagePlugin);
+    textMessageObserver.observe(textMessagePlugin);
 }
 
 void Screen::forceDisplay()

src/main.cpp

@@ -22,6 +22,7 @@
 #include "meshwifi/meshhttp.h"
 #include "meshwifi/meshwifi.h"
 #include "sleep.h"
+#include "plugins/Plugins.h"
 #include "target_specific.h"
 #include <OneButton.h>
 #include <Wire.h>
@@ -275,6 +276,8 @@ RadioInterface *rIf = NULL;
 
 void setup()
 {
+    concurrency::hasBeenSetup = true;
+
 #ifdef SEGGER_STDOUT_CH
     SEGGER_RTT_ConfigUpBuffer(SEGGER_STDOUT_CH, NULL, NULL, 1024, SEGGER_RTT_MODE_NO_BLOCK_TRIM);
 #endif
@@ -390,15 +393,15 @@ void setup()
     readFromRTC(); // read the main CPU RTC at first (in case we can't get GPS time)
 
 #ifdef GENIEBLOCKS
-    //gps setup
-    pinMode (GPS_RESET_N, OUTPUT);
+    // gps setup
+    pinMode(GPS_RESET_N, OUTPUT);
     pinMode(GPS_EXTINT, OUTPUT);
     digitalWrite(GPS_RESET_N, HIGH);
     digitalWrite(GPS_EXTINT, LOW);
-    //battery setup
+    // battery setup
     // If we want to read battery level, we need to set BATTERY_EN_PIN pin to low.
     // ToDo: For low power consumption after read battery level, set that pin to high.
-    pinMode (BATTERY_EN_PIN, OUTPUT);
+    pinMode(BATTERY_EN_PIN, OUTPUT);
     digitalWrite(BATTERY_EN_PIN, LOW);
 #endif
 
@@ -439,14 +442,17 @@ void setup()
 
     service.init();
 
+    // Now that the mesh service is created, create any plugins
+    setupPlugins();
+
     // Do this after service.init (because that clears error_code)
 #ifdef AXP192_SLAVE_ADDRESS
-    if(!axp192_found)
+    if (!axp192_found)
         recordCriticalError(CriticalErrorCode_NoAXP192); // Record a hardware fault for missing hardware
-#endif    
+#endif
 
-    // Don't call screen setup until after nodedb is setup (because we need
-    // the current region name)
+        // Don't call screen setup until after nodedb is setup (because we need
+        // the current region name)
 #if defined(ST7735_CS) || defined(HAS_EINK)
     screen->setup();
 #else

src/mesh/MeshService.cpp

@@ -60,7 +60,8 @@ static int32_t sendOwnerCb()
     currentGeneration = radioGeneration;
 
     DEBUG_MSG("Sending our nodeinfo to mesh (wantReplies=%d)\n", requestReplies);
-    nodeInfoPlugin.sendOurNodeInfo(NODENUM_BROADCAST, requestReplies); // Send our info (don't request replies)
+    assert(nodeInfoPlugin);
+    nodeInfoPlugin->sendOurNodeInfo(NODENUM_BROADCAST, requestReplies); // Send our info (don't request replies)
 
     return getPref_send_owner_interval() * getPref_position_broadcast_secs() * 1000;
 }
@@ -134,7 +135,8 @@ bool MeshService::reloadConfig()
 /// The owner User record just got updated, update our node DB and broadcast the info into the mesh
 void MeshService::reloadOwner()
 {
-    nodeInfoPlugin.sendOurNodeInfo();
+    assert(nodeInfoPlugin);
+    nodeInfoPlugin->sendOurNodeInfo();
     nodeDB.saveToDisk();
 }
 
@@ -193,10 +195,11 @@ void MeshService::sendNetworkPing(NodeNum dest, bool wantReplies)
     assert(node);
 
     DEBUG_MSG("Sending network ping to 0x%x, with position=%d, wantReplies=%d\n", dest, node->has_position, wantReplies);
+    assert(positionPlugin && nodeInfoPlugin);
     if (node->has_position)
-        positionPlugin.sendOurPosition(dest, wantReplies);
+        positionPlugin->sendOurPosition(dest, wantReplies);
     else
-        nodeInfoPlugin.sendOurNodeInfo(dest, wantReplies);
+        nodeInfoPlugin->sendOurNodeInfo(dest, wantReplies);
 }
 
 
@@ -264,7 +267,8 @@ int MeshService::onGPSChanged(const meshtastic::GPSStatus *unused)
         currentGeneration = radioGeneration;
 
         DEBUG_MSG("Sending position to mesh (wantReplies=%d)\n", requestReplies);
-        positionPlugin.sendOurPosition(NODENUM_BROADCAST, requestReplies);
+        assert(positionPlugin);
+        positionPlugin->sendOurPosition(NODENUM_BROADCAST, requestReplies);
     }
 
     return 0;

src/mesh/NodeDB.cpp

@@ -5,6 +5,7 @@
 #include "FS.h"
 
 #include "CryptoEngine.h"
+#include "FSCommon.h"
 #include "GPS.h"
 #include "MeshRadio.h"
 #include "NodeDB.h"
@@ -16,7 +17,6 @@
 #include "error.h"
 #include "mesh-pb-constants.h"
 #include "meshwifi/meshwifi.h"
-#include "FSCommon.h"
 #include <pb_decode.h>
 #include <pb_encode.h>
 
@@ -28,7 +28,7 @@ MyNodeInfo &myNodeInfo = devicestate.my_node;
 RadioConfig &radioConfig = devicestate.radio;
 ChannelSettings &channelSettings = radioConfig.channel_settings;
 
-/** The current change # for radio settings.  Starts at 0 on boot and any time the radio settings 
+/** The current change # for radio settings.  Starts at 0 on boot and any time the radio settings
  * might have changed is incremented.  Allows others to detect they might now be on a new channel.
  */
 uint32_t radioGeneration;
@@ -41,8 +41,6 @@ DeviceState versions used to be defined in the .proto file but really only this
 #define DEVICESTATE_CUR_VER 11
 #define DEVICESTATE_MIN_VER DEVICESTATE_CUR_VER
 
-
-
 // FIXME - move this somewhere else
 extern void getMacAddr(uint8_t *dmac);
 
@@ -90,7 +88,7 @@ const char *getChannelName()
     static char buf[32];
 
     char suffix;
-    if(channelSettings.psk.size != 1) {
+    if (channelSettings.psk.size != 1) {
         // We have a standard PSK, so generate a letter based hash.
         uint8_t code = 0;
         for (int i = 0; i < activePSKSize; i++)
@@ -140,32 +138,40 @@ bool NodeDB::resetRadioConfig()
     }
 
     // Convert the old string "Default" to our new short representation
-    if(strcmp(channelSettings.name, "Default") == 0)
+    if (strcmp(channelSettings.name, "Default") == 0)
         *channelSettings.name = '\0';
 
     // Convert the short "" representation for Default into a usable string
     channelName = channelSettings.name;
-    if(!*channelName) { // emptystring
+    if (!*channelName) { // emptystring
         // Per mesh.proto spec, if bandwidth is specified we must ignore modemConfig enum, we assume that in that case
         // the app fucked up and forgot to set channelSettings.name
-        channelName = "Unset";
-        if(channelSettings.bandwidth == 0) switch(channelSettings.modem_config) {
+
+        if (channelSettings.bandwidth != 0)
+            channelName = "Unset";
+        else
+            switch (channelSettings.modem_config) {
             case ChannelSettings_ModemConfig_Bw125Cr45Sf128:
-                channelName = "Medium"; break;
+                channelName = "Medium";
+                break;
             case ChannelSettings_ModemConfig_Bw500Cr45Sf128:
-                channelName = "ShortFast"; break;
+                channelName = "ShortFast";
+                break;
             case ChannelSettings_ModemConfig_Bw31_25Cr48Sf512:
-                channelName = "LongAlt"; break;
+                channelName = "LongAlt";
+                break;
             case ChannelSettings_ModemConfig_Bw125Cr48Sf4096:
-                channelName = "LongSlow"; break;
+                channelName = "LongSlow";
+                break;
             default:
-                channelName = "Invalid"; break;
-        }
+                channelName = "Invalid";
+                break;
+            }
     }
 
     // Convert any old usage of the defaultpsk into our new short representation.
-    if(channelSettings.psk.size == sizeof(defaultpsk) && 
-            memcmp(channelSettings.psk.bytes, defaultpsk, sizeof(defaultpsk)) == 0) {
+    if (channelSettings.psk.size == sizeof(defaultpsk) &&
+        memcmp(channelSettings.psk.bytes, defaultpsk, sizeof(defaultpsk)) == 0) {
         *channelSettings.psk.bytes = 1;
         channelSettings.psk.size = 1;
     }
@@ -173,10 +179,10 @@ bool NodeDB::resetRadioConfig()
     // Convert the short single byte variants of psk into variant that can be used more generally
     memcpy(activePSK, channelSettings.psk.bytes, channelSettings.psk.size);
     activePSKSize = channelSettings.psk.size;
-    if(activePSKSize == 1) {
+    if (activePSKSize == 1) {
         uint8_t pskIndex = activePSK[0];
         DEBUG_MSG("Expanding short PSK #%d\n", pskIndex);
-        if(pskIndex == 0)
+        if (pskIndex == 0)
             activePSKSize = 0; // Turn off encryption
         else {
             memcpy(activePSK, defaultpsk, sizeof(defaultpsk));
@@ -271,12 +277,13 @@ void NodeDB::init()
     myNodeInfo.node_num_bits = sizeof(NodeNum) * 8;
     myNodeInfo.packet_id_bits = sizeof(PacketId) * 8;
 
-    myNodeInfo.error_code = CriticalErrorCode_None; // For the error code, only show values from this boot (discard value from flash)
+    myNodeInfo.error_code =
+        CriticalErrorCode_None; // For the error code, only show values from this boot (discard value from flash)
     myNodeInfo.error_address = 0;
 
     // likewise - we always want the app requirements to come from the running appload
     myNodeInfo.min_app_version = 20120; // format is Mmmss (where M is 1+the numeric major number. i.e. 20120 means 1.1.20
- 
+
     // Note! We do this after loading saved settings, so that if somehow an invalid nodenum was stored in preferences we won't
     // keep using that nodenum forever. Crummy guess at our nodenum (but we will check against the nodedb to avoid conflicts)
     pickNewNodeNum();

src/plugins/NodeInfoPlugin.cpp

@@ -6,7 +6,7 @@
 #include "configuration.h"
 #include "main.h"
 
-NodeInfoPlugin nodeInfoPlugin;
+NodeInfoPlugin *nodeInfoPlugin;
 
 bool NodeInfoPlugin::handleReceivedProtobuf(const MeshPacket &mp, const User &p)
 {

src/plugins/NodeInfoPlugin.h

@@ -29,4 +29,4 @@ class NodeInfoPlugin : public ProtobufPlugin<User>
     virtual MeshPacket *allocReply();
 };
 
-extern NodeInfoPlugin nodeInfoPlugin;
+extern NodeInfoPlugin *nodeInfoPlugin;

src/plugins/Plugins.cpp

@@ -0,0 +1,20 @@
+#include "plugins/NodeInfoPlugin.h"
+#include "plugins/PositionPlugin.h"
+#include "plugins/ReplyPlugin.h"
+#include "plugins/RemoteHardwarePlugin.h"
+#include "plugins/TextMessagePlugin.h"
+
+/**
+ * Create plugin instances here.  If you are adding a new plugin, you must 'new' it here (or somewhere else)
+ */
+void setupPlugins() {
+    nodeInfoPlugin = new NodeInfoPlugin();
+    positionPlugin = new PositionPlugin();
+    textMessagePlugin = new TextMessagePlugin();
+
+    // Note: if the rest of meshtastic doesn't need to explicitly use your plugin, you do not need to assign the instance
+    // to a global variable.
+
+    new RemoteHardwarePlugin();
+    new ReplyPlugin();
+}

src/plugins/Plugins.h

@@ -0,0 +1,6 @@
+#pragma once
+
+/**
+ * Create plugin instances here.  If you are adding a new plugin, you must 'new' it here (or somewhere else)
+ */
+void setupPlugins();

src/plugins/PositionPlugin.cpp

@@ -5,7 +5,7 @@
 #include "Router.h"
 #include "configuration.h"
 
-PositionPlugin positionPlugin;
+PositionPlugin *positionPlugin;
 
 bool PositionPlugin::handleReceivedProtobuf(const MeshPacket &mp, const Position &p)
 {

src/plugins/PositionPlugin.h

@@ -30,4 +30,4 @@ class PositionPlugin : public ProtobufPlugin<Position>
     virtual MeshPacket *allocReply();
 };
 
-extern PositionPlugin positionPlugin;
+extern PositionPlugin *positionPlugin;

src/plugins/RemoteHardwarePlugin.cpp

@@ -6,8 +6,6 @@
 #include "configuration.h"
 #include "main.h"
 
-RemoteHardwarePlugin remoteHardwarePlugin;
-
 #define NUM_GPIOS 64
 
 // Because (FIXME) we currently don't tell API clients status on sent messages

src/plugins/ReplyPlugin.cpp

@@ -5,9 +5,6 @@
 
 #include <assert.h>
 
-// Create an a static instance of our plugin - this registers with the plugin system
-ReplyPlugin replyPlugin;
-
 MeshPacket *ReplyPlugin::allocReply()
 {
     assert(currentRequest); // should always be !NULL

src/plugins/TextMessagePlugin.cpp

@@ -3,7 +3,7 @@
 #include "NodeDB.h"
 #include "PowerFSM.h"
 
-TextMessagePlugin textMessagePlugin;
+TextMessagePlugin *textMessagePlugin;
 
 bool TextMessagePlugin::handleReceived(const MeshPacket &mp)
 {

src/plugins/TextMessagePlugin.h

@@ -22,4 +22,4 @@ class TextMessagePlugin : public SinglePortPlugin, public Observable<const MeshP
     virtual bool handleReceived(const MeshPacket &mp);
 };
 
-extern TextMessagePlugin textMessagePlugin;
+extern TextMessagePlugin *textMessagePlugin;