diff --git a/src/meshUtils.h b/src/meshUtils.h index 9a80ac51f..2b7915778 100644 --- a/src/meshUtils.h +++ b/src/meshUtils.h @@ -5,6 +5,16 @@ #include #include +/// Keep a function out of line even when the compiler would rather inline it. Use on helpers that +/// hold a large object on the stack: inlining several of them into one caller makes that caller's +/// frame reserve every helper's locals at once, which on our 8 KB Arduino loopTask is enough to +/// overflow the stack (see issue #11237). +#if defined(__GNUC__) +#define NOINLINE __attribute__((noinline)) +#else +#define NOINLINE +#endif + /// C++ v17+ clamp function, limits a given value to a range defined by lo and hi template constexpr const T &clamp(const T &v, const T &lo, const T &hi) { diff --git a/src/modules/AdminModule.cpp b/src/modules/AdminModule.cpp index d57fa142e..4359709d8 100644 --- a/src/modules/AdminModule.cpp +++ b/src/modules/AdminModule.cpp @@ -681,22 +681,41 @@ bool AdminModule::handleReceivedProtobuf(const meshtastic_MeshPacket &mp, meshta #endif default: - meshtastic_AdminMessage res = meshtastic_AdminMessage_init_default; - AdminMessageHandleResult handleResult = MeshModule::handleAdminMessageForAllModules(mp, r, &res); - - if (handleResult == AdminMessageHandleResult::HANDLED_WITH_RESPONSE) { - setPassKey(&res); - myReply = allocDataProtobuf(res); - } else if (mp.decoded.want_response) { - LOG_DEBUG("Module API did not respond to admin message. req.variant=%d", r->which_payload_variant); - } else if (handleResult != AdminMessageHandleResult::HANDLED) { - // Probably a message sent by us or sent to our local node. FIXME, we should avoid scanning these messages - LOG_DEBUG("Module API did not handle admin message %d", r->which_payload_variant); - } + handleViaModuleApi(mp, r); break; } // Allow any observers (e.g. the UI) to handle/respond + handleViaObservers(r); + + // If asked for a response and it is not yet set, generate an 'ACK' response + if (mp.decoded.want_response && !myReply) { + myReply = allocErrorResponse(meshtastic_Routing_Error_NONE, &mp); + } + if (mp.pki_encrypted && myReply) { + myReply->pki_encrypted = true; + } + return handled; +} + +void AdminModule::handleViaModuleApi(const meshtastic_MeshPacket &mp, meshtastic_AdminMessage *r) +{ + meshtastic_AdminMessage res = meshtastic_AdminMessage_init_default; + AdminMessageHandleResult handleResult = MeshModule::handleAdminMessageForAllModules(mp, r, &res); + + if (handleResult == AdminMessageHandleResult::HANDLED_WITH_RESPONSE) { + setPassKey(&res); + myReply = allocDataProtobuf(res); + } else if (mp.decoded.want_response) { + LOG_DEBUG("Module API did not respond to admin message. req.variant=%d", r->which_payload_variant); + } else if (handleResult != AdminMessageHandleResult::HANDLED) { + // Probably a message sent by us or sent to our local node. FIXME, we should avoid scanning these messages + LOG_DEBUG("Module API did not handle admin message %d", r->which_payload_variant); + } +} + +void AdminModule::handleViaObservers(const meshtastic_AdminMessage *r) +{ AdminMessageHandleResult observerResult = AdminMessageHandleResult::NOT_HANDLED; meshtastic_AdminMessage observerResponse = meshtastic_AdminMessage_init_default; AdminModule_ObserverData observerData = { @@ -714,15 +733,6 @@ bool AdminModule::handleReceivedProtobuf(const meshtastic_MeshPacket &mp, meshta } else if (observerResult == AdminMessageHandleResult::HANDLED) { LOG_DEBUG("Observer handled admin message"); } - - // If asked for a response and it is not yet set, generate an 'ACK' response - if (mp.decoded.want_response && !myReply) { - myReply = allocErrorResponse(meshtastic_Routing_Error_NONE, &mp); - } - if (mp.pki_encrypted && myReply) { - myReply->pki_encrypted = true; - } - return handled; } void AdminModule::handleGetModuleConfigResponse(const meshtastic_MeshPacket &mp, meshtastic_AdminMessage *r) diff --git a/src/modules/AdminModule.h b/src/modules/AdminModule.h index 1bf98be10..d81f39d47 100644 --- a/src/modules/AdminModule.h +++ b/src/modules/AdminModule.h @@ -3,6 +3,7 @@ #include #endif #include "ProtobufModule.h" +#include "meshUtils.h" #include #if HAS_WIFI #include "mesh/wifi/WiFiAPClient.h" @@ -48,16 +49,24 @@ class AdminModule : public ProtobufModule, public Obser /** * Getters + * + * Each of the NOINLINE ones below builds a whole meshtastic_AdminMessage (480 bytes) on the + * stack. They are only ever reached from one case of handleReceivedProtobuf()'s switch, but + * when the compiler inlines them the dispatcher's frame has to reserve a slot for every one of + * them at once - measured at 3008 bytes on ESP32-S3, 37% of the 8 KB Arduino loopTask stack + * that also has to carry PhoneAPI, the router, nanopb and LittleFS below it. Keeping them out + * of line means only the request actually being served pays for its response buffer. + * See issue #11237. */ void handleGetModuleConfigResponse(const meshtastic_MeshPacket &req, meshtastic_AdminMessage *p); - void handleGetOwner(const meshtastic_MeshPacket &req); - void handleGetConfig(const meshtastic_MeshPacket &req, uint32_t configType); - void handleGetModuleConfig(const meshtastic_MeshPacket &req, uint32_t configType); - void handleGetChannel(const meshtastic_MeshPacket &req, uint32_t channelIndex); - void handleGetDeviceMetadata(const meshtastic_MeshPacket &req); - void handleGetDeviceConnectionStatus(const meshtastic_MeshPacket &req); - void handleGetNodeRemoteHardwarePins(const meshtastic_MeshPacket &req); - void handleGetDeviceUIConfig(const meshtastic_MeshPacket &req); + NOINLINE void handleGetOwner(const meshtastic_MeshPacket &req); + NOINLINE void handleGetConfig(const meshtastic_MeshPacket &req, uint32_t configType); + NOINLINE void handleGetModuleConfig(const meshtastic_MeshPacket &req, uint32_t configType); + NOINLINE void handleGetChannel(const meshtastic_MeshPacket &req, uint32_t channelIndex); + NOINLINE void handleGetDeviceMetadata(const meshtastic_MeshPacket &req); + NOINLINE void handleGetDeviceConnectionStatus(const meshtastic_MeshPacket &req); + NOINLINE void handleGetNodeRemoteHardwarePins(const meshtastic_MeshPacket &req); + NOINLINE void handleGetDeviceUIConfig(const meshtastic_MeshPacket &req); /** * Setters */ @@ -102,6 +111,13 @@ class AdminModule : public ProtobufModule, public Obser /// Whether a response (variant responseVariant, module-config subtype moduleConfigTag or 0) /// from mp.from answers a request we sent; consumes the matched request so it can't be replayed. bool responseIsSolicited(const meshtastic_MeshPacket &mp, pb_size_t responseVariant, pb_size_t moduleConfigTag); + + /// Offer an admin message we have no case for to the module API, and let observers (e.g. the UI) + /// see every admin message. Both build a response on the stack, so like the getters above they + /// stay out of line to keep handleReceivedProtobuf()'s frame small. + NOINLINE void handleViaModuleApi(const meshtastic_MeshPacket &mp, meshtastic_AdminMessage *r); + NOINLINE void handleViaObservers(const meshtastic_AdminMessage *r); + void handleStoreDeviceUIConfig(const meshtastic_DeviceUIConfig &uicfg); void handleSendInputEvent(const meshtastic_AdminMessage_InputEvent &inputEvent); void reboot(int32_t seconds);