Arduino freezes after a random amount of time (Probably a UART problem)

Projects Programming
1

Hello guys,
I am making a tricopter drone using Arduino Uno and a ESP8266 for communication.

The ESP receives the commands via WiFi and sends them to the Arduino via the hardware Serial. The Arduino proceses the received data and uses it for the PID controllers. However after a random amount of time (sometimes an hour, sometimes only 6 minutes) the Arduino stops responding. I suppose it is a UART problem because if I start the Arduino with the ESP disconnected the Arduino does not freeze.

This is the function I use to receive data:

#define MAX_DELAY 140000
#define DEBUG_COMM 0

int comm_count, thro=1000, yaw=1500, pitch=1500, roll=1500, thro_tmp, yaw_tmp, pitch_tmp, roll_tmp;
int comm_status = 0; // 0 - disconnected, 1 - connected, 2 - lost connection
int comm_errors = 0;
long time_delay = 0, time_load=0;
bool rdy=false;

void setup() {
  Serial.begin(115200);
  pinMode(13,OUTPUT);
  pinMode(4,OUTPUT);
}

void loop() {

  getCommands();

}

void getCommands() {
  
  while (Serial.available()) {
    if (Serial.peek() == '

The format of the commands is $|throttle|yaw|roll|pitch|, '#' is used as an "Arm" command and '!' as a "Disarm" command.

Please tell me if you see something that is not a good practice or something that does not look correct to you.

This function is called in the loop. It still freezes even if this is the only function in the code, so I think that this is what causes he problem.) {
     Serial.read();
     rdy=true;
     time_delay=micros();
     
     if(thro_tmp==0 || yaw_tmp==0 || pitch_tmp==0 || roll_tmp==0){
       thro=1000;
       yaw=1500;
       pitch=1500;
       roll=1500;
     }
     else{  
       thro=thro_tmp+1;
       yaw = constrain(0.7yaw+0.3(yaw_tmp+1),1000,2000);
       pitch = constrain(0.7pitch+0.3(pitch_tmp+1),1000,2000);
       roll = constrain(0.7roll+0.3(roll_tmp+1),1000,2000);
       if(yaw>=1492 && yaw<=1508){
         yaw=1500;
       }
       if(pitch>=1492 && pitch<=1508){
         pitch=1500;
       }
       if(roll>=1492 && roll<=1508){
         roll=1500;
       }
 
 
       if(DEBUG_COMM){
           Serial.print(thro);
           Serial.print("\t");
           Serial.print(yaw);
           Serial.print("\t");
           Serial.print(pitch);
           Serial.print("\t");
           Serial.println(roll);
       }
     }
     thro_tmp=0;
     yaw_tmp=0;
     pitch_tmp=0;
     roll_tmp=0;
     comm_count = 0;
     comm_errors = 0;
   }
   else if (Serial.peek() == '#') {
     Serial.read();
     comm_status = 1;
     comm_errors=0;
     time_delay=micros();
   }
   else if (Serial.peek() == '!') {
     Serial.read();
     comm_status = 0;
     comm_errors=0;
   }
   
   else if(Serial.peek()>='0' && Serial.peek()<='9') {
       
     while(Serial.peek()>='0' && Serial.peek()<='9'){
       if (comm_count < 4) {
         thro_tmp+=(Serial.read()-'0')*pow(10,(3-comm_count));
         comm_count++;
       }
       else if(comm_count>=4 && comm_count<8){
         yaw_tmp+=(Serial.read()-'0')*pow(10,(3-(comm_count-4)));
         comm_count++;
       }
       else if(comm_count>=8 && comm_count<12){
         roll_tmp+=(Serial.read()-'0')*pow(10,(3-(comm_count-8)));
         comm_count++;
       }
       else if(comm_count>=12 && comm_count<16){
         pitch_tmp+=(Serial.read()-'0')*pow(10,(3-(comm_count-12)));
         comm_count++;
       }
     }
   }
   else{
      while(Serial.available()){
        Serial.read();
        delayMicroseconds(500);
      }
      if(!rdy)time_load=millis();
   }
 }
   
 if(!Serial.available()) {
 
   if(comm_status==1 && micros()-time_delay>MAX_DELAY){
     comm_status = 2;
     Serial.end();
   }
 }

}


