Trying to use c_string instead of String

Good evening,

I am used to using String in other languages, so I started using it in my Arduino project, but I just read a few topics explaining that it's bad practice in Arduino, so I'm moving away from it.

I've read :
http://forum.arduino.cc/index.php?topic=396099.msg2726746#msg2726746
http://forum.arduino.cc/index.php?topic=382944.msg2640860#msg2640860

I'm trying to advertise a Bluetooth Service on the Adafruit Feather 32u4 board, and I was running this code which worked perfectly fine, but used Strings :

String advertisingInstruction = "AT+GAPSETADVDATA=";

String advertisingCustomServiceFlags = "02-01-06";
String advertisingCustomServiceUUIDS = "-11-06-B8-C7-D7-BA-B6-AE-B1-A9-CD-40-BF-41-0E-D0-3D-06";
String advertisingCustomService = advertisingCustomServiceFlags + advertisingCustomServiceUUIDS;

String advertising = advertisingInstruction + advertisingCustomService;

ble.sendCommandCheckOK(advertising.c_str());

From the last link I read, I tried to convert it to this :

char advertisingInstruction[] = "AT+GAPSETADVDATA=";

char advertisingCustomServiceFlags[] = "02-01-06";
char advertisingCustomServiceUUIDS[] = "-11-06-B8-C7-D7-BA-B6-AE-B1-A9-CD-40-BF-41-0E-D0-3D-06";
char advertisingCustomService[] = "";
strcat(advertisingCustomService, advertisingCustomServiceFlags);
strcat(advertisingCustomService, advertisingCustomServiceUUIDS);

char advertising[] = "";
strcat(advertising, advertisingInstruction);
strcat(advertising, advertisingCustomService);

ble.sendCommandCheckOK(advertising);

The code compiled without any error showing up, but the Serial would not output anything, and then I could not upload any Sketch to the board without resetting it manually. After reverting the code to use String again, and after resetting the board, the Serial does print my Services and Characteristics.

According to this Adafruit's page, those symptoms mean that the board is in a crashed state :

My guess is that I'm not using c_string properly, but as I'm doing what the previous article recommends (or so I think), I'd appreciate if somebody could explain what I'm doing wrong here.

My full project (which is just a communication test between an iOS app and this board, so it's rather small) is attached to this post in case more context is needed.

heartratemonitor.zip (5.96 KB)

char advertising[] = "";
strcat(advertising, advertisingInstruction);

You are not giving strcat a large enough buffer to work in.
The beauty (and problem) with Strings is that it looked after such things itself.

Hi.

First, the use of String isn't bad practice with Arduino, it's bad practice in any resource constrained environment, along with any other dynamic memory construct. Having said that, that's not the issue you're having here.

Given that we've just touched on the evils on dynamic memory, C wants to see memory at compile time. So, rather than say anything more, I'll leave you with an arrow pointing to a statement in your code,

-> char advertisingCustomService[] = "";
strcat(advertisingCustomService, advertisingCustomServiceFlags);

and a word from the library reference

23.11.3.14 char * strcat ( char *dest, const char *src )
Concatenate two strings.
The strcat() function appends the src string to the dest string overwriting the '\0' character at the end of dest, and then adds a terminating '\0' character. The strings may not overlap, and the dest string must have enough space for the result.
Returns
The strcat() function returns a pointer to the resulting string dest.

strncat is a far better choice.

There is almost certainly no need for this (even if it worked)

strcat(advertisingCustomService, advertisingCustomServiceFlags);

Just send the individual parts with

ble.sendCommandCheckOK(advertisingInstruction);
ble.sendCommandCheckOK(advertisingCustomServiceFlags);
ble.sendCommandCheckOK(advertisingCustomServiceUUIDS);

Maybe only the final item needs the CheckOK.

The receiving device won't know the difference and you will avoid using the memory for the combined string.

...R

Thank you all! With your feedback, I was able to get it working :slight_smile:

@Robin2 : I tried doing that, but could not get it to work : the phone app does not recognize the device.

Here's what I've done :

//Created my c_strings :
char advertisingInstruction[] = "AT+GAPSETADVDATA=";
char advertisingCustomServiceFlags[] = "02-01-06";
char advertisingCustomServiceUUIDS[] = "-11-06-B8-C7-D7-BA-B6-AE-B1-A9-CD-40-BF-41-0E-D0-3D-06";

//Created the empty c_string with a size based on my previously created c_strings :
char advertisingCustomService[strlen(advertisingCustomServiceFlags) + strlen(advertisingCustomServiceUUIDS) + 1] = "";

//Used strncat instead of strcat to fill in the empty c_string :
strncat(advertisingCustomService, advertisingCustomServiceFlags, strlen(advertisingCustomServiceFlags));
strncat(advertisingCustomService, advertisingCustomServiceUUIDS, strlen(advertisingCustomServiceUUIDS));

//Same for the final c_string :
char advertising[strlen(advertisingInstruction) + strlen(advertisingCustomService) + 1] = "";
strncat(advertising, advertisingInstruction, strlen(advertisingInstruction));
strncat(advertising, advertisingCustomService, strlen(advertisingCustomService));

ble.sendCommandCheckOK(advertising);

I used different c_strings for the sake of learning, but I'll group the last two c_strings in one.

Well done. Now change your post subject to include "[solved]" so we all don't keep coming back to it.

Cheers.

strncat(advertisingCustomService, advertisingCustomServiceFlags, strlen(advertisingCustomServiceFlags));
strncat(advertisingCustomService, advertisingCustomServiceUUIDS, strlen(advertisingCustomServiceUUIDS));
...
strncat(advertising, advertisingInstruction, strlen(advertisingInstruction));
strncat(advertising, advertisingCustomService, strlen(advertisingCustomService));

The point of the third parameter is to restrict the copy to the space remaining in the buffer; to eliminate the possibility of a buffer overrun.

In other words, you are not using strncat correctly.