Arduino Due Serial Hangs

[Jorgie]

I am sending a 14 byte message from the PC to the Arduino due every 1.4 seconds.
After a random period of time the Arduino stops sending a response back.
Only happened twice so far, first time after approx 5 hours and the second time after about 12 hours.

The message is being recieved and acted on, but

Code: [Select]

Serial.print("OK ");
is not resulting in any characters being received from the Due.

The code snippet is:

Code: [Select]


if(Serial.available() > 0)
  {
    number_of_bytes_received = Serial.readBytesUntil (ENDOFMSG,data,messageLength);

    result = mystrncmp (data, "IRRIG", 5);

    if (result == true)
    {
      started=true;
      [color=red]Serial.print("OK ");[/color]
      pumpStatus = digitalRead(PUMPSTATUSPIN);
      Serial.print (pumpStatus);

      messageRxd = millis();
      bank = int(data[BANKNOPOS]);
      bank = bank - 48; //convert from ascii to integer

      for ( int channelNo = 0 ; channelNo < MAXCHANNELS;channelNo++)
      {
        //Serial.print(data [CHANNELSPOS+channelNo]);
        if (data [CHANNELSPOS+channelNo]=='T'){
          digitalWrite(channelMap[bank][channelNo], HIGH);
        }
        else{
          digitalWrite(channelMap[bank][channelNo], LOW); 
        }
      }
    }
    else
    {
      Serial.print("POK");
      pumpStatus = digitalRead(PUMPSTATUSPIN);
      Serial.print (pumpStatus);
    }

  }

I'm able to send the command from the Arduino Serial monitor and my own code, which potentially rules out my software.

[MarkT]

As usual post all the code or we will be running around in circles...

[Jorgie]

Code: [Select]


unsigned long currentMillis = millis();
unsigned long messageRxd = millis();
const int MAXTIME = 6000; // Max allowed time between messages before we set everything off.

// Irrigation Message Format
// "IRRIG" + Bank, plus 8 channels = T = On or F = Off.
const int messageLength =  14;  
const int ENDOFMSG=13;//69; //="E"
char data [messageLength];

int number_of_bytes_received;

const int MAXCHANNELS = 8;
const int MAXBANKS = 5;

const int BANKNOPOS = 5;
const int CHANNELSPOS = 6;

boolean started = false;

int channelMap [MAXBANKS][MAXCHANNELS] = {
 {
   2,3,4,5,6,7,8,9                    }
 ,
 {
   10,11,12,13,22,23,24,25                    }
 ,
 {
   26,27,28,29,30,32,32,33                    }
 ,
 {
   34,35,36,37,38,39,40,41                    }
 ,
 {
   42,42,44,45,46,47,48,49                    }
};

const int PUMPSTATUSPIN = 49;


void setup(void)
{

 Serial.begin(57600);
 for ( int bankNo = 0 ; bankNo < MAXBANKS;bankNo++){
   for ( int channelNo = 0 ; channelNo < MAXCHANNELS;channelNo++)
   {
     pinMode(channelMap[bankNo][channelNo], OUTPUT);
     digitalWrite(channelMap[bankNo][channelNo], LOW);    
   }
 }
 pinMode(PUMPSTATUSPIN, INPUT_PULLUP);

}

