Merge pull request #4763 from TD-er/bugfix/DefaultTaskVarNames

[Settings] Fix setting task var names & load default names

src/_P025_ADS1115.ino

@@ -54,13 +54,13 @@ boolean Plugin_025(uint8_t function, struct EventStruct *event, String& string)
     case PLUGIN_I2C_HAS_ADDRESS:
     case PLUGIN_WEBFORM_SHOW_I2C_PARAMS:
     {
-      # define ADS1115_I2C_OPTION 4
       const uint8_t i2cAddressValues[] = { 0x48, 0x49, 0x4A, 0x4B };
+      constexpr int nrAddressOptions = sizeof(i2cAddressValues) / sizeof(i2cAddressValues[0]);
 
       if (function == PLUGIN_WEBFORM_SHOW_I2C_PARAMS) {
-        addFormSelectorI2C(F("i2c_addr"), ADS1115_I2C_OPTION, i2cAddressValues, P025_I2C_ADDR);
+        addFormSelectorI2C(F("i2c_addr"), nrAddressOptions, i2cAddressValues, P025_I2C_ADDR);
       } else {
-        success = intArrayContains(ADS1115_I2C_OPTION, i2cAddressValues, event->Par1);
+        success = intArrayContains(nrAddressOptions, i2cAddressValues, event->Par1);
       }
       break;
     }

src/_Plugin_Helper.cpp

