From 41a1dfec7942448f5275f45f3cfbd22aeef7a7c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20G=C3=B6ttgens?= Date: Thu, 29 Dec 2022 15:45:49 +0100 Subject: [PATCH 01/12] a lot of thread housekeeping. Switch them off when not needed / disabled. --- src/input/RotaryEncoderInterruptBase.cpp | 2 +- src/input/RotaryEncoderInterruptBase.h | 2 +- src/input/RotaryEncoderInterruptImpl1.cpp | 2 + src/input/cardKbI2cImpl.cpp | 2 + src/input/kbI2cBase.h | 2 +- src/mesh/http/WebServer.cpp | 13 ++- src/modules/CannedMessageModule.cpp | 8 ++ src/modules/ExternalNotificationModule.cpp | 1 + src/modules/RemoteHardwareModule.cpp | 97 ++++++++++--------- src/modules/SerialModule.cpp | 4 +- .../Telemetry/EnvironmentTelemetry.cpp | 1 + src/modules/esp32/AudioModule.cpp | 3 + src/modules/esp32/RangeTestModule.cpp | 2 + src/modules/esp32/StoreForwardModule.cpp | 4 + src/mqtt/MQTT.cpp | 18 +++- 15 files changed, 102 insertions(+), 59 deletions(-) diff --git a/src/input/RotaryEncoderInterruptBase.cpp b/src/input/RotaryEncoderInterruptBase.cpp index 6914824fd..f6df7fb2c 100644 --- a/src/input/RotaryEncoderInterruptBase.cpp +++ b/src/input/RotaryEncoderInterruptBase.cpp @@ -54,7 +54,7 @@ int32_t RotaryEncoderInterruptBase::runOnce() this->action = ROTARY_ACTION_NONE; - return 30000; // TODO: technically this can be MAX_INT + return INT32_MAX; } void RotaryEncoderInterruptBase::intPressHandler() diff --git a/src/input/RotaryEncoderInterruptBase.h b/src/input/RotaryEncoderInterruptBase.h index c468f4dc6..88d619178 100644 --- a/src/input/RotaryEncoderInterruptBase.h +++ b/src/input/RotaryEncoderInterruptBase.h @@ -7,7 +7,7 @@ enum RotaryEncoderInterruptBaseStateType { ROTARY_EVENT_OCCURRED, ROTARY_EVENT_C enum RotaryEncoderInterruptBaseActionType { ROTARY_ACTION_NONE, ROTARY_ACTION_PRESSED, ROTARY_ACTION_CW, ROTARY_ACTION_CCW }; -class RotaryEncoderInterruptBase : public Observable, private concurrency::OSThread +class RotaryEncoderInterruptBase : public Observable, public concurrency::OSThread { public: explicit RotaryEncoderInterruptBase(const char *name); diff --git a/src/input/RotaryEncoderInterruptImpl1.cpp b/src/input/RotaryEncoderInterruptImpl1.cpp index bc897deef..f5fd6855a 100644 --- a/src/input/RotaryEncoderInterruptImpl1.cpp +++ b/src/input/RotaryEncoderInterruptImpl1.cpp @@ -9,6 +9,8 @@ void RotaryEncoderInterruptImpl1::init() { if (!moduleConfig.canned_message.rotary1_enabled) { // Input device is disabled. + setInterval(INT32_MAX); + enabled = false; return; } diff --git a/src/input/cardKbI2cImpl.cpp b/src/input/cardKbI2cImpl.cpp index de0fbbd38..4b4fc764b 100644 --- a/src/input/cardKbI2cImpl.cpp +++ b/src/input/cardKbI2cImpl.cpp @@ -13,6 +13,8 @@ void CardKbI2cImpl::init() if (cardkb_found != CARDKB_ADDR) { // Input device is not detected. + setInterval(INT32_MAX); + enabled = false; return; } diff --git a/src/input/kbI2cBase.h b/src/input/kbI2cBase.h index 086078cea..2fdacbc28 100644 --- a/src/input/kbI2cBase.h +++ b/src/input/kbI2cBase.h @@ -5,7 +5,7 @@ class KbI2cBase : public Observable, - private concurrency::OSThread + public concurrency::OSThread { public: explicit KbI2cBase(const char *name); diff --git a/src/mesh/http/WebServer.cpp b/src/mesh/http/WebServer.cpp index c1844b0cb..95850f5d8 100644 --- a/src/mesh/http/WebServer.cpp +++ b/src/mesh/http/WebServer.cpp @@ -165,11 +165,20 @@ void createSSLCert() WebServerThread *webServerThread; -WebServerThread::WebServerThread() : concurrency::OSThread("WebServerThread") {} +WebServerThread::WebServerThread() : concurrency::OSThread("WebServerThread") { + if(!config.network.wifi_enabled) { + setInterval(INT32_MAX); + enabled = false; + } +} int32_t WebServerThread::runOnce() { - // DEBUG_MSG("WebServerThread::runOnce()\n"); + if(!config.network.wifi_enabled) { + setInterval(INT32_MAX); + enabled = false; + } + handleWebResponse(); if (requestRestart && (millis() / 1000) > requestRestart) { diff --git a/src/modules/CannedMessageModule.cpp b/src/modules/CannedMessageModule.cpp index 4357ee2ca..a36e95a05 100644 --- a/src/modules/CannedMessageModule.cpp +++ b/src/modules/CannedMessageModule.cpp @@ -55,10 +55,18 @@ CannedMessageModule::CannedMessageModule() if ((this->splitConfiguredMessages() <= 0) && (cardkb_found != CARDKB_ADDR)) { DEBUG_MSG("CannedMessageModule: No messages are configured. Module is disabled\n"); this->runState = CANNED_MESSAGE_RUN_STATE_DISABLED; + setInterval(INT32_MAX); + enabled = false; } else { DEBUG_MSG("CannedMessageModule is enabled\n"); this->inputObserver.observe(inputBroker); } + setInterval(INT32_MAX); + enabled = false; + } else { + this->runState = CANNED_MESSAGE_RUN_STATE_DISABLED; + setInterval(INT32_MAX); + enabled = false; } } diff --git a/src/modules/ExternalNotificationModule.cpp b/src/modules/ExternalNotificationModule.cpp index be3e6cd8b..8746f8048 100644 --- a/src/modules/ExternalNotificationModule.cpp +++ b/src/modules/ExternalNotificationModule.cpp @@ -199,6 +199,7 @@ ExternalNotificationModule::ExternalNotificationModule() } } else { DEBUG_MSG("External Notification Module Disabled\n"); + setInterval(INT32_MAX); enabled = false; } } diff --git a/src/modules/RemoteHardwareModule.cpp b/src/modules/RemoteHardwareModule.cpp index 26f1c4dba..419885822 100644 --- a/src/modules/RemoteHardwareModule.cpp +++ b/src/modules/RemoteHardwareModule.cpp @@ -48,64 +48,66 @@ static uint64_t digitalReads(uint64_t mask) RemoteHardwareModule::RemoteHardwareModule() : ProtobufModule("remotehardware", PortNum_REMOTE_HARDWARE_APP, &HardwareMessage_msg), concurrency::OSThread( - "remotehardware") + "RemoteHardwareModule") { } bool RemoteHardwareModule::handleReceivedProtobuf(const MeshPacket &req, HardwareMessage *pptr) { - auto p = *pptr; - DEBUG_MSG("Received RemoteHardware typ=%d\n", p.type); + if (moduleConfig.remote_hardware.enabled) { + auto p = *pptr; + DEBUG_MSG("Received RemoteHardware typ=%d\n", p.type); - switch (p.type) { - case HardwareMessage_Type_WRITE_GPIOS: - // Print notification to LCD screen - screen->print("Write GPIOs\n"); + switch (p.type) { + case HardwareMessage_Type_WRITE_GPIOS: + // Print notification to LCD screen + screen->print("Write GPIOs\n"); - for (uint8_t i = 0; i < NUM_GPIOS; i++) { - uint64_t mask = 1 << i; - if (p.gpio_mask & mask) { - digitalWrite(i, (p.gpio_value & mask) ? 1 : 0); + for (uint8_t i = 0; i < NUM_GPIOS; i++) { + uint64_t mask = 1 << i; + if (p.gpio_mask & mask) { + digitalWrite(i, (p.gpio_value & mask) ? 1 : 0); + } } + pinModes(p.gpio_mask, OUTPUT); + + break; + + case HardwareMessage_Type_READ_GPIOS: { + // Print notification to LCD screen + if (screen) + screen->print("Read GPIOs\n"); + + uint64_t res = digitalReads(p.gpio_mask); + + // Send the reply + HardwareMessage r = HardwareMessage_init_default; + r.type = HardwareMessage_Type_READ_GPIOS_REPLY; + r.gpio_value = res; + r.gpio_mask = p.gpio_mask; + MeshPacket *p2 = allocDataProtobuf(r); + setReplyTo(p2, req); + myReply = p2; + break; } - pinModes(p.gpio_mask, OUTPUT); - break; + case HardwareMessage_Type_WATCH_GPIOS: { + watchGpios = p.gpio_mask; + 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 + DEBUG_MSG("Now watching GPIOs 0x%llx\n", watchGpios); + break; + } - case HardwareMessage_Type_READ_GPIOS: { - // Print notification to LCD screen - if (screen) - screen->print("Read GPIOs\n"); + case HardwareMessage_Type_READ_GPIOS_REPLY: + case HardwareMessage_Type_GPIOS_CHANGED: + break; // Ignore - we might see our own replies - uint64_t res = digitalReads(p.gpio_mask); - - // Send the reply - HardwareMessage r = HardwareMessage_init_default; - r.type = HardwareMessage_Type_READ_GPIOS_REPLY; - r.gpio_value = res; - r.gpio_mask = p.gpio_mask; - MeshPacket *p2 = allocDataProtobuf(r); - setReplyTo(p2, req); - myReply = p2; - break; - } - - case HardwareMessage_Type_WATCH_GPIOS: { - watchGpios = p.gpio_mask; - 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 - DEBUG_MSG("Now watching GPIOs 0x%llx\n", watchGpios); - break; - } - - case HardwareMessage_Type_READ_GPIOS_REPLY: - case HardwareMessage_Type_GPIOS_CHANGED: - break; // Ignore - we might see our own replies - - default: - DEBUG_MSG("Hardware operation %d not yet implemented! FIXME\n", p.type); - break; + default: + DEBUG_MSG("Hardware operation %d not yet implemented! FIXME\n", p.type); + break; + } } return false; @@ -113,7 +115,7 @@ bool RemoteHardwareModule::handleReceivedProtobuf(const MeshPacket &req, Hardwar int32_t RemoteHardwareModule::runOnce() { - if (watchGpios) { + if (moduleConfig.remote_hardware.enabled && watchGpios) { uint32_t now = millis(); if (now - lastWatchMsec >= WATCH_INTERVAL_MSEC) { @@ -134,6 +136,7 @@ int32_t RemoteHardwareModule::runOnce() } else { // No longer watching anything - stop using CPU enabled = false; + return INT32_MAX; } return 200; // Poll our GPIOs every 200ms (FIXME, make adjustable via protobuf arg) diff --git a/src/modules/SerialModule.cpp b/src/modules/SerialModule.cpp index eb8f0986b..00d8df9d2 100644 --- a/src/modules/SerialModule.cpp +++ b/src/modules/SerialModule.cpp @@ -222,6 +222,7 @@ int32_t SerialModule::runOnce() } else { DEBUG_MSG("Serial Module Disabled\n"); + enabled = false; return INT32_MAX; } } @@ -303,9 +304,6 @@ ProcessMessage SerialModuleRadio::handleReceived(const MeshPacket &mp) } } } - - } else { - DEBUG_MSG("Serial Module Disabled\n"); } return ProcessMessage::CONTINUE; // Let others look at this message also if they want } diff --git a/src/modules/Telemetry/EnvironmentTelemetry.cpp b/src/modules/Telemetry/EnvironmentTelemetry.cpp index 11955f3f6..500acd7f7 100644 --- a/src/modules/Telemetry/EnvironmentTelemetry.cpp +++ b/src/modules/Telemetry/EnvironmentTelemetry.cpp @@ -67,6 +67,7 @@ int32_t EnvironmentTelemetryModule::runOnce() if (!(moduleConfig.telemetry.environment_measurement_enabled || moduleConfig.telemetry.environment_screen_enabled)) { // If this module is not enabled, and the user doesn't want the display screen don't waste any OSThread time on it + enabled = false; return result; } diff --git a/src/modules/esp32/AudioModule.cpp b/src/modules/esp32/AudioModule.cpp index e7132fcc4..bd79be7cb 100644 --- a/src/modules/esp32/AudioModule.cpp +++ b/src/modules/esp32/AudioModule.cpp @@ -139,6 +139,8 @@ AudioModule::AudioModule() : SinglePortModule("AudioModule", PortNum_AUDIO_APP), DEBUG_MSG(" using %d frames of %d bytes for a total payload length of %d bytes\n", encode_frame_num, encode_codec_size, encode_frame_size); xTaskCreate(&run_codec2, "codec2_task", 30000, NULL, 5, &codec2HandlerTask); } else { + setInterval(INT32_MAX); + enabled = false; DEBUG_MSG("Codec2 disabled (AudioModule %d, Region %s, permitted %d)\n", moduleConfig.audio.codec2_enabled, myRegion->name, myRegion->audioPermitted); } } @@ -259,6 +261,7 @@ int32_t AudioModule::runOnce() return 100; } else { DEBUG_MSG("Audio Module Disabled\n"); + enabled = false; return INT32_MAX; } diff --git a/src/modules/esp32/RangeTestModule.cpp b/src/modules/esp32/RangeTestModule.cpp index d706f8ed3..5e52fea40 100644 --- a/src/modules/esp32/RangeTestModule.cpp +++ b/src/modules/esp32/RangeTestModule.cpp @@ -82,6 +82,7 @@ int32_t RangeTestModule::runOnce() return (senderHeartbeat); } else { + enabled = false; return (INT32_MAX); // This thread does not need to run as a receiver } @@ -94,6 +95,7 @@ int32_t RangeTestModule::runOnce() } #endif + enabled = false; return (INT32_MAX); } diff --git a/src/modules/esp32/StoreForwardModule.cpp b/src/modules/esp32/StoreForwardModule.cpp index 54a250d2a..ec1e3d695 100644 --- a/src/modules/esp32/StoreForwardModule.cpp +++ b/src/modules/esp32/StoreForwardModule.cpp @@ -52,6 +52,7 @@ int32_t StoreForwardModule::runOnce() return (this->packetTimeMax); } #endif + enabled = false; // Client doesn't need periodical return (INT32_MAX); } @@ -458,6 +459,9 @@ StoreForwardModule::StoreForwardModule() is_client = true; DEBUG_MSG("*** Initializing Store & Forward Module in Client mode\n"); } + } else { + setInterval(INT32_MAX); + enabled = false; } #endif } diff --git a/src/mqtt/MQTT.cpp b/src/mqtt/MQTT.cpp index df2d9b093..8dcf7eb90 100644 --- a/src/mqtt/MQTT.cpp +++ b/src/mqtt/MQTT.cpp @@ -128,12 +128,18 @@ void mqttInit() MQTT::MQTT() : concurrency::OSThread("mqtt"), pubSub(mqttClient), mqttQueue(MAX_MQTT_QUEUE) { - assert(!mqtt); - mqtt = this; + if(moduleConfig.mqtt.enabled) { - pubSub.setCallback(mqttCallback); + assert(!mqtt); + mqtt = this; - // preflightSleepObserver.observe(&preflightSleep); + pubSub.setCallback(mqttCallback); + + // preflightSleepObserver.observe(&preflightSleep); + } else { + setInterval(INT32_MAX); + enabled = false; + } } bool MQTT::connected() @@ -238,6 +244,10 @@ bool MQTT::wantsLink() const int32_t MQTT::runOnce() { + if(!moduleConfig.mqtt.enabled) { + enabled = false; + return INT32_MAX; + } bool wantConnection = wantsLink(); // If connected poll rapidly, otherwise only occasionally check for a wifi connection change and ability to contact server From 4e4a74379e1d042656ec7b56f0799d8bbc6d47dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20G=C3=B6ttgens?= Date: Thu, 29 Dec 2022 15:48:02 +0100 Subject: [PATCH 02/12] fix copy/paste error --- src/modules/CannedMessageModule.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/modules/CannedMessageModule.cpp b/src/modules/CannedMessageModule.cpp index a36e95a05..74a4aaeaf 100644 --- a/src/modules/CannedMessageModule.cpp +++ b/src/modules/CannedMessageModule.cpp @@ -61,8 +61,6 @@ CannedMessageModule::CannedMessageModule() DEBUG_MSG("CannedMessageModule is enabled\n"); this->inputObserver.observe(inputBroker); } - setInterval(INT32_MAX); - enabled = false; } else { this->runState = CANNED_MESSAGE_RUN_STATE_DISABLED; setInterval(INT32_MAX); From a8f93d5f47392aed69e6ddcfd331ecdf9af3896b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20G=C3=B6ttgens?= Date: Thu, 29 Dec 2022 22:42:05 +0100 Subject: [PATCH 03/12] Heap Debugging and Thread Disable --- src/concurrency/OSThread.cpp | 14 ++++++++++++++ src/concurrency/OSThread.h | 2 ++ 2 files changed, 16 insertions(+) diff --git a/src/concurrency/OSThread.cpp b/src/concurrency/OSThread.cpp index 88c21d1c5..c5adf297b 100644 --- a/src/concurrency/OSThread.cpp +++ b/src/concurrency/OSThread.cpp @@ -74,8 +74,16 @@ bool OSThread::shouldRun(unsigned long time) void OSThread::run() { +#ifdef DEBUG_HEAP + auto heap = ESP.getFreeHeap(); +#endif currentThread = this; auto newDelay = runOnce(); +#ifdef DEBUG_HEAP + auto newHeap = ESP.getFreeHeap(); + if (newHeap < heap) + DEBUG_MSG("Thread %s leaked heap %d -> %d (%d)\n", ThreadName.c_str(), heap, newHeap, newHeap - heap); +#endif runned(); @@ -85,6 +93,12 @@ void OSThread::run() currentThread = NULL; } +void OSThread::disable() { + enabled = false; + setInterval(INT32_MAX); + +} + /** * 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. diff --git a/src/concurrency/OSThread.h b/src/concurrency/OSThread.h index 7a86498b9..f0071c516 100644 --- a/src/concurrency/OSThread.h +++ b/src/concurrency/OSThread.h @@ -53,6 +53,8 @@ class OSThread : public Thread static void setup(); + void disable(); + /** * Wait a specified number msecs starting from the current time (rather than the last time we were run) */ From 38a1315599bbfd2c79fdae2f5456520aedb20f82 Mon Sep 17 00:00:00 2001 From: Ben Meadors Date: Thu, 29 Dec 2022 16:26:25 -0600 Subject: [PATCH 04/12] Refactor OSThread consumers to use disable() --- src/concurrency/OSThread.cpp | 4 ++-- src/input/RotaryEncoderInterruptImpl1.cpp | 3 +-- src/input/cardKbI2cImpl.cpp | 4 +--- src/mesh/http/WebServer.cpp | 10 ++++------ src/modules/CannedMessageModule.cpp | 6 ++---- src/modules/ExternalNotificationModule.cpp | 3 +-- src/modules/esp32/AudioModule.cpp | 3 +-- src/modules/esp32/StoreForwardModule.cpp | 3 +-- src/mqtt/MQTT.cpp | 3 +-- 9 files changed, 14 insertions(+), 25 deletions(-) diff --git a/src/concurrency/OSThread.cpp b/src/concurrency/OSThread.cpp index c5adf297b..53058de6a 100644 --- a/src/concurrency/OSThread.cpp +++ b/src/concurrency/OSThread.cpp @@ -93,10 +93,10 @@ void OSThread::run() currentThread = NULL; } -void OSThread::disable() { +void OSThread::disable() +{ enabled = false; setInterval(INT32_MAX); - } /** diff --git a/src/input/RotaryEncoderInterruptImpl1.cpp b/src/input/RotaryEncoderInterruptImpl1.cpp index f5fd6855a..7bf66ac55 100644 --- a/src/input/RotaryEncoderInterruptImpl1.cpp +++ b/src/input/RotaryEncoderInterruptImpl1.cpp @@ -9,8 +9,7 @@ void RotaryEncoderInterruptImpl1::init() { if (!moduleConfig.canned_message.rotary1_enabled) { // Input device is disabled. - setInterval(INT32_MAX); - enabled = false; + disable(); return; } diff --git a/src/input/cardKbI2cImpl.cpp b/src/input/cardKbI2cImpl.cpp index 4b4fc764b..3d30fb867 100644 --- a/src/input/cardKbI2cImpl.cpp +++ b/src/input/cardKbI2cImpl.cpp @@ -12,9 +12,7 @@ void CardKbI2cImpl::init() { if (cardkb_found != CARDKB_ADDR) { - // Input device is not detected. - setInterval(INT32_MAX); - enabled = false; + disable(); return; } diff --git a/src/mesh/http/WebServer.cpp b/src/mesh/http/WebServer.cpp index 95850f5d8..860c10219 100644 --- a/src/mesh/http/WebServer.cpp +++ b/src/mesh/http/WebServer.cpp @@ -166,17 +166,15 @@ void createSSLCert() WebServerThread *webServerThread; WebServerThread::WebServerThread() : concurrency::OSThread("WebServerThread") { - if(!config.network.wifi_enabled) { - setInterval(INT32_MAX); - enabled = false; + if (!config.network.wifi_enabled) { + disable(); } } int32_t WebServerThread::runOnce() { - if(!config.network.wifi_enabled) { - setInterval(INT32_MAX); - enabled = false; + if (!config.network.wifi_enabled) { + disable(); } handleWebResponse(); diff --git a/src/modules/CannedMessageModule.cpp b/src/modules/CannedMessageModule.cpp index 74a4aaeaf..682d0efdb 100644 --- a/src/modules/CannedMessageModule.cpp +++ b/src/modules/CannedMessageModule.cpp @@ -55,16 +55,14 @@ CannedMessageModule::CannedMessageModule() if ((this->splitConfiguredMessages() <= 0) && (cardkb_found != CARDKB_ADDR)) { DEBUG_MSG("CannedMessageModule: No messages are configured. Module is disabled\n"); this->runState = CANNED_MESSAGE_RUN_STATE_DISABLED; - setInterval(INT32_MAX); - enabled = false; + disable(); } else { DEBUG_MSG("CannedMessageModule is enabled\n"); this->inputObserver.observe(inputBroker); } } else { this->runState = CANNED_MESSAGE_RUN_STATE_DISABLED; - setInterval(INT32_MAX); - enabled = false; + disable(); } } diff --git a/src/modules/ExternalNotificationModule.cpp b/src/modules/ExternalNotificationModule.cpp index 8746f8048..f22bdf8f1 100644 --- a/src/modules/ExternalNotificationModule.cpp +++ b/src/modules/ExternalNotificationModule.cpp @@ -199,8 +199,7 @@ ExternalNotificationModule::ExternalNotificationModule() } } else { DEBUG_MSG("External Notification Module Disabled\n"); - setInterval(INT32_MAX); - enabled = false; + disable(); } } diff --git a/src/modules/esp32/AudioModule.cpp b/src/modules/esp32/AudioModule.cpp index bd79be7cb..25762aa3f 100644 --- a/src/modules/esp32/AudioModule.cpp +++ b/src/modules/esp32/AudioModule.cpp @@ -139,8 +139,7 @@ AudioModule::AudioModule() : SinglePortModule("AudioModule", PortNum_AUDIO_APP), DEBUG_MSG(" using %d frames of %d bytes for a total payload length of %d bytes\n", encode_frame_num, encode_codec_size, encode_frame_size); xTaskCreate(&run_codec2, "codec2_task", 30000, NULL, 5, &codec2HandlerTask); } else { - setInterval(INT32_MAX); - enabled = false; + disable(); DEBUG_MSG("Codec2 disabled (AudioModule %d, Region %s, permitted %d)\n", moduleConfig.audio.codec2_enabled, myRegion->name, myRegion->audioPermitted); } } diff --git a/src/modules/esp32/StoreForwardModule.cpp b/src/modules/esp32/StoreForwardModule.cpp index ec1e3d695..af0e60ed7 100644 --- a/src/modules/esp32/StoreForwardModule.cpp +++ b/src/modules/esp32/StoreForwardModule.cpp @@ -460,8 +460,7 @@ StoreForwardModule::StoreForwardModule() DEBUG_MSG("*** Initializing Store & Forward Module in Client mode\n"); } } else { - setInterval(INT32_MAX); - enabled = false; + disable(); } #endif } diff --git a/src/mqtt/MQTT.cpp b/src/mqtt/MQTT.cpp index 8dcf7eb90..4cd6dcee4 100644 --- a/src/mqtt/MQTT.cpp +++ b/src/mqtt/MQTT.cpp @@ -137,8 +137,7 @@ MQTT::MQTT() : concurrency::OSThread("mqtt"), pubSub(mqttClient), mqttQueue(MAX_ // preflightSleepObserver.observe(&preflightSleep); } else { - setInterval(INT32_MAX); - enabled = false; + disable(); } } From 110c80d04585825e752ed3c585bbe986d194d114 Mon Sep 17 00:00:00 2001 From: Ben Meadors Date: Thu, 29 Dec 2022 16:54:39 -0600 Subject: [PATCH 05/12] Make disable return an int32_t for runOnce usage --- src/concurrency/OSThread.cpp | 4 +++- src/concurrency/OSThread.h | 2 +- src/modules/esp32/AudioModule.cpp | 3 +-- src/mqtt/MQTT.cpp | 3 +-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/concurrency/OSThread.cpp b/src/concurrency/OSThread.cpp index 53058de6a..4e6bf1b5f 100644 --- a/src/concurrency/OSThread.cpp +++ b/src/concurrency/OSThread.cpp @@ -93,10 +93,12 @@ void OSThread::run() currentThread = NULL; } -void OSThread::disable() +int32_t OSThread::disable() { enabled = false; setInterval(INT32_MAX); + + return INT32_MAX; } /** diff --git a/src/concurrency/OSThread.h b/src/concurrency/OSThread.h index f0071c516..aa8e3e2d8 100644 --- a/src/concurrency/OSThread.h +++ b/src/concurrency/OSThread.h @@ -53,7 +53,7 @@ class OSThread : public Thread static void setup(); - void disable(); + int32_t disable(); /** * Wait a specified number msecs starting from the current time (rather than the last time we were run) diff --git a/src/modules/esp32/AudioModule.cpp b/src/modules/esp32/AudioModule.cpp index 25762aa3f..a0dffd0eb 100644 --- a/src/modules/esp32/AudioModule.cpp +++ b/src/modules/esp32/AudioModule.cpp @@ -260,8 +260,7 @@ int32_t AudioModule::runOnce() return 100; } else { DEBUG_MSG("Audio Module Disabled\n"); - enabled = false; - return INT32_MAX; + return disable(); } } diff --git a/src/mqtt/MQTT.cpp b/src/mqtt/MQTT.cpp index 4cd6dcee4..08b16f287 100644 --- a/src/mqtt/MQTT.cpp +++ b/src/mqtt/MQTT.cpp @@ -244,8 +244,7 @@ bool MQTT::wantsLink() const int32_t MQTT::runOnce() { if(!moduleConfig.mqtt.enabled) { - enabled = false; - return INT32_MAX; + return disable(); } bool wantConnection = wantsLink(); From c7c5671cca8e996a1b04465f71e39b51f40a3ca6 Mon Sep 17 00:00:00 2001 From: Ben Meadors Date: Thu, 29 Dec 2022 18:48:33 -0600 Subject: [PATCH 06/12] More disables --- src/modules/RemoteHardwareModule.cpp | 3 +-- src/modules/SerialModule.cpp | 4 +--- src/modules/Telemetry/EnvironmentTelemetry.cpp | 3 +-- src/modules/esp32/RangeTestModule.cpp | 3 +-- src/modules/esp32/StoreForwardModule.cpp | 3 +-- 5 files changed, 5 insertions(+), 11 deletions(-) diff --git a/src/modules/RemoteHardwareModule.cpp b/src/modules/RemoteHardwareModule.cpp index 419885822..f8f2fecae 100644 --- a/src/modules/RemoteHardwareModule.cpp +++ b/src/modules/RemoteHardwareModule.cpp @@ -135,8 +135,7 @@ int32_t RemoteHardwareModule::runOnce() } } else { // No longer watching anything - stop using CPU - enabled = false; - return INT32_MAX; + return disable(); } return 200; // Poll our GPIOs every 200ms (FIXME, make adjustable via protobuf arg) diff --git a/src/modules/SerialModule.cpp b/src/modules/SerialModule.cpp index 00d8df9d2..fc6448a97 100644 --- a/src/modules/SerialModule.cpp +++ b/src/modules/SerialModule.cpp @@ -221,9 +221,7 @@ int32_t SerialModule::runOnce() return (10); } else { DEBUG_MSG("Serial Module Disabled\n"); - - enabled = false; - return INT32_MAX; + return disable(); } } diff --git a/src/modules/Telemetry/EnvironmentTelemetry.cpp b/src/modules/Telemetry/EnvironmentTelemetry.cpp index 500acd7f7..b2b100257 100644 --- a/src/modules/Telemetry/EnvironmentTelemetry.cpp +++ b/src/modules/Telemetry/EnvironmentTelemetry.cpp @@ -67,8 +67,7 @@ int32_t EnvironmentTelemetryModule::runOnce() if (!(moduleConfig.telemetry.environment_measurement_enabled || moduleConfig.telemetry.environment_screen_enabled)) { // If this module is not enabled, and the user doesn't want the display screen don't waste any OSThread time on it - enabled = false; - return result; + return disable(); } if (firstTime) { diff --git a/src/modules/esp32/RangeTestModule.cpp b/src/modules/esp32/RangeTestModule.cpp index 5e52fea40..0aa25946d 100644 --- a/src/modules/esp32/RangeTestModule.cpp +++ b/src/modules/esp32/RangeTestModule.cpp @@ -82,8 +82,7 @@ int32_t RangeTestModule::runOnce() return (senderHeartbeat); } else { - enabled = false; - return (INT32_MAX); + return disable(); // This thread does not need to run as a receiver } diff --git a/src/modules/esp32/StoreForwardModule.cpp b/src/modules/esp32/StoreForwardModule.cpp index af0e60ed7..70a28ac68 100644 --- a/src/modules/esp32/StoreForwardModule.cpp +++ b/src/modules/esp32/StoreForwardModule.cpp @@ -52,8 +52,7 @@ int32_t StoreForwardModule::runOnce() return (this->packetTimeMax); } #endif - enabled = false; // Client doesn't need periodical - return (INT32_MAX); + return disable(); } /* From be91b08b3e0c594fa4b1310876053d0d76865909 Mon Sep 17 00:00:00 2001 From: Ben Meadors Date: Thu, 29 Dec 2022 18:49:40 -0600 Subject: [PATCH 07/12] Missed one --- src/modules/esp32/RangeTestModule.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/modules/esp32/RangeTestModule.cpp b/src/modules/esp32/RangeTestModule.cpp index 0aa25946d..3db8e55ad 100644 --- a/src/modules/esp32/RangeTestModule.cpp +++ b/src/modules/esp32/RangeTestModule.cpp @@ -94,8 +94,7 @@ int32_t RangeTestModule::runOnce() } #endif - enabled = false; - return (INT32_MAX); + return disable(); } MeshPacket *RangeTestModuleRadio::allocReply() From 115cb05d3bf1316054c60dba6431257bd54fc6a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20G=C3=B6ttgens?= Date: Fri, 30 Dec 2022 14:22:08 +0100 Subject: [PATCH 08/12] less verbose logging and heap free printing --- src/concurrency/OSThread.cpp | 4 +++- src/modules/SerialModule.cpp | 1 - src/modules/esp32/AudioModule.cpp | 2 -- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/concurrency/OSThread.cpp b/src/concurrency/OSThread.cpp index 4e6bf1b5f..8862fa68b 100644 --- a/src/concurrency/OSThread.cpp +++ b/src/concurrency/OSThread.cpp @@ -82,7 +82,9 @@ void OSThread::run() #ifdef DEBUG_HEAP auto newHeap = ESP.getFreeHeap(); if (newHeap < heap) - DEBUG_MSG("Thread %s leaked heap %d -> %d (%d)\n", ThreadName.c_str(), heap, newHeap, newHeap - heap); + DEBUG_MSG("------ Thread %s leaked heap %d -> %d (%d) ------\n", ThreadName.c_str(), heap, newHeap, newHeap - heap); + if (heap < newHeap) + DEBUG_MSG("++++++ Thread %s freed heap %d -> %d (%d) ++++++\n", ThreadName.c_str(), heap, newHeap, newHeap - heap); #endif runned(); diff --git a/src/modules/SerialModule.cpp b/src/modules/SerialModule.cpp index fc6448a97..11347c910 100644 --- a/src/modules/SerialModule.cpp +++ b/src/modules/SerialModule.cpp @@ -220,7 +220,6 @@ int32_t SerialModule::runOnce() return (10); } else { - DEBUG_MSG("Serial Module Disabled\n"); return disable(); } } diff --git a/src/modules/esp32/AudioModule.cpp b/src/modules/esp32/AudioModule.cpp index a0dffd0eb..548464392 100644 --- a/src/modules/esp32/AudioModule.cpp +++ b/src/modules/esp32/AudioModule.cpp @@ -140,7 +140,6 @@ AudioModule::AudioModule() : SinglePortModule("AudioModule", PortNum_AUDIO_APP), xTaskCreate(&run_codec2, "codec2_task", 30000, NULL, 5, &codec2HandlerTask); } else { disable(); - DEBUG_MSG("Codec2 disabled (AudioModule %d, Region %s, permitted %d)\n", moduleConfig.audio.codec2_enabled, myRegion->name, myRegion->audioPermitted); } } @@ -259,7 +258,6 @@ int32_t AudioModule::runOnce() } return 100; } else { - DEBUG_MSG("Audio Module Disabled\n"); return disable(); } From 65aad62702167c6a7c0125d70337e226fa6784bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20G=C3=B6ttgens?= Date: Fri, 30 Dec 2022 14:51:00 +0100 Subject: [PATCH 09/12] tryfix - no mqtt if no mqtt wanted... --- src/mesh/Router.cpp | 50 ++++++++++++++++++++++++--------------------- 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index 66e21d21d..4f60360ce 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -222,28 +222,30 @@ ErrorCode Router::send(MeshPacket *p) ChannelIndex chIndex = p->channel; // keep as a local because we are about to change it #if HAS_WIFI || HAS_ETHERNET - // check if we should send decrypted packets to mqtt + if(moduleConfig.mqtt.enabled) { + // check if we should send decrypted packets to mqtt - // truth table: - /* mqtt_server mqtt_encryption_enabled should_encrypt - * not set 0 1 - * not set 1 1 - * set 0 0 - * set 1 1 - * - * => so we only decrypt mqtt if they have a custom mqtt server AND mqtt_encryption_enabled is FALSE - */ + // truth table: + /* mqtt_server mqtt_encryption_enabled should_encrypt + * not set 0 1 + * not set 1 1 + * set 0 0 + * set 1 1 + * + * => so we only decrypt mqtt if they have a custom mqtt server AND mqtt_encryption_enabled is FALSE + */ - bool shouldActuallyEncrypt = true; - if (*moduleConfig.mqtt.address && !moduleConfig.mqtt.encryption_enabled) { - shouldActuallyEncrypt = false; + bool shouldActuallyEncrypt = true; + if (*moduleConfig.mqtt.address && !moduleConfig.mqtt.encryption_enabled) { + shouldActuallyEncrypt = false; + } + + DEBUG_MSG("Should encrypt MQTT?: %d\n", shouldActuallyEncrypt); + + // the packet is currently in a decrypted state. send it now if they want decrypted packets + if (mqtt && !shouldActuallyEncrypt) + mqtt->onSend(*p, chIndex); } - - DEBUG_MSG("Should encrypt MQTT?: %d\n", shouldActuallyEncrypt); - - // the packet is currently in a decrypted state. send it now if they want decrypted packets - if (mqtt && !shouldActuallyEncrypt) - mqtt->onSend(*p, chIndex); #endif auto encodeResult = perhapsEncode(p); @@ -253,10 +255,12 @@ ErrorCode Router::send(MeshPacket *p) } #if HAS_WIFI || HAS_ETHERNET - // the packet is now encrypted. - // check if we should send encrypted packets to mqtt - if (mqtt && shouldActuallyEncrypt) - mqtt->onSend(*p, chIndex); + if(moduleConfig.mqtt.enabled) { + // the packet is now encrypted. + // check if we should send encrypted packets to mqtt + if (mqtt && shouldActuallyEncrypt) + mqtt->onSend(*p, chIndex); + } #endif } From e73ae7cdacdad0d3eb1ce5d06991d60d52af5cd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20G=C3=B6ttgens?= Date: Fri, 30 Dec 2022 14:53:34 +0100 Subject: [PATCH 10/12] woops - was too fast there --- src/mesh/Router.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index 4f60360ce..ee6473b2b 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -221,6 +221,8 @@ ErrorCode Router::send(MeshPacket *p) if (p->which_payload_variant == MeshPacket_decoded_tag) { ChannelIndex chIndex = p->channel; // keep as a local because we are about to change it + bool shouldActuallyEncrypt = true; + #if HAS_WIFI || HAS_ETHERNET if(moduleConfig.mqtt.enabled) { // check if we should send decrypted packets to mqtt @@ -235,7 +237,6 @@ ErrorCode Router::send(MeshPacket *p) * => so we only decrypt mqtt if they have a custom mqtt server AND mqtt_encryption_enabled is FALSE */ - bool shouldActuallyEncrypt = true; if (*moduleConfig.mqtt.address && !moduleConfig.mqtt.encryption_enabled) { shouldActuallyEncrypt = false; } @@ -255,7 +256,7 @@ ErrorCode Router::send(MeshPacket *p) } #if HAS_WIFI || HAS_ETHERNET - if(moduleConfig.mqtt.enabled) { + if (moduleConfig.mqtt.enabled) { // the packet is now encrypted. // check if we should send encrypted packets to mqtt if (mqtt && shouldActuallyEncrypt) From 8364c2b1471771b630e17030eb436302213079d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20G=C3=B6ttgens?= Date: Fri, 30 Dec 2022 17:03:48 +0100 Subject: [PATCH 11/12] more verbose thread debug --- src/Power.cpp | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/Power.cpp b/src/Power.cpp index d2c5b01a0..550c8ad37 100644 --- a/src/Power.cpp +++ b/src/Power.cpp @@ -287,8 +287,18 @@ void Power::readPowerStatus() powerStatus2.getIsCharging(), powerStatus2.getBatteryVoltageMv(), powerStatus2.getBatteryChargePercent()); newStatus.notifyObservers(&powerStatus2); #ifdef DEBUG_HEAP - if (lastheap != ESP.getFreeHeap()){ - DEBUG_MSG("Heap status: %d/%d bytes free (%d), running %d threads\n", ESP.getFreeHeap(), ESP.getHeapSize(), ESP.getFreeHeap() - lastheap , concurrency::mainController.size(false)); + if (lastheap != ESP.getFreeHeap()) { + DEBUG_MSG("Threads running:"); + int running = 0; + for(int i = 0; i < MAX_THREADS; i++){ + auto thread = concurrency::mainController.get(i); + if((thread != nullptr) && (thread->enabled)) { + DEBUG_MSG(" %s", thread->ThreadName.c_str()); + running++; + } + } + DEBUG_MSG("\n"); + DEBUG_MSG("Heap status: %d/%d bytes free (%d), running %d/%d threads\n", ESP.getFreeHeap(), ESP.getHeapSize(), ESP.getFreeHeap() - lastheap, running, concurrency::mainController.size(false)); lastheap = ESP.getFreeHeap(); } #endif From 092a753a6f938342fed358a8eb165f376ef94659 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20G=C3=B6ttgens?= Date: Fri, 30 Dec 2022 20:27:35 +0100 Subject: [PATCH 12/12] yea, well --- src/concurrency/OSThread.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/concurrency/OSThread.cpp b/src/concurrency/OSThread.cpp index 4bf4baa8c..2e5ee0ba8 100644 --- a/src/concurrency/OSThread.cpp +++ b/src/concurrency/OSThread.cpp @@ -82,9 +82,9 @@ void OSThread::run() #ifdef DEBUG_HEAP auto newHeap = ESP.getFreeHeap(); if (newHeap < heap) - DEBUG_MSG("------ Thread %s leaked heap %d -> %d (%d) ------\n", ThreadName.c_str(), heap, newHeap, newHeap - heap); + LOG_DEBUG("------ Thread %s leaked heap %d -> %d (%d) ------\n", ThreadName.c_str(), heap, newHeap, newHeap - heap); if (heap < newHeap) - DEBUG_MSG("++++++ Thread %s freed heap %d -> %d (%d) ++++++\n", ThreadName.c_str(), heap, newHeap, newHeap - heap); + LOG_DEBUG("++++++ Thread %s freed heap %d -> %d (%d) ++++++\n", ThreadName.c_str(), heap, newHeap, newHeap - heap); #endif runned();