Is there a better way than using sprintf to format my data. [SOLVED]

Fellow Arduinoins, I have need of some advice.

In the code I use for my HydroSolar project I gather data from sensors on the analogue ports and I maintain various statistical data on the readings.
I then format this data into a json style format which I then use a html POST method to suffle the data to my host site where it is received and dealt with there.

Now the specific code I have for formatting the various values to json are held, not in a string of course, but in a character array using the int sprintf ( char * str, const char * format, ... ); method.
Here is one of my routines that uses this code:

void statPost() {
        char postData[425];                // allocate enough space in the char array

        sprintf(postData, "{\"id\":\"stat\",\"vT_minHour\":%1i,\"vT_maxHour\":%2i,\"vT_minDay\":%3i,\"vT_maxDay\":%4i,\"iT_inHour\":%5lu,\"iT_inDay\":%6lu,\"iT_inWeek\":%7lu,"
                "\"iS_peekHour\":%8i,\"iS_peekDay\":%9i,\"iS_inHour\":%10lu,\"iS_inDay\":%11lu,\"iS_inWeek\":%12lu,"
                "\"vB_minHour\":%13i,\"vB_maxHour\":%14i,\"vB_minDay\":%15i,\"vB_maxDay\":%16i,\"iB_inDay\":%17lu,\"iB_outDay\":%18lu}",
                vacHydro.minHour, vacHydro.maxHour, vacHydro.minDay, vacHydro.maxDay, idcHydro.AmpsHour, idcHydro.AmpsDay, idcHydro.AmpsWeek,
                idcSolar.peekHour, idcSolar.peekDay, idcSolar.AmpsHour, idcSolar.AmpsDay, idcSolar.AmpsWeek,
                vdcBattery.minHour, vdcBattery.maxHour, vdcBattery.minDay, vdcBattery.maxDay, battery.inAmpsDay, battery.outAmpsDay);

        if(!postPage(postData)) {
                ++fltCountPOST;
        }
        else {
                ++okCountPOST;
        }
}

I would like to see how others might do this differently, or is this another way to do this?
To me, it seems a difficult method to maintain.


Paul

So what is your actual problem ?

Your code could be more readable if you broke your strings down.
C/C++ allows you to do this

"The " 
"quick " 
"brown "
"fox"

is seen by the compiler as "The quick brown fox"

@michinyon, I didn't say I had an actual problem, more that the code I presented is somewhat difficult to maintain fot the reasons I explained.
For example, if I were to insert additional parameters to build up my formatted json char array, I need to carefully work out the placement of the substitution and then aslo for the format within the sprint statement.

@AWOL, ok, I take you point, though I understand that that would only apply to the first part where I have the parameter substitution, am I right?
The remainder are my data variables, e.g. vacHydro.minHour
I guess, you can see that it is unreadable, I agree, it's an eye strain.

I guess what I am asking, is there a better method in terms of programming that I can achieve the same result as this.

As a second part to my question, how big can I keep going with adding more parameters to such a sprintf statement, I'm guessing there is a practicle limit when it comes to Arduino.


Paul

You do know you are putting many arrays (JSON) into one array? It might be a bit of a memory issue on the Arduino to make an vector array but that would make it easier to access and also make it possible to loop through it more easily.

linwedil wrote:
You do know you are putting many arrays (JSON) into one array?

I wonder if you might explain that to me as I understand all I am doing is constructing a single json string containing multiple key value pairs?
My structure starts and ends with braces, and has no other braces or brakets within to suggest it is an array containing arrays.

I should have pointed in my first post out that I am using a Mega board.

Going on what AWOL was saying, I have just tinkered with the following method;