The format of the commands is $|throttle|yaw|roll|pitch|, '#' is used as an "Arm" command and '!' as a "Disarm" command.

Please tell me if you see something that is not a good practice or something that does not look correct to you. 

This function is called in the loop. It still freezes even if this is the only function in the code, so I think that this is what causes he problem.
2

I would use unsigned long for this...

 long time_delay;
...
time_delay=micros();

a long can only code up to 2,147,483,647 micro-seconds and then turns negative --> as you use micros() that will happen after ~2147 seconds which is 35 minutes and a few seconds.

When you debug, where do you send the the output if the ESP is connected on the other side?

        if(DEBUG_COMM){
            Serial.print(thro);
            Serial.print("\t");
            Serial.print(yaw);
            Serial.print("\t");
            Serial.print(pitch);
            Serial.print("\t");
            Serial.println(roll);
        }

do you disconnect it? do you send manually perfect entries? in case of wrong reception from the ESP, your input might be somewhat garbaged... are you trying to send a crapy input and see what happens ---> you need a robust parser.

for example, In your

 while(Serial.peek()>='0' && Serial.peek()<='9'){
   ...
}

you have multiple if where you actually perform a Serial.read() depending on comm_count but if for some reason comm_count becomes 16 and you still have a digit in the input then you are toast and stuck in your while loop. this is not very robust.

Note that the pow() function is slow when you doroll_tmp+=(Serial.read()-'0')*pow(10,(3-(comm_count-8))); it would be easier to do roll_tmp *=10; roll_tmp += Serial.read()-'0';this way when you receive 123 you get

roll_tmp = 0*10; roll_tmp = 0 + 1;
roll_tmp = 1*10; roll_tmp = 10 + 2;
roll_tmp = 12*10; roll_tmp = 120 + 3;

--> you end up with roll_tmp at 123 which is what you want. Slowing down emptying the Serial buffer is not a good idea especially if you have a weak parser as the ESP could saturate your input buffer and overwrite some meaningful data and then you get a crappy input to your parser.

