Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Char* conversion for default_value errornous when adding WiFiManagerParameter(); #1050

Open
4 of 11 tasks
zen85 opened this issue May 11, 2020 · 53 comments
Open
4 of 11 tasks
Labels
bug Validated BUG DEV Help Wanted Developer Needs Help Discussion Further Discussion ongoing Documentation Flagged for docs
Milestone

Comments

@zen85
Copy link

zen85 commented May 11, 2020

Basic Infos

Hardware

WiFimanager Branch/Release:

  • Master
  • Development (OTA)

Esp8266/Esp32:

  • ESP8266
  • ESP32

Hardware: ESP-12e, esp01, esp25

  • ESP01
  • ESP12 E/F/S (nodemcu, wemos, feather)
  • Other

ESP Core Version: 2.4.0, staging

  • 2.3.0
  • 2.4.0
  • 2.6.3
  • staging (master/dev)

Description

Hi,

When we add a custom WiFiManagerParameter with a normal char* to initialize the default value it has a strange conversion error. Following code:

char mqtt_broker[12] = "192.168.1.108";
custom_mqtt_broker = WiFiManagerParameter("mqttbroker", "MQTT Broker", mqtt_broker, sizeof(mqtt_broker));
Serial.println(custom_mqtt_broker.getValue());
Serial.println(mqtt_broker);`

puts out on the Serial:

⸮⸮⸮?168.1.10
192.168.1.108

Settings in IDE

Module: NodeMcu, Wemos D1

Additional libraries:

Sketch

#include <FS.h>                   //this needs to be first, or it all crashes and burns...
#include <ArduinoJson.h>          //https://github.com/bblanchon/ArduinoJson
#include <WiFiManager.h> //https://github.com/tzapu/WiFiManager WiFi Configuration Magic
#include <PubSubClient.h>
#include <ESP8266WiFi.h>

#define USEOTA
// enable OTA
#ifdef USEOTA
#include <WiFiUdp.h>
#include <ArduinoOTA.h>
#endif

#define TRIGGER_PIN 0 //Pin used to start AccessPoint to configure device - Just ground GPIO for 2 seconds

WiFiManager wm;
ESP8266WebServer server(80);




WiFiClient espClient;
PubSubClient client(espClient);

long lastReconnectAttempt = 0;

boolean reconnect() {
  Serial.println("attempting connecting to mqtt");
  if (client.connect("arduinoClient")) {

    Serial.println("MQTT CONNECTED");
    // Once connected, publish an announcement...
    client.publish("outTopic", "hello world");
    // ... and resubscribe
    client.subscribe("inTopic");
  }
  return client.connected();
}

//############################################# CUSTOM PARAMETERS FOR THE WIFI MANAGER ##########################################
//char mqtt_server[40] = "0.0.0.0";
//char mqtt_topicIN[80] ="IOT_Device/IN";
//char mqtt_topicOUT[80] ="IOT_Device/OUT";
//char mqtt_port[6] = "1883";

char output[2] = "2";
char wifi_enc[40];
char entity_version[40];
char mqtt_port[40] = "1883";
char mqtt_broker[12] = "           ";
char mqtt_pwd[40];
char session_key[40];
char entity_name[40];
char entity_type[40];
char entity_id[40];
char mqtt_retain[40];
char mqtt_qos[40];





//flag for saving data
bool shouldSaveConfig = false;

//WiFiManagerParameter custom_mqtt_server("server", "Server IP", mqtt_server, 40);
//WiFiManagerParameter custom_mqtt_port("port", "Port", mqtt_port, 6);
//WiFiManagerParameter custom_mqtt_topicIN("topicIn", "Input Topic", mqtt_topicIN, 80);
//WiFiManagerParameter custom_mqtt_topicOUT("topicOut", "Output Topic", mqtt_topicOUT, 80);
//WiFiManagerParameter custom_mqtt_messages("messages", "Messages Topic", mqtt_topicOUT, 80);
WiFiManagerParameter custom_output("output", "output", output, 2);
WiFiManagerParameter custom_mqtt_broker("mqttbroker", "MQTT Broker", mqtt_broker, 12);
WiFiManagerParameter custom_mqtt_pwd("mqttpwd", "MQTT password", mqtt_pwd, 40);
WiFiManagerParameter custom_session_key("sessionkey", "Session Key", session_key, 40);
WiFiManagerParameter custom_entity_name("entityname", "Entity Name", entity_name, 40);
WiFiManagerParameter custom_entity_type("entitytype", "Entitiy Type", entity_type, 40);
WiFiManagerParameter custom_wifi_enc("wifienc", "Wifi Enc", wifi_enc, 40);
WiFiManagerParameter custom_entity_version("entityversion", "Entity Version", entity_version, 40);
WiFiManagerParameter custom_mqtt_port("mqttport", "MQTT Port", mqtt_port, 40);
WiFiManagerParameter custom_entity_id("entityid", "Entitiy ID", entity_id, 40);
WiFiManagerParameter custom_mqtt_retain("mqttretain", "MQTT Retain", mqtt_retain, 40);
WiFiManagerParameter custom_mqtt_qos("mqttqos", "MQTT QoS", mqtt_qos, 40);


//###############################################################################################################################

//################################################### GENERAL VARIABLES #########################################################
bool blockWM = true; // Change this to false if you want your code to continue to run on the loop void even if you are not conected to any wifi.
//###############################################################################################################################


void saveConfigCallback() {

}


void saveParamCallback() {
  Serial.println("****************Should save params");
  shouldSaveConfig = true;

  if (shouldSaveConfig) {
    Serial.println("saving config");
    //mqtt_broker = custom_mqtt_broker.getValue();
    strcpy(mqtt_broker, custom_mqtt_broker.getValue());
    DynamicJsonDocument json(1024);
    //mqtt_broker = (char*) custom_mqtt_broker.getValue();
    json["output"] = output;
    json["mqtt_broker"] = mqtt_broker;
    json["mqtt_pwd"] = mqtt_pwd;
    json["session_key"] = session_key;
    json["entity_name"] = entity_name;
    json["entity_type"] = entity_type;
    json["wifi_enc"] = wifi_enc;
    json["entity_version"] = entity_version;
    json["mqtt_port"] = mqtt_port;
    json["entity_id"] = entity_id;
    json["mqtt_retain"] = mqtt_retain;
    json["mqtt_qos"] = mqtt_qos;
    // json["ip"]          = WiFi.localIP().toString();
    // json["gateway"]     = WiFi.gatewayIP().toString();
    // json["subnet"]      = WiFi.subnetMask().toString();

    File configFile = SPIFFS.open("/config.json", "w");
    if (!configFile) {
      Serial.println("failed to open config file for writing");
    }

    serializeJson(json, Serial);
    serializeJson(json, configFile);

    //json.printTo(Serial);
    //json.printTo(configFile);
    configFile.close();
    //end save
    shouldSaveConfig = false;
  }

}

void handleNotFound()
{
  String message = "File Not Found\n\n";
  message += "URI: ";
  message += server.uri();
  message += "\nMethod: ";
  message += (server.method() == HTTP_GET) ? "GET" : "POST";
  message += "\nArguments: ";
  message += server.args();
  message += "\n";
  for (uint8_t i = 0; i < server.args(); i++)
  {
    message += " " + server.argName(i) + ": " + server.arg(i) + "\n";
  }
  server.send(404, "text/plain", message);
}

/*

  This void sets the device with the WiFiManager from tzapu with custom parameters.
  Needs to be called only in the setup void.
*/
void setupDeviceWM()
{
  //Serial.println("pppppppppppppppppp");
  //Serial.println(mqtt_broker);

  // callbacks
  wm.setConfigPortalBlocking(blockWM);
  String ssid = "IOT_ESP_" + String(ESP.getChipId());

  if (wm.autoConnect(ssid.c_str()))
  {
    //if you get here you have connected to the WiFi
    Serial.println("Connected to wifi network!");
    Serial.println(ssid);
    //Serial.println(mqtt_broker);
    //Serial.println(custom_mqtt_broker.getValue());
    -Serial.println(WiFi.localIP());
    WiFi.mode(WIFI_STA);
    wm.startWebPortal();
  }
  else
  {
    Serial.println("Could not connect with WiFi!");
  }

  // call the code down to activate wifi so users can configure the device, event if it's connected to the local network
  //wm.startConfigPortal("IOT_Device");
  //
  server.onNotFound(handleNotFound);
  server.begin(); // declare this at the beggining of the code => ESP8266WebServer server(80);
}


void bindServerCallback() {
  wm.server->on("/custom", handleRoute);
  // wm.server->on("/info",handleRoute); // can override wm!
  wm.server->on("/LED", HTTP_POST, handleLED);  // Call the 'handleLED' function when a POST request is made to URI "/LED"
}

void handleRoute() {
  Serial.println("[HTTP] handle route");
  wm.server->send(200, "text/html", "<iframe name=\"dummyframe\" id=\"dummyframe\" style=\"display: none;\"></iframe><form action=\"/LED\" target=\"dummyframe\" method=\"POST\"><input type=\"submit\" value=\"Toggle LED\"></form>");

}

void handleLED() {                          // If a POST request is made to URI /LED
  digitalWrite(atoi(output), !digitalRead(atoi(output)));     // Change the state of the LED
  server.sendHeader("Location", "/");       // Add a header to respond with a new location for the browser to go to the home page again
  server.send(303);                         // Send it back to the browser with an HTTP status 303 (See Other) to redirect
}


/*

  This void needs to be called in the loop void so it can handle the WM and the webportal.
*/
void loopDeviceWM()
{
  wm.process();
  server.handleClient();
}




void callback(char* topic, byte* payload, unsigned int length) {
  Serial.print("Message arrived [");
  Serial.print(topic);
  Serial.print("] ");
  for (int i = 0; i < length; i++) {
    Serial.print((char)payload[i]);
  }
  Serial.println();
}


int setupSpiffs() {
  //read configuration from FS json
  Serial.println("mounting FS...");

  if (SPIFFS.begin()) {
    Serial.println("mounted file system");
    if (SPIFFS.exists("/config.json")) {
      //file exists, reading and loading
      Serial.println("reading config file");
      File configFile = SPIFFS.open("/config.json", "r");
      if (configFile) {
        Serial.println("opened config file");
        size_t size = configFile.size();
        // Allocate a buffer to store contents of the file.
        std::unique_ptr<char[]> buf(new char[size]);

        configFile.readBytes(buf.get(), size);
        DynamicJsonDocument jsonBuffer(1024);
        //DynamicJsonBuffer jsonBuffer;
        deserializeJson(jsonBuffer, buf.get());
        //JsonObject& json = jsonBuffer.parseObject(buf.get());
        serializeJson(jsonBuffer, Serial);
        //json.printTo(Serial);

        if (!jsonBuffer.isNull()) {
          Serial.println("\nparsed json");
          //Serial.println(custom_mqtt_broker.getValue());
          strcpy(wifi_enc, jsonBuffer["wifi_enc"]);
          strcpy(entity_version, jsonBuffer["entity_version"]);
          strcpy(mqtt_port, jsonBuffer["mqtt_port"]);
          strcpy(output, jsonBuffer["output"]);
          strcpy(mqtt_broker, jsonBuffer["mqtt_broker"]);
          strcpy(mqtt_pwd, jsonBuffer["mqtt_pwd"]);
          strcpy(session_key, jsonBuffer["session_key"]);
          strcpy(entity_name, jsonBuffer["entity_name"]);
          strcpy(entity_type, jsonBuffer["entity_type"]);
          strcpy(entity_id, jsonBuffer["entity_id"]);
          strcpy(mqtt_retain, jsonBuffer["mqtt_retain"]);
          strcpy(mqtt_qos, jsonBuffer["mqtt_qos"]);

          //Serial.println("ooooooooooooooo");
          Serial.println("loaded Json");
          Serial.println("now mqtt broker is: ");
          Serial.println(mqtt_broker);
          
          return 0;
        } else {
          Serial.println("failed to load json config");
          return 1;
        }
        configFile.close();
      }
    }
    else
    {
      Serial.println("config json doesnt exist");
      return 1;
    }
   
  } else {
    Serial.println("failed to mount FS");
    return 1;
  }
  //end read

}

void setup()
{

  pinMode(atoi(output), OUTPUT);
  Serial.begin(115200);
  delay(3000);

  //diese funktion gibt 0 zurück wenn eine config json da ist. 
  if(setupSpiffs() == 0){
    Serial.println("SO, NOW:");
    Serial.println(mqtt_broker);
    custom_mqtt_broker = WiFiManagerParameter("mqttbroker", "MQTT Broker", mqtt_broker, sizeof(mqtt_broker));
  }
  else
  {
    
  }
  //an diesem punkt sind die parameter entweder leer weil keine config existiert, oder voll weil config da war. 
  
  wm.setWebServerCallback(bindServerCallback);
  wm.setSaveParamsCallback(saveParamCallback);

  //wm.addParameter(&custom_output);
  wm.addParameter(&custom_mqtt_broker);
  //wm.addParameter(&custom_mqtt_pwd);
  //wm.addParameter(&custom_session_key);
  //wm.addParameter(&custom_entity_name);
  //wm.addParameter(&custom_entity_type);
  //wm.addParameter(&custom_wifi_enc);
  //wm.addParameter(&custom_entity_version);
  //wm.addParameter(&custom_mqtt_port);
  //wm.addParameter(&custom_entity_id);
  //wm.addParameter(&custom_mqtt_retain);
  //wm.addParameter(&custom_mqtt_qos);

  // Set cutom menu via menu[] or vector
  // const char* menu[] = {"wifi","wifinoscan","info","param","close","sep","erase","restart","exit"};
  // wm.setMenu(menu,9); // custom menu array must provide length

  std::vector<const char *> menu = {"wifi", "info", "param", "update", "close", "sep", "erase", "restart", "exit"};
  wm.setMenu(menu); // custom menu, pass vector

  // set country
  wm.setCountry("US"); // setting wifi country seems to improve OSX soft ap connectivity, may help others as well

  Serial.println("Setup mode...");
  //wifiManager.resetSettings();

  setupDeviceWM();

  pinMode(TRIGGER_PIN, INPUT);

#ifdef USEOTA
  ArduinoOTA.begin();
#endif

  wm.setClass("invert");
  Serial.println("-------------");

  Serial.println(custom_mqtt_broker.getValue());
  Serial.println(String(mqtt_broker));
  Serial.println(mqtt_broker);
  Serial.println(session_key);
  client.setServer(mqtt_broker, atoi(mqtt_port));
  client.setCallback(callback);
  lastReconnectAttempt = 0;

}

void loop()
{
#ifdef USEOTA
  ArduinoOTA.handle();
#endif
  loopDeviceWM();


  if ( digitalRead(TRIGGER_PIN) == LOW ) {
    delay(2000);
    if ( digitalRead(TRIGGER_PIN) == LOW ) {
      Serial.println("BUTTON PRESSED");
      //wm.setConfigPortalTimeout(140);

      // disable captive portal redirection
      // wm.setCaptivePortalEnable(false);

      if (!wm.startConfigPortal("OnDemandAP", "12345678")) {
        Serial.println("failed to connect and hit timeout");
        delay(3000);
      }
    }
  }



  if (!client.connected()) {
    long now = millis();
    if (now - lastReconnectAttempt > 5000) {
      lastReconnectAttempt = now;
      // Attempt to reconnect
      if (reconnect()) {
        lastReconnectAttempt = 0;
      }
    }
  } else {
    // Client connected

    client.loop();
  }

}```

