next topic »

[on:]

After many hours of frustration I can create a rather strange bit of behavior.

Using the example code below on a Mega 2560 R2 and an UNO R3 with the Serial Monitor set to NewLine 115200 baud the application seems to function as expected - that is, you send some text and it is returned to you.

Code:

const String MY_STRING_1 = "abcd";
const String MY_STRING_2 = "efgh"; 
const String MY_STRING_3 = "ijkl";

String temp1;
String temp2;
String temp3;

char   inByte;
String command;

void setup(){
    Serial.begin(115200);
}
void loop(){
  temp1 = MY_STRING_2;
  temp2 = MY_STRING_2;
  temp3 = MY_STRING_2;
  if (Serial.available() > 0) {
    inByte = Serial.read();
    if (inByte == 10 || inByte == 13){
    Serial.println("command Received: " + command);
    command = ""; 
    }
    else {
    command.concat(inByte);
    }
  }
}

Now, make one small change:

Code:

  temp1 = MY_STRING_2;
  temp3 = MY_STRING_2;
  temp2 = MY_STRING_2;

And the application no longer returns any text. If you reset the device, select "Send" with the Serial Monitor the application will return the expected result but as soon as a single (or more than one) character is sent then it stops responding.

Also, the length of the 3 constants at 4 characters is important because if they are reduced to 3 characters then no problem.

Change the variable assignment order back to the original order and all is well.

Can anyone please explain why this is happening? Surely the order of variable assignment should not matter.

Thanks.

[Reply #1 on:]

I suspect the problem is:

Code:

Serial.println("command Received: " + command);

I don't think that string constants implement the concatenate operation.  I think it's trying to do math on the address of the string.

Try this to see if it fixes the problem:

Code:

Serial.print("command Received: ");
 Serial.println(command);

[Reply #2 on:]

Confirmed. Even on a Uno.

I suspect a bug in the String class. After all, you are doing many, many assignments of one string to another.

However under v0022 of the IDE the problem does not occur.

I presume therefore that you are using version 1.0?

@johnwasser - debug prints show it doesn't even get as far as the line you quoted before it goes bonkers.

[Reply #3 on:]

Try this to see if it fixes the problem:

Code:

Serial.print("command Received: ");
 Serial.println(command);

I tried that, no dice.

[Reply #4 on:]

The changes suggested resulted in unreliable responses from the board and then as soon as more than 4 characters have been sent it stops responding.

The part that's been confusing to me is that just changing the order of the assignment "fixes" or "breaks" the program.

[Reply #5 on:]

I strongly suspect a bug in the String class library (or possibly, malloc/free).

Your code is forcing it to do thousands of string assignments (three per loop). Each one causes memory to be allocated, and deallocated. Any bug in either String or memory allocation will then cause it to fail.

It shouldn't happen, but as a workaround either don't use the String class at all, don't assign repeatedly, or go back to IDE v 0022.

[Reply #6 on:]

A bit more research appears to indicate you are falling prey to the "static variable initialization order fiasco" as described here:

http://www.parashift.com/c++-faq-lite/ctors.html#faq-10.14

If you recode to not initialize your Strings in global variable space, the problem goes away ...

Code:

#include <new.cpp>

const String MY_STRING_1 = "abcd";
const String MY_STRING_2 = "efgh"; 
const String MY_STRING_3 = "ijkl";

String * temp1;
String * temp2;
String * temp3;

char   inByte;
String command;

void setup(){
    Serial.begin(115200);
    temp1 = new String;
    temp2 = new String;
    temp3 = new String;
}


void loop(){
  *temp1 = MY_STRING_2;
  *temp3 = MY_STRING_2;
  *temp2 = MY_STRING_2;
  if (Serial.available() > 0) {
    inByte = Serial.read();
    Serial.print ("Got byte: ");
    Serial.println (inByte);
    if (inByte == 10 || inByte == 13){
    Serial.print("command Received: ");
    Serial.println (command);
    command = ""; 
    }
    else {
    command.concat(inByte);
    Serial.print ("command is now: ");
    Serial.println (command);
    }
  }
 
}

Where new.cpp is this:

Code:

void *operator new(size_t size_)
{
return malloc(size_);
}

void* operator new(size_t size_,void *ptr_)
{
return ptr_;
}

void operator delete(void *ptr_)
{
free(ptr_);
}

(Or just copy and paste that to the start of your file).

What is happening is that the three Strings (or possibly the six Strings) are being created *before* the malloc library is initialized. Thus the memory they allocate is undefined. Then when you go to free/reallocate it (in response to keyboard input) then major corruption occurs and you see what you see.

You should quite possibly not also initialize the const strings that way either.

Indeed you might consider not using the String class at all.

[Reply #7 on:]

Thanks for the time to explain something that is way too confusing for me.

Pasting the code as you suggested to the beginning of the file results in a compile error:

core.a(new.cpp.o): In function `operator delete(void*)':
...hardware\arduino\cores\arduino/new.cpp:10: multiple definition of `operator delete(void*)'
BUG_FIX.cpp.o:c:\temp\build2553925313592636845.tmp/BUG_FIX.cpp:19: first defined here
core.a(new.cpp.o): In function `operator new(unsigned int)':
...hardware\arduino\cores\arduino/new.cpp:5: multiple definition of `operator new(unsigned int)'
BUG_FIX.cpp.o:c:\temp\build2553925313592636845.tmp/BUG_FIX.cpp:9: first defined here

Placing it in a separate file as shown also results in an error:

new.cpp:0: error: declaration of 'operator new' as non-function
new.cpp:0: error: 'size_t' was not declared in this scope
new.cpp:5: error: declaration of 'operator new' as non-function
new.cpp:5: error: 'size_t' was not declared in this scope
new.cpp:5: error: expected primary-expression before 'void'
new.cpp: In function 'void operator delete(void*)':
new.cpp:12: error: 'free' was not declared in this scope

What am I missing here?

[Reply #8 on:]

If it says multiply defined just omit it. See if that works.

[Reply #9 on:]

That worked.

You mentioned not initializing the const Strings in the manner that I am doing. What would be a better method and, is this something particular to the String class or a general rule for all data types?

[Reply #10 on:]

The String class does dynamic memory allocation. "Simple" classes don't. This, for example, would have the same effect and not suffer that problem:

Code:

const char * MY_STRING_1 = "abcd";
const char * MY_STRING_2 = "efgh"; 
const char * MY_STRING_3 = "ijkl";