Arduino+Xbee+serial communication = crash || freeze?

My programming skill is very poor and I got helps from every where so my code might looks messy with little here and there. Forgive me for such question but greatly appreciated if anyone can help me again.

Here is what happened, I have tried using XBee to communicate in between 2 arduinos. It works for few seconds and “Receiver Arduino” stop working. I press the reset button and it works again for seconds then freeze again.

Here is the whole code for ReceiverArduino:

#define MAX_STRING_LEN  20
int  serIn;             // var that will hold the bytes-in read from the serialBuffer
char serInString[100];  // array that will hold the different bytes  100=100characters;
int  serInIndx  = 0;   // -> you must state how long the array will be else it won't work.
void readSerialString () {
    int sb;   
    if(Serial.available()) {
       while (Serial.available()){ 
          sb = Serial.read();             
          serInString[serInIndx] = sb;
          serInIndx++;
       }
       //Serial.print(serInString);
       Serial.println();
       for(int i=1;i<=11;i++){
         Serial.println(subStr(serInString, "|", i));
       }
    }  
}

void setup() {
   Serial.begin(9600);
}

void loop () {  
  if (Serial.available() > 0) {
   while (Serial.available()>0){
        readSerialString();
   }
  }else{
     Serial.println("waiting...");
  }
   delay(1000);
}

char* subStr (char* str, char *delim, int index) {
   char *act, *sub, *ptr;
   static char copy[MAX_STRING_LEN];
   int i;
   strcpy(copy, str);
   for (i = 1, act = copy; i <= index; i++, act = NULL) {
	sub = strtok_r(act, delim, &ptr);
	if (sub == NULL) break;
   }
   return sub;
}

Thanks for Nick’s help with this one.

And here is part of SenderArduino in loop():

  Serial.print(currentDevNum);
  int dispatchVal[] = {A_X_Value,A_Y_Value,A_Z_Value,analogRead(sliderA),B_X_Value,B_Y_Value,B_Z_Value,analogRead(sliderB),digitalRead(extraPin0),digitalRead(extraPin1),digitalRead(extraPin2),digitalRead(extraPin3)};
  for(int i=0;i<12;i++){
    String tempVar = "|";
    Serial.print(tempVar+dispatchVal[i]);
  }
  Serial.println();
  delay(10);

Whole code is way to long so I post only key part of it.

I know its probably painful to see what I am trying to do here. The whole idea is sending all variable from remote controller as a string divide by ‘|’ sign. And robot as receiver to get this string and break it into few parameters to do what it suppose to do.

Not even sure if this is the proper way to doing it so I am open to get any feedback and suggestions. Thanks again and sorry for my English if I don’t explain it clear enough. :sweat_smile:

I am still new also. However, I found that if you put Serial.println("stage one"); Then later put Serial.println("stage two");

The idea is to help you find where the code stops. If you see the stage messages you will know how far your code has gone before stopping.

Mark

It seems to me this is way too complex for what you are trying to do. As I understand it, you are sending 12 numbers from one end to the other. So why all this mucking around with long strings? Just have a simple protocol, eg.

#001|002|003| ... 012*

Now all the receiving end has to do is look for the "#" which indicates "start of stream". It sets a counter to zero. eg.

if (c == '#')
  {
  counter = 0;
  num = 0;
  }

Then when a digit arrives it adds it to the current number, eg.

if (isdigit (c))
  {
  num *= 10;  // previous value is 10 times larger
  num += c - '0';  // add new digit
  }

When a "|" arrives it stores that, eg.

if (c == '|')
  {
  command [counter++] = num;
  num = 0;
  }

When a "*" arrives it stores the last number and processes all of them, eg.

if (c == '*')
  {
  command [counter] = num;
  process_command ();
  }

That's it. No strtok. No String class. No lengthy buffers.

I agree that the code is horribly complex.

void loop () {  
  if (Serial.available() > 0) {
   while (Serial.available()>0){
        readSerialString();
.
.
.
void readSerialString () {
    int sb;   
    if(Serial.available()) {
       while (Serial.available()){ 
          sb = Serial.read();             
          serInString[serInIndx] = sb;
          serInIndx++;
       }

Why is readSerialString() called in a while loop? Before the body of the while statement is executed, the conditional is evaluated. If it is not true, the body is not executed, so the if statements prior to the while statements are unnecessary.

The "string" that you are constructing in the while loop is NOT a string. It is an array of characters.

The difference between a string and an array of characters is that the string is an array of characters [u]that is NULL terminated[/u]. Your array of characters is not NULL terminated, so it is not a string.

Because it is NOT a string, it should not be passed to any string handling functions.

Arduino+Xbee+serial communication = crush || freeze?

I was a bit worried when I read the topic that something was being crushed. Or do you mean “crash”?

serInIndx is incremented in your readSerialString routine, but it is never reset to 0. Over time (not much time probably) you will overwrite everything in RAM, this alone will cause a crash, though it may well not be your only problem.

[quote author=Nick Gammon link=topic=64009.msg466025#msg466025 date=1308129800]

Arduino+Xbee+serial communication = crush || freeze?

I was a bit worried when I read the topic that something was being crushed. Or do you mean "crash"? [/quote] LOL~ yeah, typo. Thanks again for your help!!!

Yes, I am trying to do a simple protocol but very limited for C language. So, Should I put all these code in the loop or how does that work? Shall I do this way?

void loop () {
  if (Serial.available() > 0) {
    int c = Serial.read();
      if (c == '#'){
        counter = 0;
        num = 0;
        Serial.print("#");
      }
      ...and so on
  }else{
     Serial.println("waiting...");
  }
   delay(1000);
}

Here is the picture that shows the screen from terminal with remote device:
(I might change to #123|123|123|123…* format as you mentioned)

All the arduino robots or devices will share this remote so I have first variable to identify the machine ID and the rest are X,Y position and other detail variables. When the machine received #123|123|123|123…* it will analyze it and assign each variable to proper outputs like speed of motor or ON&OFF for LEDs etc. Actually the remote itself is one of the device too. So when robots detect something and it will send variable back to remote as well.

Again, I am very limited in C programming and this language is way different from the ActionScript or JS I know, so hopefully could get helps from all you gurus. :slight_smile:

hardmouse:
Yes, I am trying to do a simple protocol but very limited for C language. So, Should I put all these code in the loop or how does that work? Shall I do this way?

Yes, but lose the delay (1000). That is building in an artificial wait of 1 second before you can process each incoming byte.

[quote author=Nick Gammon link=topic=64009.msg466533#msg466533 date=1308169971]

hardmouse: Yes, I am trying to do a simple protocol but very limited for C language. So, Should I put all these code in the loop or how does that work? Shall I do this way?

Yes, but lose the delay (1000). That is building in an artificial wait of 1 second before you can process each incoming byte. [/quote]

Sweet! It works beautifully!! Thanks Nick~~~