How to use createNestedObject in a for loop?

[probonopd]

Sorry for bringing up one more question similar to #10, #57, #84 but this appears to be strange for beginners.

I am trying to generate

{  
   "lights":{  
      "1":{  
         "name":"Lamp 1"
      },
      "2":{  
         "name":"Lamp 2"
      },
...
      "999":{  
         "name":"Lamp 999"
      }
   }
}

using a function addLight(root, i) that I need to call for a dynamic number i of lights which is determined at runtime (in the above example, i is 1...999). Looking at https://arduinojson.org/doc/pitfalls/#6-do-not-assume-that-strings-are-copied I am pretty certain that what I am doing right now is entirely wrong (and the result is a crash) but I cannot quite figure out what I need to do instead.

[bblanchon]

If you're function is written like that:

void addLight(JsonObject& root, int i)
{
    char key[4];
    char name[9];

    sprintf(key, "%d", i);
    sprintf(name, "Lamp %d", i);

    JsonObject& light = root.createNestedObject(key);
    light["name"] = name;
}

then you're leaking two dangling pointers to key and name.
That would explain the crash.
You need to keep key and name alive until the JSON is generated.

Here is an suggestion using the builder pattern:

#define MAX_LIGHTS 999

#define JSON_SIZE (JSON_OBJECT_SIZE(1)+JSON_OBJECT_SIZE(MAX_LIGHTS)+MAX_LIGHTS*JSON_OBJECT_SIZE(1))

class JsonBuilder
{
    StaticJsonBuffer<1000> _buffer;
    JsonObject& _root;
    char _keys[3][MAX_LIGHTS];
    char _names[9][MAX_LIGHTS];

public:
    JsonBuilder()
        : _root(_buffer.createJsonObject())
    {
    }

    void addLight(int i)
    {
        sprintf(_key[i], "%d", i);
        sprintf(_name[i], "Lamp %d", i);

        JsonObject& light = root.createNestedObject(_key[i]);
        light["name"] = _name[i];
    }

    void dump(Print& destination)
    {
        _root.prettyPrint(destination);
    }
};

[probonopd]

Thanks bblanchon for your detailed response. Yes, my function is currently written like it shouldn't be. So you are saying it is required to use a class to achieve this? This would increase complexity of my Arduino sketch significantly and I might be tempted to fall back to using Strings ;-).

[bblanchon]

Here is another suggestion using the memory from the JsonBuffer:

void addLight(JsonBuffer& buffer, JsonObject& root, int i)
{
    char* key = (char*)buffer.alloc(4);
    sprintf(key, "%d", i);
    JsonObject& light = root.createNestedObject(key);

    char* name = (char*)buffer.alloc(9);
    sprintf(name, "Lamp %d", i);
    light["name"] = name;
}

Don't forget to increase the size of the StaticJsonBuffer.

[probonopd]

Thanks bblanchon. I realize this is an issues tracker and not a C programming course, but maybe you are interested in the real-world problems users of your library are facing, hence I go ahead and fully respect if you won't answer.

Your example works well for the short sample but my actual function looks like

void addLightJson(JsonObject& root, int numberOfTheLight)
{
  String lightName = "" + (String) numberOfTheLight;
  JsonObject& light = root.createNestedObject(lightName.c_str());
  light["type"] = "Extended color light";
  light["name"] =  ("Hue Lamp " + (String) numberOfTheLight).c_str();
  light["modelid"] = "LCT001";
  JsonObject& state = light.createNestedObject("state");
  unsigned int brightness = strip.GetPixelColor(numberOfTheLight - 1).CalculateBrightness();
  if (brightness == 0)
  {
    state["on"] = false;
  }
  else
  {
    state["on"] = true;
  }
  state["bri"] = 254; 
  state["hue"] = 0; 
  state["sat"] = 0;
  JsonArray& array = state.createNestedArray("xy");
  array.add(0.0);
  array.add(0.0);
  state["alert"] = "none";
  state["effect"] = "none";
  state["colormode"] = "hs";
  state["reachable"] = true;
}

