Addicted to Arduino Strings - CString Rehab

Now that my firmware codes are getting bigger and more complex, memory optimization is proving to be critical.
My understanding is that character arrays are preferred and can be written just fine without casting to publish and subscribe commands. I have written below a subscription handler written with and without Duino string objects.
Suggestions or corrections welcome. I haven’t tried this method yet, but it does compile in the IDE.
Note that I am also using a response template.
The Arduino String way:

void getWeatherHandler(const char * event, const char * data){ //The cloud sends back weather data
  String weatherReturn = String(data);
  char weatherBuffer[250] = "";
  weatherReturn.toCharArray(weatherBuffer, 250);
  String condition = strtok(weatherBuffer, "\"~");
  String updatetime2 = strtok(NULL, "~");
  String epochtime = strtok(NULL, "~");
  icon = atoi(strtok(NULL, "~")); //get the weather icon number
  String useless = strtok(NULL, "/"); //getting through some useless data here
  String useless2 = strtok(NULL, "/"); //probably there is a better way?
  String useless3 = strtok(NULL, "/");
  String country = strtok(NULL, "/");
  String city = strtok(NULL, "/");
  float age = ( Time.local() - atoi(epochtime) - zone*3600)/60;
 
//send to Google
Particle.publish("WeatherGoogle", "{\"City\":\"" + city + "\",\"Country\":\"" + country + "\", \
\"Condition\": \"" + condition + "\", \"Update_Time\": \"" + updatetime2 + "\", \"Age\":\"" + String::format("%.1f",age) + "\",\
\"Icon\":\"" + (String)icon + "\", \"DeviceID\": \"" + Particle.deviceID() + "\"}", 60, PRIVATE);
}

Now with CStrings!

void getWeatherHandler(const char * event, const char * data){ //The cloud sends back weather data
    char dataCopy[250] ="";
   strcpy (dataCopy, data);
   char condition[50] = "";
   strcpy (condition, strtok(dataCopy, "\"~"));
   char updatetime2[50] = "";
   strcpy (updatetime2, strtok(NULL, "~"));
  char epochtime[50] = "";
  strcpy (epochtime, strtok(NULL, "~"));
  icon = atoi(strtok(NULL, "~")); //get the weather icon number
  char useless[50] = "";
  strcpy (useless, strtok(NULL, "/")); //just writing some tokens to the same buffer
  strcpy (useless, strtok(NULL, "/")); //to get through some useless sections of data
  strcpy (useless, strtok(NULL, "/")); //perhaps there is a better way?
  char country[50] = "";
  strcpy (country, strtok(NULL, "/"));
  char city[50] = "";
  strcpy (city, strtok(NULL, "/"));
  float age = ( Time.local() - atoi(epochtime) - zone*3600)/60;
  
//send to Google

char pubString[300] = "";
strcpy (pubString, "{\"City\":\"");
strcat (pubString, city);
strcat (pubString, "\",\"Country\":\"");
strcat (pubString, country);
strcat (pubString, "\",\"Condition\": \"");
strcat (pubString, condition);
strcat (pubString, "\", \"Update_Time\": \"");
strcat (pubString, updatetime2);

char age1[16];
snprintf(age1, sizeof(age1), "%.2f", age);
strcat (pubString, "\", \"Age\": \"");
strcat (pubString, age1);
char icon1[8];
snprintf(icon1, sizeof(icon1), "%i", icon);
strcat (pubString, "\", \"DeviceID\": \"");
strcat (pubString, Particle.deviceID());
strcat (pubString, "\"}");
Particle.publish("WeatherGoogle", pubString, 60, PRIVATE);
}

@PopQuiz, the best way to learn is to try it and report back with your questions :wink:

One thing, since your already have a mutable (non-const) copy of your string dataCopy you don’t need all the extra copies of the substrings.
strtok() will just inject zero-terminators into dataCopy handing you back a char* to the beginning of that sub-string, so you can just reuse the original array with a number of pointers pointing to the respective starting points of all the individual substrings.

