Converting away from the use of String Variables

Troubleshooting
1

I am looking for documentation or assistance to help debug the following problem.

I am in the process of improving the resilience of my Photon code by (amongst other things) minimising the use of strings.

One such string variable is a MQTT client ID which is a concatenation of the location of the sensor, the string “_client” and a random number converted to a string. This code works but makes heavy use of strings.

    .
    String location = "bedroom";
    String client_id = location + "_client" + String(random(1000,9999)); // TODO Convert away from strings.
    .

The following code is my failing attempt to use memory locations.

.
const char location[] = "bedroom"; // This works
char client_id[(sizeof(location)+10)]; // Size of location -1 for terminating character + 7 for "_client" + 4 for random number
char * client_id_pointer;               // a pointer (curently blank) that will point to client_id
client_id_pointer = &client_id[0];      // Assign client_id_pointer to the start of client_id
*client_id_pointer = String(location);  // Write location (e.g. 'bedroom') to the start of client_id
client_id_pointer = &client_id[(sizeof(location)-1))]; // move pointer to the end of 'location'
*client_id_pointer = "_client";         // Write '_client'
client_id_pointer = &client_id[(sizeof(location)+6)];
*client_id_pointer = String(random(1000,9999));
.

This fails with “client_id_pointer does not name a type” and “expected constructor, destructor, or type conversion before ‘=’ token” :scream:

Grateful for any assistance.

2

There are several things that can/should be done differently here.

Instead of having two strings location[] and client_id[] which also contains the former I’d rather think of how long will I ever have client_id[] and only reserve the one instance of the string - unless there is actual need for both strings to be available seperately.
Also none of these intermediate pointers is actually needed.
And finally, if you want to get rid of String you may want to look into snprintf() for string creation/concatenation/… and drop String(random()) and String(location) which in itself is not only redundant but also (if it worked) would point the the wrong memory location (somewhere on the heap).

In order to correctly work with strings, you need some basic understanding about memory structure, pointers and the inner workings of operators (e.g. assignment =).

Lets take this line

*client_id_pointer = "_client";

This does absolutely not what you might imagine (or at least what I think you imagine from the comments and rest of the code)

  • "_client" is creating a const char* which will point to some memory location in flash where "_client" is stored.
  • client_id_poiner is pointing somewhere in RAM
  • *client_id_pointer = 'x' would copy one byte to the exact RAM position where the pointer points, but
  • *client_id_pointer = "_client" does actually intend to copy the four-byte const char* directed at the flash memory location described above into the one-byte location and not the string that’s located there and hence fails to build

Sorry for not providing you the solution (yet), but the intention behind the above is to first encourage “curiosity” and “interest” about the topic to then have the concept better stick in anybody’s mind who comes across this thread.

2 Likes
3

@ScruffR, you make some really good points. Without even knowing it, I started falling into the String trap myself. It was so easy to create log entries mixing variables and text. It was a good reminder to go back to my roots and do it the char[] way when possible. You also make a great point about experimentation and putting in a little “sweat equity”.

From an academic exercise, here is my solution off the top of my head. I anxiously await your version to see where I can improve:

char location[] = "bedroom";
char client_id[19];
snprintf(client_id, sizeof(client_id), "%s_client%d", location, random(1000,9999));

I’m assuming you need “location” to be a separate variable. If not, you could replace the %s in snprintf() with the word “bedroom”. Don’t forget the extra space in “client_id” for the null-terminating char '\0' to make this a valid C string.

2 Likes
4

Thanks @ScruffR this got me underway again! I have now re-written with liberal use of snprintf() and have code very similar to that suggested by @syrinxtech and all appears to be working smoothly.

The only learning point I will add for anyone else reading this thread is that snprintf() does not appear to work if declared outside of a setup(), loop() or other block. However, it was no issue (and possibly read better) to declare the snprintf() statement close to the point of use of the variable.

Thanks again

1 Like
5

@NeilB, glad you got it working.