I would be quite tedious to do all the memory allocation by hand. Or is your library not really suitable for doing things like that and you would advise to go another route like using Strings instead? (In case you are wondering what I am trying to do: https://github.com/probonopd/ESP8266HueEmulator/tree/ArduinoJson)

[bblanchon]

There are again several dangling pointers here.
You have to understand that every String you create in this function will be destructed once the function exits, so they wont be available when calling JsonPrintable::printTo().

I personally hate the Arduino String class, as I think it invites developers to make useless allocations and copies, which is unacceptable in a embedded environment.

Now that I know the complexity of the task, I would recommend the builder pattern again.

class JsonBuilder
{
    DynamicJsonBuffer _buffer;
    JsonObject& _root;

public:
    JsonBuilder()
        : _root(_buffer.createJsonObject())
    {
    }

    void addLight(unsigned numberOfTheLight, unsigned brightness)
    {
        JsonObject& light = root.createNestedObject(makeLightKey(numberOfTheLight));
        light["type"] = "Extended color light";
        light["name"] =  makeLightName(numberOfTheLight);
        light["modelid"] = "LCT001";

        JsonObject& state = light.createNestedObject("state");
        state["on"] = brightness != 0;
        state["bri"] = 254; 
        state["hue"] = 0; 
        state["sat"] = 0;
        state["alert"] = "none";
        state["effect"] = "none";
        state["colormode"] = "hs";
        state["reachable"] = true;

        JsonArray& xy = state.createNestedArray("xy");
        xy.add(0.0);
        xy.add(0.0);        
    }

    void dumpTo(Print& destination) const
    {
        _root.prettyPrint(destination);
    }

private:
    const char* makeLightKey(int i)
    {
        char* key = _buffer.alloc(4);
        sprintf(key, "%d", i);
        return key;
    }

    const char* makeLightName(int i)
    {
        char* name = _buffer.alloc(32);
        sprintf(name, "Hue Lamp %d", i);
        return name;
    }
};

Anyway, you opened my eyes on the necessity to be able to easily duplicate strings.
The trick used here with JsonBuffer::alloc() is good and I'll try to make it implicit in version 5.x.
Of course, it would be an opt-in option because it still think the default behavior should avoid copies.

PS: please add your project to the wiki: https://github.com/bblanchon/ArduinoJson/wiki/Projects%20using%20ArduinoJson

[bblanchon]

I added the feature in version 5.0 beta 3.

Now String are implicitly duplicated in the JsonBuffer when necessary.
This happens with the String class, but not with char*.
Of course this will make the JsonBuffer much bigger.

Your code can now be written as:

void addLightJson(JsonObject& root, int numberOfTheLight) 
{
  String lightName = "" + (String)numberOfTheLight;
  JsonObject& light = root.createNestedObject(lightName);
  light["type"] = "Extended color light";
  light["name"] = "Hue Lamp " + (String)numberOfTheLight;
  light["modelid"] = "LCT001";
  JsonObject& state = light.createNestedObject("state");
  unsigned int brightness =
      strip.GetPixelColor(numberOfTheLight - 1).CalculateBrightness();
  if (brightness == 0) {
    state["on"] = false;
  } else {
    state["on"] = true;
  }
  state["bri"] = 254;
  state["hue"] = 0;
  state["sat"] = 0;
  JsonArray& array = state.createNestedArray("xy");
  array.add(0.0);
  array.add(0.0);
  state["alert"] = "none";
  state["effect"] = "none";
  state["colormode"] = "hs";
  state["reachable"] = true;
}

(I just removed the .c_str())

Don't hesitate to re-open the issue if there is a problem.

[probonopd]

Thank you very much for your explanation. Appreciate it.

[andrei-ivanov]

Spent some time trying to fix a leaky pointer issue and creating a JsonBuilder like in this example helped. Maybe you should include it in the pitfalls section as a possible solution.

[bblanchon]

Hi @andrei-ivanov.
I'm very interested in your feed back.
Can you share a link to a program using this?

[andrei-ivanov]

Hmm, not sure I can... but my JsonBuilder looks very similar to this one, sending state to an MQTT broker.