Arduino Unreliable Serial communication

Question

After reading the following 2 articles:

https://hackingmajenkoblog.wordpress.com/2016/02/01/reading-serial-on-the-arduino/

https://hackingmajenkoblog.wordpress.com/2016/02/04/the-evils-of-arduino-strings/

I have get rid of the complete String class from my main program and went back to using character arrays. This seems to work reliably until some extent but once I start stress testing it, quickly turns out that it is not reliable at all!

Here is the complete code, you can load it into the mega and it will produce the same results (I have removed all other unnecessary parts from the code):

#define DEBUG

char Command_PC[64], Command_Xbee[64], Command[64];
int count=0;
bool Complete_PC=false;
bool Complete_Xbee=false;

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

} // END void setup


void comm() {

/* COMM FROM PC */    
if (Serial.available()){

  #ifdef DEBUG
    Serial.println("PC: New command, collecting...");
  #endif
    count = 0;
    memset(Command_PC, 0, sizeof(Command_PC));
    while (Serial.available()){
      char character = Serial.read();
      if (character=='\n')
         {
          Complete_PC =true;
          break;
         }     
      Command_PC[count] = character;
      count++;
    }
  }

/* COMM FROM Xbee */  
if (Serial1.available()){
  #ifdef DEBUG
    Serial.println("Xbee: New command, collecting...");
  #endif
    count = 0;
    memset(Command_Xbee, 0, sizeof(Command_Xbee));
    while (Serial1.available()){
      char character = Serial1.read();
      if (character=='\n')
         {
          Complete_Xbee =true;
          break;
         }     
      Command_Xbee[count] = character;
      count++;
    }
  }

   delay(500);

 if (Complete_Xbee || Complete_PC) {

  if(Complete_Xbee)
    strcpy(Command, Command_Xbee);
  else
    strcpy(Command, Command_PC);

  #ifdef DEBUG
    Serial.print("Command received: >");
    Serial.print(Command);
    Serial.println("<");
  #endif      
           Complete_Xbee=false;
           Complete_PC=false;
 }

}

void loop()
{    
    comm();

} // END void loop

The code is simple, all it is intended to do that whether there is a command coming from the PC (USB) or Xbee (attached to hardware uart) process it and do further things. As long as you typing in commands manually it goes reliably for a while but even then after 10-15 commands it might screws up. The automated testing with:

for ((;;)); do echo HELLOWORLD > /dev/ttyACM0; done

Command received: >HELLOWORLD<
ERR_INVALIDCMD
PC: New command, collecting...
Command received: >HELLOWORLD<
ERR_INVALIDCMD
PC: New command, collecting...
Command received: >HELLOWORLD<
ERR_INVALIDCMD
PC: New command, collecting...
Command received: >HELLOWORLD<
ERR_INVALIDCMD
PC: New command, collecting...
Command received: >HELLOWORLD<
ERR_INVALIDCMD
PC: New command, collecting...
Command received: >HELLOWORD<
ERR_INVALIDCMD
PC: New command, collecting...
Command received: >HELLOWORL<
ERR_INVALIDCMD
PC: New command, collecting...
Command received: >HELLOWORLDELLOWORLD<
ERR_INVALIDCMD
PC: New command, collecting...
Command received: >HLOWORLD<
ERR_INVALIDCMD
PC: New command, collecting...
Command received: >HELOWORLD<
ERR_INVALIDCMD
PC: New command, collecting...
Command received: >HELLORLD<
ERR_INVALIDCMD
PC: New command, collecting...
Command received: >HELLOORLD<
ERR_INVALIDCMD
PC: New command, collecting...
Command received: >HELLORLD<
ERR_INVALIDCMD
PC: New command, collecting...
Command received: >HELLOWORLD<
ERR_INVALIDCMD
PC: New command, collecting...
Command received: >HELLOORLD<
ERR_INVALIDCMD
PC: New command, collecting...
Command received: >HELLORLD<
ERR_INVALIDCMD
PC: New command, collecting...
Command received: >HELLORLD<
ERR_INVALIDCMD
PC: New command, collecting...
Command received: >HELLOORLD<
ERR_INVALIDCMD
PC: New command, collecting...
Command received: >HELLORLD<
ERR_INVALIDCMD
PC: New command, collecting...
Command received: >HELLOORLD<
ERR_INVALIDCMD
PC: New command, collecting...
Command received: >HELLOWLD<
ERR_INVALIDCMD
PC: New command, collecting...
Command received: >HELLOWOD<
ERR_INVALIDCMD
PC: New command, collecting...
Command received: >HELLOWOLD<
ERR_INVALIDCMD
PC: New command, collecting...
Command received: >HELLOWLD<
ERR_INVALIDCMD
PC: New command, collecting...
Command received: >HELLOWOLD<
ERR_INVALIDCMD
PC: New command, collecting...
Command received: >HELLOWLD<
ERR_INVALIDCMD
PC: New command, collecting...