void statPost() {
	char postData[425];
	char buffer[16];

	strcat(postData, "id:stat,vT_minHour:");
	strcat(postData, itoa(vacHydro.minHour, buffer, 10));

	strcat(postData, "vT_maxHour:");
	strcat(postData, itoa(vacHydro.maxHour, buffer, 10));

	strcat(postData, "vT_minDay:");
	strcat(postData, itoa(vacHydro.minDay, buffer, 10));

	strcat(postData, "vT_maxDay:");
	strcat(postData, itoa(vacHydro.maxDay, buffer, 10));

	strcat(postData, "iT_inHour:");
	strcat(postData, itoa(idcHydro.AmpsHour, buffer, 10));

	strcat(postData, "iT_inDay:");
	strcat(postData, itoa(idcHydro.AmpsDay, buffer, 10));

	strcat(postData, "iT_inWeek:");
	strcat(postData, itoa(idcHydro.AmpsWeek, buffer, 10));

	strcat(postData, "iS_peekHour:");
	strcat(postData, itoa(idcSolar.peekHour, buffer, 10));

	strcat(postData, "iS_peekDay:");
	strcat(postData, itoa(idcSolar.peekDay, buffer, 10));

	strcat(postData, "iS_inHour:");
	strcat(postData, itoa(idcSolar.AmpsHour, buffer, 10));

	strcat(postData, "iS_inDay:");
	strcat(postData, itoa(idcSolar.AmpsDay, buffer, 10));

	strcat(postData, "iS_inWeek:");
	strcat(postData, itoa(idcSolar.AmpsWeek, buffer, 10));

	strcat(postData, "vB_minHour:");
	strcat(postData, itoa(vdcBattery.minHour, buffer, 10));

	strcat(postData, "vB_maxHour:");
	strcat(postData, itoa(vdcBattery.maxHour, buffer, 10));

	strcat(postData, "vB_minDay:");
	strcat(postData, itoa(vdcBattery.minDay, buffer, 10));

	strcat(postData, "vB_maxDay");
	strcat(postData, itoa(vdcBattery.maxDay, buffer, 10));

	strcat(postData, "vB_inAmpsDay");
	strcat(postData, itoa(battery.inAmpsDay , buffer, 10));

	strcat(postData, "vB_outAmpsDay");
	strcat(postData, itoa(battery.outAmpsDay, buffer, 10));

//	Serial.println(postData);

	if(!postPage(postData)) {
//		Serial.println("Error Post stats");
		++fltCountPOST;
	}
	else {
//		Serial.println("Post stat");
		++okCountPOST;
	}
}

Which I am thinking will achieve a similar result, except just going for a key value pair string without the json format of braces and quotes.
But after compiling it comsumes an additional 300 odd words of memory over my original code section.
I didn't load it up to the board to see the results at this stage.
But to me, it is far more readable and easy to make alterations to.

linwendil wrote:
make an vector array but that would make it easier to access and also make it possible to loop through it more easily

Ok, I could look into that I guess.

Thanks for your advice.


Paul

rockwallaby:
I wonder if you might explain that to me as I understand all I am doing is constructing a single json string containing multiple key value pairs?

JSON stores values in arrays, while your C code indeed sets it all to one string, thats not how JSON is meant to work. If you store it as a string you are not really takeing advantage of what JSON gives you.

[q="json.org"]
A collection of name/value pairs. In various languages, this is realized as an object, record, struct, dictionary, hash table, keyed list, or associative array.
An ordered list of values. In most languages, this is realized as an array, vector, list, or sequence.
[/quote]

in C/C++ I would use an array for it, or perhaps a struct. A string in C is an array.

rockwallaby:
My structure starts and ends with braces, and has no other braces or brakets within to suggest it is an array containing arrays.

I mean to say that it would be more usefull if it was an associative array (or a class with objects) as thats how JSON kind of is supposed to be used. There is a few C/C++ libaries to handle JSON to, but I don't know if there is any Arduino specific ones

rockwallaby:
Going on what AWOL was saying, I have just tinkered with the following method;

(Code deleted)
That is much better then you start yes. And it might be more memory effecient then other methods.