void loop(void)
{
 int command ;       // This is the command char, in ascii form, sent from the serial port    
 unsigned long currentMillis = millis();
 int result;
 int bank;
 int pumpStatus = digitalRead(PUMPSTATUSPIN);


 for (int x=0 ;x<messageLength;x++){
   data[x] = 0;
 }

 if(Serial.available() > 0)
 {

   number_of_bytes_received = Serial.readBytesUntil (ENDOFMSG,data,messageLength);

   result = mystrncmp (data, "IRRIG", 5);

   if (result == true)
   {
     started=true;
     Serial.print("OK ");
     pumpStatus = digitalRead(PUMPSTATUSPIN);
     Serial.print (pumpStatus);

     messageRxd = millis();
     bank = int(data[BANKNOPOS]);
     bank = bank - 48; //convert from ascii to integer


     for ( int channelNo = 0 ; channelNo < MAXCHANNELS;channelNo++)
     {
       //Serial.print(data [CHANNELSPOS+channelNo]);
       if (data [CHANNELSPOS+channelNo]=='T'){
         digitalWrite(channelMap[bank][channelNo], HIGH);
       }
       else{
         digitalWrite(channelMap[bank][channelNo], LOW);  
       }
     }
   }
   else
   {
     Serial.print("POK");
     pumpStatus = digitalRead(PUMPSTATUSPIN);
     Serial.print (pumpStatus);
   }

 }
 if (started == true){
   if (currentMillis>MAXTIME ){
     
     if (currentMillis-MAXTIME > messageRxd ){

       for ( int bankNo = 0 ; bankNo < MAXBANKS;bankNo++){
         for ( int channelNo = 0 ; channelNo < MAXCHANNELS;channelNo++)
         {

           digitalWrite(channelMap[bankNo][channelNo], LOW);

         }
       }
       messageRxd = millis();
       Serial.print("NOK");
       pumpStatus = digitalRead(PUMPSTATUSPIN);
       Serial.print (pumpStatus);
     }


   }
 }
}

boolean mystrncmp (char item1[], char item2[], int numberToCompare){

 for (int n=0; n<numberToCompare; n++){

   if (item1[n] != item2[n]){
     return false;
   }
 }
 return true;
}

[MarkT]

What is this code supposed to do?

Code: [Select]


  if (started == true){
    if (currentMillis>MAXTIME ){
     
      if (currentMillis-MAXTIME > messageRxd ){



comparing currentMillis to MAXTIME like that won't work across wrap-around.  However
this won't bite till many days.

What's happening is that occasionally you are getting corruption on the serial line and
this means that the variable bank will have a wild value, so that the array access will
exceed its bounds and corrupt memory.  A simple test of just typing junk at the thing
causes it to latch up very readily.

Add a guard like this:

Code: [Select]

      bank = int(data[BANKNOPOS]);
      bank = bank - 48; //convert from ascii to integer
      if (bank >= 0 || bank <MAXBANKS)
      {
         ....
      }


Another bug is that the section of code guarded by the comparisons above will run
continuously once the first packet is received within the MAXTIME window, since you
don't test whether you've already run that code.

Again this is very easy to observe by just typing into the serial monitor.

[Jorgie]

Hi, thanks for the response. I like many did not know that the serial messages could be corrupted on one byte only.

Since you have pointed it out and "bank" was the only part of the message not being tested I'll make that change.

I've also made changes to ensure that each section of code only runs once, since then the code has been running for over 18 hours.

What is this code supposed to do?

Code: [Select]


  if (started == true){
    if (currentMillis>MAXTIME ){
     
      if (currentMillis-MAXTIME > messageRxd ){


comparing currentMillis to MAXTIME like that won't work across wrap-around.  However
this won't bite till many days.

I'm not quite sure of your meaning, if millis wraps around then currentMillis will, for a time less than MAXTIME, not be greater than MAXTIME and that part of the code will not be executed. Or is there something I've missed?
I agree that this section of code is probably not required

[MarkT]

currentMillis will be numerically small just after reset, but it will wrap-around and again
be small 49 days and 17 hours later.   (OK perhaps you're not worried about leaving
something running for 7 weeks, but in general this will happen).

Thus the test for reaching a set point in time requires:

The comparison between a difference of two timestamps and a given interval,
differences are always correct if the intervals are less than 7 weeks.

Also that after a succesful test for such a condition the test be reset for next time.

So its possible to make something happen once a day by code like:

Code: [Select]


#define DAY (24 * 60 * 60 * 1000)
int last_time = millis () ;

...

  if (millis () - last_time >= DAY)
  {
     ... do the thing here
     last_time += DAY ;  // reset the time so test is again valid
  }


However if you just want something to happen within the first day after reset and
never again you have to cancel the test so it doesn't succeed again 7 weeks later

Code: [Select]


boolean starting_up = true ;  // guard variable

...

  if (starting_up && millis () < START_PERIOD)
  {
     ... are in the start up period...
  }
  else if (millis () >= START_PERIOD)
    starting_up = false ; // cancel the test thereafter, so that wraparound doesn't retrigger