### Debug Messages

mounted file system
reading config file
opened config file
{"output":"2","mqtt_broker":"192.168.1.108","mqtt_pwd":"","session_key":"","entity_name":"","entity_type":"","wifi_enc":"","entity_version":"","mqtt_port":"1883","entity_id":"","mqtt_retain":"","mqtt_qos":""}
parsed json
loaded Json
now mqtt broker is:
192.168.1.108
SO, NOW:
192.168.1.108
*WM: [3] allocating params bytes: 20
*WM: [2] Added Parameter: mqttbroker
*WM: [1]

Configure WiFi

Info
Setup
Update
Close


Erase
Restart
Exit

Setup mode...
*WM: [1] AutoConnect
*WM: [2] esp_wifi_set_country: US
*WM: [1] AutoConnect: ESP Already Connected
*WM: [3] STA static IP:
*WM: [2] setSTAConfig static ip not set, skipping
*WM: [1] AutoConnect: SUCCESS
*WM: [1] STA IP Address: 192.168.1.109
Connected to wifi network!
IOT_ESP_6184038
192.168.1.109
*WM: [1] Starting Web Portal
*WM: [3] dns server started with ip:
*WM: [2] HTTP server started
*WM: [2] WiFi Scan completed in 2182 ms

⸮⸮⸮?168.1.10
192.168.1.108
192.168.1.108

attempting connecting to mqtt


@tablatronix tablatronix added Question User Question member to member support DEV Help Wanted Developer Needs Help labels May 11, 2020
@rvt
Copy link

rvt commented May 12, 2020

You need to let c++ decide for the size:
So use ‘char mqtt_broker[] = "192.168.1.108";’
And male sure that the length parameter for WiFiManagerParameter is large enough for a
An IP address: 16 chars

@zen85
Copy link
Author

zen85 commented May 12, 2020

You need to let c++ decide for the size:
So use ‘char mqtt_broker[] = "192.168.1.108";’
And male sure that the length parameter for WiFiManagerParameter is large enough for a
An IP address: 16 chars

unfortunatly this did not work and does not change anything no matter what i do there... the error occurs when i do the following:

  • i setup the device, put in my wifi and/or save params.
  • the device connects... everything works... also after restarting.
  • when i enter the params page again and set a new or the same value and save it... suddenly "custom_mqtt_broker.getValue()" messes up the value and instead of "192.168.1.108" says "⸮⸮⸮?168.1.10". the variable "mqtt_broker" is fine... but since custom_mqtt_broker.getValue() does not output the right value the Webinterface is messed up.

