Merge pull request letscontrolit#3932 from TD-er/bugfix/P016_crash_load

[P016] Reduce memory usage during load/save (letscontrolit#3930)

src/_P016_IR.ino

@@ -315,14 +315,7 @@ boolean Plugin_016(uint8_t function, struct EventStruct *event, String& string)
                         2000);
 
       {
-        // For load and save of the display lines, we must not rely on the data in memory.
-        // This data in memory can be altered through write commands.
-        // Therefore we must read the lines from flash in a temporary object.
-        P016_data_struct *P016_data = new (std::nothrow) P016_data_struct();
-
-        if (nullptr != P016_data) {
-          P016_data->loadCommandLines(event); // load saved codes and commands
-
+        {
           addFormSubHeader(F("Code - command map"));
 
           int size = static_cast<int>(decode_type_t::kLastDecodeType) + 1;
@@ -366,6 +359,9 @@ boolean Plugin_016(uint8_t function, struct EventStruct *event, String& string)
           int rowCnt = 0;
 
           for (uint8_t varNr = 0; varNr < P16_Nlines; varNr++) {
+            tCommandLinesV2 line;
+            P016_data_struct::loadCommandLine(event, line, varNr);
+
             html_TR_TD();
 
             if (varNr < 9) {
@@ -375,30 +371,30 @@ boolean Plugin_016(uint8_t function, struct EventStruct *event, String& string)
             html_TD();
             {                      // Decode type
               addSelector(getPluginCustomArgName(rowCnt + 0), size, &decodeTypes[0], &decodeTypeOptions[0], NULL,
-                          static_cast<int>(P016_data->CommandLines[varNr].CodeDecodeType), false, true, EMPTY_STRING);
+                          static_cast<int>(line.CodeDecodeType), false, true, EMPTY_STRING);
             }
             html_TD();
-            addCheckBox(getPluginCustomArgName(rowCnt + 1), bitRead(P016_data->CommandLines[varNr].CodeFlags, P16_FLAGS_REPEAT));
+            addCheckBox(getPluginCustomArgName(rowCnt + 1), bitRead(line.CodeFlags, P16_FLAGS_REPEAT));
             html_TD();
             strCode = EMPTY_STRING;
 
-            if (P016_data->CommandLines[varNr].Code > 0) {
-              strCode = uint64ToString(P016_data->CommandLines[varNr].Code, 16); // convert code to hex for display
+            if (line.Code > 0) {
+              strCode = uint64ToString(line.Code, 16); // convert code to hex for display
             }
             addTextBox(getPluginCustomArgName(rowCnt + 2), strCode, P16_Cchars - 1, false, false, P016_HEX_INPUT_PATTERN, EMPTY_STRING);
 
             html_TD();
             {
               addSelector(getPluginCustomArgName(rowCnt + 3), size, &decodeTypes[0], &decodeTypeOptions[0], NULL,
-                          static_cast<int>(P016_data->CommandLines[varNr].AlternativeCodeDecodeType), false, true, EMPTY_STRING);
+                          static_cast<int>(line.AlternativeCodeDecodeType), false, true, EMPTY_STRING);
             }
             html_TD();
-            addCheckBox(getPluginCustomArgName(rowCnt + 4), bitRead(P016_data->CommandLines[varNr].AlternativeCodeFlags, P16_FLAGS_REPEAT));
+            addCheckBox(getPluginCustomArgName(rowCnt + 4), bitRead(line.AlternativeCodeFlags, P16_FLAGS_REPEAT));
             html_TD();
             strCode = EMPTY_STRING;
 
-            if (P016_data->CommandLines[varNr].AlternativeCode > 0) {
-              strCode = uint64ToString(P016_data->CommandLines[varNr].AlternativeCode, 16); // convert code to hex for display
+            if (line.AlternativeCode > 0) {
+              strCode = uint64ToString(line.AlternativeCode, 16); // convert code to hex for display
             }
             addTextBox(getPluginCustomArgName(rowCnt + 5), strCode, P16_Cchars - 1, false, false, P016_HEX_INPUT_PATTERN, EMPTY_STRING);
 
@@ -409,15 +405,12 @@ boolean Plugin_016(uint8_t function, struct EventStruct *event, String& string)
             addHtmlInt(varNr + 1);
             addHtml(':');
             addHtml(F("<TD colspan=\"5\">")); // Use as much of available width (though limited to 500px by css)
-            addTextBox(getPluginCustomArgName(rowCnt + 6), String(P016_data->CommandLines[varNr].Command), P16_Nchars - 1);
+            addTextBox(getPluginCustomArgName(rowCnt + 6), String(line.Command), P16_Nchars - 1);
 
             rowCnt += 7;
             delay(0);
           }
           html_end_table();
-
-          // Need to delete the allocated object here
-          delete P016_data;
         }
 
         if (P016_SETTINGS_VERSION != P16_SETTINGS_LATEST) {
@@ -454,87 +447,72 @@ boolean Plugin_016(uint8_t function, struct EventStruct *event, String& string)
       PCONFIG_LONG(0)     = lSettings;
       P016_CMDINHIBIT     = getFormItemInt(F("p016_cmdinhibit"));
 
-      {
-        // For load and save of the display lines, we must not rely on the data in memory.
-        // This data in memory can be altered through write commands.
-        // Therefore we must use a temporary version to store the settings.
-        P016_data_struct *P016_data = new (std::nothrow) P016_data_struct();
+      # ifdef PLUGIN_016_DEBUG
+      P016_infoLogMemory(F("before save"));
+      # endif // ifdef PLUGIN_016_DEBUG
 
-        if (nullptr != P016_data) {
-          char   strCode[P16_Cchars];
+      {
           String strError;
           strError.reserve(30); // Length of expected string, needed for strings > 11 chars
-          String   strID;
-          uint64_t iCode;
 
           int rowCnt = 0;
 
-          P016_data->CommandLines.clear(); // Start fresh
-
           for (uint8_t varNr = 0; varNr < P16_Nlines; varNr++) {
-            P016_data->CommandLines.push_back(tCommandLinesV2());
+            tCommandLinesV2 line;
+
             strError = EMPTY_STRING;
 
             // Normal Code & flags
-            P016_data->CommandLines[varNr].CodeDecodeType = static_cast<decode_type_t>(getFormItemInt(getPluginCustomArgName(rowCnt + 0)));
-            bitWrite(P016_data->CommandLines[varNr].CodeFlags, P16_FLAGS_REPEAT, isFormItemChecked(getPluginCustomArgName(rowCnt + 1)));
-            iCode = 0;
+            line.CodeDecodeType = static_cast<decode_type_t>(getFormItemInt(getPluginCustomArgName(rowCnt + 0)));
+            bitWrite(line.CodeFlags, P16_FLAGS_REPEAT, isFormItemChecked(getPluginCustomArgName(rowCnt + 1)));
+            line.Code = 0;
 
+            char strCode[P16_Cchars] = {0};
             if (!safe_strncpy(strCode, webArg(getPluginCustomArgName(rowCnt + 2)), P16_Cchars)) {
               strError += F("Code ");
               strError += (varNr + 1);
               strError += ' ';
             } else {
-              iCode = hexToULL(strCode); // convert string with hexnumbers to uint64_t
+              line.Code = hexToULL(strCode); // convert string with hexnumbers to uint64_t
             }
-            P016_data->CommandLines[varNr].Code = iCode;
 
             delay(0);
 
             // Alternate Code & flags
-            P016_data->CommandLines[varNr].AlternativeCodeDecodeType =
+            line.AlternativeCodeDecodeType =
               static_cast<decode_type_t>(getFormItemInt(getPluginCustomArgName(rowCnt + 3)));
-            bitWrite(P016_data->CommandLines[varNr].AlternativeCodeFlags, P16_FLAGS_REPEAT,
+            bitWrite(line.AlternativeCodeFlags, P16_FLAGS_REPEAT,
                      isFormItemChecked(getPluginCustomArgName(rowCnt + 4)));
-            iCode = 0;
+            line.AlternativeCode = 0;
 
             if (!safe_strncpy(strCode, webArg(getPluginCustomArgName(rowCnt + 5)), P16_Cchars)) {
               strError += F("Alt.Code ");
               strError += (varNr + 1);
               strError += ' ';
             } else {
-              iCode = hexToULL(strCode); // convert string with hexnumbers to uint64_t
+              line.AlternativeCode = hexToULL(strCode); // convert string with hexnumbers to uint64_t
             }
-            P016_data->CommandLines[varNr].AlternativeCode = iCode;
 
             // Command
-            if (!safe_strncpy(P016_data->CommandLines[varNr].Command, webArg(getPluginCustomArgName(rowCnt + 6)), P16_Nchars)) {
+            if (!safe_strncpy(line.Command, webArg(getPluginCustomArgName(rowCnt + 6)), P16_Nchars)) {
               strError += F("Command ");
               strError += (varNr + 1);
             }
-            P016_data->CommandLines[varNr].Command[P16_Nchars - 1] = 0; // Terminate string
+            line.Command[P16_Nchars - 1] = 0; // Terminate string
 
             if (!strError.isEmpty()) {
               addHtmlError(strError);
             }
 
             rowCnt += 7;
             delay(0);
-          }
-
-          # ifdef PLUGIN_016_DEBUG
-          P016_infoLogMemory(F("before save"));
-          # endif // ifdef PLUGIN_016_DEBUG
 
-          P016_data->saveCommandLines(event);
+            P016_data_struct::saveCommandLine(event, line, varNr);
+          }
 
           # ifdef PLUGIN_016_DEBUG
           P016_infoLogMemory(F("after save"));
           # endif // ifdef PLUGIN_016_DEBUG
-
-          // Need to delete the allocated object here
-          delete P016_data;
-        }
       }
 
       # ifdef PLUGIN_016_DEBUG

src/_P022_PCA9685.ino

@@ -63,6 +63,7 @@ boolean Plugin_022(uint8_t function, struct EventStruct *event, String& string)
       Device[deviceCount].ValueCount         = 0;
       Device[deviceCount].Custom             = true;
       Device[deviceCount].TimerOption        = false;
+      Device[deviceCount].ExitTaskBeforeSave = false;
       break;
     }
 

src/_P036_FrameOLED.ino

@@ -461,17 +461,6 @@ boolean Plugin_036(uint8_t function, struct EventStruct *event, String& string)
         }
       }
 
-      P036_data_struct *P036_data =
-        static_cast<P036_data_struct *>(getPluginTaskData(event->TaskIndex));
-
-      if (nullptr != P036_data) {
-        // After saving, make sure the active lines are updated.
-        P036_data->frameCounter       = 0;
-        P036_data->MaxFramesToDisplay = 0xFF;
-        P036_data->disp_resolution   = static_cast<p036_resolution>(P036_RESOLUTION);
-        P036_data->loadDisplayLines(event->TaskIndex, 1);
-      }
-
 #ifdef PLUGIN_036_DEBUG
       addLog(LOG_LEVEL_INFO, F("P036_PLUGIN_WEBFORM_SAVE Done"));
 #endif // PLUGIN_036_DEBUG

src/_P052_SenseAir.ino

@@ -158,6 +158,8 @@ boolean Plugin_052(uint8_t function, struct EventStruct *event, String& string)
       Device[deviceCount].TimerOption        = true;
       Device[deviceCount].GlobalSyncOption   = true;
       Device[deviceCount].OutputDataType     = Output_Data_type_t::Simple;
+      // FIXME TD-er: Seems to use task data, but not sure if really needed.
+      Device[deviceCount].ExitTaskBeforeSave = false;
       break;
     }
 

src/_P062_MPR121_KeyPad.ino

@@ -56,6 +56,7 @@ boolean Plugin_062(uint8_t function, struct EventStruct *event, String& string)
       Device[deviceCount].TimerOption        = true;
       Device[deviceCount].TimerOptional      = true;
       Device[deviceCount].GlobalSyncOption   = true;
+      Device[deviceCount].ExitTaskBeforeSave = false;
       break;
     }
 

src/_P070_NeoPixel_Clock.ino

@@ -166,6 +166,8 @@ boolean Plugin_070(uint8_t function, struct EventStruct *event, String& string)
         Device[deviceCount].SendDataOption = false;
         Device[deviceCount].TimerOption = false;
         Device[deviceCount].GlobalSyncOption = false;
+        // FIXME TD-er: Not sure if access to any existing task data is needed when saving
+        Device[deviceCount].ExitTaskBeforeSave = false;
         break;
       }
 

