From 96ee78156da87f107b5aa6730537537b041fbd8b Mon Sep 17 00:00:00 2001 From: Bernhard Kirchen Date: Mon, 28 Aug 2023 13:20:56 +0200 Subject: [PATCH] Fix DPL Mode 2 MQTT Status (#402) * DPL MQTT handler: modernize * there is no need to tokenize and check the topic of a received MQTT message if we only subscribe to a single topic. all messages will be for that topic. avoid testing the topic in the callback alltogether. * use std::string and std::stoi over allocating and deleting a buffer and copying charactes around. * use a switch statement to process the actual payload. * break a long line. * DPL: fix getMode() return value getMode() returned a bool. probably its return type was not adjusted when the third mode was introduced. this lead to mode 2 being cast to true implicitly, which in turn was used to construct a String, such that "1" was published as the DPL mode when in fact it was 2. make the mode an enum class to avoid such problems in the future. inline getMode() and setMode(). fix indention. --- include/MqttHandlePowerLimiter.h | 2 +- include/PowerLimiter.h | 13 +++-- src/MqttHandlePowerLimiter.cpp | 82 ++++++++++++++------------------ src/PowerLimiter.cpp | 12 +---- src/WebApi_powerlimiter.cpp | 2 +- 5 files changed, 50 insertions(+), 61 deletions(-) diff --git a/include/MqttHandlePowerLimiter.h b/include/MqttHandlePowerLimiter.h index 82d736ea..d52d202d 100644 --- a/include/MqttHandlePowerLimiter.h +++ b/include/MqttHandlePowerLimiter.h @@ -10,7 +10,7 @@ public: void loop(); private: - void onMqttMessage(const espMqttClientTypes::MessageProperties& properties, const char* topic, const uint8_t* payload, size_t len, size_t index, size_t total); + void onCmdMode(const espMqttClientTypes::MessageProperties& properties, const char* topic, const uint8_t* payload, size_t len, size_t index, size_t total); uint32_t _lastPublishStats; uint32_t _lastPublish; diff --git a/include/PowerLimiter.h b/include/PowerLimiter.h index 34e6920a..6bf1497a 100644 --- a/include/PowerLimiter.h +++ b/include/PowerLimiter.h @@ -51,8 +51,15 @@ public: void loop(); uint8_t getPowerLimiterState(); int32_t getLastRequestedPowerLimit(); - void setMode(uint8_t mode); - bool getMode(); + + enum class Mode : unsigned { + Normal = 0, + Disabled = 1, + UnconditionalFullSolarPassthrough = 2 + }; + + void setMode(Mode m) { _mode = m; } + Mode getMode() const { return _mode; } void calcNextInverterRestart(); private: @@ -63,7 +70,7 @@ private: uint32_t _lastCalculation = 0; static constexpr uint32_t _calculationBackoffMsDefault = 128; uint32_t _calculationBackoffMs = _calculationBackoffMsDefault; - uint8_t _mode = PL_MODE_ENABLE_NORMAL_OP; + Mode _mode = Mode::Normal; std::shared_ptr _inverter = nullptr; bool _batteryDischargeEnabled = false; uint32_t _nextInverterRestart = 0; // Values: 0->not calculated / 1->no restart configured / >1->time of next inverter restart in millis() diff --git a/src/MqttHandlePowerLimiter.cpp b/src/MqttHandlePowerLimiter.cpp index 56ec1a16..d5d6987e 100644 --- a/src/MqttHandlePowerLimiter.cpp +++ b/src/MqttHandlePowerLimiter.cpp @@ -7,8 +7,7 @@ #include "MqttHandlePowerLimiter.h" #include "PowerLimiter.h" #include - -#define TOPIC_SUB_POWER_LIMITER "mode" +#include MqttHandlePowerLimiterClass MqttHandlePowerLimiter; @@ -21,11 +20,10 @@ void MqttHandlePowerLimiterClass::init() using std::placeholders::_5; using std::placeholders::_6; - String topic = MqttSettings.getPrefix(); - MqttSettings.subscribe(String(topic + "powerlimiter/cmd/" + TOPIC_SUB_POWER_LIMITER).c_str(), 0, std::bind(&MqttHandlePowerLimiterClass::onMqttMessage, this, _1, _2, _3, _4, _5, _6)); + String topic = MqttSettings.getPrefix() + "powerlimiter/cmd/mode"; + MqttSettings.subscribe(topic.c_str(), 0, std::bind(&MqttHandlePowerLimiterClass::onCmdMode, this, _1, _2, _3, _4, _5, _6)); _lastPublish = millis(); - } @@ -38,60 +36,52 @@ void MqttHandlePowerLimiterClass::loop() const CONFIG_T& config = Configuration.get(); if ((millis() - _lastPublish) > (config.Mqtt_PublishInterval * 1000) ) { - MqttSettings.publish("powerlimiter/status/mode", String(PowerLimiter.getMode())); + auto val = static_cast(PowerLimiter.getMode()); + MqttSettings.publish("powerlimiter/status/mode", String(val)); - yield(); - _lastPublish = millis(); + yield(); + _lastPublish = millis(); } } -void MqttHandlePowerLimiterClass::onMqttMessage(const espMqttClientTypes::MessageProperties& properties, const char* topic, const uint8_t* payload, size_t len, size_t index, size_t total) +void MqttHandlePowerLimiterClass::onCmdMode(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(); - + // ignore messages if PowerLimiter is disabled if (!config.PowerLimiter_Enabled) { return; } - 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 "powerlimiter" - strtok_r(rest, "/", &rest); // Remove "cmd" - - setting = strtok_r(rest, "/", &rest); - - if (setting == NULL) { + std::string strValue(reinterpret_cast(payload), len); + int intValue = -1; + try { + intValue = std::stoi(strValue); + } + catch (std::invalid_argument const& e) { + MessageOutput.printf("PowerLimiter MQTT handler: cannot parse payload of topic '%s' as int: %s\r\n", + topic, strValue.c_str()); return; } - char* str = new char[len + 1]; - memcpy(str, payload, len); - str[len] = '\0'; - uint8_t payload_val = atoi(str); - delete[] str; - - if (!strcmp(setting, TOPIC_SUB_POWER_LIMITER)) { - if(payload_val == 2) { - MessageOutput.println("Power limiter full solar PT"); - PowerLimiter.setMode(PL_MODE_SOLAR_PT_ONLY); - return; - } - if(payload_val == 1) { - MessageOutput.println("Power limiter disabled"); - PowerLimiter.setMode(PL_MODE_FULL_DISABLE); - return; - } - if(payload_val == 0) { - MessageOutput.println("Power limiter enabled"); - PowerLimiter.setMode(PL_MODE_ENABLE_NORMAL_OP); - return; - } - MessageOutput.println("Power limiter enable / disable - unknown command received. Please use 0 or 1"); - } + using Mode = PowerLimiterClass::Mode; + switch (static_cast(intValue)) { + case Mode::UnconditionalFullSolarPassthrough: + MessageOutput.println("Power limiter unconditional full solar PT"); + PowerLimiter.setMode(Mode::UnconditionalFullSolarPassthrough); + break; + case Mode::Disabled: + MessageOutput.println("Power limiter disabled (override)"); + PowerLimiter.setMode(Mode::Disabled); + break; + case Mode::Normal: + MessageOutput.println("Power limiter normal operation"); + PowerLimiter.setMode(Mode::Normal); + break; + default: + MessageOutput.printf("PowerLimiter - unknown mode %d\r\n", intValue); + break; + } } \ No newline at end of file diff --git a/src/PowerLimiter.cpp b/src/PowerLimiter.cpp index 3e031a98..354f2585 100644 --- a/src/PowerLimiter.cpp +++ b/src/PowerLimiter.cpp @@ -131,7 +131,7 @@ void PowerLimiterClass::loop() return; } - if (PL_MODE_FULL_DISABLE == _mode) { + if (Mode::Disabled == _mode) { shutdown(Status::DisabledByMqtt); return; } @@ -185,7 +185,7 @@ void PowerLimiterClass::loop() return announceStatus(Status::InverterDevInfoPending); } - if (PL_MODE_SOLAR_PT_ONLY == _mode) { + if (Mode::UnconditionalFullSolarPassthrough == _mode) { // handle this mode of operation separately return unconditionalSolarPassthrough(_inverter); } @@ -398,14 +398,6 @@ int32_t PowerLimiterClass::getLastRequestedPowerLimit() { return _lastRequestedPowerLimit; } -bool PowerLimiterClass::getMode() { - return _mode; -} - -void PowerLimiterClass::setMode(uint8_t mode) { - _mode = mode; -} - bool PowerLimiterClass::canUseDirectSolarPower() { CONFIG_T& config = Configuration.get(); diff --git a/src/WebApi_powerlimiter.cpp b/src/WebApi_powerlimiter.cpp index cb7c7c4b..7aa6f4cc 100644 --- a/src/WebApi_powerlimiter.cpp +++ b/src/WebApi_powerlimiter.cpp @@ -125,7 +125,7 @@ void WebApiPowerLimiterClass::onAdminPost(AsyncWebServerRequest* request) CONFIG_T& config = Configuration.get(); config.PowerLimiter_Enabled = root[F("enabled")].as(); - PowerLimiter.setMode(PL_MODE_ENABLE_NORMAL_OP); // User input sets PL to normal operation + PowerLimiter.setMode(PowerLimiterClass::Mode::Normal); // User input sets PL to normal operation config.PowerLimiter_VerboseLogging = root[F("verbose_logging")].as(); config.PowerLimiter_SolarPassThroughEnabled = root[F("solar_passthrough_enabled")].as(); config.PowerLimiter_SolarPassThroughLosses = root[F("solar_passthrough_losses")].as();