it is so weird that this sequence of events works totally fine till it breaks...

this issue here seems to be related?
#1020

@tablatronix
Copy link
Collaborator

tablatronix commented May 12, 2020

Are you sure the parameters are still in scope?

Maybe reduce your test sketch so we can reproduce, there is too much going on there, try the examples

@rvt
Copy link

rvt commented May 12, 2020

You are re-assigning custom_mqtt_broker in your setup pfunction , don’t do it like that.

@zen85
Copy link
Author

zen85 commented May 13, 2020

ok... progress :)

Are you sure the parameters are still in scope?

Maybe reduce your test sketch so we can reproduce, there is too much going on there, try the examples

i did that and then

You are re-assigning custom_mqtt_broker in your setup pfunction , don’t do it like that.

got rid of the weirdness... but i cant find a way to update the value of custom_mqtt_broker on runtime... so the value i get back at custom_mqtt_broker.getValue() will always be 192.168.1.108 no matter what i do in the portal and therefor the param-interface will always show that value. is there a method to do this cleanly?

#include <FS.h>                   //this needs to be first, or it all crashes and burns...
#include <ArduinoJson.h>          //https://github.com/bblanchon/ArduinoJson
#include <WiFiManager.h> //https://github.com/tzapu/WiFiManager WiFi Configuration Magic
#include <PubSubClient.h>
#include <ESP8266WiFi.h>


WiFiManager wm;
ESP8266WebServer server(80);


//############################################# CUSTOM PARAMETERS FOR THE WIFI MANAGER ##########################################

char mqtt_broker[16] = "192.168.100.108";


WiFiManagerParameter custom_mqtt_broker("mqttbroker", "MQTT Broker", mqtt_broker, 16);



//################################################### GENERAL VARIABLES #########################################################
bool blockWM = true; // Change this to false if you want your code to continue to run on the loop void even if you are not conected to any wifi.
//###############################################################################################################################



void saveParamCallback() {
  Serial.println("****************Should save params");
  Serial.println(mqtt_broker);
  Serial.println(custom_mqtt_broker.getValue());

  strcpy(mqtt_broker, custom_mqtt_broker.getValue());
  
  Serial.println(mqtt_broker);
  Serial.println(custom_mqtt_broker.getValue());
  
  DynamicJsonDocument json(1024);
 
  json["mqtt_broker"] = mqtt_broker;
 
    
  File configFile2 = SPIFFS.open("/config.json", "w");
    if (!configFile2) {
      Serial.println("failed to open config file for writing");
    }
    
  serializeJson(json, Serial);
  serializeJson(json, configFile2);

  configFile2.close();
  //end save
  }



void handleNotFound()
{
  String message = "File Not Found\n\n";
  message += "URI: ";
  message += server.uri();
  message += "\nMethod: ";
  message += (server.method() == HTTP_GET) ? "GET" : "POST";
  message += "\nArguments: ";
  message += server.args();
  message += "\n";
  for (uint8_t i = 0; i < server.args(); i++)
  {
    message += " " + server.argName(i) + ": " + server.arg(i) + "\n";
  }
  server.send(404, "text/plain", message);
}

/*
  This void sets the device with the WiFiManager from tzapu with custom parameters.
  Needs to be called only in the setup void.
*/

void setupDeviceWM()
{
  //Serial.println("setupDeviceWM() is running");


  // callbacks
  wm.setConfigPortalBlocking(blockWM);
  
  String ssid = "IOT_ESP_" + String(ESP.getChipId());

  if (wm.autoConnect(ssid.c_str()))
  {
    //if you get here you have connected to the WiFi
    Serial.println("Connected to wifi network!");
    Serial.println(ssid);
    Serial.println(WiFi.localIP());
    
    WiFi.mode(WIFI_STA);
    wm.startWebPortal();
  }
  else
  {
    Serial.println("Could not connect with WiFi!");
  }

  // call the code down to activate wifi so users can configure the device, event if it's connected to the local network
  //wm.startConfigPortal("IOT_Device");
  //
  server.onNotFound(handleNotFound);
  server.begin(); // declare this at the beggining of the code => ESP8266WebServer server(80);
}



/*
  This void needs to be called in the loop void so it can handle the WM and the webportal.
*/


void loopDeviceWM()
{
  wm.process();
  server.handleClient();
}


int setupSpiffs() {
  //read configuration from FS json
  Serial.println("mounting FS...");

  if (SPIFFS.begin()) {
    Serial.println("mounted file system");
    if (SPIFFS.exists("/config.json")) {
      //file exists, reading and loading
      Serial.println("reading config file");
      File configFile = SPIFFS.open("/config.json", "r");
      if (configFile) {
        Serial.println("opened config file");
        size_t size = configFile.size();
        // Allocate a buffer to store contents of the file.
        std::unique_ptr<char[]> buf(new char[size]);

        configFile.readBytes(buf.get(), size);
        DynamicJsonDocument jsonBuffer(1024);
        deserializeJson(jsonBuffer, buf.get());
        serializeJson(jsonBuffer, Serial);
      
        if (!jsonBuffer.isNull()) {
          Serial.println("\nparsed json");

          strcpy(mqtt_broker, jsonBuffer["mqtt_broker"]);

          Serial.println("loaded Json");
          Serial.println("now mqtt broker is: ");
          Serial.println(mqtt_broker);
          
          return 0;
          
        } else {
          Serial.println("failed to load json config");
          return 1;
        }
        configFile.close();
      }
    }
    else
    {
      Serial.println("config json doesnt exist");
      return 1;
    }
   
  } else {
    Serial.println("failed to mount FS");
    return 1;
  }
  //end read

}

void setup()
{

  Serial.begin(115200);
  delay(3000);

  //diese funktion gibt 0 zurück wenn eine config json da ist. 
  if(setupSpiffs() == 0){
    Serial.println("SO, NOW:");
    Serial.println(mqtt_broker);
    //custom_mqtt_broker = WiFiManagerParameter("mqttbroker", "MQTT Broker", mqtt_broker, 16);
  }
  else
  {
    
  }
  // at this point the parameters are either empty because there is no config or full because there is a config
  

  wm.setSaveParamsCallback(saveParamCallback);

 
  wm.addParameter(&custom_mqtt_broker);


  std::vector<const char *> menu = {"wifi", "info", "param", "close", "sep", "erase", "restart", "exit"};
  wm.setMenu(menu); // custom menu, pass vector

  // set country
  wm.setCountry("US"); // setting wifi country seems to improve OSX soft ap connectivity, may help others as well

  Serial.println("Setup mode...");
  //wifiManager.resetSettings();
  
  setupDeviceWM();




  wm.setClass("invert");
  Serial.println("-------------");
  Serial.println(String(custom_mqtt_broker.getValue()));
  Serial.println(custom_mqtt_broker.getValue());
  Serial.println(String(mqtt_broker));
  Serial.println(mqtt_broker);


}

void loop()
{
  
  loopDeviceWM();

}

@tablatronix
Copy link
Collaborator

I have no idea, your sketch still contains other stuff not needed to reproduce, hell you have 2 webservers running...

@zen85
Copy link
Author

zen85 commented May 13, 2020

you are right - you need to know my motivation to know why i left that part in:

the basic idea behind the sketch was to take the sketch posted on the end of this:
#656

and to merge it with the FS Example.

i guess the question turned into
"how can i define WiFiManagerParameter custom_mqtt_broker("mqttbroker", "MQTT Broker", mqtt_broker, 40) ;
as global and change the value of custom_mqtt_broker after i read the spiffs.

@zen85
Copy link
Author

zen85 commented May 13, 2020

i refactored and still got to the root of the problem:

when i configure the first time everything is cool. after entering the configportal while on wireless with the local ip the fields are empty... therefor when i change a value in the custom parameters i rewrite the existing ones with "" - i wanted to get around of that and so my code above seems, and quite probably is, kind of weird...

so here is this version for reproducing that:

#include <FS.h>          // this needs to be first, or it all crashes and burns...
#include <WiFiManager.h> // https://github.com/tzapu/WiFiManager
#include <ArduinoJson.h> // https://github.com/bblanchon/ArduinoJson

#ifdef ESP32
  #include <SPIFFS.h>
#endif

//define your default values here, if there are different values in config.json, they are overwritten.
char mqtt_server[40];
char mqtt_port[6]  = "8080";
char api_token[32] = "YOUR_API_TOKEN";

//default custom static IP
char static_ip[16] = "10.0.1.56";
char static_gw[16] = "10.0.1.1";
char static_sn[16] = "255.255.255.0";

//flag for saving data
bool shouldSaveConfig = false;

//callback notifying us of the need to save config
void saveConfigCallback () {
  Serial.println("Should save config");
  shouldSaveConfig = true;
}