Thank you linwendil for your effort in explaing that clearly to me, I appreciate it.

linwendil:
JSON stores values in arrays, while your C code indeed sets it all to one string, thats not how JSON is meant to work. If you store it as a string you are not really takeing advantage of what JSON gives you.

Yes, you are correct.

My use for using a json formatted string is simply that I use this format to POST data from the Arduino to a remote server, which stores the data in either file or SQL database.
When a client connects to the remote server site, the server simply passes out that data again as json formatted data as text in a html POST back to the client.
On the the client side javascript, I require it as json format as I use Backbone.js Model View Controller which is tied with jquery.

I'm looking to make some changes so that I might move from using json as a wrapper for my KVP data to just the KVP data itself.
Presently, it is all working [check sig], from Arduino through to host site and then to client, I am looking to make improvements I guess.

linwendil:
There is a few C/C++ libaries to handle JSON to, but I don't know if there is any Arduino specific

Here is one you may be interested in looking at specifically for Arduino, GitHub - lasselukkari/aJson: aJson is an Arduino library to enable JSON processing with Arduino. It easily enables you to decode, create, manipulate and encode JSON directly from and to data structures.


Paul

A further refinement of your new method might be to take this:

	strcat(postData, "vT_maxHour:");
	strcat(postData, itoa(vacHydro.maxHour, buffer, 10));

and put it in a function that takes two char*s and an int so you can pass postData, the identifying string and the value. Slightly less code.

Even though you're using a mega, I'd still be reluctant to use such a large buffer for the postData. Can you send the values one at a time? Add a timestamp if you need the datapoints to all have the same time on them.

wildbill:
A further refinement of your new method might be to ... put it in a function that takes two char*s and an int so you can pass postData, the identifying string and the value. Slightly less code.
Even though you're using a mega, I'd still be reluctant to use such a large buffer for the postData. Can you send the values one at a time? Add a timestamp if you need the datapoints to all have the same time on them.

Good point wildbill, especially since this is not the only routine I have for posting data out of the Arduino.
This one just happens to be one of the larger ones, and my thing is that if I wish to have more routines that send data out of the Arduino, well, it may become a problem with having such large char buffers, even though they only have local scope but the data is passed in the call to the actual posting routine.

I don't wish to send each key value pair as a separate post if that is what you mean, the overhead in terms of http requests would see my router go berserk, and maybe smoke from the Wiznet chip. :stuck_out_tongue:

The timestamp is again a good idea, though I shouldn't need it, I send small packets of data to the host site at a rate of every five seconds for close to real-time sensor readings and every 60 seconds for statistical data like the code discussed. I have the php script on the server set the time for any data I store to SQL database.

Thanks again for your input.


Paul

Apart from the advice above, you could "stream" the data.

http://arduiniana.org/libraries/streaming/

I assume it eventually "goes somewhere" like another computer or a disk. You can stream to anything derived from the Stream class, as I recall (or Printable, maybe).

Hello Nick,

Spot on, I did think of that as my last thought yesterday.
I do use the stream operator in some other sections of my code and it dawned on me that it could be a solution.

So I coded up the following as a quick thought, that needs more work;

