From 42b24d45105b9c352e7ee3b5018eee21e6a6f0e8 Mon Sep 17 00:00:00 2001 From: Ben Meadors Date: Tue, 3 Jan 2023 14:32:28 -0600 Subject: [PATCH 1/7] Yank mqtt service envelope queue --- src/mqtt/MQTT.cpp | 55 ++++++----------------------------------------- 1 file changed, 6 insertions(+), 49 deletions(-) diff --git a/src/mqtt/MQTT.cpp b/src/mqtt/MQTT.cpp index f4004139a..f72271324 100644 --- a/src/mqtt/MQTT.cpp +++ b/src/mqtt/MQTT.cpp @@ -21,10 +21,6 @@ String statusTopic = "msh/2/stat/"; String cryptTopic = "msh/2/c/"; // msh/2/c/CHANNELID/NODEID String jsonTopic = "msh/2/json/"; // msh/2/json/CHANNELID/NODEID -static MemoryDynamic staticMqttPool; - -Allocator &mqttPool = staticMqttPool; - void MQTT::mqttCallback(char *topic, byte *payload, unsigned int length) { mqtt->onPublish(topic, payload, length); @@ -126,7 +122,7 @@ void mqttInit() new MQTT(); } -MQTT::MQTT() : concurrency::OSThread("mqtt"), pubSub(mqttClient), mqttQueue(MAX_MQTT_QUEUE) +MQTT::MQTT() : concurrency::OSThread("mqtt"), pubSub(mqttClient) { if(moduleConfig.mqtt.enabled) { @@ -253,36 +249,9 @@ int32_t MQTT::runOnce() if (!pubSub.loop()) { if (wantConnection) { reconnect(); - - // If we succeeded, empty the queue one by one and start reading rapidly, else try again in 30 seconds (TCP connections are EXPENSIVE so try rarely) - if (pubSub.connected()) { - if (!mqttQueue.isEmpty()) { - // FIXME - this size calculation is super sloppy, but it will go away once we dynamically alloc meshpackets - ServiceEnvelope *env = mqttQueue.dequeuePtr(0); - static uint8_t bytes[MeshPacket_size + 64]; - size_t numBytes = pb_encode_to_bytes(bytes, sizeof(bytes), &ServiceEnvelope_msg, env); - - String topic = cryptTopic + env->channel_id + "/" + owner.id; - LOG_INFO("publish %s, %u bytes from queue\n", topic.c_str(), numBytes); - - - pubSub.publish(topic.c_str(), bytes, numBytes, false); - - if (moduleConfig.mqtt.json_enabled) { - // handle json topic - auto jsonString = this->downstreamPacketToJson(env->packet); - if (jsonString.length() != 0) { - String topicJson = jsonTopic + env->channel_id + "/" + owner.id; - LOG_INFO("JSON publish message to %s, %u bytes: %s\n", topicJson.c_str(), jsonString.length(), jsonString.c_str()); - pubSub.publish(topicJson.c_str(), jsonString.c_str(), false); - } - } - mqttPool.release(env); - } - return 20; - } else { - return 30000; - } + + // If we succeeded, start reading rapidly, else try again in 30 seconds (TCP connections are EXPENSIVE so try rarely) + return pubSub.connected() ? 20 : 30000; } else return 5000; // If we don't want connection now, check again in 5 secs } else { @@ -304,7 +273,7 @@ void MQTT::onSend(const MeshPacket &mp, ChannelIndex chIndex) if (ch.settings.uplink_enabled) { const char *channelId = channels.getGlobalId(chIndex); // FIXME, for now we just use the human name for the channel - ServiceEnvelope *env = mqttPool.allocZeroed(); + ServiceEnvelope env = ServiceEnvelope_init_default; env->channel_id = (char *)channelId; env->gateway_id = owner.id; env->packet = (MeshPacket *)∓ @@ -314,7 +283,7 @@ void MQTT::onSend(const MeshPacket &mp, ChannelIndex chIndex) // FIXME - this size calculation is super sloppy, but it will go away once we dynamically alloc meshpackets static uint8_t bytes[MeshPacket_size + 64]; - size_t numBytes = pb_encode_to_bytes(bytes, sizeof(bytes), &ServiceEnvelope_msg, env); + size_t numBytes = pb_encode_to_bytes(bytes, sizeof(bytes), &ServiceEnvelope_msg, &env); String topic = cryptTopic + channelId + "/" + owner.id; LOG_DEBUG("publish %s, %u bytes\n", topic.c_str(), numBytes); @@ -330,19 +299,7 @@ void MQTT::onSend(const MeshPacket &mp, ChannelIndex chIndex) pubSub.publish(topicJson.c_str(), jsonString.c_str(), false); } } - } else { - LOG_INFO("MQTT not connected, queueing packet\n"); - if (mqttQueue.numFree() == 0) { - LOG_WARN("NOTE: MQTT queue is full, discarding oldest\n"); - ServiceEnvelope *d = mqttQueue.dequeuePtr(0); - if (d) - mqttPool.release(d); - } - // make a copy of serviceEnvelope and queue it - ServiceEnvelope *copied = mqttPool.allocCopy(*env); - assert(mqttQueue.enqueue(copied, 0)); } - mqttPool.release(env); } } From 5867038abf196cfd99580ff7f19fcf6b886e047e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20G=C3=B6ttgens?= Date: Tue, 3 Jan 2023 22:09:35 +0100 Subject: [PATCH 2/7] trybuildfix mqtt system --- src/mqtt/MQTT.cpp | 6 +++--- src/mqtt/MQTT.h | 6 ------ 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/src/mqtt/MQTT.cpp b/src/mqtt/MQTT.cpp index f72271324..49ad586dd 100644 --- a/src/mqtt/MQTT.cpp +++ b/src/mqtt/MQTT.cpp @@ -274,9 +274,9 @@ void MQTT::onSend(const MeshPacket &mp, ChannelIndex chIndex) const char *channelId = channels.getGlobalId(chIndex); // FIXME, for now we just use the human name for the channel ServiceEnvelope env = ServiceEnvelope_init_default; - env->channel_id = (char *)channelId; - env->gateway_id = owner.id; - env->packet = (MeshPacket *)∓ + env.channel_id = (char *)channelId; + env.gateway_id = owner.id; + env.packet = (MeshPacket *)∓ // don't bother sending if not connected... if (pubSub.connected()) { diff --git a/src/mqtt/MQTT.h b/src/mqtt/MQTT.h index ddbacbcc4..9c813711a 100644 --- a/src/mqtt/MQTT.h +++ b/src/mqtt/MQTT.h @@ -13,8 +13,6 @@ #include #endif -#define MAX_MQTT_QUEUE 32 - /** * Our wrapper/singleton for sending/receiving MQTT "udp" packets. This object isolates the MQTT protocol implementation from * the two components that use it: MQTTPlugin and MQTTSimInterface. @@ -55,10 +53,6 @@ class MQTT : private concurrency::OSThread bool connected(); protected: - PointerQueue mqttQueue; - - int reconnectCount = 0; - virtual int32_t runOnce() override; private: From cad5c9b70c7d242eb897bc624e2eade9497261cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20G=C3=B6ttgens?= Date: Tue, 3 Jan 2023 22:17:04 +0100 Subject: [PATCH 3/7] removed too much --- src/mqtt/MQTT.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/mqtt/MQTT.h b/src/mqtt/MQTT.h index 9c813711a..33fbbb8eb 100644 --- a/src/mqtt/MQTT.h +++ b/src/mqtt/MQTT.h @@ -53,6 +53,9 @@ class MQTT : private concurrency::OSThread bool connected(); protected: + + int reconnectCount = 0; + virtual int32_t runOnce() override; private: From cab5fcf5ae3c2e9650f8747994ad0efa3df3e72b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20G=C3=B6ttgens?= Date: Tue, 3 Jan 2023 22:35:24 +0100 Subject: [PATCH 4/7] no excessive heap debugging on release builds --- arch/esp32/esp32.ini | 1 - arch/esp32/esp32s2.ini | 1 - arch/esp32/esp32s3.ini | 1 - 3 files changed, 3 deletions(-) diff --git a/arch/esp32/esp32.ini b/arch/esp32/esp32.ini index 0011cc39f..70654e8ec 100644 --- a/arch/esp32/esp32.ini +++ b/arch/esp32/esp32.ini @@ -26,7 +26,6 @@ build_flags = -DCONFIG_NIMBLE_CPP_LOG_LEVEL=2 -DCONFIG_BT_NIMBLE_MAX_CCCDS=20 -DESP_OPENSSL_SUPPRESS_LEGACY_WARNING - -DDEBUG_HEAP lib_deps = ${arduino_base.lib_deps} diff --git a/arch/esp32/esp32s2.ini b/arch/esp32/esp32s2.ini index ca4f576d6..cd47c4ca1 100644 --- a/arch/esp32/esp32s2.ini +++ b/arch/esp32/esp32s2.ini @@ -27,7 +27,6 @@ build_flags = -DCONFIG_BT_NIMBLE_MAX_CCCDS=20 -DESP_OPENSSL_SUPPRESS_LEGACY_WARNING -DHAS_BLUETOOTH=0 - -DDEBUG_HEAP lib_deps = ${arduino_base.lib_deps} diff --git a/arch/esp32/esp32s3.ini b/arch/esp32/esp32s3.ini index b276ceff9..ce64fdbe2 100644 --- a/arch/esp32/esp32s3.ini +++ b/arch/esp32/esp32s3.ini @@ -26,7 +26,6 @@ build_flags = -DCONFIG_NIMBLE_CPP_LOG_LEVEL=2 -DCONFIG_BT_NIMBLE_MAX_CCCDS=20 -DESP_OPENSSL_SUPPRESS_LEGACY_WARNING - -DDEBUG_HEAP lib_deps = ${arduino_base.lib_deps} From 6d989a29dd8975f862ea840c2b0a244fb1feebfb Mon Sep 17 00:00:00 2001 From: thebentern Date: Tue, 3 Jan 2023 22:34:34 +0000 Subject: [PATCH 5/7] [create-pull-request] automated change --- version.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.properties b/version.properties index 5c253336e..816f7c2ac 100644 --- a/version.properties +++ b/version.properties @@ -1,4 +1,4 @@ [VERSION] major = 2 minor = 0 -build = 11 +build = 12 From 78b6916b1b020810f7f131a2c6878f2a759ead3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20G=C3=B6ttgens?= Date: Wed, 4 Jan 2023 14:45:28 +0100 Subject: [PATCH 6/7] External Notification Hotfix --- src/modules/ExternalNotificationModule.cpp | 79 ++++++++++------------ src/modules/ExternalNotificationModule.h | 14 ++++ 2 files changed, 51 insertions(+), 42 deletions(-) diff --git a/src/modules/ExternalNotificationModule.cpp b/src/modules/ExternalNotificationModule.cpp index b24b0bbc0..e0014f98c 100644 --- a/src/modules/ExternalNotificationModule.cpp +++ b/src/modules/ExternalNotificationModule.cpp @@ -42,26 +42,22 @@ int32_t ExternalNotificationModule::runOnce() if (!moduleConfig.external_notification.enabled) { return INT32_MAX; // we don't need this thread here... } else { -#ifndef ARCH_PORTDUINO if ((nagCycleCutoff < millis()) && !rtttl::isPlaying()) { -#else - if (nagCycleCutoff < millis()) { -#endif + // let the song finish if we reach timeout nagCycleCutoff = UINT32_MAX; LOG_INFO("Turning off external notification: "); for (int i = 0; i < 2; i++) { - if (getExternal(i)) { - setExternalOff(i); - externalTurnedOn[i] = 0; - LOG_INFO("%d ", i); - } + setExternalOff(i); + externalTurnedOn[i] = 0; + LOG_INFO("%d ", i); } LOG_INFO("\n"); + isNagging = false; return INT32_MAX; // save cycles till we're needed again } // If the output is turned on, turn it back off after the given period of time. - if (nagCycleCutoff != UINT32_MAX) { + if (isNagging) { if (externalTurnedOn[0] + (moduleConfig.external_notification.output_ms ? moduleConfig.external_notification.output_ms : EXT_NOTIFICATION_MODULE_OUTPUT_MS) < millis()) { @@ -80,16 +76,14 @@ int32_t ExternalNotificationModule::runOnce() } // now let the PWM buzzer play -#ifndef ARCH_PORTDUINO if (moduleConfig.external_notification.use_pwm) { if (rtttl::isPlaying()) { rtttl::play(); - } else if (nagCycleCutoff >= millis()) { + } else if (isNagging && (nagCycleCutoff >= millis())) { // start the song again if we have time left rtttl::begin(config.device.buzzer_gpio, rtttlConfig.ringtone); } } -#endif return 25; } } @@ -140,10 +134,9 @@ bool ExternalNotificationModule::getExternal(uint8_t index) } void ExternalNotificationModule::stopNow() { -#ifndef ARCH_PORTDUINO rtttl::stop(); -#endif nagCycleCutoff = 1; // small value + isNagging = false; setIntervalFromNow(0); } @@ -230,6 +223,7 @@ ProcessMessage ExternalNotificationModule::handleReceived(const MeshPacket &mp) if (moduleConfig.external_notification.alert_bell) { if (containsBell) { LOG_INFO("externalNotificationModule - Notification Bell\n"); + isNagging = true; setExternalOn(0); if (moduleConfig.external_notification.nag_timeout) { nagCycleCutoff = millis() + moduleConfig.external_notification.nag_timeout * 1000; @@ -242,6 +236,7 @@ ProcessMessage ExternalNotificationModule::handleReceived(const MeshPacket &mp) if (moduleConfig.external_notification.alert_bell_vibra) { if (containsBell) { LOG_INFO("externalNotificationModule - Notification Bell (Vibra)\n"); + isNagging = true; setExternalOn(1); if (moduleConfig.external_notification.nag_timeout) { nagCycleCutoff = millis() + moduleConfig.external_notification.nag_timeout * 1000; @@ -254,12 +249,11 @@ ProcessMessage ExternalNotificationModule::handleReceived(const MeshPacket &mp) if (moduleConfig.external_notification.alert_bell_buzzer) { if (containsBell) { LOG_INFO("externalNotificationModule - Notification Bell (Buzzer)\n"); + isNagging = true; if (!moduleConfig.external_notification.use_pwm) { setExternalOn(2); } else { -#ifndef ARCH_PORTDUINO rtttl::begin(config.device.buzzer_gpio, rtttlConfig.ringtone); -#endif } if (moduleConfig.external_notification.nag_timeout) { nagCycleCutoff = millis() + moduleConfig.external_notification.nag_timeout * 1000; @@ -271,6 +265,7 @@ ProcessMessage ExternalNotificationModule::handleReceived(const MeshPacket &mp) if (moduleConfig.external_notification.alert_message) { LOG_INFO("externalNotificationModule - Notification Module\n"); + isNagging = true; setExternalOn(0); if (moduleConfig.external_notification.nag_timeout) { nagCycleCutoff = millis() + moduleConfig.external_notification.nag_timeout * 1000; @@ -279,33 +274,33 @@ ProcessMessage ExternalNotificationModule::handleReceived(const MeshPacket &mp) } } - if (!moduleConfig.external_notification.use_pwm) { - if (moduleConfig.external_notification.alert_message_vibra) { - LOG_INFO("externalNotificationModule - Notification Module (Vibra)\n"); - setExternalOn(1); - if (moduleConfig.external_notification.nag_timeout) { - nagCycleCutoff = millis() + moduleConfig.external_notification.nag_timeout * 1000; - } else { - nagCycleCutoff = millis() + moduleConfig.external_notification.output_ms; - } - } - - if (moduleConfig.external_notification.alert_message_buzzer) { - LOG_INFO("externalNotificationModule - Notification Module (Buzzer)\n"); - if (!moduleConfig.external_notification.use_pwm) { - setExternalOn(2); - } else { -#ifndef ARCH_PORTDUINO - rtttl::begin(config.device.buzzer_gpio, rtttlConfig.ringtone); -#endif - } - if (moduleConfig.external_notification.nag_timeout) { - nagCycleCutoff = millis() + moduleConfig.external_notification.nag_timeout * 1000; - } else { - nagCycleCutoff = millis() + moduleConfig.external_notification.output_ms; - } + + if (moduleConfig.external_notification.alert_message_vibra) { + LOG_INFO("externalNotificationModule - Notification Module (Vibra)\n"); + isNagging = true; + setExternalOn(1); + if (moduleConfig.external_notification.nag_timeout) { + nagCycleCutoff = millis() + moduleConfig.external_notification.nag_timeout * 1000; + } else { + nagCycleCutoff = millis() + moduleConfig.external_notification.output_ms; } } + + if (moduleConfig.external_notification.alert_message_buzzer) { + LOG_INFO("externalNotificationModule - Notification Module (Buzzer)\n"); + isNagging = true; + if (!moduleConfig.external_notification.use_pwm) { + setExternalOn(2); + } else { + rtttl::begin(config.device.buzzer_gpio, rtttlConfig.ringtone); + } + if (moduleConfig.external_notification.nag_timeout) { + nagCycleCutoff = millis() + moduleConfig.external_notification.nag_timeout * 1000; + } else { + nagCycleCutoff = millis() + moduleConfig.external_notification.output_ms; + } + } + setIntervalFromNow(0); // run once so we know if we should do something } diff --git a/src/modules/ExternalNotificationModule.h b/src/modules/ExternalNotificationModule.h index 50af360c1..097ae96b3 100644 --- a/src/modules/ExternalNotificationModule.h +++ b/src/modules/ExternalNotificationModule.h @@ -5,6 +5,18 @@ #include "configuration.h" #ifndef ARCH_PORTDUINO #include +#else +// Noop class for portduino. +class rtttl +{ + public: + explicit rtttl() {} + static bool isPlaying() { return false; } + static void play() {} + static void begin(byte a, const char * b) {}; + static void stop() {} + static bool done() { return true; } +}; #endif #include #include @@ -39,6 +51,8 @@ class ExternalNotificationModule : public SinglePortModule, private concurrency: virtual int32_t runOnce() override; + bool isNagging = false; + virtual AdminMessageHandleResult handleAdminMessageForModule(const MeshPacket &mp, AdminMessage *request, AdminMessage *response) override; }; From e5d9f1f946306c3d955c62f263f40d78677fa558 Mon Sep 17 00:00:00 2001 From: thebentern Date: Wed, 4 Jan 2023 16:13:43 +0000 Subject: [PATCH 7/7] [create-pull-request] automated change --- version.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.properties b/version.properties index 816f7c2ac..40e7926a9 100644 --- a/version.properties +++ b/version.properties @@ -1,4 +1,4 @@ [VERSION] major = 2 minor = 0 -build = 12 +build = 13