From 463226082fa8afaede2439aa5f4795ed7d72f72c Mon Sep 17 00:00:00 2001 From: Bernhard Kirchen Date: Sat, 30 Dec 2023 18:31:50 +0100 Subject: [PATCH] clean up Huawei MQTT handler * bind the callback to a topic (enum value) such that there is no need to tokenize the full topic (string) to find out what value is being processed. tokenizing is expensive. * get rid of using the config in the callback, which improves thread-safety since the MQTT callback is running in the MQTT thread. * prefer C++ method stof to convert MQTT value to a float, which saves us from using new and delete for a buffer in particular. * prefer switch statements over if-else-trees. * split long lines. * get rid of topic #defines. * fix indention. --- include/MqttHandleHuawei.h | 14 +++- src/MqttHandleHuawei.cpp | 161 ++++++++++++++++++------------------- 2 files changed, 91 insertions(+), 84 deletions(-) diff --git a/include/MqttHandleHuawei.h b/include/MqttHandleHuawei.h index 0ced25ed..f518ed9d 100644 --- a/include/MqttHandleHuawei.h +++ b/include/MqttHandleHuawei.h @@ -15,7 +15,19 @@ public: private: void loop(); - void onMqttMessage(const espMqttClientTypes::MessageProperties& properties, const char* topic, const uint8_t* payload, size_t len, size_t index, size_t total); + + enum class Topic : unsigned { + LimitOnlineVoltage, + LimitOnlineCurrent, + LimitOfflineVoltage, + LimitOfflineCurrent, + Mode + }; + + void onMqttMessage(Topic t, + const espMqttClientTypes::MessageProperties& properties, + const char* topic, const uint8_t* payload, size_t len, + size_t index, size_t total); Task _loopTask; diff --git a/src/MqttHandleHuawei.cpp b/src/MqttHandleHuawei.cpp index f14bfe1f..4330dc7c 100644 --- a/src/MqttHandleHuawei.cpp +++ b/src/MqttHandleHuawei.cpp @@ -10,12 +10,6 @@ #include "WebApi_Huawei.h" #include -#define TOPIC_SUB_LIMIT_ONLINE_VOLTAGE "limit_online_voltage" -#define TOPIC_SUB_LIMIT_ONLINE_CURRENT "limit_online_current" -#define TOPIC_SUB_LIMIT_OFFLINE_VOLTAGE "limit_offline_voltage" -#define TOPIC_SUB_LIMIT_OFFLINE_CURRENT "limit_offline_current" -#define TOPIC_SUB_MODE "mode" - MqttHandleHuaweiClass MqttHandleHuawei; void MqttHandleHuaweiClass::init(Scheduler& scheduler) @@ -25,19 +19,22 @@ void MqttHandleHuaweiClass::init(Scheduler& scheduler) _loopTask.setIterations(TASK_FOREVER); _loopTask.enable(); - using std::placeholders::_1; - using std::placeholders::_2; - using std::placeholders::_3; - using std::placeholders::_4; - using std::placeholders::_5; - using std::placeholders::_6; + String const& prefix = MqttSettings.getPrefix(); - String topic = MqttSettings.getPrefix(); - MqttSettings.subscribe(String(topic + "huawei/cmd/" + TOPIC_SUB_LIMIT_ONLINE_VOLTAGE).c_str(), 0, std::bind(&MqttHandleHuaweiClass::onMqttMessage, this, _1, _2, _3, _4, _5, _6)); - MqttSettings.subscribe(String(topic + "huawei/cmd/" + TOPIC_SUB_LIMIT_ONLINE_CURRENT).c_str(), 0, std::bind(&MqttHandleHuaweiClass::onMqttMessage, this, _1, _2, _3, _4, _5, _6)); - MqttSettings.subscribe(String(topic + "huawei/cmd/" + TOPIC_SUB_LIMIT_OFFLINE_VOLTAGE).c_str(), 0, std::bind(&MqttHandleHuaweiClass::onMqttMessage, this, _1, _2, _3, _4, _5, _6)); - MqttSettings.subscribe(String(topic + "huawei/cmd/" + TOPIC_SUB_LIMIT_OFFLINE_CURRENT).c_str(), 0, std::bind(&MqttHandleHuaweiClass::onMqttMessage, this, _1, _2, _3, _4, _5, _6)); - MqttSettings.subscribe(String(topic + "huawei/cmd/" + TOPIC_SUB_MODE).c_str(), 0, std::bind(&MqttHandleHuaweiClass::onMqttMessage, this, _1, _2, _3, _4, _5, _6)); + auto subscribe = [&prefix, this](char const* subTopic, Topic t) { + String fullTopic(prefix + "huawei/cmd/" + subTopic); + MqttSettings.subscribe(fullTopic.c_str(), 0, + std::bind(&MqttHandleHuaweiClass::onMqttMessage, this, t, + std::placeholders::_1, std::placeholders::_2, + std::placeholders::_3, std::placeholders::_4, + std::placeholders::_5, std::placeholders::_6)); + }; + + subscribe("limit_online_voltage", Topic::LimitOnlineVoltage); + subscribe("limit_online_current", Topic::LimitOnlineCurrent); + subscribe("limit_offline_voltage", Topic::LimitOfflineVoltage); + subscribe("limit_offline_current", Topic::LimitOfflineCurrent); + subscribe("mode", Topic::Mode); _lastPublish = millis(); @@ -86,81 +83,79 @@ void MqttHandleHuaweiClass::loop() } -void MqttHandleHuaweiClass::onMqttMessage(const espMqttClientTypes::MessageProperties& properties, const char* topic, const uint8_t* payload, size_t len, size_t index, size_t total) +void MqttHandleHuaweiClass::onMqttMessage(Topic t, + const espMqttClientTypes::MessageProperties& properties, + const char* topic, const uint8_t* payload, size_t len, + size_t index, size_t total) { - const CONFIG_T& config = Configuration.get(); - - char token_topic[MQTT_MAX_TOPIC_STRLEN + 40]; // respect all subtopics - strncpy(token_topic, topic, MQTT_MAX_TOPIC_STRLEN + 40); // convert const char* to char* - - char* setting; - char* rest = &token_topic[strlen(config.Mqtt.Topic)]; - - strtok_r(rest, "/", &rest); // Remove "huawei" - strtok_r(rest, "/", &rest); // Remove "cmd" - - setting = strtok_r(rest, "/", &rest); - - if (setting == NULL) { + std::string strValue(reinterpret_cast(payload), len); + float payload_val = -1; + try { + payload_val = std::stof(strValue); + } + catch (std::invalid_argument const& e) { + MessageOutput.printf("Huawei MQTT handler: cannot parse payload of topic '%s' as float: %s\r\n", + topic, strValue.c_str()); return; } - char* strlimit = new char[len + 1]; - memcpy(strlimit, payload, len); - strlimit[len] = '\0'; - float payload_val = strtof(strlimit, NULL); - delete[] strlimit; - std::lock_guard mqttLock(_mqttMutex); - if (!strcmp(setting, TOPIC_SUB_LIMIT_ONLINE_VOLTAGE)) { - // Set voltage limit - MessageOutput.printf("Limit Voltage: %f V\r\n", payload_val); - _mqttCallbacks.push_back(std::bind(&HuaweiCanClass::setValue, - &HuaweiCan, payload_val, HUAWEI_ONLINE_VOLTAGE)); + switch (t) { + case Topic::LimitOnlineVoltage: + MessageOutput.printf("Limit Voltage: %f V\r\n", payload_val); + _mqttCallbacks.push_back(std::bind(&HuaweiCanClass::setValue, + &HuaweiCan, payload_val, HUAWEI_ONLINE_VOLTAGE)); + break; - } else if (!strcmp(setting, TOPIC_SUB_LIMIT_OFFLINE_VOLTAGE)) { - // Set current limit - MessageOutput.printf("Offline Limit Voltage: %f V\r\n", payload_val); - _mqttCallbacks.push_back(std::bind(&HuaweiCanClass::setValue, - &HuaweiCan, payload_val, HUAWEI_OFFLINE_VOLTAGE)); + case Topic::LimitOfflineVoltage: + MessageOutput.printf("Offline Limit Voltage: %f V\r\n", payload_val); + _mqttCallbacks.push_back(std::bind(&HuaweiCanClass::setValue, + &HuaweiCan, payload_val, HUAWEI_OFFLINE_VOLTAGE)); + break; - } else if (!strcmp(setting, TOPIC_SUB_LIMIT_ONLINE_CURRENT)) { - // Set current limit - MessageOutput.printf("Limit Current: %f A\r\n", payload_val); - _mqttCallbacks.push_back(std::bind(&HuaweiCanClass::setValue, - &HuaweiCan, payload_val, HUAWEI_ONLINE_CURRENT)); + case Topic::LimitOnlineCurrent: + MessageOutput.printf("Limit Current: %f A\r\n", payload_val); + _mqttCallbacks.push_back(std::bind(&HuaweiCanClass::setValue, + &HuaweiCan, payload_val, HUAWEI_ONLINE_CURRENT)); + break; - } else if (!strcmp(setting, TOPIC_SUB_LIMIT_OFFLINE_CURRENT)) { - // Set current limit - MessageOutput.printf("Offline Limit Current: %f A\r\n", payload_val); - _mqttCallbacks.push_back(std::bind(&HuaweiCanClass::setValue, - &HuaweiCan, payload_val, HUAWEI_OFFLINE_CURRENT)); + case Topic::LimitOfflineCurrent: + MessageOutput.printf("Offline Limit Current: %f A\r\n", payload_val); + _mqttCallbacks.push_back(std::bind(&HuaweiCanClass::setValue, + &HuaweiCan, payload_val, HUAWEI_OFFLINE_CURRENT)); + break; - } else if (!strcmp(setting, TOPIC_SUB_MODE)) { - // Control power on/off - if(payload_val == 3) { - MessageOutput.println("[Huawei MQTT::] Received MQTT msg. New mode: Full internal control"); - _mqttCallbacks.push_back(std::bind(&HuaweiCanClass::setMode, - &HuaweiCan, HUAWEI_MODE_AUTO_INT)); - } + case Topic::Mode: + switch (static_cast(payload_val)) { + case 3: + MessageOutput.println("[Huawei MQTT::] Received MQTT msg. New mode: Full internal control"); + _mqttCallbacks.push_back(std::bind(&HuaweiCanClass::setMode, + &HuaweiCan, HUAWEI_MODE_AUTO_INT)); + break; - if(payload_val == 2) { - MessageOutput.println("[Huawei MQTT::] Received MQTT msg. New mode: Internal on/off control, external power limit"); - _mqttCallbacks.push_back(std::bind(&HuaweiCanClass::setMode, - &HuaweiCan, HUAWEI_MODE_AUTO_EXT)); - } + case 2: + MessageOutput.println("[Huawei MQTT::] Received MQTT msg. New mode: Internal on/off control, external power limit"); + _mqttCallbacks.push_back(std::bind(&HuaweiCanClass::setMode, + &HuaweiCan, HUAWEI_MODE_AUTO_EXT)); + break; - if(payload_val == 1) { - MessageOutput.println("[Huawei MQTT::] Received MQTT msg. New mode: Turned ON"); - _mqttCallbacks.push_back(std::bind(&HuaweiCanClass::setMode, - &HuaweiCan, HUAWEI_MODE_ON)); - } - - if(payload_val == 0) { - MessageOutput.println("[Huawei MQTT::] Received MQTT msg. New mode: Turned OFF"); - _mqttCallbacks.push_back(std::bind(&HuaweiCanClass::setMode, - &HuaweiCan, HUAWEI_MODE_OFF)); - } - } + case 1: + MessageOutput.println("[Huawei MQTT::] Received MQTT msg. New mode: Turned ON"); + _mqttCallbacks.push_back(std::bind(&HuaweiCanClass::setMode, + &HuaweiCan, HUAWEI_MODE_ON)); + break; + + case 0: + MessageOutput.println("[Huawei MQTT::] Received MQTT msg. New mode: Turned OFF"); + _mqttCallbacks.push_back(std::bind(&HuaweiCanClass::setMode, + &HuaweiCan, HUAWEI_MODE_OFF)); + break; + + default: + MessageOutput.printf("[Huawei MQTT::] Invalid mode %.0f\r\n", payload_val); + break; + } + break; + } } \ No newline at end of file