src/_P073_7DGT.ino

@@ -704,6 +704,8 @@ boolean Plugin_073(uint8_t function, struct EventStruct *event, String& string)
       Device[deviceCount].TimerOption        = false;
       Device[deviceCount].TimerOptional      = false;
       Device[deviceCount].GlobalSyncOption   = true;
+      // FIXME TD-er: Not sure if access to any existing task data is needed when saving
+      Device[deviceCount].ExitTaskBeforeSave = false;
       break;
     }
 

src/_P075_Nextion.ino

@@ -121,6 +121,9 @@ boolean Plugin_075(uint8_t function, struct EventStruct *event, String& string)
       Device[deviceCount].TimerOption = true;
       Device[deviceCount].TimerOptional = true;         // Allow user to disable interval function.
       Device[deviceCount].GlobalSyncOption = true;
+      // FIXME TD-er: Not sure if access to any existing task data is needed when saving
+      Device[deviceCount].ExitTaskBeforeSave = false;
+
       break;
     }
 

src/_P085_AcuDC243.ino

@@ -105,6 +105,7 @@ boolean Plugin_085(uint8_t function, struct EventStruct *event, String& string)
       Device[deviceCount].SendDataOption     = true;
       Device[deviceCount].TimerOption        = true;
       Device[deviceCount].GlobalSyncOption   = true;
