[Cleanup] Add CommandArgParser to simplify processing commands

src/_Plugin_Helper.h

@@ -30,6 +30,7 @@
 #include "src/Globals/Services.h"
 
 #include "src/Helpers/_Plugin_init.h"
+#include "src/Helpers/CommandArgParser.h"
 #include "src/Helpers/ESPEasy_math.h"
 #include "src/Helpers/ESPEasy_Storage.h"
 #include "src/Helpers/ESPEasy_time_calc.h"

src/src/DataStructs/KeyValueStruct.cpp

@@ -132,6 +132,17 @@ KeyValueStruct::KeyValueStruct(const String& key,
   _values.emplace_back(ValueStruct(std::move(val)));
 }
 
+void KeyValueStruct::clear()
+{
+  _key.clear();
+  _values.clear();
+  _format = Format::Default;
+#if FEATURE_TASKVALUE_UNIT_OF_MEASURE
+  _uomIndex = 0;
+#endif
+  _isArray = false;
+}
+
 KeyValueStruct KeyValueStruct::makeHexFormatted(const __FlashStringHelper *key, uint64_t val, uint8_t minNrDigits)
 {
   KeyValueStruct kv(

src/src/DataStructs/KeyValueStruct.h

@@ -109,6 +109,8 @@ struct KeyValueStruct
      }
    */
 
+  void                  clear();
+
   static KeyValueStruct makeHexFormatted(const __FlashStringHelper *key,
                                          uint64_t                   val,
                                          uint8_t                    minNrDigits = 0);

src/src/DataStructs/ValueStruct.cpp

@@ -10,6 +10,8 @@
 #define VALUE_STRUCT_SSO_FIRST_CHAR   bytes_all[VALUE_STRUCT_SSO_FIRST_CHAR_INDEX]
 #define VALUE_STRUCT_SSO_MAX_SIZE          14
 
+const ValueStruct INVALID_VALUESTRUCT{};
+
 // ********************************************************************************
 // ValueStruct
 // ********************************************************************************
@@ -386,23 +388,23 @@ ValueStruct ValueStruct::makeFromString(const String& val)
   return ValueStruct(val);
 }
 
-String ValueStruct::toString() const
+String ValueStruct::toString(bool unformatted) const
 {
   ValueType valueType;
 
-  return toString(valueType);
+  return toString(valueType, unformatted);
 }
 
-String ValueStruct::toString(ValueType& valueType) const
+String ValueStruct::toString(ValueType& valueType, bool unformatted) const
 {
   PrintToString p;
 
-  print(p, valueType);
+  print(p, valueType, unformatted);
   String res(p.getMove());
   return res;
 }
 
-int64_t ValueStruct::toInt() const
+int64_t ValueStruct::toInt(int64_t defaultValue) const
 {
   switch (getValueType())
   {
@@ -435,10 +437,11 @@ int64_t ValueStruct::toInt() const
       break;
     case ValueStruct::ValueType::String:
     case ValueStruct::ValueType::FlashString:
+      return toString().toInt();
     case ValueStruct::ValueType::Unset:
       break;
   }
-  return toString().toInt();
+  return defaultValue;
 }
 
 double ValueStruct::toFloat() const
@@ -500,12 +503,12 @@ size_t ValueStruct::formatCase(Print& out, String&& str) const
   return out.print(str);
 }
 