void setupSpiffs(){
  //clean FS, for testing
  // SPIFFS.format();

  //read configuration from FS json
  Serial.println("mounting FS...");

  if (SPIFFS.begin()) {
    Serial.println("mounted file system");
    if (SPIFFS.exists("/config.json")) {
      //file exists, reading and loading
      Serial.println("reading config file");
      File configFile = SPIFFS.open("/config.json", "r");
      if (configFile) {
        Serial.println("opened config file");
        size_t size = configFile.size();
        // Allocate a buffer to store contents of the file.
        std::unique_ptr<char[]> buf(new char[size]);

        configFile.readBytes(buf.get(), size);
       DynamicJsonDocument jsonBuffer(1024);
        //DynamicJsonBuffer jsonBuffer;
        deserializeJson(jsonBuffer, buf.get());
        //JsonObject& json = jsonBuffer.parseObject(buf.get());
        serializeJson(jsonBuffer, Serial);
        //json.printTo(Serial);



        if (!jsonBuffer.isNull()) {        
    
          Serial.println("\nparsed json");

          strcpy(mqtt_server, jsonBuffer["mqtt_server"]);
          strcpy(mqtt_port, jsonBuffer["mqtt_port"]);
          strcpy(api_token, jsonBuffer["api_token"]);

          // if(json["ip"]) {
          //   Serial.println("setting custom ip from config");
          //   strcpy(static_ip, json["ip"]);
          //   strcpy(static_gw, json["gateway"]);
          //   strcpy(static_sn, json["subnet"]);
          //   Serial.println(static_ip);
          // } else {
          //   Serial.println("no custom ip in config");
          // }

        } else {
          Serial.println("failed to load json config");
        }
      }
    }
  } else {
    Serial.println("failed to mount FS");
  }
  //end read
}

  WiFiManager wm;
  WiFiManagerParameter custom_mqtt_server("server", "mqtt server", mqtt_server, 40);
  WiFiManagerParameter custom_mqtt_port("port", "mqtt port", mqtt_port, 6);
  WiFiManagerParameter custom_api_token("api", "api token", "", 32);

void setup() {
  // put your setup code here, to run once:
  Serial.begin(115200);
  Serial.println();

  setupSpiffs();

  // WiFiManager, Local intialization. Once its business is done, there is no need to keep it around


  //set config save notify callback
  wm.setSaveConfigCallback(saveConfigCallback);

  // setup custom parameters
  // 
  // The extra parameters to be configured (can be either global or just in the setup)
  // After connecting, parameter.getValue() will get you the configured value
  // id/name placeholder/prompt default length

  //add all your parameters here
  wm.addParameter(&custom_mqtt_server);
  wm.addParameter(&custom_mqtt_port);
  wm.addParameter(&custom_api_token);

  // set static ip
  // IPAddress _ip,_gw,_sn;
  // _ip.fromString(static_ip);
  // _gw.fromString(static_gw);
  // _sn.fromString(static_sn);
  // wm.setSTAStaticIPConfig(_ip, _gw, _sn);

  //reset settings - wipe credentials for testing
  //wm.resetSettings();

  //automatically connect using saved credentials if they exist
  //If connection fails it starts an access point with the specified name
  //here  "AutoConnectAP" if empty will auto generate basedcon chipid, if password is blank it will be anonymous
  //and goes into a blocking loop awaiting configuration
  if (!wm.autoConnect("AutoConnectAP", "password")) {
    Serial.println("failed to connect and hit timeout");
    delay(3000);
    // if we still have not connected restart and try all over again
    ESP.restart();
    delay(5000);
  }

  // always start configportal for a little while
  // wm.setConfigPortalTimeout(60);
  // wm.startConfigPortal("AutoConnectAP","password");

  //if you get here you have connected to the WiFi
  Serial.println("connected...yeey :)");

  WiFi.mode(WIFI_STA);
  wm.startWebPortal();
  
  //read updated parameters
  strcpy(mqtt_server, custom_mqtt_server.getValue());
  strcpy(mqtt_port, custom_mqtt_port.getValue());
  strcpy(api_token, custom_api_token.getValue());

  //save the custom parameters to FS
  if (shouldSaveConfig) {
    Serial.println("saving config");
     DynamicJsonDocument jsonBuffer(1024);
  
   // JsonObject& json = jsonBuffer.createObject();
    jsonBuffer["mqtt_server"] = mqtt_server;
    jsonBuffer["mqtt_port"]   = mqtt_port;
    jsonBuffer["api_token"]   = api_token;

    // json["ip"]          = WiFi.localIP().toString();
    // json["gateway"]     = WiFi.gatewayIP().toString();
    // json["subnet"]      = WiFi.subnetMask().toString();

    File configFile = SPIFFS.open("/config.json", "w");
    if (!configFile) {
      Serial.println("failed to open config file for writing");
    }

    serializeJson(jsonBuffer, Serial);
    serializeJson(jsonBuffer, configFile);


    //jsonBuffer.prettyPrintTo(Serial);
    //jsonBuffer.printTo(configFile);
    configFile.close();
    //end save
    shouldSaveConfig = false;
  }

  Serial.println("local ip");
  Serial.println(WiFi.localIP());
  Serial.println(WiFi.gatewayIP());
  Serial.println(WiFi.subnetMask());
}

void loop() {
  // put your main code here, to run repeatedly:

  wm.process();
}````

@tablatronix
Copy link
Collaborator

ok this is pretty much the example, so where exactly is the problem, where in the code can we see the issue ? can you add a line in there or serial print ?

@zen85
Copy link
Author

zen85 commented May 13, 2020

ok this is pretty much the example, so where exactly is the problem, where in the code can we see the issue ? can you add a line in there or serial print ?

ok - above sketch never saved changed parameters to spiff... could not work anyway... but please try this one. there is an obvious serial print saying what i expect and this is not matching up with what i get at the end of the setup - so as you can see "mqtt_broker" is alright - i can use this to connect ... but custom_mqtt_broker.getValue() is not. and that also shows on the portal making it unusable.

#include <FS.h>                   //this needs to be first, or it all crashes and burns...
#include <ArduinoJson.h>          //https://github.com/bblanchon/ArduinoJson
#include <WiFiManager.h> //https://github.com/tzapu/WiFiManager WiFi Configuration Magic



WiFiManager wm;

//############################################# CUSTOM PARAMETERS FOR THE WIFI MANAGER ##########################################

char mqtt_broker[16] = "192.168.100.108";


WiFiManagerParameter custom_mqtt_broker("mqttbroker", "MQTT Broker", mqtt_broker, 16);



//################################################### GENERAL VARIABLES #########################################################
bool blockWM = true; // Change this to false if you want your code to continue to run on the loop void even if you are not conected to any wifi.
//###############################################################################################################################



void saveParamCallback() {
  Serial.println("****************Should save params");
  Serial.println(mqtt_broker);
  Serial.println(custom_mqtt_broker.getValue());

  strcpy(mqtt_broker, custom_mqtt_broker.getValue());
  
  Serial.println(mqtt_broker);
  Serial.println(custom_mqtt_broker.getValue());
  
  DynamicJsonDocument json(1024);
 
  json["mqtt_broker"] = mqtt_broker;
 
    
  File configFile2 = SPIFFS.open("/config.json", "w");
    if (!configFile2) {
      Serial.println("failed to open config file for writing");
    }
    
  serializeJson(json, Serial);
  serializeJson(json, configFile2);

  configFile2.close();
  //end save
  }



/*
  This void sets the device with the WiFiManager from tzapu with custom parameters.
  Needs to be called only in the setup void.
*/

void setupDeviceWM()
{
  //Serial.println("setupDeviceWM() is running");


  // callbacks
  wm.setConfigPortalBlocking(blockWM);
  
  String ssid = "IOT_ESP_" + String(ESP.getChipId());

  if (wm.autoConnect(ssid.c_str()))
  {
    //if you get here you have connected to the WiFi
    Serial.println("Connected to wifi network!");
    Serial.println(ssid);
    Serial.println(WiFi.localIP());
    
    WiFi.mode(WIFI_STA);
    wm.startWebPortal();
  }
  else
  {
    Serial.println("Could not connect with WiFi!");
  }

  // call the code down to activate wifi so users can configure the device, event if it's connected to the local network
  //wm.startConfigPortal("IOT_Device");
  //
  }



/*
  This void needs to be called in the loop void so it can handle the WM and the webportal.
*/


void loopDeviceWM()
{
  wm.process();
  
}


int setupSpiffs() {
  //read configuration from FS json
  Serial.println("mounting FS...");

  if (SPIFFS.begin()) {
    Serial.println("mounted file system");
    if (SPIFFS.exists("/config.json")) {
      //file exists, reading and loading
      Serial.println("reading config file");
      File configFile = SPIFFS.open("/config.json", "r");
      if (configFile) {
        Serial.println("opened config file");
        size_t size = configFile.size();
        // Allocate a buffer to store contents of the file.
        std::unique_ptr<char[]> buf(new char[size]);

        configFile.readBytes(buf.get(), size);
        DynamicJsonDocument jsonBuffer(1024);
        deserializeJson(jsonBuffer, buf.get());
        serializeJson(jsonBuffer, Serial);
      
        if (!jsonBuffer.isNull()) {
          Serial.println("\nparsed json");

          strcpy(mqtt_broker, jsonBuffer["mqtt_broker"]);

          Serial.println("loaded Json");
          Serial.println("now mqtt broker is: ");
          Serial.println(mqtt_broker);
          
          return 0;
          
        } else {
          Serial.println("failed to load json config");
          return 1;
        }
        configFile.close();
      }
    }
    else
    {
      Serial.println("config json doesnt exist");
      return 1;
    }
   
  } else {
    Serial.println("failed to mount FS");
    return 1;
  }
  //end read

}

