bblanchon / ArduinoJson

📟 JSON library for Arduino and embedded C++. Simple and efficient.
https://arduinojson.org
MIT License
6.73k stars 1.12k forks source link

Parsing a json file on SD card is unreliable (on Arduino Yún) #223

Closed med44 closed 8 years ago

med44 commented 8 years ago

Hi Benoît,

This code is such a good idea to start with. Thanks for the great work.

I'm using your library to open and parse a settings.json file that gets created by PHP on my Yun via an HTML input form. That file just has 12 objects as follows:

{"LI":"1","LH":"22","LM":"35","LDH":"00","LDM":"00","LDS":"15","WI":"1","WH":"22","WM":"36","WDH":"00","WDM":"00","WDS":"15"}

On the Arduino side, I have code that is able to load it and parse it upon restart and these values basically update some timers that I have on other parts of the code. Every time I update the settings.json fall, the Arduino is instructed to open the file (using FileIO) and parse the new values.

However I'm having a tricky problem with what seems to be memory management of the buffer. Sometimes after resetting and updating the form once the parsing works and is able to update after I hit submit , but then with subsequent data entry it stops updating it and reports parseObject fails consistenly.

I read through your wiki several times to try and setup the buffer in as static (using object declaration or giving it just a byte size) and lastly I tried it as a dynamic buffer. With all three methods I am getting parse fail messages, but with:

StaticJsonBuffer<400> jsonBuffer;

It at least loads the values once or twice. This is by far the buffer setting that provides better results, but it's still too unstable for my needs. Here is the code as I have it now:

void updateSet() {

  File dataFile = FileSystem.open(filePath, FILE_READ);   //open file (path has been set elsewhere and works)
  String json = dataFile.readString();                    // read data to 'json' variable
  dataFile.close();                                       // close file

// ArdunoJson.h //

//  const int BUFFER_SIZE = JSON_OBJECT_SIZE(12); //alternate way of declaring buffer size (but doesn't work for me) 
//  StaticJsonBuffer<BUFFER_SIZE> jsonBuffer; 

  StaticJsonBuffer<400> jsonBuffer;       // buffer must be big enough to hold the whole JSON string
  JsonObject& root = jsonBuffer.parseObject(json);    // breaks the JSON string into individual items, saves into JsonObject 'root'

  Serial.println(json);

  if (!root.success()) {
    Serial.println("parseObject() failed");
    return;   //if parsing failed, this will exit the function and not change the values to 00

  } else { // update variables

    lightDay = root["LI"];
    lightTime = writeTime( root["LH"] , root["LM"] , 0 );
    lightDuration = writeTime( root["LDH"] , root["LDM"] , root["LDS"] );
    lightEnd = addTime( lightTime, lightDuration);

    waterDay = root["WI"];
    waterTime = writeTime( root["WH"] , root["WM"] , 0 );
    waterDuration = writeTime( root["WDH"] , root["WDM"] , root["WDS"] );
    waterEnd = addTime( waterTime, waterDuration);

    Serial.println("values updated");
    Serial.print("lightTime: ");
    Serial.println(lightTime);
    Serial.print("lightDay: ");
    Serial.println(lightDay);

    Serial.print("waterTime: ");
    Serial.println(waterTime);
    Serial.print("waterDay: ");
    Serial.println(waterDay);

    counterLight = lightDay; 
    counterWater = waterDay;

  } // end of if(!root.success())
}

According to my understanding of buffer size allocation from reading your wiki, the settings.json string above should be 12 objects (using JSON_OBJECT_SIZE(12)) or 126 bytes (if I use StaticJsonBuffer<126>). However I repeatedly get the parseObject() failed message with these two approaches. I tried the DynamicJsonBuffer too and it reported the parseObject() failed error as well.

Upon reading more about this issue, I also saw you recommend using char json[] = instead of String. However I'm trying to put that code together to work using the FileIO class but am not sure how to extract the bytes from the settings.json file into a char array. I was looking for an example of this too but can't find it.

Do you see something that I'm not seeing on my code? I've been trying to troubleshoot this on my own for about a week now and all the forum questions I looked don't have this particular problem I'm having. I appreciate any insight you may have.

Thanks!

bblanchon commented 8 years ago

Hi Carlos,

First of all, thank you for taking the time write a very detail and well written explanation of your problem.