I’m curious what your second comment regarding snprintf meant. That function is typically part of <stdio.h> and I’ve used it it in many different blocks of code, including setup(), loop(), and my own functions and libraries. Can you help me understand your statement by providing context or examples?

Are you saying you used it outside of a typical function definition, ie:

setup()
{}
loop()
{}
void func1()
{}
void func2()
{}
snprintf(......);
void func3()
{}
1 Like
6

By way of example, @syrinxtech, this works…

// This #include statement was automatically added by the Particle IDE.
#include <DS18.h>
#include <MQTT.h>
.
.  (other initialising code)
.
// Initialise function name
int localReset(String command);
// ... and variables
double tempC = 17;
unsigned long old_time = 0;
unsigned long lastTimeSync = 0;
// ... and constants
const char type[] = "thermometers";
const char location[] = "bedroom";
char client_id[23];
char mqtt_dir[25];
.
.
.
void setup() {
    // connect to the MQTT Broker
    snprintf(client_id, sizeof(client_id), "%s_client%d", location, random(1000,9999)); 
    client.connect(client_id); 
.
.
}
// END OF SETUP

void loop() {
.
.
        if (client.isConnected()) {
            // Publish the temperature to MQTT queue for the room
            snprintf(mqtt_dir, sizeof(mqtt_dir),"%s/%s", type, location);
            client.publish(mqtt_dir, String(sensor.celsius()));
        } else {
            Particle.publish("mqtt", "Not Connected... attempting reconnect");
            snprintf(client_id, sizeof(client_id), "%s_client%d", location, random(1000,9999));
            client.connect(client_id);
        }
.
.

… but this doesn’t

// This #include statement was automatically added by the Particle IDE.
#include <DS18.h>
#include <MQTT.h>
.
.  (other initialising code)
.
// Initialise function name
int localReset(String command);
// ... and variables
double tempC = 17;
unsigned long old_time = 0;
unsigned long lastTimeSync = 0;
// ... and constants
const char type[] = "thermometers";
const char location[] = "bedroom";
char client_id[23];
char mqtt_dir[25];
snprintf(client_id, sizeof(client_id), "%s_client%d", location, random(1000,9999)); 
snprintf(mqtt_dir, sizeof(mqtt_dir),"%s/%s", type, location);
.
.
.
void setup() {
    // connect to the MQTT Broker
    client.connect(client_id); 
.
.
}
// END OF SETUP

void loop() {
.
.
        if (client.isConnected()) {
            // Publish the temperature to MQTT queue for the room
            client.publish(mqtt_dir, String(sensor.celsius()));
        } else {
            Particle.publish("mqtt", "Not Connected... attempting reconnect");
            snprintf(client_id, sizeof(client_id), "%s_client%d", location, random(1000,9999));
            client.connect(client_id);
        }
.
.
7

This is true in all C/C++ programs: only declarative items (like variable declarations) can appear outside of functions in global scope. Calling functions like snprintf() or similar will not work. Another way to think of it is that everything in global scope needs to be fully defined at compile time.

1 Like
8

@bko, would an exception be function prototypes (which I guess you would consider a declarative item)?

Wouldn’t this be OK in global scope?

int func1(int, float, char);

Yes, they normally would go in an .h file…but sometimes on short programs I cheat.

9

Yes, function prototypes are forward declarations so that the compiler “knows” how you are going to call a function that it has not compiled yet or will be linked to later.

10

Glad my suggestions and guiding hints did help.
In essence the way you wrote the code there is what I was thinking of.

The only minor change I'd make is in the global declaration

const char location[] = "bedroom";

The difference here is that location will only occupy memory once in flash.
Without the const modifyer the compiler would create one instance of the string literal in flash and also reserve an additional instance of the "mutable" array in RAM plus the code to copy the literal from flash into RAM. As I'd assume the literal will never need to be changed at run time declaring it const will spare you all the extra effort.

Some extra hint about snprintf() and my suspicion about the reason for random(1000,9999).
If the reason for the lower limit is the wish to ensure a four digit number, there is also another possibility is this

snprintf(client_id, sizeof(client_id), "%s_client%04d", location, random(0,9999));

%04d is the format string for an integer which should take at least four places and use leading zeros (where required - 0000..0999).

2 Likes
11

@ScruffR, I always try and learn from everyone, even if it sometimes means how NOT to do things.

Thanks for the tip on “const”.

How does adding PROGMEM to a declaration like that change things, if any? For example, is there a difference between:

const String location = "bedroom";

and

const String location PROGMEM = "bedroom";

12

On the Particle devices there is no division in memory space between flash and RAM so there is no such thing as PROGMEM. When you write it in code on these devices it will just vanish completely as it’s only an empty macro to make AVR code easier portable.
But in your particular example it does make little sense even if it existed, since String is an object which stores the string on the heap which by definition can’t be in the PROGMEM area.

13

@ScruffR, thanks again.

i’m trying to figure out how to change my code to pass String parameters by reference instead of value. Is it as easy as putting the “address of” operator in front of the calling and called String variable names? I’m used to using “&” and “*” when working with variables like ints, floats, etc.

Sorry, I hope I’m not using up my allotment of questions…but does using the String required by the Particle functions impose the risk of creating heap fragmentation? Should I convert the String using c_str() before using it?

How dangerous is this line?

String emailID = LOCATION + "-" + ((Time.month() < 10) ? "0" + String(Time.month()) : String(Time.month())) +((Time.day() < 10) ? "0" + String(Time.day()) : String(Time.day())) + String(Time.year() - 2000) + "-" + String(emailIDIndex);

When I think of trying to re-write that line using C strings I cringe!

14

@ScruffR, does that mean that the “F macro” doesn’t work either, but is just a hollow macro?

15

Why? I think using c strings and snprintf is easier to write, shorter, and more readable. If I read your code correctly, it would look like so,

char emailID[30];
snprintf(emailID, 30, "LOCATION-%02d%02d%d-%d", Time.month(), Time.day(), Time.year() - 2000, emailIDIndex);

I was assuming that LOCATION is a string (not a variable) and that emailIDIndex is an int

2 Likes
16

@Ric, you are correct on both assumptions.

Wow, that is much easier. I guess I got hung up on the leading zeros instead of applying what I’ve used lots of other times. Damn tertiary operators!

Thanks, much appreciated.

17

Exactly that.

1 Like
18

@scruffr, I guess I’ve spent too much time in the AT328 world! :frowning_face:

19

Although there seem to be two schools of thought about the "best" way to pass objects by reference, I'm usually going with the dedicated C++ feature of using the special construct of C++ references over passing object pointers.

someObject_t x(1,2,3);

void takePointer(someObject_t* param) {
  param->method();
}
void takeReference(someObject_t& param) {
  param.method();
}

void usage() {
  takePointer(&x);
  takeReference(x);
}

simplified: When using a reference the reference "becomes" the actual object and can be used "in place" just like the original, while with pointers you do have to treat the param differently (i.e. as pointer) than the original object.

BTW, for your intent to format your date, instead of taking the Time apart and then putting it back together in a different way, I'd just use the dedicated Time.format() function which in turn just takes a format string to immediately hand you the data you want.
http://www.cplusplus.com/reference/ctime/strftime/

For your example above this would look like this

char emailID[30];
snprintf(emailID, sizeof(emailID), "LOCATION-%s-%d", (const char*)Time.format("%m%d%y"), emailIDIndex);

BTW²: I know this is the American way of writing dates, but if you ever wanted to sort a string in a simple manner, I'd always go with the intrinsically sortable format "%y%m%d" instead - especially since without date delimiters the human way of looking at dates in the cognitively "comfortable" way is out of question, why then not follow the logic consequence and make the format more suitable for machines?

20

Ooooh!.... I like that! :smiley:

1 Like