void setup()
{

  Serial.begin(115200);
  delay(3000);

  //diese funktion gibt 0 zurück wenn eine config json da ist. 
  if(setupSpiffs() == 0){
    Serial.println("SO, NOW:");
    Serial.println(mqtt_broker);
    //custom_mqtt_broker = WiFiManagerParameter("mqttbroker", "MQTT Broker", mqtt_broker, 16);
  }
  else
  {
    
  }
  // at this point the parameters are either empty because there is no config or full because there is a config
  

  wm.setSaveParamsCallback(saveParamCallback);

 
  wm.addParameter(&custom_mqtt_broker);


  std::vector<const char *> menu = {"wifi", "info", "param", "close", "sep", "erase", "restart", "exit"};
  wm.setMenu(menu); // custom menu, pass vector

  // set country
  wm.setCountry("US"); // setting wifi country seems to improve OSX soft ap connectivity, may help others as well

  Serial.println("Setup mode...");
  //wifiManager.resetSettings();
  
  setupDeviceWM();




  wm.setClass("invert");
  Serial.println("**** Issue here **** - the next four lines should be the mqtt broker adress after changing in portal and restarting but apparently custom_mqtt_broker.getValue() never gets it value of the config.json");
  Serial.println(String(custom_mqtt_broker.getValue()));
  Serial.println(custom_mqtt_broker.getValue());
  Serial.println(String(mqtt_broker));
  Serial.println(mqtt_broker);


}

void loop()
{
  
  loopDeviceWM();

}

@tablatronix
Copy link
Collaborator

Why are you copying it back into the same variable though ?

strcpy(mqtt_broker, jsonBuffer["mqtt_broker"]);

@tablatronix
Copy link
Collaborator

you also should not be doing this much stuff in your callbacks, just set some flag and then save your spiffs later.

@zen85
Copy link
Author

zen85 commented May 13, 2020

Why are you copying it back into the same variable though ?

strcpy(mqtt_broker, jsonBuffer["mqtt_broker"]);

this is just done in setup... "mqtt_broker" is always alright. it behaves perfectly even when i put it in the code... but when i enter the configportal > setup it shows the first value it had and the changes are not reflected there... if i could just put something like "custom_mqtt_broker = mqtt_broker" everything would work :) . if i comment that out nothing is updated...

the callback also works fine i think. the config.json reflects everything i change. otherwise "strcpy(mqtt_broker, jsonBuffer["mqtt_broker"]);" would not work. its just the UI thats unusable because it will always say "192.168.100.108" even if i change it in the configportal while on wifi.

@tablatronix
Copy link
Collaborator

tablatronix commented May 13, 2020

Looks fine to me

*WM: [1] Starting Web Portal 
*WM: [3] dns server started with ip: 
*WM: [2] HTTP server started 
*WM: [2] WiFi Scan completed in 2182 ms
**** Issue here **** - the next four lines should be the mqtt broker adress after changing in portal and restarting but apparently custom_mqtt_broker.getValue() never gets it value of the config.json
192.168.100.108
192.168.100.108
192.168.100.108
192.168.100.108
*WM: [2] <- HTTP Root 
*WM: [3] -> 192.168.1.84 
*WM: [3] lastconxresult: WL_CONNECTED
*WM: [2] WiFi Scan completed in 2183 ms
*WM: [2] <- HTTP Param 
*WM: [3] lastconxresult: WL_CONNECTED
*WM: [3] Sent param page 
*WM: [2] <- HTTP WiFi save  
*WM: [3] Method: POST
*WM: [2] Parameters 
*WM: [2] -------------------- 
*WM: [2] mqttbroker: 192.168.100.112
*WM: [2] -------------------- 
****************Should save params
192.168.100.108
192.168.100.112
192.168.100.112
192.168.100.112
{"mqtt_broker":"192.168.100.112"}*WM: [3] Sent param save page 

I had to remove spiffs, it was crashing my esp, maybe your problem is with spiffs..

Not really understanding what you are trying to do though..
I am going to assume your issue is loading or saving to spiffs or json, so remove all the other WM code and use plain variables and see if you have the same problem.

Is this a typo?
apparently custom_mqtt_broker.getValue() never gets it value of the config.json

because that doesn't make sense, wm params have nothing to do with json.. they are whatever you set them to or what you submit them as in the http get request.

@zen85
Copy link
Author

zen85 commented May 13, 2020

yes... it works so far... if you restart the esp now it does not anymore

and it gets extremly close to working if i put:
if(setupSpiffs() == 0){
Serial.println("SO, NOW:");
Serial.println(mqtt_broker);
custom_mqtt_broker = WiFiManagerParameter("mqttbroker", "MQTT Broker", mqtt_broker, sizeof(mqtt_broker));
}

@tablatronix
Copy link
Collaborator

tablatronix commented May 13, 2020

If it is not set after resetting then you are not reading it from spiffs properly and then setting it..
Look for a bug there, either the spiffs read or the json decode, the answer was already posted above and you are still using the same variable char mqtt_broker[16] with the same size , reread the comments above by @rvt

@zen85
Copy link
Author

zen85 commented May 13, 2020

If it is not set after resetting then you are not reading it from spiffs properly and then setting it..
Look for a bug there, either the spiffs read or the json decode

but it is set properly... restart it after you did what you just did. and then enter the portal on the wifi, go to setup... now you will still see 192.168.100.108 and not 192.168.100.112 while the value of mqtt_broker is correctly 192.168.100.112.

here it is with the answer from above - no change:


#include <FS.h>                   //this needs to be first, or it all crashes and burns...
#include <ArduinoJson.h>          //https://github.com/bblanchon/ArduinoJson
#include <WiFiManager.h> //https://github.com/tzapu/WiFiManager WiFi Configuration Magic



WiFiManager wm;

//############################################# CUSTOM PARAMETERS FOR THE WIFI MANAGER ##########################################

char mqtt_broker[] = "192.168.100.108";


WiFiManagerParameter custom_mqtt_broker("mqttbroker", "MQTT Broker", mqtt_broker, sizeof(mqtt_broker));



//################################################### GENERAL VARIABLES #########################################################
bool blockWM = true; // Change this to false if you want your code to continue to run on the loop void even if you are not conected to any wifi.
//###############################################################################################################################



void saveParamCallback() {
  Serial.println("****************Should save params");
  Serial.println(mqtt_broker);
  Serial.println(custom_mqtt_broker.getValue());

  strcpy(mqtt_broker, custom_mqtt_broker.getValue());
  
  Serial.println(mqtt_broker);
  Serial.println(custom_mqtt_broker.getValue());
  
  DynamicJsonDocument json(1024);
 
  json["mqtt_broker"] = mqtt_broker;
 
    
  File configFile2 = SPIFFS.open("/config.json", "w");
    if (!configFile2) {
      Serial.println("failed to open config file for writing");
    }
    
  serializeJson(json, Serial);
  serializeJson(json, configFile2);

  configFile2.close();
  //end save
  }



/*
  This void sets the device with the WiFiManager from tzapu with custom parameters.
  Needs to be called only in the setup void.
*/

void setupDeviceWM()
{
  //Serial.println("setupDeviceWM() is running");


  // callbacks
  wm.setConfigPortalBlocking(blockWM);
  
  String ssid = "IOT_ESP_" + String(ESP.getChipId());

  if (wm.autoConnect(ssid.c_str()))
  {
    //if you get here you have connected to the WiFi
    Serial.println("Connected to wifi network!");
    Serial.println(ssid);
    Serial.println(WiFi.localIP());
    
    WiFi.mode(WIFI_STA);
    wm.startWebPortal();
  }
  else
  {
    Serial.println("Could not connect with WiFi!");
  }

  // call the code down to activate wifi so users can configure the device, event if it's connected to the local network
  //wm.startConfigPortal("IOT_Device");
  //
  }



/*
  This void needs to be called in the loop void so it can handle the WM and the webportal.
*/


void loopDeviceWM()
{
  wm.process();
  
}


int setupSpiffs() {
  //read configuration from FS json
  Serial.println("mounting FS...");

  if (SPIFFS.begin()) {
    Serial.println("mounted file system");
    if (SPIFFS.exists("/config.json")) {
      //file exists, reading and loading
      Serial.println("reading config file");
      File configFile = SPIFFS.open("/config.json", "r");
      if (configFile) {
        Serial.println("opened config file");
        size_t size = configFile.size();
        // Allocate a buffer to store contents of the file.
        std::unique_ptr<char[]> buf(new char[size]);

        configFile.readBytes(buf.get(), size);
        DynamicJsonDocument jsonBuffer(1024);
        deserializeJson(jsonBuffer, buf.get());
        serializeJson(jsonBuffer, Serial);
      
        if (!jsonBuffer.isNull()) {
          Serial.println("\nparsed json");

          strcpy(mqtt_broker, jsonBuffer["mqtt_broker"]);

          Serial.println("loaded Json");
          Serial.println("now mqtt broker is: ");
          Serial.println(mqtt_broker);
          
          return 0;
          
        } else {
          Serial.println("failed to load json config");
          return 1;
        }
        configFile.close();
      }
    }
    else
    {
      Serial.println("config json doesnt exist");
      return 1;
    }
   
  } else {
    Serial.println("failed to mount FS");
    return 1;
  }
  //end read

}