void statsCmd(WebServer &server, WebServer::ConnectionType type, char *url_tail, bool tail_complete) {
        if (type == WebServer::POST) {
                server.httpFail();
                return;
        }
        server.httpSuccess("application/json");                                //server.httpSuccess(false, "application/json");
        if (type == WebServer::HEAD) {
                return;
        }
        server << "id:stat,vT_minHour:" << vacHydro.minHour;
        server << ",vT_maxHour:" << vacHydro.maxHour;
        server << ",vT_minDay:" << vacHydro.minDay;
        server << ",vT_maxDay:" << vacHydro.maxDay;
        server << ",iT_inHour:" << idcHydro.AmpsHour;
        server << ",iT_inDay:" << idcHydro.AmpsDay;
        server << ",iT_inWeek:" << idcHydro.AmpsWeek;
        server << ",iS_peekHour:" << idcSolar.peekHour;
        server << ",iS_peekDay:" << idcSolar.peekDay;
        server << ",iS_inHour:" << idcSolar.AmpsHour;
        server << ",iS_inDay:" << idcSolar.AmpsDay;
        server << ",iS_inWeek:" << idcSolar.AmpsWeek;
        server << ",vB_minHour:" << vdcBattery.minHour;
        server << ",vB_maxHour:" << vdcBattery.maxHour;
        server << ",vB_minDay:" << vdcBattery.minDay;
        server << ",vB_maxDay:" << vdcBattery.maxDay;
        server << ",vB_inAmpsDay:" << battery.inAmpsDay;
        server << ",vB_outAmpsDay:" << battery.outAmpsDay;
}

And then complied the code all good, though I would need to make other changes to this before it is functional.
The difference is that I need to add the stream code within the actual POSTing code section, or at least call it, as I don't beleive you can pass the stream as a paramater into a function.

Off Topic:
Incidentlly, if you recall that TimeThree issue from a while back, I hadn't yet found the root cause of the problem, but it essentially stopped being a problem as soon as I changed how my application as a whole dealt with client access. While the client side script retreived data directly from the Arduino, the problem of TimerThree persisted. As soon as I stopped direct access to the Arduino, and instead via a host site, the problem ceased. I haved wondered if it was something that the Wiznet Ethernet or WebDuino routines would clobber something around those timer capture registers. I still monitor those data we did then, and no problems since the change. The change was made as the maximum connections the Wiznet could handle was four.

On Topic:
Back to stream, yes, you are correct, as can be seen from above, what I am doing is to move a set of data out from the Arduino to the host site .
The nice thing about stream is that I do away with needing those large holding char arrays.
I will think a little more about using this method in the coming days.

Many thanks for your input Nick.


Paul

rockwallaby:
The difference is that I need to add the stream code within the actual POSTing code section, or at least call it, as I don't beleive you can pass the stream as a paramater into a function.

Yeah you can. Pass by reference. Example:

#include <Streaming.h>

class myMenu 
  {
  private:
    Stream & port_; 
  public:
    myMenu (Stream & port) : port_ (port) { }
    void begin ();
  };

void myMenu::begin ()
  {
  port_ << "Menu initialized." << endl; 
  }

myMenu menu (Serial);

void setup ()
  {
  Serial.begin (115200);
  menu.begin ();
  }  // end of setup

void loop () { }

Well, I didn't know that.
It's good to have such knowledgeable folks here that I can learn from.
I shall look at that today and will like to give that a try.

If it works well, I'll mark my question solved then.

Thank you Nick.


Paul

A function with many parameters will put them on the stack before calling the function, leading to increased run-time memory usage in addition to the compile-time overhead.

I don't think you're using "%14i" and etc correctly. that number would normally be a field WIDTH, so it doesn't make sense for it to increment from argument to argument...

Here is another way to do it:

void statPost() {
    char postData[425];                // allocate enough space in the char array
    char *p = &postData[0];

    p += sprintf(p, "{\"id\":\"stat\"};
    p += sprintf(p, ",\"vT_minHour\":%i",  vacHydro.minHour);
    p += sprintf(p, ",\"vT_maxHour\":%i", vacHydro.maxHour);
    // etc
}

I'm not sure whether this would end up bigger or smaller (My guess is slightly bigger because of the additional function calls), but it is easier to read and modify.

westfw, again, thanks for your input too.
I did look at your idea and it works fine, and like you guessed, it does take a little extra, in my example, just over 200 words more.
And yes, what you say about my use of the incrementing number, well, I am not sure how I got to doing that, but I have removed those and have taken a look on the cplusplus site.
But it has been worthwhile for me to hear all your ideas, thanks guys.
I'll mark it solved then.


Paul