Brings out the bug right away. Is there any problem with this code? Any ideas how to make this 100% reliable even if it takes slowing the main program down?

I have noticed that by adding: delay(500); it makes it more stable. If this is not there it will even fail on the first manually entered command like:

PC: New command, collecting...
PC: New command, collecting...
PC: New command, collecting...
Command received: >lloworld<

By adding a delay to my test script:

for ((;;)); do echo HELLOWORLD > /dev/ttyACM0 && sleep 1; done

It runs stable for a while but after 15 mins testing it start misbehaving again:

Command received: >HELLOWORLD<
PC: New command, collecting...
Command received: >HELLOWORLD<
PC: New command, collecting...
PC: New command, collecting...
Command received: ><
PC: New command, collecting...
PC: New command, collecting...
Command received: ><
PC: New command, collecting...
PC: New command, collecting...
Command received: ><
PC: New command, collecting...
PC: New command, collecting...
Command received: ><

score 3 · Accepted Answer · 2016-11-12 07:00:11Z

To me, your problem is because the program is reading the characters faster than they are sent by the computer. Then, some situations occur where all the characters of the buffer have been read (Serial.available() = 0) but the '\n' character has not been sent by the computer yet. The program will then exit the loop without setting the Complete_PC variable to true.

In that case, the program will write several times "PC: New command, collecting..." without writing any result (Complete_PC not set to true). It can also drop the first letters, as they were read during the previous loop and the Command_PC array and count were reset in between.

The delay(500) command improve the stability because it increase the chances that the whole line is received when the program is not reading it at the same time.

Try something like this:

while ((!Complete_PC)&&(count<60))
{
    int character = Serial.read();
    if (character=='\n')
    {
        Complete_PC =true;
    }
    else if (character!=-1)
    {
        Command_PC[count] = (char) character;
        count++;
    }
}

Majenko · Accepted Answer · 2016-11-10 10:15:18Z

1

As @Edesign has mentioned your reception methodology is slightly flawed. A better arrangement would be:

static int Count_PC = 0;
static bool Complete_PC = false;

if (Serial.available()) {
  char character = Serial.read();
  if (character=='\n') {
    Complete_PC =true;
    Count_PC = 0;
  } else {
    if (Count_PC < 62) { // Don't overflow the array...!
      Command_PC[Count_PC++] = character;
      Command_PC[Count_PC] = 0;
    }
  }
}

The count and complete flags are now static - that is, they retain their values between iterations of the loop. You only look at one character per iteration of the loop (you could enhance it by changing the first if to a while if you like, but this way plays better with handling multiple data sources), and add that to the string. Only when the \n is received do you flag it as complete. There is no clearing of the whole array, it's not needed - just ensure that you always add a \0 to the end of the string each time you add a character.

The current character position (Count_PC) is reset when you receive the \n so that it is ready to receive the next command - be sure that you process the command that same loop (which you do anyway).

answered Nov 10, 2016 at 10:15

Majenko♦

104k5 gold badges76 silver badges134 bronze badges