void setup()
{

  Serial.begin(115200);
  delay(3000);

  //diese funktion gibt 0 zurück wenn eine config json da ist. 
  if(setupSpiffs() == 0){
    Serial.println("SO, NOW:");
    Serial.println(mqtt_broker);
    custom_mqtt_broker = WiFiManagerParameter("mqttbroker", "MQTT Broker", mqtt_broker, sizeof(mqtt_broker));
  }
  else
  {
    
  }
  // at this point the parameters are either empty because there is no config or full because there is a config
  

  wm.setSaveParamsCallback(saveParamCallback);

 
  wm.addParameter(&custom_mqtt_broker);


  std::vector<const char *> menu = {"wifi", "info", "param", "close", "sep", "erase", "restart", "exit"};
  wm.setMenu(menu); // custom menu, pass vector

  // set country
  wm.setCountry("US"); // setting wifi country seems to improve OSX soft ap connectivity, may help others as well

  Serial.println("Setup mode...");
  //wifiManager.resetSettings();
  
  setupDeviceWM();




  wm.setClass("invert");
  Serial.println("**** Issue here **** - the next four lines should be the mqtt broker adress after changing in portal and restarting but apparently custom_mqtt_broker.getValue() never gets it value of the config.json");
  Serial.println(String(custom_mqtt_broker.getValue()));
  Serial.println(custom_mqtt_broker.getValue());
  Serial.println(String(mqtt_broker));
  Serial.println(mqtt_broker);


}

void loop()
{
  
  loopDeviceWM();

}

@tablatronix
Copy link
Collaborator

tablatronix commented May 13, 2020

I cannot test that part, my spiffs is not working, reread my previous port I edited it.

you know you can init a param without a value and then set value later ?

your code just does this over and over on every restart, why would the value change ?


char mqtt_broker[16] = "192.168.100.108";


WiFiManagerParameter custom_mqtt_broker("mqttbroker", "MQTT Broker", mqtt_broker, 16);

@tablatronix
Copy link
Collaborator

tablatronix commented May 13, 2020

oops I commented out the spiffs part

I see it now

custom_mqtt_broker = WiFiManagerParameter("mqttbroker", "MQTT Broker", mqtt_broker, sizeof(mqtt_broker));

Yeah so my guess is you are reading this in wrong into the same var[16] char, you need to not do that, as mentioned above.

@tablatronix
Copy link
Collaborator

  wm.setBreakAfterConfig(true);
  wm.setSaveParamsCallback(saveParamCallback);

 
  wm.addParameter(&custom_mqtt_broker);
  Serial.println(String(custom_mqtt_broker.getValue())); // <--- what does this say ?

I am curious is it corrupt only after submitting and getting it? Or is this also corrupt ?

@zen85
Copy link
Author

zen85 commented May 13, 2020

if i dont do
custom_mqtt_broker = WiFiManagerParameter("mqttbroker", "MQTT Broker", mqtt_broker, sizeof(mqtt_broker));
nothing is updated at all and the value will always be 192.168.100.108.... if i do that i get at least "@⸮⸮?168.100.102"

Serial.println(String(custom_mqtt_broker.getValue())); // <--- what does this say ?

SO, NOW:
192.168.100.102
*WM: [3] allocating params bytes: 20
*WM: [2] Added Parameter: mqttbroker
@⸮⸮?168.100.102

@tablatronix
Copy link
Collaborator

interesting

@tablatronix
Copy link
Collaborator

tablatronix commented May 13, 2020

oh sorry, I have that commented out, do not do // if(setupSpiffs() == 0){
remove that whole block

Just set and get the param value, that is all

@tablatronix
Copy link
Collaborator

tablatronix commented May 13, 2020

Then change this
strcpy(mqtt_broker, jsonBuffer["mqtt_broker"]);
to use a different VARIABLE
say mqtt_broker_RESTORE[];

then change your spiffs restore to

   if(setupSpiffs() == 0){
     Serial.println("SO, NOW:");
     Serial.println(mqtt_broker);
     //custom_mqtt_broker = WiFiManagerParameter("mqttbroker", "MQTT Broker", mqtt_broker_RESTORE, 16); // <-- NEW VAR
  }

Sorry it took me like 30 minutes and 2 reboots to get my damn serial ports working, and now I have to format spiffs to actually run your code.

@zen85
Copy link
Author

zen85 commented May 13, 2020

Then change this
strcpy(mqtt_broker, jsonBuffer["mqtt_broker"]);
to use a different VARIABLE
say mqtt_broker_RESTORE[];

then change your spiffs restore to

   if(setupSpiffs() == 0){
     Serial.println("SO, NOW:");
     Serial.println(mqtt_broker);
     //custom_mqtt_broker = WiFiManagerParameter("mqttbroker", "MQTT Broker", mqtt_broker_RESTORE, 16); // <-- NEW VAR
  }

Sorry it took me like 30 minutes and 2 reboots to get my damn serial ports working, and now I have to format spiffs to actually run your code.

heyhey.... its completly amazing that there is somebody out there... i sunk about 20 hours in this now and it feels truly heartwarming that you are taking care...

so i changed my code to this but i still have the same outcome:

#include <FS.h>                   //this needs to be first, or it all crashes and burns...
#include <ArduinoJson.h>          //https://github.com/bblanchon/ArduinoJson
#include <WiFiManager.h> //https://github.com/tzapu/WiFiManager WiFi Configuration Magic



WiFiManager wm;

//############################################# CUSTOM PARAMETERS FOR THE WIFI MANAGER ##########################################

char mqtt_broker[] = "192.168.100.108";
char mqtt_broker_RESTORE[sizeof(mqtt_broker)];

WiFiManagerParameter custom_mqtt_broker("mqttbroker", "MQTT Broker", mqtt_broker, sizeof(mqtt_broker));



//################################################### GENERAL VARIABLES #########################################################
bool blockWM = true; // Change this to false if you want your code to continue to run on the loop void even if you are not conected to any wifi.
//###############################################################################################################################



void saveParamCallback() {
  Serial.println("****************Should save params");
  Serial.println(mqtt_broker);
  Serial.println(custom_mqtt_broker.getValue());

  strcpy(mqtt_broker, custom_mqtt_broker.getValue());
  
  Serial.println(mqtt_broker);
  Serial.println(custom_mqtt_broker.getValue());
  
  DynamicJsonDocument json(1024);
 
  json["mqtt_broker"] = mqtt_broker;
 
    
  File configFile2 = SPIFFS.open("/config.json", "w");
    if (!configFile2) {
      Serial.println("failed to open config file for writing");
    }
    
  serializeJson(json, Serial);
  serializeJson(json, configFile2);

  configFile2.close();
  //end save
  }



/*
  This void sets the device with the WiFiManager from tzapu with custom parameters.
  Needs to be called only in the setup void.
*/

void setupDeviceWM()
{
  //Serial.println("setupDeviceWM() is running");


  // callbacks
  wm.setConfigPortalBlocking(blockWM);
  
  String ssid = "IOT_ESP_" + String(ESP.getChipId());

  if (wm.autoConnect(ssid.c_str()))
  {
    //if you get here you have connected to the WiFi
    Serial.println("Connected to wifi network!");
    Serial.println(ssid);
    Serial.println(WiFi.localIP());
    
    WiFi.mode(WIFI_STA);
    wm.startWebPortal();
  }
  else
  {
    Serial.println("Could not connect with WiFi!");
  }

  // call the code down to activate wifi so users can configure the device, event if it's connected to the local network
  //wm.startConfigPortal("IOT_Device");
  //
  }



/*
  This void needs to be called in the loop void so it can handle the WM and the webportal.
*/


void loopDeviceWM()
{
  wm.process();
  
}


int setupSpiffs() {
  //read configuration from FS json
  Serial.println("mounting FS...");

  if (SPIFFS.begin()) {
    Serial.println("mounted file system");
    if (SPIFFS.exists("/config.json")) {
      //file exists, reading and loading
      Serial.println("reading config file");
      File configFile = SPIFFS.open("/config.json", "r");
      if (configFile) {
        Serial.println("opened config file");
        size_t size = configFile.size();
        // Allocate a buffer to store contents of the file.
        std::unique_ptr<char[]> buf(new char[size]);

        configFile.readBytes(buf.get(), size);
        DynamicJsonDocument jsonBuffer(1024);
        deserializeJson(jsonBuffer, buf.get());
        serializeJson(jsonBuffer, Serial);
      
        if (!jsonBuffer.isNull()) {
          Serial.println("\nparsed json");

          strcpy(mqtt_broker_RESTORE, jsonBuffer["mqtt_broker"]);

          Serial.println("loaded Json");
          Serial.println("now mqtt broker is: ");
          Serial.println(mqtt_broker);
          
          return 0;
          
        } else {
          Serial.println("failed to load json config");
          return 1;
        }
        configFile.close();
      }
    }
    else
    {
      Serial.println("config json doesnt exist");
      return 1;
    }
   
  } else {
    Serial.println("failed to mount FS");
    return 1;
  }
  //end read

}