@@ -192,6 +192,7 @@ int checkDeviceVTypeForTask(struct EventStruct *event) {
     if (validTaskIndex(event->TaskIndex)) {
       String dummy;
 
+      event->idx = -1;
       if (PluginCall(PLUGIN_GET_DEVICEVTYPE, event, dummy)) {
         return event->idx; // pconfig_index
       }

src/src/DataStructs/Caches.cpp

@@ -256,6 +256,7 @@ void Caches::updateExtraTaskSettingsCache()
     if (it != extraTaskSettings_cache.end()) {
       // We need to keep the original checksum, from when loaded from storage
       tmp.md5checksum = it->second.md5checksum;
+      tmp.defaultTaskDeviceValueName = it->second.defaultTaskDeviceValueName;
 
       // Now clear it so we can create a fresh copy.
       extraTaskSettings_cache.erase(it);
@@ -312,22 +313,16 @@ void Caches::updateExtraTaskSettingsCache_afterLoad_Save()
     return;
   }
 
-  // Check if we need to update the cache
-  auto it = extraTaskSettings_cache.find(ExtraTaskSettings.TaskIndex);
-
-  if (it != extraTaskSettings_cache.end()) {
-    if (ExtraTaskSettings.computeChecksum() == it->second.md5checksum) {
-      return;
-    }
-  }
-
   // First update all other values
   updateExtraTaskSettingsCache();
 
   // Iterator has changed
-  it = extraTaskSettings_cache.find(ExtraTaskSettings.TaskIndex);
+  auto it = extraTaskSettings_cache.find(ExtraTaskSettings.TaskIndex);
 
   if (it != extraTaskSettings_cache.end()) {
+    for (size_t i = 0; i < VARS_PER_TASK; ++i) {
+      bitWrite(it->second.defaultTaskDeviceValueName, i, ExtraTaskSettings.isDefaultTaskVarName(i));
+    }
     it->second.md5checksum = ExtraTaskSettings.computeChecksum();
   }
 }

src/src/DataStructs/Caches.h

@@ -41,6 +41,7 @@ struct ExtraTaskSettings_cache_t {
   String       TaskDeviceName;
   ChecksumType md5checksum;
   uint8_t      decimals[VARS_PER_TASK] = { 0 };
+  uint8_t      defaultTaskDeviceValueName{};
   #if FEATURE_PLUGIN_STATS
   uint8_t enabledPluginStats = 0;
   #endif // if FEATURE_PLUGIN_STATS

src/src/DataStructs/ExtraTaskSettingsStruct.cpp

@@ -125,6 +125,15 @@ void ExtraTaskSettingsStruct::clearTaskDeviceValueName(taskVarIndex_t taskVarInd
   }
 }
 
+void ExtraTaskSettingsStruct::clearDefaultTaskDeviceValueNames()
+{
+  for (int i = 0; i < VARS_PER_TASK; ++i) {
+    if (isDefaultTaskVarName(i)) {
+      clearTaskDeviceValueName(i);
+    }
+  }
+}
+
 void ExtraTaskSettingsStruct::setAllowedRange(taskVarIndex_t taskVarIndex, const float& minValue, const float& maxValue)
 {
   if (validTaskVarIndex(taskVarIndex)) {
@@ -207,6 +216,20 @@ bool ExtraTaskSettingsStruct::anyEnabledPluginStats() const
 
 #endif // if FEATURE_PLUGIN_STATS
 
+bool ExtraTaskSettingsStruct::isDefaultTaskVarName(taskVarIndex_t taskVarIndex) const
+{
+  if (!validTaskVarIndex(taskVarIndex)) { return false; }
+  return bitRead(VariousBits[taskVarIndex], 1);
+}
+
+void ExtraTaskSettingsStruct::isDefaultTaskVarName(taskVarIndex_t taskVarIndex, bool isDefault)
+{
+  if (validTaskVarIndex(taskVarIndex)) {
+    bitWrite(VariousBits[taskVarIndex], 1, isDefault);
+  }
+}
+
+
 void ExtraTaskSettingsStruct::populateDeviceValueNamesSeq(
   const __FlashStringHelper *valuename,
   size_t                     nrValues,

src/src/DataStructs/ExtraTaskSettingsStruct.h

@@ -40,6 +40,7 @@ struct ExtraTaskSettingsStruct
   void          setTaskDeviceValueName(taskVarIndex_t taskVarIndex, const __FlashStringHelper * str);
 
   void          clearTaskDeviceValueName(taskVarIndex_t taskVarIndex);
+  void          clearDefaultTaskDeviceValueNames();
 
   void          setAllowedRange(taskVarIndex_t taskVarIndex,
                                 const float  & minValue,
@@ -60,6 +61,11 @@ struct ExtraTaskSettingsStruct
   bool          anyEnabledPluginStats() const;
 #endif // if FEATURE_PLUGIN_STATS
 
+  bool          isDefaultTaskVarName(taskVarIndex_t taskVarIndex) const;
+  void          isDefaultTaskVarName(taskVarIndex_t taskVarIndex,
+                                     bool           isDefault);
+
+
   void          populateDeviceValueNamesSeq(const __FlashStringHelper *valuename,
                                             size_t                     nrValues,
                                             uint8_t                    defaultDecimals,

src/src/Globals/Plugins.cpp

@@ -244,6 +244,33 @@ void queueTaskEvent(const __FlashStringHelper * eventName, taskIndex_t taskIndex
   queueTaskEvent(String(eventName), taskIndex, String(value1));
 }
 
+void loadDefaultTaskValueNames_ifEmpty(taskIndex_t TaskIndex) {
+  String  oldNames[VARS_PER_TASK];
+  uint8_t oldNrDec[VARS_PER_TASK];
+  LoadTaskSettings(TaskIndex);
+
+  for (uint8_t i = 0; i < VARS_PER_TASK; ++i) {
+    oldNames[i] = ExtraTaskSettings.TaskDeviceValueNames[i];
+    oldNrDec[i] = ExtraTaskSettings.TaskDeviceValueDecimals[i];
+    oldNames[i].trim();
+  }
+
+  struct EventStruct TempEvent(TaskIndex);
+  String dummy;
+  // the plugin call should populate ExtraTaskSettings with its default values.
+  PluginCall(PLUGIN_GET_DEVICEVALUENAMES, &TempEvent, dummy);
+
+  // Restore the settings that were already set by the user
+  for (uint8_t i = 0; i < VARS_PER_TASK; ++i) {
+    const bool isDefault = oldNames[i].isEmpty();
+    ExtraTaskSettings.isDefaultTaskVarName(i, isDefault);
+    if (!isDefault) {
+      ExtraTaskSettings.setTaskDeviceValueName(i, oldNames[i]);
+      ExtraTaskSettings.TaskDeviceValueDecimals[i] = oldNrDec[i];
+    }
+  }
+}
+
 /**
  * Call the plugin of 1 task for 1 function, with standard EventStruct and optional command string
  */
@@ -894,7 +921,7 @@ bool PluginCall(uint8_t Function, struct EventStruct *event, String& str)
         if (Function == PLUGIN_GET_DEVICEVALUECOUNT) {
           // Check if we have a valid value count.
           if (Output_Data_type_t::Simple == Device[DeviceIndex].OutputDataType) {
-            if (event->Par1 < 1 || event->Par1 > 4) {
+            if (event->Par1 < 1 || event->Par1 > VARS_PER_TASK) {
               // Output_Data_type_t::Simple only allows for 1 .. 4 output types.
               // Apparently the value is not correct, so use the default.
               event->Par1 = Device[DeviceIndex].ValueCount;

src/src/Globals/Plugins.h

@@ -91,6 +91,8 @@ String        getPluginNameFromPluginID(pluginID_t pluginID);
 bool prepare_I2C_by_taskIndex(taskIndex_t taskIndex, deviceIndex_t DeviceIndex);
 void post_I2C_by_taskIndex(taskIndex_t taskIndex, deviceIndex_t DeviceIndex);
 
+void loadDefaultTaskValueNames_ifEmpty(taskIndex_t TaskIndex);
+
 /*********************************************************************************************\
 * Function call to all or specific plugins
 \*********************************************************************************************/

src/src/Helpers/ESPEasy_Storage.cpp

@@ -1054,6 +1054,8 @@ String SaveTaskSettings(taskIndex_t TaskIndex)
   String err;
 
   if (!Cache.matchChecksumExtraTaskSettings(TaskIndex, ExtraTaskSettings.computeChecksum())) {
+    // Clear task device value names before saving, will generate again when loading them later.
+    ExtraTaskSettings.clearDefaultTaskDeviceValueNames();
     ExtraTaskSettings.validate(); // Validate before saving will reduce nr of saves as it is more likely to not have changed the next time it will be saved.
 
     // Call to validate() may have changed the content, so re-compute the checksum.
@@ -1100,7 +1102,11 @@ String LoadTaskSettings(taskIndex_t TaskIndex)
   if (!validDeviceIndex(DeviceIndex)) {
     // No need to load from storage, as there is no plugin assigned to this task.
     ExtraTaskSettings.TaskIndex = TaskIndex; // Needed when an empty task was requested
-    Cache.updateExtraTaskSettingsCache_afterLoad_Save();
+
+    // FIXME TD-er: Do we need to keep a cache of an empty task?
+    // Maybe better to do this? 
+    Cache.clearTaskCache(TaskIndex); 
+//    Cache.updateExtraTaskSettingsCache_afterLoad_Save();
     return EMPTY_STRING;
   }
   #ifndef BUILD_NO_RAM_TRACKER
@@ -1122,15 +1128,8 @@ String LoadTaskSettings(taskIndex_t TaskIndex)
       ExtraTaskSettings.TaskDeviceValueDecimals[i] = 0;
     }      
   }
-
-  if (ExtraTaskSettings.TaskDeviceValueNames[0][0] == 0) {
-    // if field set empty, reload defaults
-    struct EventStruct TempEvent(TaskIndex);
-    String tmp;
-
-    // the plugin call should populate ExtraTaskSettings with its default values.
-    PluginCall(PLUGIN_GET_DEVICEVALUENAMES, &TempEvent, tmp);
-  }
+  loadDefaultTaskValueNames_ifEmpty(TaskIndex);
+
   ExtraTaskSettings.validate();
   Cache.updateExtraTaskSettingsCache_afterLoad_Save();
   STOP_TIMER(LOAD_TASK_SETTINGS);

src/src/Helpers/_Plugin_SensorTypeHelper.cpp

@@ -116,15 +116,17 @@ void sensorTypeHelper_webformLoad(struct EventStruct *event, uint8_t pconfigInde
 
 void sensorTypeHelper_saveOutputSelector(struct EventStruct *event, uint8_t pconfigIndex, uint8_t valueIndex, const String& defaultValueName)
 {
-  if (defaultValueName.equals(ExtraTaskSettings.TaskDeviceValueNames[valueIndex])) {
+  const bool isDefault = defaultValueName.equals(ExtraTaskSettings.TaskDeviceValueNames[valueIndex]);
+  if (isDefault) {
     ExtraTaskSettings.clearTaskDeviceValueName(valueIndex);
   }
+  ExtraTaskSettings.isDefaultTaskVarName(valueIndex, isDefault);
   pconfig_webformSave(event, pconfigIndex);
 }
 
 void pconfig_webformSave(struct EventStruct *event, uint8_t pconfigIndex)
 {
-  PCONFIG(pconfigIndex) = getFormItemInt(PCONFIG_LABEL(pconfigIndex), 0);
+  PCONFIG(pconfigIndex) = getFormItemInt(PCONFIG_LABEL(pconfigIndex), PCONFIG(pconfigIndex));
 }
 
 void sensorTypeHelper_loadOutputSelector(

src/src/PluginStructs/P025_data_struct.cpp

@@ -27,29 +27,44 @@ P025_data_struct::P025_data_struct(struct EventStruct *event) {
   _fullScaleFactor = 1.0f;
 
   if (P025_VOLT_OUT_GET) {
-    if (P025_GAIN == 0) { 
-      _fullScaleFactor = 6.144 / 32768.0f; 
+    if (P025_GAIN == 0) {
+      _fullScaleFactor = 6144;
     } else {
       const uint8_t shift = 13 - P025_GAIN;
-      _fullScaleFactor = (1 << shift) / 32768000.0f;
+      _fullScaleFactor = (1 << shift);
     }
+    _fullScaleFactor /= 32768000.0f;
   }
 }
 
 bool P025_data_struct::read(float& value) const {
-  if (!waitReady025(5)) { return false; }
+  if (!waitReady025(5)) {
+    //    addLog(LOG_LEVEL_INFO, F("ADS1115: Not Ready at start read"));
+    return false;
+  }
 
   if (!I2C_write16_reg(_i2cAddress, P025_CONFIG_REGISTER, _configRegisterValue)) {
+    //    addLog(LOG_LEVEL_INFO, F("ADS1115: Start measurement failed"));
     return false;
   }
 
   // See https://github.com/letscontrolit/ESPEasy/issues/3159#issuecomment-660546091
-  if (!waitReady025(10)) { return false; }
+  if (!waitReady025(10)) {
+    //    addLog(LOG_LEVEL_INFO, F("ADS1115: Not Ready after start measurement"));
+
+    return false;
+  }
 
   int16_t raw = 0;
-  if (!readConversionRegister025(raw)) return false;
+
+  if (!readConversionRegister025(raw)) {
+    addLog(LOG_LEVEL_INFO, F("ADS1115: Cannot read from conversion register"));
+    return false;
+  }
 
   value = _fullScaleFactor * raw;
+
+  //  addLog(LOG_LEVEL_INFO, strformat(F("ADS1115: RAW value: %d, output value: %f"), raw, value));
   return true;
 }
 
@@ -65,14 +80,11 @@ bool P025_data_struct::waitReady025(unsigned long timeout_ms) const {
   const unsigned long timeout = millis() + timeout_ms;
 
   while (!timeOutReached(timeout)) {
-    bool is_ok = false;
-
     // bit15=0 performing a conversion   =1 not performing a conversion
+    bool is_ok       = false;
     const bool ready = (I2C_read16_reg(_i2cAddress, P025_CONFIG_REGISTER, &is_ok) & 0x8000) != 0;
 
-    if (!is_ok) { return false; }
-
-    if (ready) { return true; }
+    if (ready && is_ok) { return true; }
     delay(1);
   }
   return false;

src/src/WebServer/DevicesPage.cpp

@@ -249,7 +249,6 @@ void handle_devices_CopySubmittedSettings(taskIndex_t taskIndex, pluginID_t task
 
   Settings.TaskDeviceNumber[taskIndex] = taskdevicenumber;
 
-
   uint8_t flags = 0;
 
   if (Device[DeviceIndex].Type == DEVICE_TYPE_I2C) {
@@ -278,10 +277,12 @@ void handle_devices_CopySubmittedSettings(taskIndex_t taskIndex, pluginID_t task
     Settings.I2C_Flags[taskIndex] = flags;
   }
 
-  struct EventStruct TempEvent(taskIndex);
-
+  // Must load from file system to make sure all caches and checksums match.
   ExtraTaskSettings.clear();
   ExtraTaskSettings.TaskIndex = taskIndex;
+  Cache.clearTaskCache(taskIndex);
+
+  struct EventStruct TempEvent(taskIndex);
 
   // Save selected output type.
   switch (Device[DeviceIndex].OutputDataType) {
@@ -295,32 +296,13 @@ void handle_devices_CopySubmittedSettings(taskIndex_t taskIndex, pluginID_t task
     case Output_Data_type_t::All:
     {
       int pconfigIndex = checkDeviceVTypeForTask(&TempEvent);
+      Sensor_VType VType = TempEvent.sensorType;
 
       if ((pconfigIndex >= 0) && (pconfigIndex < PLUGIN_CONFIGVAR_MAX)) {
-        Sensor_VType VType = static_cast<Sensor_VType>(getFormItemInt(PCONFIG_LABEL(pconfigIndex), 0));
+        VType = static_cast<Sensor_VType>(getFormItemInt(PCONFIG_LABEL(pconfigIndex), 0));
         Settings.TaskDevicePluginConfig[taskIndex][pconfigIndex] = static_cast<int>(VType);
-        ExtraTaskSettings.clearUnusedValueNames(getValueCountFromSensorType(VType));
-
-        // nr output values has changed, generate new variable names
-        String  oldNames[VARS_PER_TASK];
-        uint8_t oldNrDec[VARS_PER_TASK];
-
-        for (uint8_t i = 0; i < VARS_PER_TASK; ++i) {
-          oldNames[i] = ExtraTaskSettings.TaskDeviceValueNames[i];
-          oldNrDec[i] = ExtraTaskSettings.TaskDeviceValueDecimals[i];
-        }
-
-        String dummy;
-        PluginCall(PLUGIN_GET_DEVICEVALUENAMES, &TempEvent, dummy);
-
-        // Restore the settings that were already set by the user
-        for (uint8_t i = 0; i < VARS_PER_TASK; ++i) {
-          if (!oldNames[i].isEmpty()) {
-            ExtraTaskSettings.setTaskDeviceValueName(i, oldNames[i]);
-            ExtraTaskSettings.TaskDeviceValueDecimals[i] = oldNrDec[i];
-          }
-        }
       }
+      ExtraTaskSettings.clearUnusedValueNames(getValueCountFromSensorType(VType));
       break;
     }
   }
@@ -421,9 +403,18 @@ void handle_devices_CopySubmittedSettings(taskIndex_t taskIndex, pluginID_t task
     }    
   }
 
+  // Store all PCONFIG values on the web page
+  // Must be done after PLUGIN_WEBFORM_SAVE, to allow tasks to clear the default task value names
+  // Output type selectors are typically stored in PCONFIG 
+  for (int pconfigIndex = 0; pconfigIndex < PLUGIN_CONFIGVAR_MAX; ++pconfigIndex) {
+    pconfig_webformSave(&TempEvent, pconfigIndex);
+  }
   // ExtraTaskSettings may have changed during PLUGIN_WEBFORM_SAVE, so again update the cache.
   Cache.updateExtraTaskSettingsCache();
 
+  loadDefaultTaskValueNames_ifEmpty(taskIndex);
+  Cache.updateExtraTaskSettingsCache();
+
   // notify controllers: CPlugin::Function::CPLUGIN_TASK_CHANGE_NOTIFICATION
   for (controllerIndex_t x = 0; x < CONTROLLER_MAX; x++)
   {