diff --git a/platformio.ini b/platformio.ini index 588231b1a..54777ef17 100644 --- a/platformio.ini +++ b/platformio.ini @@ -9,7 +9,7 @@ ; https://docs.platformio.org/page/projectconf.html [platformio] -;default_envs = tbeam +default_envs = tbeam ;default_envs = tbeam0.7 ;default_envs = heltec ;default_envs = tlora-v1 @@ -18,7 +18,7 @@ ;default_envs = lora-relay-v1 # nrf board ;default_envs = eink ;default_envs = nrf52840dk-geeksville -default_envs = native # lora-relay-v1 # nrf52840dk-geeksville # linux # or if you'd like to change the default to something like lora-relay-v1 put that here +;default_envs = native # lora-relay-v1 # nrf52840dk-geeksville # linux # or if you'd like to change the default to something like lora-relay-v1 put that here [common] ; common is not currently used @@ -397,4 +397,4 @@ lib_deps = ;extends = esp32_base ;board = genieblocks_lora ;build_flags = -; ${esp32_base.build_flags} -D GENIEBLOCKS \ No newline at end of file +; ${esp32_base.build_flags} -D GENIEBLOCKS diff --git a/src/mesh/MeshPlugin.cpp b/src/mesh/MeshPlugin.cpp index 45c7bcf18..1c14001f2 100644 --- a/src/mesh/MeshPlugin.cpp +++ b/src/mesh/MeshPlugin.cpp @@ -86,8 +86,10 @@ void MeshPlugin::callPlugins(const MeshPacket &mp) /// We only call plugins that are interested in the packet (and the message is destined to us or we are promiscious) bool wantsPacket = (isDecoded || pi.encryptedOk) && (pi.isPromiscuous || toUs) && pi.wantPacket(&mp); + DEBUG_MSG("Plugin %s wantsPacket=%d\n", pi.name, wantsPacket); + assert(!pi.myReply); // If it is !null it means we have a bug, because it should have been sent the previous time + if (wantsPacket) { - // DEBUG_MSG("Plugin %s wantsPacket=%d\n", pi.name, wantsPacket); pluginFound = true; /// received channel (or NULL if not decoded) @@ -124,6 +126,14 @@ void MeshPlugin::callPlugins(const MeshPacket &mp) } else { DEBUG_MSG("Plugin %s considered\n", pi.name); } + + // If the requester didn't ask for a response we might need to discard unused replies to prevent memory leaks + if (pi.myReply) { + DEBUG_MSG("Discarding an unneeded response\n"); + packetPool.release(pi.myReply); + pi.myReply = NULL; + } + if (handled) { DEBUG_MSG("Plugin %s handled and skipped other processing\n", pi.name); break; @@ -136,7 +146,7 @@ void MeshPlugin::callPlugins(const MeshPacket &mp) if (mp.decoded.want_response && toUs) { if (currentReply) { - DEBUG_MSG("Sending response\n"); + printPacket("Sending response", currentReply); service.sendToMesh(currentReply); currentReply = NULL; } else { @@ -154,6 +164,13 @@ void MeshPlugin::callPlugins(const MeshPacket &mp) DEBUG_MSG("No plugins interested in portnum=%d\n", mp.decoded.portnum); } +MeshPacket *MeshPlugin::allocReply() +{ + auto r = myReply; + myReply = NULL; // Only use each reply once + return r; +} + /** Messages can be received that have the want_response bit set. If set, this callback will be invoked * so that subclasses can (optionally) send a response back to the original sender. Implementing this method * is optional @@ -176,7 +193,7 @@ void MeshPlugin::sendResponse(const MeshPacket &req) void setReplyTo(MeshPacket *p, const MeshPacket &to) { assert(p->which_payloadVariant == MeshPacket_decoded_tag); // Should already be set by now - p->to = getFrom(&to); + p->to = getFrom(&to); // Make sure that if we are sending to the local node, we use our local node addr, not 0 p->channel = to.channel; // Use the same channel that the request came in on // No need for an ack if we are just delivering locally (it just generates an ignored ack) diff --git a/src/mesh/MeshPlugin.h b/src/mesh/MeshPlugin.h index 011e680ab..937c37dfe 100644 --- a/src/mesh/MeshPlugin.h +++ b/src/mesh/MeshPlugin.h @@ -69,6 +69,11 @@ class MeshPlugin */ static const MeshPacket *currentRequest; + /** + * If your handler wants to send a response, simply set currentReply and it will be sent at the end of response handling. + */ + MeshPacket *myReply = NULL; + /** * Initialize your plugin. This setup function is called once after all hardware and mesh protocol layers have * been initialized @@ -87,8 +92,12 @@ class MeshPlugin virtual bool handleReceived(const MeshPacket &mp) { return false; } /** Messages can be received that have the want_response bit set. If set, this callback will be invoked - * so that subclasses can (optionally) send a response back to the original sender. */ - virtual MeshPacket *allocReply() { return NULL; } + * so that subclasses can (optionally) send a response back to the original sender. + * + * Note: most implementers don't need to override this, instead: If while handling a request you have a reply, just set + * the protected reply field in this instance. + * */ + virtual MeshPacket *allocReply(); /*** * @return true if you want to be alloced a UI screen frame @@ -106,6 +115,7 @@ class MeshPlugin * the RoutingPlugin to avoid sending redundant acks */ static MeshPacket *currentReply; + friend class ReliableRouter; /** Messages can be received that have the want_response bit set. If set, this callback will be invoked diff --git a/src/plugins/AdminPlugin.cpp b/src/plugins/AdminPlugin.cpp index 3848bd2f6..87989937b 100644 --- a/src/plugins/AdminPlugin.cpp +++ b/src/plugins/AdminPlugin.cpp @@ -19,7 +19,7 @@ void AdminPlugin::handleGetChannel(const MeshPacket &req, uint32_t channelIndex) AdminMessage r = AdminMessage_init_default; r.get_channel_response = channels.getByIndex(channelIndex); r.which_variant = AdminMessage_get_channel_response_tag; - reply = allocDataProtobuf(r); + myReply = allocDataProtobuf(r); } } @@ -36,7 +36,7 @@ void AdminPlugin::handleGetRadio(const MeshPacket &req) r.get_radio_response.preferences.ls_secs = getPref_ls_secs(); r.which_variant = AdminMessage_get_radio_response_tag; - reply = allocDataProtobuf(r); + myReply = allocDataProtobuf(r); } } @@ -57,7 +57,7 @@ bool AdminPlugin::handleReceivedProtobuf(const MeshPacket &mp, const AdminMessag case AdminMessage_set_channel_tag: DEBUG_MSG("Client is setting channel %d\n", r->set_channel.index); if (r->set_channel.index < 0 || r->set_channel.index >= (int)MAX_NUM_CHANNELS) - reply = allocErrorResponse(Routing_Error_BAD_REQUEST, &mp); + myReply = allocErrorResponse(Routing_Error_BAD_REQUEST, &mp); else handleSetChannel(r->set_channel); break; @@ -66,7 +66,7 @@ bool AdminPlugin::handleReceivedProtobuf(const MeshPacket &mp, const AdminMessag uint32_t i = r->get_channel_request - 1; DEBUG_MSG("Client is getting channel %u\n", i); if (i >= MAX_NUM_CHANNELS) - reply = allocErrorResponse(Routing_Error_BAD_REQUEST, &mp); + myReply = allocErrorResponse(Routing_Error_BAD_REQUEST, &mp); else handleGetChannel(mp, i); break; @@ -141,13 +141,6 @@ void AdminPlugin::handleSetRadio(const RadioConfig &r) service.reloadConfig(); } -MeshPacket *AdminPlugin::allocReply() -{ - auto r = reply; - reply = NULL; // Only use each reply once - return r; -} - AdminPlugin::AdminPlugin() : ProtobufPlugin("Admin", PortNum_ADMIN_APP, AdminMessage_fields) { // restrict to the admin channel for rx diff --git a/src/plugins/AdminPlugin.h b/src/plugins/AdminPlugin.h index 71dd5536a..e4fc91d6c 100644 --- a/src/plugins/AdminPlugin.h +++ b/src/plugins/AdminPlugin.h @@ -6,8 +6,6 @@ */ class AdminPlugin : public ProtobufPlugin { - MeshPacket *reply = NULL; - public: /** Constructor * name is for debugging output @@ -21,10 +19,6 @@ class AdminPlugin : public ProtobufPlugin */ virtual bool handleReceivedProtobuf(const MeshPacket &mp, const AdminMessage *p); - /** Messages can be received that have the want_response bit set. If set, this callback will be invoked - * so that subclasses can (optionally) send a response back to the original sender. */ - virtual MeshPacket *allocReply(); - private: void handleSetOwner(const User &o); void handleSetChannel(const Channel &cc); diff --git a/src/plugins/RemoteHardwarePlugin.cpp b/src/plugins/RemoteHardwarePlugin.cpp index ac7e70e58..d31e70608 100644 --- a/src/plugins/RemoteHardwarePlugin.cpp +++ b/src/plugins/RemoteHardwarePlugin.cpp @@ -10,12 +10,13 @@ // Because (FIXME) we currently don't tell API clients status on sent messages // we need to throttle our sending, so that if a gpio is bouncing up and down we -// don't generate more messages than the net can send. So we limit watch messages to +// don't generate more messages than the net can send. So we limit watch messages to // a max of one change per 30 seconds #define WATCH_INTERVAL_MSEC (30 * 1000) /// Set pin modes for every set bit in a mask -static void pinModes(uint64_t mask, uint8_t mode) { +static void pinModes(uint64_t mask, uint8_t mode) +{ for (uint8_t i = 0; i < NUM_GPIOS; i++) { if (mask & (1 << i)) { pinMode(i, mode); @@ -24,7 +25,8 @@ static void pinModes(uint64_t mask, uint8_t mode) { } /// Read all the pins mentioned in a mask -static uint64_t digitalReads(uint64_t mask) { +static uint64_t digitalReads(uint64_t mask) +{ uint64_t res = 0; pinModes(mask, INPUT_PULLUP); @@ -40,10 +42,9 @@ static uint64_t digitalReads(uint64_t mask) { return res; } - RemoteHardwarePlugin::RemoteHardwarePlugin() - : ProtobufPlugin("remotehardware", PortNum_REMOTE_HARDWARE_APP, HardwareMessage_fields), - concurrency::OSThread("remotehardware") + : ProtobufPlugin("remotehardware", PortNum_REMOTE_HARDWARE_APP, HardwareMessage_fields), concurrency::OSThread( + "remotehardware") { } @@ -69,26 +70,26 @@ bool RemoteHardwarePlugin::handleReceivedProtobuf(const MeshPacket &req, const H case HardwareMessage_Type_READ_GPIOS: { // Print notification to LCD screen - if(screen) + if (screen) screen->print("Read GPIOs\n"); uint64_t res = digitalReads(p.gpio_mask); // Send the reply - HardwareMessage reply = HardwareMessage_init_default; - reply.typ = HardwareMessage_Type_READ_GPIOS_REPLY; - reply.gpio_value = res; - MeshPacket *p = allocDataProtobuf(reply); + HardwareMessage r = HardwareMessage_init_default; + r.typ = HardwareMessage_Type_READ_GPIOS_REPLY; + r.gpio_value = res; + MeshPacket *p = allocDataProtobuf(r); setReplyTo(p, req); - service.sendToMesh(p); + myReply = p; break; } case HardwareMessage_Type_WATCH_GPIOS: { watchGpios = p.gpio_mask; - lastWatchMsec = 0; // Force a new publish soon + lastWatchMsec = 0; // Force a new publish soon previousWatch = ~watchGpios; // generate a 'previous' value which is guaranteed to not match (to force an initial publish) - enabled = true; // Let our thread run at least once + enabled = true; // Let our thread run at least once DEBUG_MSG("Now watching GPIOs 0x%llx\n", watchGpios); break; } @@ -101,31 +102,31 @@ bool RemoteHardwarePlugin::handleReceivedProtobuf(const MeshPacket &req, const H DEBUG_MSG("Hardware operation %d not yet implemented! FIXME\n", p.typ); break; } - - return true; // handled + + return false; } -int32_t RemoteHardwarePlugin::runOnce() { - if(watchGpios) { +int32_t RemoteHardwarePlugin::runOnce() +{ + if (watchGpios) { uint32_t now = millis(); - if(now - lastWatchMsec >= WATCH_INTERVAL_MSEC) { + if (now - lastWatchMsec >= WATCH_INTERVAL_MSEC) { uint64_t curVal = digitalReads(watchGpios); - if(curVal != previousWatch) { + if (curVal != previousWatch) { previousWatch = curVal; DEBUG_MSG("Broadcasting GPIOS 0x%llx changed!\n", curVal); // Something changed! Tell the world with a broadcast message - HardwareMessage reply = HardwareMessage_init_default; - reply.typ = HardwareMessage_Type_GPIOS_CHANGED; - reply.gpio_value = curVal; - MeshPacket *p = allocDataProtobuf(reply); + HardwareMessage r = HardwareMessage_init_default; + r.typ = HardwareMessage_Type_GPIOS_CHANGED; + r.gpio_value = curVal; + MeshPacket *p = allocDataProtobuf(r); service.sendToMesh(p); } } - } - else { + } else { // No longer watching anything - stop using CPU enabled = false; }