void setup()
{

  Serial.begin(115200);
  delay(3000);

  //diese funktion gibt 0 zurück wenn eine config json da ist. 
  
   if(setupSpiffs() == 0){
     Serial.println("SO, NOW:");
     Serial.println(mqtt_broker);
      custom_mqtt_broker = WiFiManagerParameter("mqttbroker", "MQTT Broker", mqtt_broker_RESTORE, 16); // <-- NEW VAR
  }

  
  // at this point the parameters are either empty because there is no config or full because there is a config
  
  wm.setBreakAfterConfig(true);
  wm.setSaveParamsCallback(saveParamCallback);

 
  wm.addParameter(&custom_mqtt_broker);
  Serial.println(String(custom_mqtt_broker.getValue())); // <--- what does this say ?

  std::vector<const char *> menu = {"wifi", "info", "param", "close", "sep", "erase", "restart", "exit"};
  wm.setMenu(menu); // custom menu, pass vector

  // set country
  wm.setCountry("US"); // setting wifi country seems to improve OSX soft ap connectivity, may help others as well

  Serial.println("Setup mode...");
  //wifiManager.resetSettings();
  
  setupDeviceWM();




  wm.setClass("invert");
  Serial.println("**** Issue here **** - the next four lines should be the mqtt broker adress after changing in portal and restarting but apparently custom_mqtt_broker.getValue() never gets it value of the config.json");
  Serial.println(String(custom_mqtt_broker.getValue()));
  Serial.println(custom_mqtt_broker.getValue());
  Serial.println(String(mqtt_broker));
  Serial.println(mqtt_broker);


}

void loop()
{
  
  loopDeviceWM();

}

@tablatronix
Copy link
Collaborator

Do you know how to init format spiffs?

@tablatronix
Copy link
Collaborator

nm, // SPIFFS.format();

@tablatronix
Copy link
Collaborator

And you have the same problem with the autoconnectwithparams example?

@tablatronix
Copy link
Collaborator

tablatronix commented May 14, 2020

in the mean time use setValue, it has something to do with calling WiFiManagerParameter twice and it not setting the or overriding the first obj memory properly

This should work fine, adjust length to match whatever you set it to before etc.
custom_mqtt_broker.setValue(mqtt_broker_RESTORE,20);

@tablatronix
Copy link
Collaborator

tablatronix commented May 14, 2020

So here is the reproduction

char mqtt_broker[] = "192.168.100.108";

WiFiManagerParameter custom_mqtt_broker("mqttbroker", "MQTT Broker", mqtt_broker, 20);

void setup()
{

  Serial.begin(115200);
  delay(3000);

  Serial.println("custom_mqtt_broker FIRST");
  Serial.println(String(custom_mqtt_broker.getValue()));
  custom_mqtt_broker = WiFiManagerParameter("mqttbroker", "MQTT Broker", mqtt_broker, 20);

  Serial.println("custom_mqtt_broker SECOND");
  Serial.println(String(custom_mqtt_broker.getValue()));

  custom_mqtt_broker.setValue(mqtt_broker, 20);
  Serial.println("custom_mqtt_broker THIRD");
  Serial.println(String(custom_mqtt_broker.getValue()));

  wm.addParameter(&custom_mqtt_broker);
}
....
custom_mqtt_broker FIRST
192.168.100.108
custom_mqtt_broker SECOND
P�<-- garbage memory
custom_mqtt_broker THIRD
192.168.100.108
*WM: [3] allocating params bytes: 20
*WM: [2] Added Parameter: 

What I don't understand is why setvalue is fine, and calling the constructor again is not, while they both internally call setValue and memset the same..

@tablatronix tablatronix added Discussion Further Discussion ongoing and removed Question User Question member to member support labels May 14, 2020
@tablatronix
Copy link
Collaborator

tablatronix commented May 14, 2020

Maybe someone can look at this for me and help me fix it or understand wtf is going on in the param class or with constructors, it should be the same memory, and if I check the ID its the same object..

@tablatronix
Copy link
Collaborator

tablatronix commented May 14, 2020

custom_mqtt_broker FIRST
Address of _value is 0x3fff008c
192.168.100.108
preexisting ID