+      Device[deviceCount].ExitTaskBeforeSave = false;
       break;
     }
 

src/_P087_SerialProxy.ino

@@ -65,6 +65,8 @@ boolean Plugin_087(uint8_t function, struct EventStruct *event, String& string)
       Device[deviceCount].SendDataOption     = true;
       Device[deviceCount].TimerOption        = true;
       Device[deviceCount].GlobalSyncOption   = false;
+      // FIXME TD-er: Not sure if access to any existing task data is needed when saving
+      Device[deviceCount].ExitTaskBeforeSave = false;
       break;
     }
 

src/_P094_CULReader.ino

@@ -73,6 +73,8 @@ boolean Plugin_094(uint8_t function, struct EventStruct *event, String& string)
       Device[deviceCount].TimerOption        = true;
       Device[deviceCount].GlobalSyncOption   = false;
 //      Device[deviceCount].DuplicateDetection = true;
+      // FIXME TD-er: Not sure if access to any existing task data is needed when saving
+      Device[deviceCount].ExitTaskBeforeSave = false;
       break;
     }
 

src/_P104_max7219_Dotmatrix.ino

@@ -131,6 +131,7 @@ boolean Plugin_104(uint8_t function, struct EventStruct *event, String& string)
       Device[deviceCount].TimerOption        = false;
       Device[deviceCount].TimerOptional      = false;
       Device[deviceCount].GlobalSyncOption   = true;