So you're working on the microcontroler side of the Arduino Yun, a platform with very few RAM. On that kind of platform, you need to be very picky about the memory usage and fragmentation. The best way to do that is to stick to fixed memory allocation, and the first thing to do it to get rid of the String class.

First, you know exactly the format of the JSON input and it looks like it's 125 characters long. We'll reserve 150 bytes for that, just in case you add more digits, and read it from FileIO:

char json[150];
File dataFile = FileSystem.open(filePath, FILE_READ);
dataFile.readBytes(json, size(json));
dataFile.close();     

Then, you know that the JSON consist in an object with 12 members, so let's create a StaticJsonBuffer of the right size and parse:

StaticJsonBuffer<JSON_OBJECT_SIZE(12)> jsonBuffer;
JsonObject& root = jsonBuffer.parseObject(json);

Voila!

Now the updateSet() function requires 150 + 126 = 276 bytes of RAM, which is 11% of the total 2.5KB available on that MCU. So I bet this should work fine.

I didn't test the code, in case of a problem, try to print the content of json to see if it's read correctly. In particular, you may need to insert the 0 terminator yourself based on the return value of readBytes().

Read more about memory fragmentation in this article about ArduinoJson 1.0 and about the String class in this article about ArduinoJson 5.0.

med44 commented 8 years ago

This is great, it now works as expected and is very stable. I was suspicious of Strings as I read your advice, but I realize I was using wrong code snippets to read the bytes from the files, define the the char array and load them into it. Your code example was exactly what I was missing.

Thanks for responding so quickly and nailing this for me.

A final question I have is when you say this function as I made it will use 150 + 126 = 276 bytes of RAM does this mean that the JSON string is getting duplicated as it's being processed? I though that was true for Strings as you mentioned it elsewhere, but I want to check if the code still does that with chars. Or is the 126 bytes calculation derived from something else? If you want to point to an explanation of this that works too.

Just as a an FYI for this thread, I am reposting below my updated code that works with your solution. I figured it may help others who are trying to achieve this.

void updateSet() {

  byte bufferSize = 150; //define number of bytes expected on the JSON thread
  char json[bufferSize]; //create a char array buffer that is 150 bytes in size in this case

  //Opens a 'settings.json' text file on SD that has has a JSON string stored in filePath = /mnt/sd/arduino/www/[your_sketch_name]
  File dataFile = FileSystem.open(filePath, FILE_READ);
  dataFile.readBytes(json, bufferSize);
  dataFile.close();

  Serial.println(json); //prints whatever JSON string values were read

  StaticJsonBuffer<JSON_OBJECT_SIZE(12)> jsonBuffer; //We know our JSON string just has 12 objects only and no arrays, so our buffer is set to handle that many
  JsonObject& root = jsonBuffer.parseObject(json);    // breaks the JSON string into individual items, saves into JsonObject 'root'

  if (!root.success()) {
    Serial.println("parseObject() failed");
    return;   //if parsing failed, this will exit the function and not change the values to 00

  } else { // update variables

    lightDay = root["LI"];
    lightTime = writeTime( root["LH"] , root["LM"] , 0 );
    lightDuration = writeTime( root["LDH"] , root["LDM"] , root["LDS"] );
    lightEnd = addTime( lightTime, lightDuration);

    waterDay = root["WI"];
    waterTime = writeTime( root["WH"] , root["WM"] , 0 );
    waterDuration = writeTime( root["WDH"] , root["WDM"] , root["WDS"] );
    waterEnd = addTime( waterTime, waterDuration);

    Serial.println("values updated");
    Serial.print("lightTime: ");
    Serial.println(lightTime);
    Serial.print("lightDay: ");
    Serial.println(lightDay);

    Serial.print("waterTime: ");
    Serial.println(waterTime);
    Serial.print("waterDay: ");
    Serial.println(waterDay);
    Serial.println();

    counterLight = lightDay; //update day counters for light
    counterWater = waterDay; //update day counters for irrigation

  }
}
bblanchon commented 8 years ago

With this program, no string duplication happens in ArduinoJson.

The exact quantity of RAM used in this function is:

sizeof(json) + sizeof(jsonBuffer)

which is around 150 + 126 = 276 bytes (probably 2 more bytes for the internal state of StaticJsonBuffer).

Those 126 bytes are used to store the tree representation of the JSON object.

med44 commented 8 years ago

I see what you mean, it all makes sense now and it makes me understand better how the memory requirements for parsing JSON on arduino work. Thanks again.