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

@@ -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;

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<InverterAbstract> _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()

src/MqttHandlePowerLimiter.cpp

@@ -7,8 +7,7 @@
 #include "MqttHandlePowerLimiter.h"
 #include "PowerLimiter.h"
 #include <ctime>
-
-#define TOPIC_SUB_POWER_LIMITER "mode"
+#include <string>
 
 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,15 +36,17 @@ 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<unsigned>(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();
 
@@ -55,43 +55,33 @@ void MqttHandlePowerLimiterClass::onMqttMessage(const espMqttClientTypes::Messag
         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<const char*>(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<Mode>(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;
     }
 }

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();

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<bool>();
-    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<bool>();
     config.PowerLimiter_SolarPassThroughEnabled = root[F("solar_passthrough_enabled")].as<bool>();
     config.PowerLimiter_SolarPassThroughLosses = root[F("solar_passthrough_losses")].as<uint8_t>();