Strange behaviours when concatenating strings

Projects Programming
1

I'm noticing some very strange behaviours when concatenating certain strings. Basically the serial output goes haywire and either I get nothing or junk. So I'm thinking there's some kind of overflow or memory management issue. I'm really stumped. Here's a simple example:

void travelNumberOfPositions(byte motorId, int positions) {
// sendDebugMsg("Travelling " + String(positions) + " positions for motorId(s): " + String(int(motorId)), DEBUG_ON);
issueMotorCommand(TRVL, motorId, positions);
}

Anything I do trying to work with that byte parameter causes these problems. Suggestions?

2

Not enough code posted to form an opinion.
I don't use String much, and certainly not for debug.

3

As I see this "debug message" I can imagine you are running out of RAM. Strings can take a lot of memory and Arduino has just 2KB . It is not a PC with several gigs.

You need to minimize your strings or move them (partly) to PROGMEM - Arduino Playground - PROGMEM

4

ElPablo:
// sendDebugMsg("Travelling " + String(positions) + " positions for motorId(s): " + String(int(motorId)), DEBUG_ON);

All this String concatenation is likely to be leading to dynamic memory fragmentation.

5

Thank you all for the replies. I'm looking into minimizing string usage and moving them to progmem.

6

It's not String usage that, in and of itself, that is the problem. It is all the concatenation of String objects that is the problem. Stop trying to concatenate the strings the way you are, using several print statements, instead, to minimize Sting copying/concatenation issues.

7

In my case it seems to be enough to just use string literals as parameters. I stopped concatenating the strings and just used literals but I still got the same strange behaviors. Commenting out the line resolved the problem.

8

I think we would need to see more to comment. A single string literal, passed to a function, in itself should cause no problems at all. Unless you perhaps have hundreds of them? Or, perhaps the debug function does something strange with the string argument.

9

Happy to share the sketch and since I'm fairly new to working with the Arduino would welcome any feedback or suggestions in general:

https://github.com/Pablo1/Avatar/blob/master/arduino/avatar/avatar.pde

10

Well I'm puzzled about this:

void sendDebugMsg(String message, int level) {
  if (debugLevel >= level) {
    char buffer[255] = {
      '\0' };
    message.toCharArray(buffer, message.length() + 1);
    cmdMessenger.sendCmd(DEBUG_MESSAGE, buffer);
  }
}

What, ultimately, is this doing? Sending a debug message to the serial port?

You seem to be misusing toCharArray here. The length is supposed to be the buffer length (ie. sizeof (buffer)) not the string (message) length which it can work out for itself.

In any case, you seem to be using a lot more memory for debugging than is required. With about one exception, all the debug messages are literals, eg.

  sendDebugMsg("Issuing travel command", DEBUG_ON);

So this is being promoted to a String type (which involves dynamic memory allocation) when you call sendDebugMsg, and then immediately you use a 255 byte buffer (bearing in mind the whole processor only has 2048 bytes of RAM) and copy the message back into a char array, which you then send. Why not just send char * in the first place? eg.

void sendDebugMsg(char * message, int level) {
  if (debugLevel >= level) {
    cmdMessenger.sendCmd(DEBUG_MESSAGE, buffer);
  }
}
11

OP, your code is full of strings like this "SendAllParams Command Received". Too many of the above (31 bytes for this string * 32=1KByte) sucks up your SRAM. These strings should all be moved to PROGMEM as pointed out by others. Plus, you program Arduino like a Java programmer, bashing strings together. You should get yourself familiar with the concept of sprintf, the standard way of C. I don't care what your programming background is, but if your math is good, you know this size of char array will only make your program more unstable.

char buffer[350];

At least make some realistic estimate of the size you need. You're also filling them with zeros to start with, very bad if you want to save time. Plus this action certainly dooms the overflown memory but yeah it makes the symptom more obvious so "well done" I guess. Otherwise you may not see your problem so easily, more or less come-and-go instead.

12

Thanks folks for the thoughtful comments. Just using sprintf and the keeping the string literals as arrays and just using pointers seems to have resolved the problems I was experiencing.