--> I see you have a start symbol ($;#,!), I would suggest you add a termination symbol as well and read a full line into a buffer rather than doing byte by byte analysis (that would allow your loop to run too, which might be a good idea if you need other things to be done instead of waiting for Serial). --> Study Serial Input Basics to see how to handle Serial input well.

Also note that using WiFi might not be a great idea in terms of distance you can control your tricopter...

3

Thank you for the reply!

I will make some of the suggested changes and see what the results will be.

I will change "time_delay" to be of type unsigned long.

I will change this loop:

while(Serial.peek()>='0' && Serial.peek()<='9'){
   ...
}

so that the charecter is read in the begining of each step and the loop breaks if "comm_count">=16. In that way any possible situation where the comm_count is >=16 but there are still charecters in the buffer should be eliminated.

Also I will replace the "pow()" function with the equation you suggested.

With regard to your question about where does the output go when i debug. The output from the debuging (thro,yaw,pitch,roll values ) goes to the ESP and to the Arduino's USB to Serial converter since the ESP is connected to the hardware Serial (pins 0,1) of the Arduino. In that way I can see the debug output in the serial monitor. I know this isn't a very correct way to use the hardware Serial and it can cause problems but I use debuging in that way very rarely and most of the time it is turned off. Do you think this is causing any trouble with the curent program?

About the terminating symbol, this will be a little harder to implement, but I will have that in mind.

Also this tricopter is not realy intended to be used at longer ranges than 10-20 meters, it is more a starter/learner one, so i think WiFi range will be enough.

Thank you for your advice, I will post the results as soon as I can!

4

Are the motors running during this? I'd be far more suspicious of EMI if they are.

5

@MarkT they are not running, but turning them on doesn't affect the results.

6

Ok, so I changed the code. Now it seems to work fine. It has been working for about 2 days.

#define MAX_COMM_ERRORS 140
#define MAX_DELAY 140000
#define DEBUG_COMM 0

int comm_count, thro=1000, yaw=1500, pitch=1500, roll=1500, thro_tmp, yaw_tmp, pitch_tmp, roll_tmp;
int comm_status = 0; // 0 - disconnected, 1 - connected, 2 - lost connection
int comm_errors = 0;
long time_delay = 0, time_load=0,loop_timer=0;
bool blink=true;
bool rdy=false;




void setup() {
  // put your setup code here, to run once:
  //Serial.begin(115200);
  Serial.begin(115200);
  pinMode(13,OUTPUT);
  pinMode(4,OUTPUT);
  loop_timer = micros();
}

void loop() {

  
  if(comm_status==1){
    digitalWrite(4,1);

  }
  else{
    digitalWrite(4,0);
  }
  
  getCommands();

 
}

void getCommands() {
  char c;
  
  while (Serial.available()) {
    c=Serial.read();
    if (c == '

Thank you for the advice J-M-L !
) {
      rdy=true;
      //Serial.println(micros()-time_delay);
      time_delay=micros();
     
      if(thro_tmp==0 || yaw_tmp==0 || pitch_tmp==0 || roll_tmp==0){
        thro=1000;
        yaw=1500;
        pitch=1500;
        roll=1500;
      }
      else{ 
        //thro = constrain((0.5thro+0.5(thro_tmp+101)),1000,2000);
        thro=thro_tmp;
        yaw = constrain(0.7yaw+0.3(yaw_tmp),1000,2000);
        pitch = constrain(0.7pitch+0.3(pitch_tmp),1000,2000);
        roll = constrain(0.7roll+0.3(roll_tmp),1000,2000);
        if(yaw>=1492 && yaw<=1508){
          yaw=1500;
        }
        if(pitch>=1492 && pitch<=1508){
          pitch=1500;
        }
        if(roll>=1492 && roll<=1508){
          roll=1500;
        }
 
 
        if(DEBUG_COMM){
            Serial.print(thro);
            Serial.print("\t");
            Serial.print(yaw);
            Serial.print("\t");
            Serial.print(pitch);
            Serial.print("\t");
            Serial.println(roll);
        }
      }
      thro_tmp=0;
      yaw_tmp=0;
      pitch_tmp=0;
      roll_tmp=0;
      comm_count = 0;
      comm_errors = 0;
    }
    else if (c == '#') {
      comm_status = 1;
      comm_errors=0;
      time_delay=micros();
    }
    else if (c == '!') {
      comm_status = 0;
      comm_errors=0;
    }
   
    else if(c>='0' && c<='9') {
       
        if (comm_count < 4) {
          thro_tmp *=10;
          thro_tmp += c-'0';
          comm_count++;
        }
        else if(comm_count>=4 && comm_count<8){
          yaw_tmp *=10;
          yaw_tmp += c-'0';
          comm_count++;
        }
        else if(comm_count>=8 && comm_count<12){
          roll_tmp *=10;
          roll_tmp += c-'0';
          comm_count++;
        }
        else if(comm_count>=12 && comm_count<16){
          pitch_tmp *=10;
          pitch_tmp += c-'0';
          comm_count++;
        }
     
    }
    else{
      while(Serial.available()){
        Serial.read();
        delayMicroseconds(500);
      }
      if(!rdy)time_load=millis();
    }
  }
   
  if(!Serial.available()) {

if(comm_status==1 && micros()-time_delay>MAX_DELAY){
      comm_status = 2;
    }
  }

}


Thank you for the advice J-M-L !
7

Great news! (You still need to use unsigned long for timing)

That code

while(Serial.available()){
         Serial.read();
         delayMicroseconds(500);
       }

does not guarantee you will fully empty the Serial buffer so it’s un-necessary, you are better off ignoring input until you receive the start character (and in case something weird happens you might be getting rid of useful input)

Also using any delay() is a bad idea for overall code performance / user exeperience, let the loop() run its course and deal with the flow of data when it arrives, ignoring what needs to be ignored

8

I was using unsigned long, I think something messed up when I copied the code here :o

I will try to change the part of code you mentioned and see what happens.

9

#define MAX_DELAY 140000

Good practice to add L or UL if the value is out of int range. Sometimes when doing math with #define-ed constants you will get results you werent expecting.

Gone through this frustration once.

10

Thank you ! I will make sure I fix that.