+      Device[deviceCount].ExitTaskBeforeSave = false;
       break;
     }
 

src/src/DataStructs/DeviceStruct.cpp

@@ -5,7 +5,8 @@ DeviceStruct::DeviceStruct() :
   OutputDataType(Output_Data_type_t::Default),
   PullUpOption(false), InverseLogicOption(false), FormulaOption(false),
   Custom(false), SendDataOption(false), GlobalSyncOption(false),
-  TimerOption(false), TimerOptional(false), DecimalsOnly(false) {}
+  TimerOption(false), TimerOptional(false), DecimalsOnly(false),
+  ExitTaskBeforeSave(true) {}
 
 bool DeviceStruct::connectedToGPIOpins() const {
   switch(Type) {

src/src/DataStructs/DeviceStruct.h

@@ -93,6 +93,7 @@ struct DeviceStruct
   bool TimerOption        : 1;       // Allow to set the "Interval" timer for the plugin.
   bool TimerOptional      : 1;       // When taskdevice timer is not set and not optional, use default "Interval" delay (Settings.Delay)
   bool DecimalsOnly       : 1;       // Allow to set the number of decimals (otherwise treated a 0 decimals)
+  bool ExitTaskBeforeSave : 1;       // Optimization in memory usage, Do not exit when task data is needed during save.
 };
 typedef std::vector<DeviceStruct> DeviceVector;
 

src/src/DataTypes/ControllerIndex.cpp

@@ -1,4 +1,4 @@
-#include "ControllerIndex.h"
+#include "../DataTypes/ControllerIndex.h"
 
 #include "../CustomBuild/ESPEasyLimits.h"