So no need for estra arrays and strcpy().

Instead of a bunch of strcat() calls, you could use a single snprintf() statement.

BTW, you could also use a single sscanf() to pluck apart your original const char* into individual char[] strings.

2 Likes

My $0.02…I would replace all of the string commands with the “n” version, ie:

strcpy -> strncpy
strcat -> strncat

Better safe than sorry.

Great suggestions, thanks.

Ok, so how do I call the individual substrings in the arguments of snprintf()? With pointers for each one?
I have:

  char dataCopy[250] ="";
  strcpy (dataCopy, data);
  char * token1 =  strtok(dataCopy, "\"~");
  char * token2 =  strtok(NULL, "~");
  char * token3 = strtok(NULL, "~");
  char * token4 = strtok(NULL, "~"); 
  char * token5 = strtok(NULL, "/"); //useless token
  char * token6 = strtok(NULL, "/"); //useless token
  char * token7 = strtok(NULL, "/");
  char * token8 =  strtok(NULL, "/");
  char * token9 =  strtok(NULL, "/");
  float age = ( Time.local() - atoi(token1) - zone*3600)/60;
  int icon =atoi(token2);
  char pubString[300] = "";
  snprintf(pubString, sizeof(pubString), "{\"Condition\":\"%s\",\"Update_Time\":\"%s\",\"Condition\":\"%s\",\"Update_Time\":\"%s\",\"Age\":\"%.1f\",\"Icon\":\"%i\",\"DeviceID\":\"%s\"}", token3,token7,token8,token9,age,icon, Particle.deviceID() );

For one, if you want to use as little memory as possible, you can (should) replace this

  char dataCopy[250] ="";

with

  char dataCopy[strlen(data) + 1];

This only allocates the actually needed space for the copy of data.
Similarly you could make pubString[300] a bit smaller if you calculated the overhead your JSON introduces ontop of the size of data (e.g. char pubString[strlen(data) + overhead]).

Not sure, what your question really is, since what you are doing in your snprintf() is probably the same as I'd suggest.
I'd maybe use more descriptive variable names than tokenX or an array char* token[9] but generally the idea is the same.
What is the problem you are seeing with the way you have written your snprintf()?

But since you are not actually using all tokens, you could just drop the returned pointers, so you could save the few bytes for the otherwise required pointer variables.

I'm also not quite sure why your first token would be delimited with a single double quote (") or a tilde (~). Double quotes usually come in pairs which might throw off your tokenizing logic.

1 Like

I have several functions used for publishing on timers.

char locationString[150] = "";
snprintf(locationString,sizeof(locationString), "{"MyCountry": "%s","SearchTerm":"%s","Key":"%s","API":"postalcodes"}",myCountry,myZip,key);
Particle.publish("locationHook", locationString, 60, PRIVATE);

Which is better optimization for memory, to initialize a single char array globally and have all the functions write to it just before publishing, or initialize a local array within each function and publish that?

Also, my understanding is that I don't need to clear local variables. Would it be a good idea to clear a global array between writes?

That's not the whole story.
First, a statement like char s[150] = "" does not initialize the whole string with zeros, but only places a \0 byte at s[0]. But in general, you don't need to initialize a local variable if you know that you'll just overwrite its content, no matter what. But if you have code paths that may leave the variable untouched or may alter the current value then you have to make sure the starting value is determined/initialized.

The same goes for globals.

As for the question loval vs. global, that depends on the use-case.
But since local variables are allocated on the stack and on µC the stack is limited - especially when running functions on a different thread than the application thread - it might be worth considering to have big arrays not allocated on stack.

2 Likes

How big is “big?” My biggest array is the publication string and 220 provides enough overhead for all of my publish functions to write to it, and overwriting is no problem since the string is published immediately.
Instead of using 4 local varialbles, I could use one global and write to it over and over. Would that be vastly preferable, just a little better, or not make much of a difference at all?

It wouldn’t make much difference on the application thread, but if your function would be called on the Software Timer thread (with 1K stack) it might be the difference between running or crashing :wink: