From 3636b87db02549a1c01cdc8cc4ba89f843adf3eb Mon Sep 17 00:00:00 2001 From: Kevin Hester Date: Fri, 8 Jan 2021 11:52:43 +0800 Subject: [PATCH 1/4] formatting --- src/mesh/NodeDB.cpp | 49 ++++++++++++++++++++++++++------------------- 1 file changed, 28 insertions(+), 21 deletions(-) diff --git a/src/mesh/NodeDB.cpp b/src/mesh/NodeDB.cpp index 16ea5c16..a87a5e49 100644 --- a/src/mesh/NodeDB.cpp +++ b/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 #include @@ -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(); From 7aacfd66ef7e924dbc83f00be5ba952be4207f98 Mon Sep 17 00:00:00 2001 From: Kevin Hester Date: Fri, 8 Jan 2021 13:15:49 +0800 Subject: [PATCH 2/4] add assertIsSetup() and use it from OSThread constructor fixes nasty bug @mc-hamster discovered with plugin order of operations --- docs/software/plugin-api.md | 1 + src/concurrency/OSThread.cpp | 34 ++++++++++++++++++++++++++++ src/concurrency/OSThread.h | 20 +++++++++++++--- src/graphics/Screen.cpp | 2 +- src/main.cpp | 22 +++++++++++------- src/mesh/MeshService.cpp | 14 ++++++++---- src/plugins/NodeInfoPlugin.cpp | 2 +- src/plugins/NodeInfoPlugin.h | 2 +- src/plugins/Plugins.cpp | 20 ++++++++++++++++ src/plugins/Plugins.h | 6 +++++ src/plugins/PositionPlugin.cpp | 2 +- src/plugins/PositionPlugin.h | 2 +- src/plugins/RemoteHardwarePlugin.cpp | 2 -- src/plugins/ReplyPlugin.cpp | 3 --- src/plugins/TextMessagePlugin.cpp | 2 +- src/plugins/TextMessagePlugin.h | 2 +- 16 files changed, 108 insertions(+), 28 deletions(-) create mode 100644 src/plugins/Plugins.cpp create mode 100644 src/plugins/Plugins.h diff --git a/docs/software/plugin-api.md b/docs/software/plugin-api.md index e0699a6b..360acb51 100644 --- a/docs/software/plugin-api.md +++ b/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 REPLY_APP to 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 "meshtastic --dest 1234 --ping" diff --git a/src/concurrency/OSThread.cpp b/src/concurrency/OSThread.cpp index 58a0d59c..5e20debf 100644 --- a/src/concurrency/OSThread.cpp +++ b/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 diff --git a/src/concurrency/OSThread.h b/src/concurrency/OSThread.h index 3be149d5..7a86498b 100644 --- a/src/concurrency/OSThread.h +++ b/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 diff --git a/src/graphics/Screen.cpp b/src/graphics/Screen.cpp index 92e38337..34ab53ac 100644 --- a/src/graphics/Screen.cpp +++ b/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() diff --git a/src/main.cpp b/src/main.cpp index 029f753e..dffbfc9b 100644 --- a/src/main.cpp +++ b/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 #include @@ -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 diff --git a/src/mesh/MeshService.cpp b/src/mesh/MeshService.cpp index 075c0007..073c4814 100644 --- a/src/mesh/MeshService.cpp +++ b/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; diff --git a/src/plugins/NodeInfoPlugin.cpp b/src/plugins/NodeInfoPlugin.cpp index 876f5ea0..54aea4cc 100644 --- a/src/plugins/NodeInfoPlugin.cpp +++ b/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) { diff --git a/src/plugins/NodeInfoPlugin.h b/src/plugins/NodeInfoPlugin.h index 968d5f8b..aabeb005 100644 --- a/src/plugins/NodeInfoPlugin.h +++ b/src/plugins/NodeInfoPlugin.h @@ -29,4 +29,4 @@ class NodeInfoPlugin : public ProtobufPlugin virtual MeshPacket *allocReply(); }; -extern NodeInfoPlugin nodeInfoPlugin; \ No newline at end of file +extern NodeInfoPlugin *nodeInfoPlugin; \ No newline at end of file diff --git a/src/plugins/Plugins.cpp b/src/plugins/Plugins.cpp new file mode 100644 index 00000000..7a43c45f --- /dev/null +++ b/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(); +} \ No newline at end of file diff --git a/src/plugins/Plugins.h b/src/plugins/Plugins.h new file mode 100644 index 00000000..7d2a20cf --- /dev/null +++ b/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(); \ No newline at end of file diff --git a/src/plugins/PositionPlugin.cpp b/src/plugins/PositionPlugin.cpp index fe4f5773..f9f86bf6 100644 --- a/src/plugins/PositionPlugin.cpp +++ b/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) { diff --git a/src/plugins/PositionPlugin.h b/src/plugins/PositionPlugin.h index 9153a228..6a1eb05f 100644 --- a/src/plugins/PositionPlugin.h +++ b/src/plugins/PositionPlugin.h @@ -30,4 +30,4 @@ class PositionPlugin : public ProtobufPlugin virtual MeshPacket *allocReply(); }; -extern PositionPlugin positionPlugin; \ No newline at end of file +extern PositionPlugin *positionPlugin; \ No newline at end of file diff --git a/src/plugins/RemoteHardwarePlugin.cpp b/src/plugins/RemoteHardwarePlugin.cpp index e63e05cf..26b78c80 100644 --- a/src/plugins/RemoteHardwarePlugin.cpp +++ b/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 diff --git a/src/plugins/ReplyPlugin.cpp b/src/plugins/ReplyPlugin.cpp index 788edc55..a8c60743 100644 --- a/src/plugins/ReplyPlugin.cpp +++ b/src/plugins/ReplyPlugin.cpp @@ -5,9 +5,6 @@ #include -// 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 diff --git a/src/plugins/TextMessagePlugin.cpp b/src/plugins/TextMessagePlugin.cpp index 1e742454..4ad08070 100644 --- a/src/plugins/TextMessagePlugin.cpp +++ b/src/plugins/TextMessagePlugin.cpp @@ -3,7 +3,7 @@ #include "NodeDB.h" #include "PowerFSM.h" -TextMessagePlugin textMessagePlugin; +TextMessagePlugin *textMessagePlugin; bool TextMessagePlugin::handleReceived(const MeshPacket &mp) { diff --git a/src/plugins/TextMessagePlugin.h b/src/plugins/TextMessagePlugin.h index 1a6a4507..f7eba65f 100644 --- a/src/plugins/TextMessagePlugin.h +++ b/src/plugins/TextMessagePlugin.h @@ -22,4 +22,4 @@ class TextMessagePlugin : public SinglePortPlugin, public Observable Date: Fri, 8 Jan 2021 13:21:14 +0800 Subject: [PATCH 3/4] update protos --- proto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proto b/proto index 75078afe..dfe7bc12 160000 --- a/proto +++ b/proto @@ -1 +1 @@ -Subproject commit 75078afe43934f4ce15ef86ebc6950658a170145 +Subproject commit dfe7bc1217a00c23eecb9dfcf1d56fe95ebddc3b From 71be71d63d2b304257121c4befc49b05177a01ce Mon Sep 17 00:00:00 2001 From: Kevin Hester Date: Fri, 8 Jan 2021 13:31:28 +0800 Subject: [PATCH 4/4] add note about how to send messages thanks @rw-w for the question --- docs/software/plugin-api.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/software/plugin-api.md b/docs/software/plugin-api.md index 4fd477ae..34f1224d 100644 --- a/docs/software/plugin-api.md +++ b/docs/software/plugin-api.md @@ -55,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.