Turning off motors if Serial Communication fails

I am in the process of building an ROV and would like to add a safety feature to the code. If the the onboard Arduino stops getting commands over serial I would like to shut if off after 1 second or so of no communication with the surface. I have tried a few things but really am lost here at keeeping track of elapsed time. Here is what I have so far, it does not work, it only reduces motor speed to near zero.

if (Serial.available ()){
    processInput ();

  }
  else 
  {
     current = millis();
  
  if (current-last > interval)
  {
  last = current;
  digitalWrite(M1A, LOW);
  digitalWrite(M1B, LOW);
  }
  }

Last needs to be updated when you get a character as this is the last time a clear was received.

And also it should not updated when the motors are turned off. (last=current). You want them to stay off, right? :slight_smile:

I should have more clearly said it should only be updated when you get a char. Basically needs to be moved to the if not the else part.

Heres what I have so far:

if (Serial.available ())

  {

    processInput ();

    last = millis();

  }

  else 
  {

    current = millis();

    if (current-last > interval)

    {
      digitalWrite(M1A, LOW);
      digitalWrite(M1B, LOW);
    }

It still will not shut off the motor, what am I missing?

This works but not without the delay, it only slows the motor without the delay

  if (Serial.available ())

  {

    processInput ();

    last = millis();

  }

  else 
  {
    
    current = millis();

    if (current-last > interval)

    {
      digitalWrite(M1B, LOW);
      delay(1000);
    }

Here is the whole code:

//-------------------------------------------------------------------

#include <Servo.h> 
Servo servo; 

//PINS
const int  M1A = 4; // M1---M4 ARE HORIZONTAL THRUSTERS
const int  M1B = 3;


//INCOMING DATA
const char SOP = '<';
const char EOP = '>';
enum { 
  Case, Speed };


//VARIABLES
int whichNumber = Case;
int Casev, Speedv;
int pwmright;            //1
int pwmleft;             //2

unsigned long last;
long interval = 500;
unsigned long current;



//-------------------------------------------------------------------


void setup ()
{ 
  Serial.begin (9600);
  pinMode (M1A, OUTPUT);
  pinMode (M1B, OUTPUT);


} 

//------------------------------------------------------------------- 


void processNumber (int x)
{

  switch (whichNumber)
  {

  case Case: 
    Casev = x;
    whichNumber = Speed;
    break;

  case Speed: 
    Speedv = x;
    whichNumber = Case;
    break;


  }
}  


//------------------------------------------------------------------- 

void processInput ()
{
  static int receivedNumber = 0; //float
  static boolean False = false;
  byte c = Serial.read ();    //byte

  switch (c)
  {

  case EOP:  
    if (False) 
      processNumber (- receivedNumber); 
    else
      processNumber (receivedNumber); 

    // fall through to start a new number
  case SOP: 
    receivedNumber = 0; 
    False = false;
    break;

  case '0' ... '9': 
    receivedNumber *= 10;           // receivedNumber = recievedNumber*10 increments decimal places
    receivedNumber += c -'0';      //receivedNumber = recievedNumber + (c - '0') goes from char to decimal         
    break;

  case '-':
    False = true;
    break;

  } 
}  


//-------------------------------------------------------------------

void loop ()
{

  // Check sensors and print values (temperature, depth, accelorometer)


  // Process input, turn off if no data available

  if (Serial.available ())

  {

    processInput ();

    last = millis();

  }

  else 
  {
    
    current = millis();

    if (current-last > interval)

    {
      digitalWrite(M1B, LOW);
      delay(1000);
    }



    // Test order of recieved commands
    int var1;
    int var2; 

    if (Casev < 13) {
      var1 = Casev;
      var2 = Speedv;
    }
    else {
      var1 = Speedv;
      var2 = Casev;
    }


    switch (var1)
    {

      //RIGHT  
    case 1:
      pwmright = var2;
      moveright();
      break;

      //LEFT
    case 2:
      pwmleft = var2;
      moveleft();
      break;

    case 3:
      digitalWrite(M1A, LOW);
      digitalWrite(M1B, LOW);

    } 
  }
}
//-------------------------------------------------------------------1 

void moveright()
{
  digitalWrite(M1A, HIGH);
  analogWrite(M1B, pwmright);
}

//------------------------------------------------------------------- 2

void moveleft() 
{
  digitalWrite(M1A, LOW);
  analogWrite(M1B, pwmleft);
}

Any way to not use delay?

GOT it, thanks guys

I put the switch inside the if statement and the else after, works like a charm

Which means it might be even better to put it in processInput().

So then you could write loop() very cleanly as

void loop() 
{
  int now=millis();

  if (Serial.available())         // Serial Received "Event"
  { 
    processInput();
    last=now;  // reset serial timer
  }

  if ( now - last > interval)       // Serial Timer Expired "Event"
  {
    digitalWrite( M1B, LOW);  // turn off motors
  }
  
}

Note that "else" is not actually necessary.

Grabbing now=millis() at the start of the loop is a often handy.

Better variable names might include:
last ==> lastSerialProcessed or serialTimerStarted (or comm instead of serial)
interval ==> serialTimeout or serialTimeOutInterval

Cheers,
John

OH! And I forgot to mention what got me thinking about variable names....

This is priceless!

  case '-':
    False = true;
    break;

To be honest, I grabbed that part of the code from somewhere on the interwebs and tweaked it to make it work for me. I do not fully understand it, any simple way to explain to me how that part of the code works?

Also, note taken about better variable names

mech_eng:
To be honest, I grabbed that part of the code from somewhere on the interwebs and tweaked it to make it work for me. I do not fully understand it, any simple way to explain to me how that part of the code works?

It looks like it is reading command data (left, right, speed) from Serial in a format like <123><2><-23><1><345>. What type of device or program is sending to it?

John