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

Can't fill JsonArray in for loop #10

Closed
franklinvv opened this issue Aug 3, 2014 · 7 comments
Closed

Can't fill JsonArray in for loop #10

franklinvv opened this issue Aug 3, 2014 · 7 comments
Labels
question v5 ArduinoJson 5

Comments

@franklinvv
Copy link

(Disclaimer: I'm not at all an expert in C/C++)

I am trying to build a JsonArray in a for loop on an Arduino Mega 2560, iterating through an array. Here is a (somewhat) minimal example of what I'm trying to do.

struct Person {
    int id;
    char name[32];
};

Person boss;
boss.id = 1;
strcpy(boss.name, "Jeff");
Person employee;
employee.id = 2;
strcpy(employee.name, "John");

Person persons[2] = {boss, employee};

JsonArray<2> json;
for(int i = 0; i < 2; i++) {
    JsonObject<2> object;
    object.add("id", persons[i].id);
    object.add("name", persons[i].name);
    json.add(object);
}

Serial.print(json);

The expected result is:

[
    {
        id: 1,
        name: "Jeff"
    },
    {
        id: 2,
        name: "John"
    }
]

However, I get this:

[
    {
        id: 2,
        name: "John"
    },
    {
        id: 2,
        name: "John"
    }
]

Any idea why I can't construct an JsonArray this way?

bblanchon added a commit that referenced this issue Aug 4, 2014
@bblanchon
Copy link
Owner

Then is a pitfall here.:confused:

When you call:

json.add(object);

the JsonArray saves the address of the object.
This is designed in order to avoid copying the data.

The bytes of object are reused for the second iteration of the loop.
It's exactly the same as calling add() with the same object twice, because the address doesn't change.

In fact, if you look at the code, you have only declared one JsonObject and you expect to have two in the JsonArray. This is not possible with this library.

To fix this problem, you need to have two JsonObject in memory, like this:

JsonObject<2> object[2];

for (int i = 0; i < 2; i++)
{
    object[i].add("id", persons[i].id);
    object[i].add("name", persons[i].name);

    json.add(object[i]);
}

However, I'm very glad you posted that issue because it helped me discover a bug in version 3.1: nested objects in arrays are not working.
Please stick with version 3.0 until I release 3.2 later in the day.

@franklinvv
Copy link
Author

Ah, I understand. I see how my example code can be fixed, thanks to your information. I created that piece of example code to make it easier to understand for you.

However, my actual code is a bit different, and I don't see how I can fix that. I create 2 objects of a class called LightSwitch. This class contains a method getJson() which returns fields (id and title) as a JsonObject.

From the class LightSwitch:

LightSwitch::LightSwitch(int id, char* title) {
    this->id = id;
    strcpy(this->title, title);
}

JsonObject<2> LightSwitch::getJson() {
    JsonObject<2> json;
    json.add("id", this->id);
    json.add("title", this->title);
    return json;
}

Then, I try to use this method to construct a larger JsonObject:

LightSwitch switch1 = LightSwitch(1, "Expedit");
LightSwitch switch2 = LightSwitch(2, "Docent");
LightSwitch lightSwitches[2] = {switch1, switch2};

JsonArray<2> jsonLightSwitches;
for(int i = 0; i < 2; i++) {
  jsonLightSwitches.add(lightSwitches[i].getJson());
}

I can't see how I should fix this problem. :( If I understand you correctly, the problem now lies in getJson()?

@bblanchon
Copy link
Owner

The problem is that the JsonObject<2> are temporary variables:

  1. getJson() creates it
  2. add() stores the address
  3. then the variable is out of scope.

To solve this issue, you could add a JsonObject<2> field in your LightSwitch class.

JsonObject<2>& LightSwitch::getJson() 
{
    this->json = JsonObject<2>();
    this->json.add("id", this->id);
    this->json.add("title", this->title);
    return this->json;
}

Don't forget the &, it's important to return a reference to the field, and not a copy.

@bblanchon
Copy link
Owner

Even better, make LightSwitch implement Printable.

virtual size_t LightSwitch::printTo(Print& p) const
{
    JsonObject<2> json;
    json.add("id", this->id);
    json.add("title", this->title);
    return json.printTo(p); 
}

for(int i = 0; i < 2; i++) 
{
    jsonLightSwitches.add(lightSwitches[i]);
}

bblanchon added a commit that referenced this issue Aug 4, 2014
@franklinvv
Copy link
Author

OOPS: my comment below is void. I simply forgot to update your library ... :/ Everything works perfectly now.
THANKS, Benoît!

Your second suggestion looks like a tidy way to code it. I took over your idea, but I can't get it to work. :(

It still prints the same object twice, just as before. Is that because the json object in the implementation of printTo() still is a temporary variable? I tried to turn the json object into a field of the class, but that didn't work either.

This is what I tried:

size_t LightSwitch::printTo(Print& p) const
{
    JsonObject<2> json;
    json.add("id", this->id);
    json.add("title", this->title);
    return json.printTo(p);
}

I figure that it's not necessary to avoid using a temporary variable if you call printTo() but if I do this (as you suggested to avoid a temporary variable):

size_t LightSwitch::printTo(Print& p) const {
    this->json = JsonObject<2>();
    this->json.add("id", this->id);
    this->json.add("title", this->title);
    return this->json.printTo(p);
}

Then I get this error from the Arduino IDE:

LightSwitch.ino: In member function 'virtual size_t LightSwitch::printTo(Print&) const':
LightSwitch:11: error: passing 'const ArduinoJson::Generator::JsonObject<2>' as 'this' argument of 'ArduinoJson::Generator::JsonObject<2>& ArduinoJson::Generator::JsonObject<2>::operator=(const ArduinoJson::Generator::JsonObject<2>&)' discards qualifiers
LightSwitch:12: error: passing 'const ArduinoJson::Generator::JsonObject<2>' as 'this' argument of 'void ArduinoJson::Generator::JsonObjectBase::add(const char*, T) [with T = int]' discards qualifiers
LightSwitch:13: error: passing 'const ArduinoJson::Generator::JsonObject<2>' as 'this' argument of 'void ArduinoJson::Generator::JsonObjectBase::add(const char*, T) [with T = const char*]' discards qualifiers

I suppose I am still doing something wrong? Thanks for your help so far!

@ersinpw
Copy link

ersinpw commented Nov 4, 2017

@bblanchon I have a similar problem, I only see values of the last object. But the code here is outdated I guess? Do you have a suggestion how to solve this in the newest version of ArduinoJson?

@r-anwar
Copy link

r-anwar commented Nov 27, 2017

@bblanchon Hi! I have the same problem like ersinpw.

I used the builder pattern with no success. The last entry overrides all elements in 'users' field.

class JSONBuilder
{
DynamicJsonBuffer _buffer;
JsonObject& _root;

public:
JSONBuilder()
: _root(_buffer.createObject()) {
}

void getUsersResult(User users[], int size) {

    JsonArray& usersJSON = _root.createNestedArray("users");

    for(int i = 0; i < size; i++) {
        User user = users[i];

        usersJSON.add(user.toJson(_buffer.createObject()));
    }
}

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

String dumpTo(String destination) const {
    _root.prettyPrintTo(destination);
    Serial.println(destination);
    return destination;
}

private:
};

Please, may you can help me?

Best regards

Repository owner locked and limited conversation to collaborators Sep 21, 2018
@bblanchon bblanchon added the v5 ArduinoJson 5 label Feb 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
question v5 ArduinoJson 5
Projects
None yet
Development

No branches or pull requests

4 participants