custom_mqtt_broker SECOND
Address of _value is 0x3ffeffac
`�
custom_mqtt_broker THIRD
Address of _value is 0x3ffeffac
192.168.100.108
*WM: [3] allocating params bytes: 20
*WM: [2] Added Parameter: 

hmm ok so something I do not understand about constructors.. NOT the same memory..

@rvt
Copy link

rvt commented May 14, 2020

WiFiManagerParameter is missing a copy assignment operator. https://en.cppreference.com/w/cpp/language/copy_assignment

Doing this: WiFiManagerParameter custom_mqtt_broker("mqttbroker", "MQTT Broker", mqtt_broker, 20);

is different from doing this: custom_mqtt_broker = WiFiManagerParameter("mqttbroker", "MQTT Broker", mqtt_broker, 20);

what happens in that single line is this:

    • Create a new object WiFiManagerParameter(...) will call setValue(...) etc..
    • Assign the newly created object to custom_mqtt_broker, c++ generates a assignment operator where each value is copied copied, but remember... char pointers is a address, only the address get's copied, not the string!
    • After assignment the created object in 1 get's deleted. so the pointer _value is not valid anymore, also not in the copied version because they point to the same deleted address.

Hope that clarifies...

Is there any reason you want re-assign custom_mqtt_broker? Some comments are in German and although I got taught German at school 25 years ago it's not that good anymore...

@tablatronix
Copy link
Collaborator

tablatronix commented May 14, 2020

Thanks, Yeah I was reading about correct assignment but did not really understand what it meant. I am mostly curious how to handle this, how to make it work as expected or prevent it. As it is something users are going to try.

I have some reading to do on c++

@rvt
Copy link

rvt commented May 14, 2020

I agree that users will try to do this so we must prevent users from doing it or add the required code to handle it.

I will make a pull request to handle this.

@dontsovcmc
Copy link
Contributor

Maybe it's better to deny coping of WiFiParameter and add function "setValue()"?
No reasons to copy WiFiParameters.

@tablatronix
Copy link
Collaborator

tablatronix commented May 14, 2020

We have setValue and we can also check _id and deny I can throw a compiler error. It would be cool to just allow it though if its not too much trouble. At this point I really want to see how , there is also already an init helper in the constructor so maybe we can do something with that to reinit safely

@tablatronix
Copy link
Collaborator

tablatronix commented May 14, 2020

So is this correct or the gist?

Not sure if rhs is ref or pointer, I will wait for an actual PR from someone more confident in this than I, But I am trying to learn lol

WiFiManagerParameter& operator=(const WiFiManagerParameter& rhs);

...

WiFiManagerParameter& WiFiManagerParameter::operator=(const WiFiManagerParameter& rhs){
(*this->_value) = (*rhs._value);
// other stuff, copy other vars? _id etc
  return *this;
}

Does this leak memory though?

Also I am not sure we are cleaning up this in the deconstructor properly.

@rvt
Copy link

rvt commented May 14, 2020

We could use WiFiManagerParameter(const WiFiManagerParameter&) = delete; WiFiManagerParameter& operator=(const WiFiManagerParameter&) = delete;

No reasons to copy WiFiParameters.

Somebody will always find a reason :)

@dontsovcmc
Copy link
Contributor

dontsovcmc commented May 14, 2020

I found the reason: More code, more bugs.

Here is one of them:

We have memory leak if call setValue() in our program.
_value = new char[_length + 1]; //_value is't null cause setValue called early in constructor

@tablatronix i think:

WiFiManagerParameter& WiFiManagerParameter::operator=(const WiFiManagerParameter& rhs){
this->init(rhs.id, rhs.label, rhs.defaultValue, rhs.length, rhs.custom, rhs.labelPlacement);
return *this;
}

@rvt
Copy link

rvt commented May 14, 2020

I like this one better:

WiFiManagerParameter& operator=( WiFiManagerParameter rhs) {
      if (&rhs == this) {
        return *this;
      }
      std::swap(_id , rhs._id);
      std::swap(_label , rhs._label);
      std::swap(_value , rhs._value);
      std::swap(_value, rhs._value);
      std::swap(_length , rhs._length);
      std::swap(_labelPlacement , rhs._labelPlacement);
      std::swap(_customHTML , rhs._customHTML);

      return *this;
    }

_value = new char[_length + 1]; //_value is't null cause setValue called early in constructor

That will be deleted again in the destructor

The copy assignment could be further optimized by using move semantics.
But the whole dynamic allocation within an embedded system should be prevent at all cost.

@dontsovcmc Perhaps you made a good point, just prevent copy assignment in the first place?

That will show also good design methodology for embedded software developers.

@dontsovcmc
Copy link
Contributor

dontsovcmc commented May 14, 2020

@rvt "That will be deleted again in the destructor."
No, I mean:

char value[16] = "192.168.100.108";
WiFiManagerParameter custom_mqtt_broker("mqttbroker", "MQTT Broker", value, 20);
custom_mqtt_broker.setValue(new_value); //here _value = new char[_length + 1]; without memory free

@rvt
Copy link

rvt commented May 14, 2020

@dontsovcmc yes you are absolutely right, setValue does not release memory of previously allocated memory... I guess that´s why we love immutables :)

@zen85
Copy link
Author

zen85 commented May 14, 2020

@tablatronix @rvt
sorry i left the discussion, it was 3 in the morning where i live and i had to work.
When i woke up i checked the thread in an instant and tried the solution before i had to leave and you guys made my day. it finally works. thank you so much!

Is there any reason you want re-assign custom_mqtt_broker? Some comments are in German and although I got taught German at school 25 years ago it's not that good anymore...

I have to admit that i did not know about setValue() and never stumpled upon it in my extensive research. I am just wondering why this is considered a "meantime"-solution since it does exactly what i needed? is it not optimal to solve it like that? i am sorry for the german comments... i changed them to english. but no.... there was no particula reason to redeclare it - i just tried so much and this was the only thing that produced some kind of meaningful return.

also: the working sketch for anybody making it down here:

#include <FS.h>                   //this needs to be first, or it all crashes and burns...
#include <ArduinoJson.h>          //https://github.com/bblanchon/ArduinoJson
#include <WiFiManager.h> //https://github.com/tzapu/WiFiManager WiFi Configuration Magic



WiFiManager wm;

//############################################# CUSTOM PARAMETERS FOR THE WIFI MANAGER ##########################################

char mqtt_broker[] = "192.168.100.108";
//char mqtt_broker_RESTORE[sizeof(mqtt_broker)];

WiFiManagerParameter custom_mqtt_broker("mqttbroker", "MQTT Broker", mqtt_broker, sizeof(mqtt_broker));



//################################################### GENERAL VARIABLES #########################################################
bool blockWM = true; // Change this to false if you want your code to continue to run on the loop void even if you are not conected to any wifi.
//###############################################################################################################################



void saveParamCallback() {
  Serial.println("****************Should save params");
  Serial.println(mqtt_broker);
  Serial.println(custom_mqtt_broker.getValue());

  strcpy(mqtt_broker, custom_mqtt_broker.getValue());
  
  Serial.println(mqtt_broker);
  Serial.println(custom_mqtt_broker.getValue());
  
  DynamicJsonDocument json(1024);
 
  json["mqtt_broker"] = mqtt_broker;
 
    
  File configFile2 = SPIFFS.open("/config.json", "w");
    if (!configFile2) {
      Serial.println("failed to open config file for writing");
    }
    
  serializeJson(json, Serial);
  serializeJson(json, configFile2);

  configFile2.close();
  //end save
  }



/*
  This void sets the device with the WiFiManager from tzapu with custom parameters.
  Needs to be called only in the setup void.
*/

void setupDeviceWM()
{
  //Serial.println("setupDeviceWM() is running");


  // callbacks
  wm.setConfigPortalBlocking(blockWM);
  
  String ssid = "IOT_ESP_" + String(ESP.getChipId());

  if (wm.autoConnect(ssid.c_str()))
  {
    //if you get here you have connected to the WiFi
    Serial.println("Connected to wifi network!");
    Serial.println(ssid);
    Serial.println(WiFi.localIP());
    
    WiFi.mode(WIFI_STA);
    wm.startWebPortal();
  }
  else
  {
    Serial.println("Could not connect with WiFi!");
  }

  // call the code down to activate wifi so users can configure the device, event if it's connected to the local network
  //wm.startConfigPortal("IOT_Device");
  //
  }



/*
  This void needs to be called in the loop void so it can handle the WM and the webportal.
*/


void loopDeviceWM()
{
  wm.process();
  
}


int setupSpiffs() {
  //read configuration from FS json
  Serial.println("mounting FS...");

  if (SPIFFS.begin()) {
    Serial.println("mounted file system");
    if (SPIFFS.exists("/config.json")) {
      //file exists, reading and loading
      Serial.println("reading config file");
      File configFile = SPIFFS.open("/config.json", "r");
      if (configFile) {
        Serial.println("opened config file");
        size_t size = configFile.size();
        // Allocate a buffer to store contents of the file.
        std::unique_ptr<char[]> buf(new char[size]);

        configFile.readBytes(buf.get(), size);
        DynamicJsonDocument jsonBuffer(1024);
        deserializeJson(jsonBuffer, buf.get());
        serializeJson(jsonBuffer, Serial);
      
        if (!jsonBuffer.isNull()) {
          Serial.println("\nparsed json");

          strcpy(mqtt_broker, jsonBuffer["mqtt_broker"]);

          Serial.println("loaded Json");
          Serial.println("now mqtt broker is: ");
          Serial.println(mqtt_broker);
          
          return 0;
          
        } else {
          Serial.println("failed to load json config");
          return 1;
        }
        configFile.close();
      }
    }
    else
    {
      Serial.println("config json doesnt exist");
      return 1;
    }
   
  } else {
    Serial.println("failed to mount FS");
    return 1;
  }
  //end read

}

void setup()
{

  Serial.begin(115200);
  delay(3000);

  //if this function returns "0" there is already a config.json
  
   if(setupSpiffs() == 0){
     Serial.println("SO, NOW:");
     Serial.println(mqtt_broker);
      //custom_mqtt_broker = WiFiManagerParameter("mqttbroker", "MQTT Broker", mqtt_broker_RESTORE, 16); // <-- NEW VAR
      custom_mqtt_broker.setValue(mqtt_broker, sizeof(mqtt_broker));
  }

  
  // at this point the parameters are either empty because there is no config or full because there is a config
  
  wm.setBreakAfterConfig(true);
  wm.setSaveParamsCallback(saveParamCallback);

 
  wm.addParameter(&custom_mqtt_broker);
  Serial.println(String(custom_mqtt_broker.getValue())); // <--- what does this say ?

  std::vector<const char *> menu = {"wifi", "info", "param", "close", "sep", "erase", "restart", "exit"};
  wm.setMenu(menu); // custom menu, pass vector

  // set country
  wm.setCountry("US"); // setting wifi country seems to improve OSX soft ap connectivity, may help others as well

  Serial.println("Setup mode...");
  //wifiManager.resetSettings();
  
  setupDeviceWM();




  wm.setClass("invert");
  Serial.println(String(custom_mqtt_broker.getValue()));
  Serial.println(custom_mqtt_broker.getValue());
  Serial.println(String(mqtt_broker));
  Serial.println(mqtt_broker);


}

void loop()
{
  
  loopDeviceWM();

} 

@tablatronix
Copy link
Collaborator

Thanks all, yeah that is why I asked if this would leak memory, I assumed it would from the NEW value each time if there was any destruction allowed via assignment.

Ill let you all discuss this a little

@tablatronix
Copy link
Collaborator

Made copy assignment private, so compiler will error out if you try this again.

Leaving open for improvements or alternatives.

@tablatronix tablatronix added this to the dev milestone May 15, 2020
@tablatronix tablatronix added the Documentation Flagged for docs label May 15, 2020
@rvt
Copy link

rvt commented May 15, 2020

Made copy assignment private, so compiler will error out if you try this again.

Not sure what compiler you are using but with c++11 you can do this:
WiFiManagerParameter& operator=(const WiFiManagerParameter&) = delete;

so you don't have to set it private if you just want to prevent all users (including the class self) prevent using the copy assignment.

@tablatronix
Copy link
Collaborator

ooh

@jonathanvanpeteghem
Copy link

Hello,

I am curently using .setValue()
The reason is that when i update the parameters, and later go back to the configportal (via button for example) then I want to see the updated values. And .setValue seems the only way for that.

But my question is, is this now safe to use? Forgive my ignorance. I have read the post 1050 / 1341 and 1346. But I cannot understand if the memory leak issue is now gone or not?

In my case I always restart after going to the configportal (because, when returning from configportal i have to go back to an assync webserver, and that only works with a restart, since the cleanup from the wm webserver leaves "stuff behind")
==> so i guess with my restart, i wont notice the memory leak, since my restart cleaned it up.

But in anycase, it would be kind if you can let me know if the .setValue() is now leak safe.

Best Regards to Tablatronix! Thank you so much for making this wifimanager something that we can use daily.

@tablatronix
Copy link
Collaborator

This is 2 years old, so no idea, pretty sure there have been at least 2 PRs to this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Validated BUG DEV Help Wanted Developer Needs Help Discussion Further Discussion ongoing Documentation Flagged for docs
Projects
None yet
Development

No branches or pull requests

5 participants