-size_t ValueStruct::print(Print& out, ValueType& valueType) const
+size_t ValueStruct::print(Print& out, ValueType& valueType, bool unformatted) const
 {
   valueType = getValueType();
 
   if (_isSSO) {
-    if (getCaseFormat() == CaseFormat::KeepCase) {
+    if (getCaseFormat(unformatted) == CaseFormat::KeepCase) {
       return out.write((const char *)&VALUE_STRUCT_SSO_FIRST_CHAR);
     }
     return formatCase(out, String((const char *)&VALUE_STRUCT_SSO_FIRST_CHAR));
@@ -521,7 +524,7 @@ size_t ValueStruct::print(Print& out, ValueType& valueType) const
     {
       if (str_val == nullptr) { return 0; }
 
-      if (getCaseFormat() == CaseFormat::KeepCase) {
+      if (getCaseFormat(unformatted) == CaseFormat::KeepCase) {
         return out.write((const uint8_t *)str_val, _size);
       }
       return formatCase(out, String((const uint8_t *)str_val, _size));
@@ -530,7 +533,7 @@ size_t ValueStruct::print(Print& out, ValueType& valueType) const
     {
       if (str_val == nullptr) { return 0; }
 
-      if (getCaseFormat() == CaseFormat::KeepCase) {
+      if (getCaseFormat(unformatted) == CaseFormat::KeepCase) {
         return out.print((const __FlashStringHelper *)str_val);
       }
       return formatCase(out, String((const __FlashStringHelper *)str_val));
@@ -558,25 +561,25 @@ size_t ValueStruct::print(Print& out, ValueType& valueType) const
     case ValueStruct::ValueType::Int:
     {
       if ((_size > 32) || _minNrDigits) {
-        return out.print(ll2String(i64_val, DEC, _minNrDigits, '\0', getCaseFormat() == CaseFormat::ToUpper));
+        return out.print(ll2String(i64_val, DEC, _minNrDigits, '\0', getCaseFormat(unformatted) == CaseFormat::ToUpper));
       }
       auto v = static_cast<int32_t>(i64_val);
       return out.print(v);
     }
     case ValueStruct::ValueType::UInt:
     {
-      const auto format = getPreferredFormat();
+      const auto format = getPreferredFormat(unformatted);
 
       if (format == PreferredFormat::Bin) {
         return out.print(concat(F("0b"), ull2String(u64_val, BIN, _minNrDigits)));
       }
 
       if (format == PreferredFormat::Hex) {
-        return out.print(concat(F("0x"), ull2String(u64_val, HEX, _minNrDigits)));
+        return out.print(concat(F("0x"), ull2String(u64_val, HEX, _minNrDigits, '\0', getCaseFormat(unformatted) == CaseFormat::ToUpper)));
       }
 
       if ((_size > 32) || _minNrDigits) {
-        return out.print(ull2String(u64_val, DEC, _minNrDigits, '\0', getCaseFormat() == CaseFormat::ToUpper));
+        return out.print(ull2String(u64_val, DEC, _minNrDigits));
       }
       auto v = static_cast<uint32_t>(u64_val);
       return out.print(v);
@@ -602,3 +605,44 @@ bool ValueStruct::isEmpty() const
   }
   return false;
 }
+
+bool ValueStruct::equals(const __FlashStringHelper *cmdStr, bool ignoreCase) const
+{
+  if (getValueType() == ValueStruct::ValueType::Unset) { return false; }
+  return equals(String(cmdStr), ignoreCase);
+}
+
+bool ValueStruct::equals(const String& cmdStr, bool ignoreCase) const
+{
+  if (getValueType() == ValueStruct::ValueType::Unset) { return false; }
+
+  if (ignoreCase) {
+    return toString().equalsIgnoreCase(cmdStr);
+  }
+
+  return toString().equals(cmdStr);
+}
+
+#ifndef BUILD_NO_DEBUG
+
+String ValueStruct::debug() const
+{
+  const String normalStr      = toString();
+  const String unformattedStr = toString(true);
+  String logstr               = strformat(
+    F("t:%d,'%s'"),
+    static_cast<int>(getValueType()),
+    normalStr.c_str());
+
+  if (!normalStr.equals(unformattedStr)) {
+    logstr += strformat(F("u:'%s'"), unformattedStr.c_str());
+  }
+
+  if (!operator bool()) {
+    logstr += '!';
+  }
+
+  return logstr;
+}
+
+#endif // ifndef BUILD_NO_DEBUG

src/src/DataStructs/ValueStruct.h

@@ -95,14 +95,16 @@ public:
     return static_cast<ValueStruct::ValueType>(_valueType);
   }
 
-  ValueStruct::PreferredFormat getPreferredFormat() const
+  ValueStruct::PreferredFormat getPreferredFormat(bool unformatted = false) const
   {
+    if (unformatted) { return ValueStruct::PreferredFormat::Default; }
     return static_cast<ValueStruct::PreferredFormat>(_preferredFormat);
   }
 
   void                    setPreferredFormat(ValueStruct::PreferredFormat format);
 
-  ValueStruct::CaseFormat getCaseFormat() const {
+  ValueStruct::CaseFormat getCaseFormat(bool unformatted = false) const {
+    if (unformatted) { return ValueStruct::CaseFormat::KeepCase; }
     return static_cast<ValueStruct::CaseFormat>(_caseFormat);
   }
 
@@ -110,6 +112,8 @@ public:
 
   void         setMinNrDigits(uint8_t minNrDigits) { _minNrDigits = (uint64_t)minNrDigits; }
 
+  void         clear();
+
   ValueStruct& operator=(ValueStruct&& rhs);
   ValueStruct& operator=(const ValueStruct& rhs) = delete;
 
@@ -127,11 +131,12 @@ public:
   static ValueStruct makeFromString(const __FlashStringHelper *val);
   static ValueStruct makeFromString(const String& val);
 
-  String             toString() const;
+  String             toString(bool unformatted = false) const;
 
-  String             toString(ValueType& valueType) const;
+  String             toString(ValueType& valueType,
+                              bool       unformatted = false) const;
 
-  int64_t            toInt() const;
+  int64_t            toInt(int64_t defaultValue = 0) const;
 
   double             toFloat() const;
 
@@ -139,17 +144,29 @@ public:
 
   bool               isEmpty() const;
 
-  void               clear();
-
   bool               isSet() const { return getValueType() != ValueStruct::ValueType::Unset; }
 
+  bool               equals(const __FlashStringHelper *cmdStr,
+                            bool                       ignoreCase = false) const;
+  bool               equals(const String& cmdStr,
+                            bool          ignoreCase = false) const;
+
+  bool               equalsIgnoreCase(const __FlashStringHelper *cmdStr) const { return equals(cmdStr, true); }
+
+  bool               equalsIgnoreCase(const String& cmdStr) const              { return equals(cmdStr, true); }
+
+#ifndef BUILD_NO_DEBUG
+  String             debug() const;
+#endif
+
 private:
 
   size_t formatCase(Print  & out,
                     String&& str) const;
 
   size_t print(Print    & out,
-               ValueType& valueType) const;
+               ValueType& valueType,
+               bool       unformatted = false) const;
 
   union {
     struct {
@@ -164,7 +181,7 @@ private:
       uint64_t _caseFormat      : 2;
 
       // --- End of 1st byte
-      
+
       uint64_t _trimTrailingZeros : 1;
       uint64_t _minNrDigits       : 7;  // For printing ints with leading zeroes
       uint64_t _nrDecimals        : 8;
@@ -190,3 +207,5 @@ private:
 
 
 }; // class ValueStruct
+
+extern const ValueStruct INVALID_VALUESTRUCT;

src/src/Helpers/CommandArgParser.cpp

@@ -0,0 +1,172 @@
+#include "../Helpers/CommandArgParser.h"
+
+#include "../Globals/RulesCalculate.h"
+#include "../Helpers/Rules_calculate.h"
+#include "../Helpers/StringConverter.h"
+
+bool CommandArgParser::isValid() const
+{
+  if (_hasCommand && !getCommand().isSet()) {
+    return false;
+  }
+
+  if (_hasSubcommand && !getSubCommand().isSet()) {
+    return false;
+  }
+  return _kv.operator bool();
+}
+
+void CommandArgParser::clear()
+{
+  _hasCommand    = false;
+  _hasSubcommand = false;
+  _kv.clear();
+}
+
+bool CommandArgParser::_init(
+  const String& string,
+  char          separator,
+  bool          argumentsOnly,
+  bool          keepRawStrings)
+
+{
+  _kv.clear();
+  uint8_t indexFind = 0;
+
+  unsigned int string_pos = 0, argc_pos = 0;
+  const char  *string_c_str = string.c_str();
+
+  String arg_str;
+
+  if (!argumentsOnly) {
+    if (!GetArgv(string_c_str, string.length(), arg_str, ++indexFind, string_pos, argc_pos, separator)) {
+      return false;
+    }
+    _kv = KeyValueStruct(arg_str);
+    _kv._key.setCaseFormat(ValueStruct::CaseFormat::ToLower);
+  }
+
+  while (GetArgv(string_c_str, string.length(), arg_str, ++indexFind, string_pos, argc_pos, separator))
+  {
+    if (_hasSubcommand && (indexFind == 2)) {
+      // In order to match subcommand, set format to lower case
+      ValueStruct v(arg_str);
+      v.setCaseFormat(ValueStruct::CaseFormat::ToLower);
+      _kv.appendValue(std::move(v));
+    } else {
+      if (keepRawStrings) { _kv.appendValue(arg_str); }
+      else {
+        if (!arg_str.isEmpty() && (arg_str[0] == '=')) {
+          ESPEASY_RULES_FLOAT_TYPE param{};
+
+          // Starts with an '=', so Calculate starting at next position
+          CalculateReturnCode returnCode = Calculate(arg_str.substring(1), param);
+
+          if (!isError(returnCode)) {
+            _kv.appendValue(ValueStruct(param));
+          } else {
+            _kv.appendValue(ValueStruct::makeFromString(arg_str));
+          }
+        } else {
+          _kv.appendValue(ValueStruct::makeFromString(arg_str));
+        }
+      }
+    }
+  }
+
+  return isValid();
+}
+
+const ValueStruct& CommandArgParser::getCommand() const
+{
+  return _kv._key;
+}
+
+const ValueStruct& CommandArgParser::getSubCommand() const
+{
+  if (_hasSubcommand && (_kv._values.size() > 0)) {
+    return _kv._values[0];
+  }
+  return INVALID_VALUESTRUCT;
+}
+
+bool CommandArgParser::commandEquals(const __FlashStringHelper *cmdStr) const
+{
+  // Command is always compared using ignore case
+  return getCommand().equalsIgnoreCase(cmdStr);
+}
+
+bool CommandArgParser::commandEquals(const String& cmdStr) const
+{
+  // Command is always compared using ignore case
+  return getCommand().equalsIgnoreCase(cmdStr);
+}
+
+bool CommandArgParser::subCommandEquals(const __FlashStringHelper *cmdStr) const
+{
+  // SubCommand is always compared using ignore case
+  return getSubCommand().equalsIgnoreCase(cmdStr);
+}
+
+bool CommandArgParser::subCommandEquals(const String& cmdStr) const
+{
+  // SubCommand is always compared using ignore case
+  return getSubCommand().equalsIgnoreCase(cmdStr);
+}
+
+const ValueStruct& CommandArgParser::getArg(uint8_t index) const
+{
+  if (_hasSubcommand) { ++index; }
+
+  if (index < _kv._values.size()) {
+    return _kv._values[index];
+  }
+
+  return INVALID_VALUESTRUCT;
+}
+
+int CommandArgParser::getArgInt(uint8_t index, int defaultValue) const
+{
+  return getArg(index).toInt(defaultValue);
+}
+
+size_t CommandArgParser::getNrArgs() const {
+  if (_hasSubcommand && _kv._values.size()) { return _kv._values.size() - 1; }
+  return _kv._values.size();
+}
+
+#ifndef BUILD_NO_DEBUG
+
+void CommandArgParser::debug(const __FlashStringHelper *comment, uint8_t logLevel) const
+{
+  if (!loglevelActiveFor(logLevel)) { return; }
+  String logstr = comment;
+
+  if (!logstr.isEmpty()) { logstr += F(": "); }
+
+  logstr += concat(
+    F("CommandArgParser: cmd:"),
+    getCommand().toString());
+
+  if (_hasSubcommand) {
+    logstr += concat(
+      F(" sub:"),
+      getSubCommand().toString());
+  }
+
+  if (!_kv.operator bool()) {
+    logstr += F(" notSet");
+  }
+
+  for (uint8_t i = 0; i < getNrArgs(); ++i)
+  {
+    const ValueStruct& v = getArg(i);
+    logstr += strformat(F(" [%d]%s"), i, v.debug().c_str());
+  }
+
+  if (!logstr.isEmpty()) {
+    addLog(logLevel, logstr);
+  }
+}
+
+#endif // ifndef BUILD_NO_DEBUG

src/src/Helpers/CommandArgParser.h

@@ -0,0 +1,133 @@
+#pragma once
+
+#include "../../ESPEasy_common.h"
+
+#include "../DataStructs/KeyValueStruct.h"
+
+struct CommandArgParser {
+
+  CommandArgParser() = default;
+
+  bool isValid() const;
+
+  operator bool() const {
+    return isValid();
+  }
+
+  void clear();
+
+  // consider given string as a list of:
+  // - command
+  // - 0 or more arguments
+  bool readCommand(
+    const String& string,
+    char          separator      = ',',
+    bool          keepRawStrings = false)
+  {
+    _hasCommand    = true;
+    _hasSubcommand = false;
+    return _init(string, separator, false, keepRawStrings);
+  }
+
+  bool readCommandAndMatch(
+    const String             & string,
+    const __FlashStringHelper *cmdToMatch,
+    char                       separator      = ',',
+    bool                       keepRawStrings = false)
+  {
+    _hasCommand    = true;
+    _hasSubcommand = false;
+    return
+      _init(string, separator, false, keepRawStrings) &&
+      commandEquals(cmdToMatch);
+  }
+
+  // consider given string as a list of:
+  // - command
+  // - subcommmand
+  // - 0 or more arguments
+  bool readCommandSubCommand(
+    const String& string,
+    char          separator      = ',',
+    bool          keepRawStrings = false)
+  {
+    _hasCommand    = true;
+    _hasSubcommand = true;
+    return _init(string, separator, false, keepRawStrings);
+  }
+
+  bool readCommandSubCommandAndMatch(
+    const String             & string,
+    const __FlashStringHelper *cmdToMatch,
+    char                       separator      = ',',
+    bool                       keepRawStrings = false)
+  {
+    _hasCommand    = true;
+    _hasSubcommand = true;
+    return
+      _init(string, separator, false, keepRawStrings) &&
+      commandEquals(cmdToMatch);
+  }
+
+  // consider given string as a list of:
+  // - 0 or more arguments
+  bool readArgumentsOnly(
+    const String& string,
+    char          separator      = ',',
+    bool          keepRawStrings = false)
+  {
+    _hasCommand    = false;
+    _hasSubcommand = false;
+    return _init(string, separator, true, keepRawStrings);
+  }
+
+private:
+
+  bool _init(
+    const String& string,
+    char          separator,
+    bool          argumentsOnly,
+    bool          keepRawStrings);
+
+public:
+
+  // When initialized with readCommand or readCommandSubCommand, returns the command
+  // Otherwise return an unset ValueStruct
+  const ValueStruct& getCommand() const;
+
+  // When initialized with readCommandSubCommand, returns the subcommand
+  // Otherwise return an unset ValueStruct
+  const ValueStruct& getSubCommand() const;
+
+  bool               commandEquals(const __FlashStringHelper *cmdStr) const;
+  bool               commandEquals(const String& cmdStr) const;
+
+  bool               subCommandEquals(const __FlashStringHelper *cmdStr) const;
+  bool               subCommandEquals(const String& cmdStr) const;
+
+  // Returns found argument, or an uninitialized ValueStruct when index refers to a non-existent
+  // N.B. When initialized with readCommandSubCommand, index 0 is the first argument after the subcommand.
+  // Otherwise index 0 is the first argument
+  const ValueStruct& getArg(uint8_t index) const;
+
+  // Returns int-representation of the requested argument or defaultValue if argument is not present or not valid.
+  int                getArgInt(uint8_t index,
+                               int     defaultValue = 0) const;
+
+  size_t             getNrArgs() const;
+
+  bool               hasArgs() const { return getNrArgs() > 0; }
+
+#ifndef BUILD_NO_DEBUG
+  void               debug(const __FlashStringHelper *comment,
+                           uint8_t                    logLevel = LOG_LEVEL_DEBUG) const;
+#endif // ifndef BUILD_NO_DEBUG
+
+private:
+
+  KeyValueStruct _kv;
+
+  bool _hasCommand{};
+  bool _hasSubcommand{};
+
+};

src/src/Helpers/StringConverter.cpp

@@ -1676,6 +1676,8 @@ bool GetArgv(const char  *string,
              unsigned int &argc_pos,
              char         separator)
 {
+  // FIXME TD-er: Setting to 0 again, must fix not starting from beginning each time
+//  string_pos = 0; argc_pos = 0;
   int  pos_begin, pos_end;
   bool hasArgument = GetArgvBeginEnd(
     string,
@@ -1784,6 +1786,7 @@ bool GetArgvBeginEnd(const char        *string,
         argc_pos++;
         if (argc_pos == argc)
         {
+          string_pos++;
           return true;
         }
         // new Argument separator found

src/src/Helpers/StringParser.cpp

@@ -1110,45 +1110,3 @@ void parseCommandString(struct EventStruct *event, const String& string)
     event->ParN[i] = parseCommandArgumentInt(string, i + 1);
   }
 }
-
-
-KeyValueStruct parseArguments(
-  const String& string, 
-  char separator, 
-  bool keepRawStrings)
-{
-  String  result;
-  uint8_t indexFind = 1;
-
-  unsigned int string_pos = 0, argc_pos = 0;
-  const char  *string_c_str = string.c_str();
-
-  if (!GetArgv(string_c_str, string.length(), result, indexFind, string_pos, argc_pos, separator)) {
-    return KeyValueStruct();
-  }
-  KeyValueStruct res(result);
-  res._key.setCaseFormat(ValueStruct::CaseFormat::ToLower);
-
-  while (GetArgv(string_c_str, string.length(), result, indexFind, string_pos, argc_pos, separator))
-  {
-    if (keepRawStrings) { res.appendValue(result); }
-    else {
-      if (!result.isEmpty() && (result[0] == '=')) {
-        ESPEASY_RULES_FLOAT_TYPE param{};
-
-        // Starts with an '=', so Calculate starting at next position
-        CalculateReturnCode returnCode = Calculate(result.substring(1), param);
-
-        if (!isError(returnCode)) {
-          res.appendValue(ValueStruct(param));
-        } else {
-          res.appendValue(ValueStruct::makeFromString(result));
-        }
-      } else {
-        res.appendValue(ValueStruct::makeFromString(result));
-      }
-    }
-  }
-
-  return res;
-}

src/src/Helpers/StringParser.h

@@ -116,9 +116,5 @@ int parseCommandArgumentInt(const String& string,
 void parseCommandString(struct EventStruct *event,
                         const String      & string);
 
-KeyValueStruct parseArguments(
-  const String& string, 
-  char separator, 
-  bool keepRawStrings = false);
 
 #endif

src/src/PluginStructs/P126_data_struct.cpp

@@ -89,7 +89,8 @@ bool P126_data_struct::plugin_read(struct EventStruct *event) {
 
   for (uint16_t index = P126_CONFIG_SHOW_OFFSET; index < _chipCount && index < last && varNr < VARS_PER_TASK; index += 4, ++varNr) {
     const uint32_t result = getChannelState(index, min(VARS_PER_TASK, _chipCount - index));
-    if (UserVar.getUint32(event->TaskIndex, varNr) != result) changed = true;
+
+    if (UserVar.getUint32(event->TaskIndex, varNr) != result) { changed = true; }
     UserVar.setUint32(event->TaskIndex, varNr, result);
   }
   return changed;
@@ -99,174 +100,204 @@ bool P126_data_struct::plugin_write(struct EventStruct *event,
                                     const String      & string) {
   bool success = false;
   bool updated = false;
-  String command = parseString(string, 1);
 
-  if (equals(command, F("shiftout"))) {
-    const String subcommand = parseString(string, 2);
-    const bool   hc_update  = subcommand.indexOf(F("noupdate")) == -1;
+  CommandArgParser parsedCmd;
 
-    if (equals(subcommand, F("set")) || equals(subcommand, F("setnoupdate"))) {
-      const uint8_t  pin   = event->Par2;
-      const uint16_t value = event->Par3;
+  if (!parsedCmd.readCommandSubCommandAndMatch(string, F("shiftout"))) {
+    return false;
+  }
+
+# ifndef BUILD_NO_DEBUG
+  parsedCmd.debug(F("init success"), LOG_LEVEL_INFO);
+# endif
+
+  const bool hc_update = parsedCmd.getSubCommand().toString().indexOf(F("noupdate")) == -1;
+
+  if (parsedCmd.subCommandEquals(F("set")) || 
+      parsedCmd.subCommandEquals(F("setnoupdate"))) {
+    const uint8_t  pin   = parsedCmd.getArgInt(0, 0);
+    const uint16_t value = parsedCmd.getArgInt(1, -1);
+
+    if (validChannel(pin) && ((value == 0) || (value == 1))) {
+      shift->set(pin - 1, value, hc_update);
+
+      if (hc_update) { updated = true; }
+      success = true;
+    }
+# ifdef P126_DEBUG_LOG
+
+    if (loglevelActiveFor(LOG_LEVEL_DEBUG)) {
+      addLogMove(LOG_LEVEL_DEBUG, strformat(
+                   F("%s, pin: %d, value: %d"),
+                   parsedCmd.getCommand().toString().c_str(),
+                   pin,
+                   value));
+    }
+# endif // ifdef P126_DEBUG_LOG
+  } else if (parsedCmd.subCommandEquals(F("update"))) {
+    shift->updateRegisters();
+    updated = true;
+    success = true;
+  } else if (parsedCmd.subCommandEquals(F("setall")) ||
+             parsedCmd.subCommandEquals(F("setallnoupdate"))) {
+    success = true;
+    std::vector<uint8_t> value;
+    value.resize(_chipCount, 0);             // Initialize vector to 0's
+
+    const uint8_t *pvalue = shift->getAll(); // Get current state
+
+    for (uint8_t i = 0; i < _chipCount; ++i) {
+      value[i] = pvalue[i];
+    }
+
+    uint32_t par   = 0u;
+    uint8_t  param = 3; // Start with an offset
+    uint8_t  width = 4;
+    uint8_t  idx   = 0;
+    String   arg   = parseString(string, param);
+
+    while (!arg.isEmpty() && idx < _chipCount && success) {
+      int colon    = arg.indexOf(':'); // First colon: Chip-index, range 1.._chipCount
+      int32_t itmp = 0;
+
+      if (colon != -1) {
+        const String cis = arg.substring(0, colon);
+        arg = arg.substring(colon + 1);
+
+        if (!cis.isEmpty() && validIntFromString(cis, itmp) && (itmp > 0) && (itmp <= _chipCount)) {
+          idx = itmp - 1;       // Actual range is 0.._chipCount - 1
+        } else {
+          success = false;      // Cancel entire operation on error
+        }
+      }
+      colon = arg.indexOf(':'); // Second colon: data width, range 1..4 bytes
+      width = 4;                // Set default data width to 4 = 32 bits
+
+      if (colon != -1) {
+        const String lis = arg.substring(0, colon);
+        arg = arg.substring(colon + 1);
+
+        if (!lis.isEmpty() && validIntFromString(lis, itmp) && (itmp > 0) && (itmp <= 4)) {
+          width = itmp;
+        } else {
+          success = false; // Cancel entire operation on error
+        }
+      }
+      int64_t tmp = 0;
+      par = 0u; // reset
+
+      if (validInt64FromString(arg, tmp)) {
+        par = static_cast<uint32_t>(tmp);
+      }
 
-      if (validChannel(pin) && ((value == 0) || (value == 1))) {
-        shift->set(pin - 1, value, hc_update);
-        if (hc_update) updated = true;
-        success = true;
         # ifdef P126_DEBUG_LOG
 
+      if (loglevelActiveFor(LOG_LEVEL_INFO)) {
+        addLogMove(
+          LOG_LEVEL_INFO,
+          strformat(
+            F("%s: arg: %s, tmp: %s/0x%x, par: %d/0x%x, chip:%d, width:%d"),
+            parsedCmd.getCommand().toString().c_str(),
+            arg.c_str(),
+            ull2String(tmp).c_str(),
+            tmp,
+            par,
+            par,
+            idx,
+            width));
+      }
+        # endif // ifdef P126_DEBUG_LOG
+
+      param++; // Process next argument
+      arg = parseString(string, param);
+
+      for (uint8_t n = 0; n < width && idx < _chipCount; ++n, ++idx) {
+        value[idx] = ((par >> (n * 8)) & 0xff);
+
+          # ifdef P126_DEBUG_LOG
+
         if (loglevelActiveFor(LOG_LEVEL_DEBUG)) {
-          addLogMove(LOG_LEVEL_DEBUG, strformat(F("%s, pin: %d, value: %d"), command.c_str(), pin, value));
+          addLogMove(LOG_LEVEL_DEBUG,
+                     strformat(F("%s: value[%d] : %d/0x%x, n * 8: %d/%d"),
+                               parsedCmd.getCommand().toString().c_str(),
+                               idx,
+                               value[idx],
+                               value[idx],
+                               n,
+                               n * 8));
         }
-        # endif // ifdef P126_DEBUG_LOG
-      }
-    } else if (equals(subcommand, F("update"))) {
-      shift->updateRegisters();
-      updated = true;
-      success = true;
-    } else if (equals(subcommand, F("setall")) || equals(subcommand, F("setallnoupdate"))) {
-      success = true;
-      std::vector<uint8_t> value;
-      value.resize(_chipCount, 0);             // Initialize vector to 0's
-
-      const uint8_t *pvalue = shift->getAll(); // Get current state
-
-      for (uint8_t i = 0; i < _chipCount; ++i) {
-        value[i] = pvalue[i];
-      }
-
-      uint32_t par   = 0u;
-      uint8_t  param = 3; // Start with an offset
-      uint8_t  width = 4;
-      uint8_t  idx   = 0;
-      String   arg   = parseString(string, param);
-
-      while (!arg.isEmpty() && idx < _chipCount && success) {
-        int colon    = arg.indexOf(':'); // First colon: Chip-index, range 1.._chipCount
-        int32_t itmp = 0;
-
-        if (colon != -1) {
-          const String cis = arg.substring(0, colon);
-          arg = arg.substring(colon + 1);
-
-          if (!cis.isEmpty() && validIntFromString(cis, itmp) && (itmp > 0) && (itmp <= _chipCount)) {
-            idx = itmp - 1;       // Actual range is 0.._chipCount - 1
-          } else {
-            success = false;      // Cancel entire operation on error
-          }
-        }
-        colon = arg.indexOf(':'); // Second colon: data width, range 1..4 bytes
-        width = 4;                // Set default data width to 4 = 32 bits
-
-        if (colon != -1) {
-          const String lis = arg.substring(0, colon);
-          arg = arg.substring(colon + 1);
-
-          if (!lis.isEmpty() && validIntFromString(lis, itmp) && (itmp > 0) && (itmp <= 4)) {
-            width = itmp;
-          } else {
-            success = false; // Cancel entire operation on error
-          }
-        }
-        int64_t tmp = 0;
-        par = 0u; // reset
-
-        if (validInt64FromString(arg, tmp)) {
-          par = static_cast<uint32_t>(tmp);
-        }
-
-        # ifdef P126_DEBUG_LOG
-
-        if (loglevelActiveFor(LOG_LEVEL_INFO)) {
-          addLogMove(LOG_LEVEL_INFO,
-                     strformat(F("%s: arg: %s, tmp: %s/0x%x, par: %d/0x%x, chip:%d, width:%d"),
-                               command.c_str(), arg.c_str(), ull2String(tmp).c_str(), tmp, par, par, idx, width));
-        }
-        # endif // ifdef P126_DEBUG_LOG
-
-        param++; // Process next argument
-        arg = parseString(string, param);
-
-        for (uint8_t n = 0; n < width && idx < _chipCount; ++n, ++idx) {
-          value[idx] = ((par >> (n * 8)) & 0xff);
-
-          # ifdef P126_DEBUG_LOG
-
-          if (loglevelActiveFor(LOG_LEVEL_DEBUG)) {
-            addLogMove(LOG_LEVEL_DEBUG,
-                       strformat(F("%s: value[%d] : %d/0x%x, n * 8: %d/%d"),
-                                 command.c_str(), idx, value[idx], value[idx], n, n * 8));
-          }
           # endif // ifdef P126_DEBUG_LOG
-        }
       }
-
-      if (success) {
-        shift->setAll(&value[0], hc_update);
-        if (hc_update) updated = true;
-      }
-    } else if (equals(subcommand, F("setalllow"))) {
-      shift->setAllLow();
-      updated = true;
-      success = true;
-    } else if (equals(subcommand, F("setallhigh"))) {
-      shift->setAllHigh();
-      updated = true;
-      success = true;
-    } else if (equals(subcommand, F("setoffset"))) {
-      if ((event->Par2 >= 0) && (event->Par2 <= P126_MAX_SHOW_OFFSET)) {
-        uint8_t previousOffset = P126_CONFIG_SHOW_OFFSET;
-        P126_CONFIG_SHOW_OFFSET = event->Par2;
-
-        if (P126_CONFIG_SHOW_OFFSET >= P126_CONFIG_CHIP_COUNT) {
-          P126_CONFIG_SHOW_OFFSET = 0;
-        }
-        P126_CONFIG_SHOW_OFFSET -= (P126_CONFIG_SHOW_OFFSET % 4);
-
-        if ((P126_CONFIG_CHIP_COUNT > 4) &&
-            (P126_CONFIG_SHOW_OFFSET > P126_CONFIG_CHIP_COUNT - 4) &&
-            (P126_CONFIG_CHIP_COUNT < P126_MAX_SHOW_OFFSET)) {
-          P126_CONFIG_SHOW_OFFSET -= 4;
-        }
-
-        // Reset State_A..D values when changing the offset
-        if ((previousOffset != P126_CONFIG_SHOW_OFFSET) && P126_CONFIG_FLAGS_GET_VALUES_RESTORE) {
-          for (uint8_t varNr = 0; varNr < VARS_PER_TASK; ++varNr) {
-            UserVar.setUint32(event->TaskIndex, varNr, 0u);
-          }
-          # ifdef P126_DEBUG_LOG
-          addLog(LOG_LEVEL_INFO, F("SHIFTOUT: 'Offset for display' changed: state values reset."));
-          # endif // ifdef P126_DEBUG_LOG
-        }
-        success = true;
-      }
-    } else if (equals(subcommand, F("setchipcount"))) {
-      if ((event->Par2 >= 1) && (event->Par2 <= P126_MAX_CHIP_COUNT)) {
-        P126_CONFIG_CHIP_COUNT = event->Par2;
-        _chipCount             = event->Par2;
-        shift->setSize(P126_CONFIG_CHIP_COUNT);
-        success = true;
-      }
-    # ifdef P126_SHOW_VALUES
-    } else if (equals(subcommand, F("sethexbin"))) {
-      if ((event->Par2 == 0) || (event->Par2 == 1)) {
-        uint32_t lSettings = P126_CONFIG_FLAGS;
-        bitWrite(lSettings, P126_FLAGS_VALUES_DISPLAY, event->Par2 == 1);
-        P126_CONFIG_FLAGS = lSettings;
-        success           = true;
-      }
-    # endif // ifdef P126_SHOW_VALUES
     }
-    # ifdef P126_DEBUG_LOG
 
     if (success) {
-      addLog(LOG_LEVEL_DEBUG, string);
+      shift->setAll(&value[0], hc_update);
+
+      if (hc_update) { updated = true; }
     }
-    # endif // ifdef P126_DEBUG_LOG
+  } else if (parsedCmd.subCommandEquals(F("setalllow"))) {
+    shift->setAllLow();
+    updated = true;
+    success = true;
+  } else if (parsedCmd.subCommandEquals(F("setallhigh"))) {
+    shift->setAllHigh();
+    updated = true;
+    success = true;
+  } else if (parsedCmd.subCommandEquals(F("setoffset"))) {
+    if ((event->Par2 >= 0) && (event->Par2 <= P126_MAX_SHOW_OFFSET)) {
+      uint8_t previousOffset = P126_CONFIG_SHOW_OFFSET;
+      P126_CONFIG_SHOW_OFFSET = event->Par2;
+
+      if (P126_CONFIG_SHOW_OFFSET >= P126_CONFIG_CHIP_COUNT) {
+        P126_CONFIG_SHOW_OFFSET = 0;
+      }
+      P126_CONFIG_SHOW_OFFSET -= (P126_CONFIG_SHOW_OFFSET % 4);
+
+      if ((P126_CONFIG_CHIP_COUNT > 4) &&
+          (P126_CONFIG_SHOW_OFFSET > P126_CONFIG_CHIP_COUNT - 4) &&
+          (P126_CONFIG_CHIP_COUNT < P126_MAX_SHOW_OFFSET)) {
+        P126_CONFIG_SHOW_OFFSET -= 4;
+      }
+
+      // Reset State_A..D values when changing the offset
+      if ((previousOffset != P126_CONFIG_SHOW_OFFSET) && P126_CONFIG_FLAGS_GET_VALUES_RESTORE) {
+        for (uint8_t varNr = 0; varNr < VARS_PER_TASK; ++varNr) {
+          UserVar.setUint32(event->TaskIndex, varNr, 0u);
+        }
+          # ifdef P126_DEBUG_LOG
+        addLog(LOG_LEVEL_INFO, F("SHIFTOUT: 'Offset for display' changed: state values reset."));
+          # endif // ifdef P126_DEBUG_LOG
+      }
+      success = true;
+    }
+  } else if (parsedCmd.subCommandEquals(F("setchipcount"))) {
+    if ((event->Par2 >= 1) && (event->Par2 <= P126_MAX_CHIP_COUNT)) {
+      P126_CONFIG_CHIP_COUNT = event->Par2;
+      _chipCount             = event->Par2;
+      shift->setSize(P126_CONFIG_CHIP_COUNT);
+      success = true;
+    }
+    # ifdef P126_SHOW_VALUES
+  } else if (parsedCmd.subCommandEquals(F("sethexbin"))) {
+    if ((event->Par2 == 0) || (event->Par2 == 1)) {
+      uint32_t lSettings = P126_CONFIG_FLAGS;
+      bitWrite(lSettings, P126_FLAGS_VALUES_DISPLAY, event->Par2 == 1);
+      P126_CONFIG_FLAGS = lSettings;
+      success           = true;
+    }
+    # endif // ifdef P126_SHOW_VALUES
   }
+    # ifdef P126_DEBUG_LOG
+
+  if (success) {
+    addLog(LOG_LEVEL_DEBUG, string);
+  }
+    # endif // ifdef P126_DEBUG_LOG
+
   // TODO TD-er: Do we need to schedule a taskRun to send out events and/or send data to controller(s), or just a plugin_read(event) ??
-  if (updated) //plugin_read(event);
-  Scheduler.schedule_task_device_timer(event->TaskIndex, 10);
+  if (updated) { // plugin_read(event);
+    Scheduler.schedule_task_device_timer(event->TaskIndex, 10);
+  }
   return success;
 }
 

src/src/PluginStructs/P126_data_struct.h

@@ -72,7 +72,7 @@ private:
   uint32_t getChannelState(uint8_t offset,
                                  uint8_t size) const;
 
-  bool validChannel(uint channel) const {
+  bool validChannel(uint16_t channel) const {
     return channel > 0 && channel <= (_chipCount * 8);
   }