I have tested this code and it comes up with about the same results. Running or ((;;)); do echo HELLOWORLD > /dev/ttyACM0 && sleep 1; done will produce ommand received: >< PC: New command, collecting... PC: New command, collecting... Command received: >< PC: New command, collecting... PC: New command, collecting... Command received: >< empty commands after about 10 mins. If I then stop the stress test for a second then restart it then it is working again for a while.
– qlesk
Nov 10, 2016 at 10:41
Also with your code if my delay(500); is not present then it returns empty right away for the word hello instead of receiving at least a couple of characters like ello.
– qlesk
Nov 10, 2016 at 10:44
You will have to show how you have fed this code into your existing code - how you handle the command and reset things will also have a bearing on things. As well as when you are doing your debug printing...
– Majenko ♦
Nov 10, 2016 at 10:47
pastebin.ca/3737758 the difference is I still kept my while loop, breaking not breaking at \n made no difference during the tests however I believe that if I don't have the break statement there it is more stable, because if the end of the line is reached and there would be some junk characters after it in the buffer those would be erased by the Serial.read() and would not end up in my final Command_PC of course.
– qlesk
Nov 10, 2016 at 11:19
@qlesk When testing this code, did you completely removed the whole if statement after /* COMM FROM PC */? Because those lines are supposed to deal with 1 letter at each loop, so it will never work if you keep resetting the array with the memset command for example.
– Edesign
Nov 10, 2016 at 11:24

| Show 2 more comments

qlesk · Accepted Answer · 2016-11-10 13:58:47Z

@Edesign, @Majenko: thank you for your help, it turns out that the hardware DOES matter as well. It was partially a hardware issue. Regarding my initial code, you can get that code remove the delay(500); from it and load it into an Arduino Leonardo and it will pass my first initial stress test without any delays and it works stable even after an hour testing.

I believe that both the Mega and the Leonardo have USB2 connections:

Bus 001 Device 003: ID 2341:0042 Arduino SA Mega 2560 R3 (CDC ACM) Bus 003 Device 013: ID 2341:8036 Arduino SA

Although this is was a Keyestudio MEGA, a cheap chinese counterfeit which would explain why did it have this issue. My initial code was not wrong but as you have pointed out it could have been improved, what I did.

Code1:

static int Count_PC = 0;
static bool Complete_PC = false;

if (Serial.available()) {
  char character = Serial.read();
  if (character=='\n') {
    Complete_PC =true;
    Count_PC = 0;
  } else {
    if (Count_PC < 62) { // Don't overflow the array...!
      Command_PC[Count_PC++] = character;
      Command_PC[Count_PC] = 0;
    }
  }
}

Code2:

/* COMM FROM PC */   
if (Serial.available()){

  #ifdef DEBUG
    Serial.println("PC: New command, collecting...");
  #endif
    memset(Command_PC, 0, sizeof(Command_PC));
    while (Serial.available()){
      char character = Serial.read();
      if (character=='\n')
         {
          Complete_PC =true;
          Count_PC = 0;
          //break;
         } else {
             if (Count_PC < 62) { // Don't overflow the array...!
                 Command_PC[Count_PC++] = character;
                 Command_PC[Count_PC] = 0;
             }
         }
  }
}

The difference between his code and my last code is that I do the while looping inside the function while his relies on the main loop cycles to complete the string.

Both are working but if the main program have a lot to do, his version will noticeably make the responses more sluggish. Which can be easily tested with:

 for ((;;)); do date > /dev/ttyACM0; done

Stack Exchange Network

Arduino Unreliable Serial communication

3 Answers 3

Your Answer

Not the answer you're looking for? Browse other questions tagged
serial
communication
or ask your own question.

Hot Network Questions

Arduino Unreliable Serial communication

3 Answers 3

Your Answer

Sign up or log in

Post as a guest

Not the answer you're looking for? Browse other questions tagged serialcommunication or ask your own question.

Related

Hot Network Questions

Not the answer you're looking for? Browse other questions tagged
serial
communication
or ask your own question.