letscontrolit / ESPEasy

Easy MultiSensor device based on ESP8266/ESP32
http://www.espeasy.com
Other
3.26k stars 2.21k forks source link

Bug in Plugin _P109_RESOL_DeltaSol_Pro.ino #2634

Closed tammycutzet closed 1 year ago

tammycutzet commented 4 years ago

Summarize of the problem/feature request

When adding the DeltaSol as a Device i got: Bug in PLUGIN_WEBFORM_LOAD, should not append to string, use addHtml() instead

Expected behavior

I expect to choose the registers for various items.

Actual behavior

Bug in PLUGIN_WEBFORM_LOAD, should not append to string, use addHtml() instead image

I googled and found that String should be replaced by addHtml(). Im not good in coding, but i found the part in the code that causes this trouble.

case PLUGIN_WEBFORM_LOAD:
      {
        int16_t choice = Settings.TaskDevicePluginConfig[event->TaskIndex][0];

        String options[RESOL_REGISTER_OPTIONS];
        uint optionValues[RESOL_REGISTER_OPTIONS];

        optionValues[0] = REG_TEMP_SENSOR_1;
        options[0] = F("Temperature sensor 1");
        optionValues[1] = REG_TEMP_SENSOR_2;
        options[1] = F("Temperature sensor 2");
        optionValues[2] = REG_TEMP_SENSOR_3;
        options[2] = F("Temperature sensor 3");
        optionValues[3] = REG_RELAIS_1;
        options[3] = F("Relais 1");
        optionValues[4] = REG_RELAIS_2;
        options[4] = F("Relais 2");

        string += F("<TR><TD>Register:<TD><select name='plugin_109_register'>");
        for (byte x = 0; x < RESOL_REGISTER_OPTIONS; x++)
        {
          string += F("<option value='");
          string += optionValues[x];
          string += "'";
          if (choice == optionValues[x])
            string += F(" selected");
          string += ">";
          string += options[x];
          string += F("</option>");
        }
        string += F("</select>");

        success = true;
        break;
      }

It is not a problem to change string +="" to addHtml(). But i have trouble with string += optionValues[x];

Could you get me some support here? Hope that i dont bother anybody with this issue, or if this is the wrong way.

Cheers

uzi18 commented 4 years ago

You have similar solution here: https://github.com/letscontrolit/ESPEasy/blob/9fb79a237096041618706f5e4dc71e8c770b6e07/src/_P001_Switch.ino#L185

please look also some lines above

TD-er commented 4 years ago

string = ....; should be replaced by addHtml(...); string += ....; should be replaced by addHtml(...);

+= means append to (make it longer), but we don't have to return the full string, we can send it directly to the browser by sending with addHtml.

tammycutzet commented 4 years ago

Many thanks for your feedback! I understand that i need to send the possible choises to html and need to get back the choice that had been taken by the user via webinterface. This will then be processed and defines which value will be hold in the paritcular register. Im not to deep into coding and this plugin. I will try to work thru and try to understand.

tammycutzet commented 4 years ago

I think i just got it! Thank you again for your help! Attached is the code. Im a beginner, but i will try to contribute it, so others maybe can use it. I will find out how to do so!

    case PLUGIN_WEBFORM_LOAD:
      {
        int16_t choice = Settings.TaskDevicePluginConfig[event->TaskIndex][0];

        String registerOptions[5];
        registerOptions[0] = F("Temperature sensor 1");
        registerOptions[1] = F("Temperature sensor 2");
        registerOptions[2] = F("Temperature sensor 3");
        registerOptions[3] = F("Relais 1");
        registerOptions[4] = F("Relais 2");
        int registerOptionValues[5] =
        { REG_TEMP_SENSOR_1, REG_TEMP_SENSOR_2, REG_TEMP_SENSOR_3, REG_RELAIS_1, REG_RELAIS_2 };
        addFormSelector(F("Register Type"), F("plugin_109_register"), 5, registerOptions, registerOptionValues, choice);**

        success = true;
        break;
      }

    case PLUGIN_WEBFORM_SAVE:
      {
        Settings.TaskDevicePluginConfig[event->TaskIndex][0] = getFormItemInt(F("plugin_109_register"));
        Plugin_109_init = false; // Force device setup next time
        success = true;
        break;
      }     
uzi18 commented 4 years ago

it looks fine now =) you can modify this file on GitHub and prepare PR

tammycutzet commented 4 years ago

I was a little bit too fast. Im not able to debug the case PLUGIN_WEBFORM_SAVE: section. Maybe some one more skilled coluld have a look to it?

tonhuisman commented 1 year ago

This issue is no longer relevant, and can be closed.