Arduinojson: StaticJsonBuffer.clear()

[firepick1]

For minimal memory usage under really tight constraints, I'm finding it convenient to introduce a new method, clear(), to StaticJsonBuffer. This allows the StaticJsonBuffer to be re-used directly with resorting to memory allocation. The existing recommendation to create StaticJsonBuffer for the duration of a task doesn't quite work for me because the client code needs to:
1) parse a json request
2) execute the parsed request incrementally over a series of period time intervals via callback method
3) return a json result formulated by updating the original, arbitrary, json request

The above requirements essentially require that the StaticJsonBuffer have a lifetime spanning multiple procedure invocations. It's certainly possible to implement this with new/delete but I'd prefer to minimize heap use and keep memory management static as much as possible, especially for large memory chunks. As a proposed solution, I'm finding that a single StaticJsonBuffer with a clear() method lets me handle the above in a simpler way:
https://github.com/firepick1/ArduinoJson/commit/34a5fbf80bd3b7fcc99d6bd54101356efca910a3

[bblanchon]

I've have a strong opinion against that feature. Let me explain.

In a previous draft of ArduinoJson, I added this function because I found it very convenient in my tests.
Unfortunately, it became extremely tempting to write things like:

// STEP1: parse input
StaticJsonBuffer<200> jsonBuffer;
JsonObject& inputObject = jsonBuffer.parseObject(inputJson);

...etc...

// STEP2: generate output
jsonBuffer.clear();
JsonObject& outputObject = jsonBuffer.createObject();
outputObject["hello"] = inputObject["world"];

I became so incitating to make mistake like this that I decided to remove it.

Arduino is a environment used by programmer that are not experienced in C or C++.
Many of them come from managed languages like C# or Java.
When you look at the code above, it's not obvious that calling jsonBuffer.clear() will corrupt inputObject.
Then its very hard to understand what's wrong with this code, especially because it may work for the first member if the memory hasn't been erased yet.

That's why I came up with the current design that enforce programmers to use JsonBuffer in short scopes, along with JsonObject& and JsonArray&.

However, I didn't hide the assignment operator...
This mean that an advanced user, like you, can still write:

jsonBuffer = StaticJsonBuffer<200>();

to reset the buffer :wink:

[firepick1]

Benoit,

This is actually quite the fascinating issue. I do acknowledge the danger of clear()'ing a buffer with active items. I did think about exactly that for several days before slipping the clear() back into my fork.

However, I couldn't find any other solution to my problem. The solution you present requires stack allocation of StaticJsonBuffer<200>(). In my case, I have StaticJsonBuffer<2000-5000>. That's how much JSON I'm piping through--lot's of it. It's large because I'm sending a 4-dimensional stroke vector the the Arduino, with lots of points to define the stroke. To save room, I use a single StaticJsonBuffer to hold the JSON request as well as the response. It's much like your example of input/output buffer, but I just use a single buffer for input AND output. The only difference is that I clear the input/output buffer before each request, not between input and output.

More particularly, the proposed assignment solution requires stack allocation of a fresh buffer. On an Arduino this will blow up the stack with a buffer like mine that consumes most of available memory. But a clear() won't. A delete/new could accomplish the same as a clear, but my fear on the allocation approach was that the size of the allocated chunk would be a problem for a fragmented heap.

For now I'm happy with the clear() solution in my fork. I totally understand the danger you describe, but have no better solution. I suppose we could do something like this:

jsonBuffer = 0;

But that seems a bit arcane and is also basically just clear().

(btw, I merged your Issue68 change back into my fork and discarded my hack. All my unit tests pass and thanks again!)

[bblanchon]

Writing jsonBuffer = StaticJsonBuffer<5000>(); will not blow the stack.
The compiler will inline it and it will be exactly the same as the clear() function.
Even if it doesn't inline the statement, it will just move the stack pointer back and forth, that's all.

If you don't want to repeat the size of the buffer in the assignment, you can use this template function:

template<typename T>
void clear(T& instance)
{
    instance = T();
}

[firepick1]

Benoit, I can see that you've got a deeper understanding of the C++ compilers than I do--and I've written compilers! Nice point.

And your global template solution is one I didn't think of. A nice alternative.

I'll make one last appeal...

ArduinoJson is beautiful, elegant and simple.

The clear() method is beautiful, elegant and simple. It is also dangerous. I'm a rock climber and I prefer beauty, elegance and simplicity over safety. It's an emotional thing. A wild thing. Plus I can't resist poking you and saying, "Benoit, you had the same idea to start with"

Come join the crazy crowd and run with scissors!

:)

[wyyyy]

That works thanks.

[WillTurman]

@bblanchon I found this looking for a buffer clear() function or similar. Thank you for writing out your explanation for why you chose to avoid an implementation like this and providing a way to accomplish the clear() functionality with an assignment instead of a function. Much appreciated!

[bblanchon]

Thanks @WillTurman.
I copied the answer in the FAQ, so it should be easier to find.

[mischmerz]

Here's a problem I am trying to work around: I have a variety of json strings incoming that I need to discover and parse. So I created a function

JsonObject& serviceJson(String data)
{
StaticJsonBuffer<256> jsonBuffer;
if (data) {
return(jsonBuffer.parseObject(data));
}
}

.. and call it like so ..

JsonObject& root = serviceJson(rest);

I then check the entries with

JsonObject& power = root["cmd"];
if (!power.success()) {
const char *xx = root["cmd"];
mode=1;
} else {
mode=2;
Serial.println("POWER");
const char *xx = power["power"];
}

and that works fine. However - after about 10 hours my arduino (actually an ESP8266) crashes while accessing an JSON string that is completely similar as other string it has parsed before. I assume I am running out of memory. Is there anything I should change?

Michaela
Thanks @bblanchon

[bblanchon]

Hi Michaela,

This function leaks a reference to a destructed variable:

JsonObject& serviceJson(String data) {
  StaticJsonBuffer<256> jsonBuffer;
  return jsonBuffer.parseObject(data);
}

Indeed, the returned JsonObject is stored in jsonBuffer, but this variable is destructed as soon as the function leaves.
Unfortunately, it's not possible to write such a function; you need to make sure that the JsonBuffer outlives the JsonObject.
I'm very surprised that your program was able to run for 10 hours without crashing :-)

BTW, if you're interested in learning more about this topic, there is a complete walk-through in the chapter "Troubleshooting / Program crashes" of Mastering ArduinoJson

Regards,
Benoit

[mischmerz]

@bblanchon - I appreciate the answer Benoit. I was confused thinking that the word "Static" in "StaticJsonBuffer" would be a hint that the buffer is statically allocated. Which .. as you explained it .. is not the case. I changed the code accordingly.

Thanks again.
Michaela

[hongquan]

"Static" in "StaticJsonBuffer" is just to be constrast with "DynamicJsonBuffer". And in this context, "Dynamic" means heap-allocated, so "Static" will mean stack-allocated.