help with robot skid- steer code

i am trying to write a code for a rc controlled skid=steer robot but i am having issues with the safety side of the coding. the forword/backward and left/ right works well but when the robot is powered up and i turn the rc transmitter off the microcontroller tells the motor controller to run at full speed. i can not seem to change that. any ideas for a safer way? this robot is over 200 lbs and is an lod powered wheel chair. it can easily push a car. any ideas on how to change the code so that it safer if it loses signal from the rc, that way it just cant go crazy? or how to totally over haul the skid-steer code? my hardware is: chipkit max32, (2) pololu simple motor controlers(serial ttl), flysky fs=ct6b rc

unsigned long counter = 0;
unsigned long Channel1Value;
unsigned long Channel2Value;
unsigned long lastgood1;
unsigned long lastgood2;
unsigned long InitialSteer;
unsigned long InitialThrottle;
boolean Right;
//motor stuff
long Steer;
int Thrust;
int RightMotor;
int LeftMotor;
int Chan1 = 11;
int Chan2 = 10;
int newVal1;
int newVal2;
#define MotorLimit 3200
int sensor = A0;
int dist;
/**************************************************************
 * Subroutine to exit saft start;
 ***************************************************************/
void exitSafeStartBoth()  
{  
  Serial3.print(0x83, BYTE);  
}  

/**************************************************************
 * Subroutine to control right motor= motorRight(speed, direction);
 ***************************************************************/
void setMotorSpeedleft(int speedleft)
{
  if (speedleft < 0)
  {
    Serial3.print(0xAA, BYTE);
    Serial3.print(0xa, BYTE);
    Serial3.print(0x6, BYTE); // motor reverse command
    speedleft = -speedleft; // make speed positive
  }
  else
  {
    Serial3.print(0xAA, BYTE);
    Serial3.print(0xa, BYTE);  
    Serial3.print(0x5, BYTE); // motor forward command
  }
  Serial3.print((unsigned char)(speedleft & 0x1F), BYTE);
  Serial3.print((unsigned char)(speedleft >> 5), BYTE);
}

/**************************************************************
 * Subroutine to control left motor
 ***************************************************************/
void setMotorSpeedright(int speedright)
{
  if (speedright < 0)
  {
    Serial3.print(0xAA, BYTE);
    Serial3.print(0xd, BYTE);
    Serial3.print(0x06, BYTE); // motor reverse command
    speedright = -speedright; // make speed positive
  }
  else
  {
    Serial3.print(0xAA, BYTE);
    Serial3.print(0xd, BYTE);  
    Serial3.print(0x05, BYTE); // motor forward command
  }
  Serial3.print((unsigned char)(speedright & 0x1F), BYTE);
  Serial3.print((unsigned char)(speedright >> 5), BYTE);
}



void setup()
{
  Serial3.begin(19200);
  Serial.begin(19200);
  Serial.println("Ready");
  pinMode (Chan2, INPUT); // connect Rx channel 2 to PD4, which is labled "D4" on the Arduino board
  pinMode (Chan1, INPUT); //  connect Rx channel 3 to PD5, which is labled "D5" on the Arduino board
  pinMode (sensor, INPUT);
  InitialSteer = pulseIn (Chan1, HIGH); //read RC channel 1
  lastgood1 = InitialSteer;
  InitialThrottle = pulseIn (Chan2, HIGH); //read RC channel 2
  lastgood2 = InitialThrottle; 
}

void loop()
{
  dist = analogRead(sensor);
  //  counter++;  // this just increments a counter for benchmarking the impact of the pulseIn's on CPU performance
  newVal1 = pulseIn(Chan1, HIGH, 50000);
  newVal2 = pulseIn(Chan2, HIGH, 35000);
  if(newVal1 >= Channel1Value ||
    newVal1 <= Channel1Value)
    Channel1Value = newVal1;

  if(newVal2 >= Channel2Value ||
    newVal2 <= Channel2Value)
    Channel2Value = newVal2; //read RC channel 2

    if (Channel1Value == 0) {
    Channel1Value = lastgood1;
  } 
  else {
    lastgood1 = Channel1Value;
  }

  if (Channel1Value < InitialSteer) {
    Right = false;
  } 
  else {
    Right = true;
  }

  if (Right) {
    Steer = Channel1Value - InitialSteer;
  } 
  else {
    Steer = InitialSteer - Channel1Value;
  }
  Steer = constrain (Steer, -MotorLimit, MotorLimit); //just in case

  //Thrust = Channel2Value - InitialThrottle;
  Thrust = map(Channel2Value, 3645, 5565, -(MotorLimit), MotorLimit); 
  Thrust = constrain (Thrust, -(MotorLimit), MotorLimit); //just in case

  Serial.print ("Channel 1: ");  // if you turn on your serial monitor you can see the readings.
  Serial.println (Channel1Value);
  Serial.print ("Channel 1: ");  // if you turn on your serial monitor you can see the readings.
  Serial.println (Channel1Value);
  Serial.print("Channel 2: ");
  Serial.println (Channel2Value);
  Serial.println(LeftMotor);
  Serial.println(RightMotor);
  Serial.println(dist);
  Serial.println("");
  Serial.println("");
  Serial.println("");
  Serial.println("");



  if (Right == true) // turn right 
  {
    RightMotor = Thrust - Steer; // reduce power to the motor in the direction you want to go 
    // if (RightMotor < 0) RightMotor = 0; //mike changed this so that they will run backwards
    LeftMotor = Thrust + Steer; // increase power to the motor opposite the direction you want to go
    if (LeftMotor > 3200) LeftMotor = 3200;
  }
  if (Right == false) //turn left
  {
    LeftMotor = Thrust - Steer; // reduce power to the motor in the direction you want to go 
    // if (LeftMotor < 0) LeftMotor = 0; //mike changed this so that they will run backwards
    RightMotor = Thrust + Steer; // increase power to the motor opposite the direction you want to go
    if (RightMotor > 3200) RightMotor = 3200;
  }

  if (Channel1Value <= 0) {
    LeftMotor = 0;
    RightMotor = 0; 
  }

  if (Channel1Value > 3600) { 
    exitSafeStartBoth();
  }

  LeftMotor = constrain (LeftMotor, -3200, 3200);
  RightMotor = constrain (RightMotor, -3200, 3200);
  setMotorSpeedleft(LeftMotor);
  setMotorSpeedright(RightMotor);
  delay(50);
}

and i turn the rc transmitter off the microcontroller tells the motor controller to run at full speed

I assume there is a rc receiver tied to the micro that receives transmissions from the transmitter?

If this receiver doesn't have any safety mechanism to detect loss of signal, then chances are it's just sending noise to the micro, and there isn't much you're going to be able to do about that. The safety really needs to be in the receiver. At best, it'll be extremely difficult to detect loss of signal in the micro, and likely impossible to do it reliably. And with a 200lb robot, it NEEDS to be reliable.

yes i have the six Chanel receiver plugged into the mirco. the output of the receiver drops too 0 when the transmitter is turned off.

...and this is why I always reccommend that one thinks about and designs the safety systems of such large robots -first- before anything else is designed or built. It should -never- be an afterthought.

Ok - do you have a spare channel on that 6-channel receiver? If not, better get a larger kit; use that spare channel to send a steady output to the receiver, and read that with the Arduino. If it varies by a certain amount for more than a second, shutdown - no if's, and's, or but's.

Also - have the Arduino check for a button press on a digital input - hook up a large, red, flashing button to this, mount it to an easy-to-access spot on the robot, with plenty of caution tape and "emergency stop" signage.

Finally - add an emergency strobe flasher to the robot, to warn others.

Note - this isn't a perfect solution; ideally, you'd want the receiver to be able to output its reception power level value, and key off of that for emergency shutdown (in addition to the side-channel heartbeat, I might add). Maybe there are r/c control kits out there that allow for that, or maybe there is a way to hack your kit if it doesn't have it - I don't know. I'd look into it, though.

Also - you might want to investigate a separate 900 MHz or 2.4 GHz remote key-fob automotive relay transmitter/receiver kit; hit the button for emergency stop (wire the relay to a larger relay or something that cuts the main power bus).

this part of the code i cant seem to get to fuction

if (Channel1Value <= 0) {
    LeftMotor = 0;
    RightMotor = 0; 
  }

when the transmitter is off the receiver outputs a zero.

How about a diode OR of all the rx channels driving a hardware watchdog?
Any high will keep the watchdog kicked.

  delay(50);

Ordinary R/C frames are 20ms long - that isn't going to do any thing for your responsiveness, I think.

int Chan1 = 11;

It is a constant, isn't it?
Why not make it one?

Why is "dist" a global?
Why are "LeftMotor" and "RightMotor" globals?

  newVal1 = pulseIn(Chan1, HIGH, 50000);
  newVal2 = pulseIn(Chan2, HIGH, 35000);
  if(newVal1 >= Channel1Value ||
    newVal1 <= Channel1Value)
    Channel1Value = newVal1;

  if(newVal2 >= Channel2Value ||
    newVal2 <= Channel2Value)
    Channel2Value = newVal2; //read RC channel 2

    if (Channel1Value == 0) {
    Channel1Value = lastgood1;
  } 
  else {
    lastgood1 = Channel1Value;
  }

Your indenting leaves a lot to be desired. The IDE includes a tool to fix this for you. Look in the Tools menu, and try out the Auto Format tool.

Consistent use of { and } after if statements, especially compound if statements, and putting the { on a new line, would improve the readability of your code, too.

  if (Right == true) // turn right 
  {
    RightMotor = Thrust - Steer; // reduce power to the motor in the direction you want to go 
    // if (RightMotor < 0) RightMotor = 0; //mike changed this so that they will run backwards
    LeftMotor = Thrust + Steer; // increase power to the motor opposite the direction you want to go
    if (LeftMotor > 3200) LeftMotor = 3200;
  }
  if (Right == false) //turn left
  {
    LeftMotor = Thrust - Steer; // reduce power to the motor in the direction you want to go 
    // if (LeftMotor < 0) LeftMotor = 0; //mike changed this so that they will run backwards
    RightMotor = Thrust + Steer; // increase power to the motor opposite the direction you want to go
    if (RightMotor > 3200) RightMotor = 3200;
  }

Are there any circumstances where both blocks of code are going to be executed? If Right is not true, it must be false, right? So, no need for a second if test. The second if statement should be replaced with else.

  if (speedleft < 0)
  {
    Serial3.print(0xAA, BYTE);
    Serial3.print(0xa, BYTE);
    Serial3.print(0x6, BYTE); // motor reverse command
    speedleft = -speedleft; // make speed positive
  }
  else
  {
    Serial3.print(0xAA, BYTE);
    Serial3.print(0xa, BYTE);  
    Serial3.print(0x5, BYTE); // motor forward command
  }

Move the duplicate code before the if test.

if (Right == true) // turn right 
  {
    RightMotor = Thrust - Steer; // reduce power to the motor in the direction you want to go 
    // if (RightMotor < 0) RightMotor = 0; //mike changed this so that they will run backwards
    LeftMotor = Thrust + Steer; // increase power to the motor opposite the direction you want to go
    if (LeftMotor > 3200) LeftMotor = 3200;
  }
  if (Right == false) //turn left

"else" is a useful construct for booleans.

Actually, the majority of those if statements aren't even necessary. The following code:

  if (Right) {
    Steer = Channel1Value - InitialSteer;
  } 
  else {
    Steer = InitialSteer - Channel1Value;
  }
  
   if (Right == true) // turn right 
  {
    RightMotor = Thrust - Steer; // reduce power to the motor in the direction you want to go 
    // if (RightMotor < 0) RightMotor = 0; //mike changed this so that they will run backwards
    LeftMotor = Thrust + Steer; // increase power to the motor opposite the direction you want to go
    if (LeftMotor > 3200) LeftMotor = 3200;
  }
  if (Right == false) //turn left
  {
    LeftMotor = Thrust - Steer; // reduce power to the motor in the direction you want to go 
    // if (LeftMotor < 0) LeftMotor = 0; //mike changed this so that they will run backwards
    RightMotor = Thrust + Steer; // increase power to the motor opposite the direction you want to go
    if (RightMotor > 3200) RightMotor = 3200;
  }

Can be reduced to:

  Steer = Channel1Value - InitialSteer;
  RightMotor = Thrust - Steer;
  LeftMotor = Thrust + Steer;

  if (RightMotor > 3200) RightMotor = 3200;
  if (LeftMotor > 3200) LeftMotor = 3200;

And why are you doing all this:

 newVal1 = pulseIn(Chan1, HIGH, 50000);
  newVal2 = pulseIn(Chan2, HIGH, 35000);
  if(newVal1 >= Channel1Value ||
    newVal1 <= Channel1Value)
    Channel1Value = newVal1;

  if(newVal2 >= Channel2Value ||
    newVal2 <= Channel2Value)
    Channel2Value = newVal2; //read RC channel 2

    if (Channel1Value == 0) {
    Channel1Value = lastgood1;
  } 
  else {
    lastgood1 = Channel1Value;
  }

Instead of just this:

  Channel1Value = pulseIn(Chan1, HIGH, 50000);
  Channel2Value = pulseIn(Chan2, HIGH, 35000);

I see no reason for those intermediate checks.

My first thought was "wow!"

:slight_smile:

Polou make a couple of RC switches, the one I'm using for my aircraft landing and navigation lights has 2 outputs.

One output ONLY comes up after it can "see" a valid control frame, the other when the controlling channel becomes active.
The first one I'm using to turn on my nav and collision lights, the second I'm using for the main landing lights.

Point I'm making here is all these software ideas are great, include them!
But from someone that has been hurt more times than I care to think about "on the job", you need a failsafe!

With the the Polou RC switch use the valid frame AND your landing gear or flap switch.
Using both a hardware and software solution together will be safer.

Believe me when I tell you, getting clobbered by a heavy piece of equipment is not the fun time mentioned in the brochures!
:slight_smile:

And why are you doing all this:
Code:
newVal1 = pulseIn(Chan1, HIGH, 50000);
newVal2 = pulseIn(Chan2, HIGH, 35000);
if(newVal1 >= Channel1Value ||
newVal1 <= Channel1Value)
Channel1Value = newVal1;

if(newVal2 >= Channel2Value ||
newVal2 <= Channel2Value)
Channel2Value = newVal2; //read RC channel 2

if (Channel1Value == 0) {
Channel1Value = lastgood1;
}
else {
lastgood1 = Channel1Value;
}

Instead of just this:
Code:
Channel1Value = pulseIn(Chan1, HIGH, 50000);
Channel2Value = pulseIn(Chan2, HIGH, 35000);

I see no reason for those intermediate checks.

I use this code because often the micro returns a zero from the receiver while the receiver is still on.

AWOL:

int Chan1 = 11;

It is a constant, isn't it?
Why not make it one?

Why is "dist" a global?
Why are "LeftMotor" and "RightMotor" globals?

what's is the better way to write it? As you all can tell i'm not that good with writing code, but I'm very glad for all that awesome help that I am receiving :slight_smile:

PaulS:

  newVal1 = pulseIn(Chan1, HIGH, 50000);

newVal2 = pulseIn(Chan2, HIGH, 35000);
 if(newVal1 >= Channel1Value ||
   newVal1 <= Channel1Value)
   Channel1Value = newVal1;

if(newVal2 >= Channel2Value ||
   newVal2 <= Channel2Value)
   Channel2Value = newVal2; //read RC channel 2

if (Channel1Value == 0) {
   Channel1Value = lastgood1;
 }
 else {
   lastgood1 = Channel1Value;
 }



Your indenting leaves a lot to be desired. The IDE includes a tool to fix this for you. Look in the Tools menu, and try out the Auto Format tool.

Consistent use of { and } after if statements, especially compound if statements, and putting the { on a new line, would improve the readability of your code, too.

is this what you were talking about?

newVal1 = pulseIn(Chan1, HIGH, 50000);
  newVal2 = pulseIn(Chan2, HIGH, 35000);
  if(newVal1 >= Channel1Value || newVal1 <= Channel1Value)
  {
    Channel1Value = newVal1;
  }
  if(newVal2 >= Channel2Value || newVal2 <= Channel2Value)
  {
    Channel2Value = newVal2; //read RC channel 2
  }
  if (Channel1Value == 0)
  {
    Channel1Value = lastgood1;
  } 
  else 
  {
    lastgood1 = Channel1Value;
  }

  if (Channel1Value < InitialSteer) 
  {
    Right = false;
  } 
  else
  {
    Right = true;
  }

PaulS, I'm not sure what you mean here

Code:
if (speedleft < 0)
{
Serial3.print(0xAA, BYTE);
Serial3.print(0xa, BYTE);
Serial3.print(0x6, BYTE); // motor reverse command
speedleft = -speedleft; // make speed positive
}
else
{
Serial3.print(0xAA, BYTE);
Serial3.print(0xa, BYTE);
Serial3.print(0x5, BYTE); // motor forward command
}
Move the duplicate code before the if test.

I had an idea, or some one said so, but what if i changed the code to

{
Void Loop
{
  if (newVal1 > 0 || newVal2 > 0)
  {
    //  counter++;  // this just increments a counter for benchmarking the impact of the pulseIn's on CPU performance
    newVal1 = pulseIn(Chan1, HIGH, 50000);
    newVal2 = pulseIn(Chan2, HIGH, 35000);
    if(newVal1 >= Channel1Value || newVal1 <= Channel1Value)
    {
      Channel1Value = newVal1;
    }
    if(newVal2 >= Channel2Value || newVal2 <= Channel2Value)
    {
      Channel2Value = newVal2; //read RC channel 2
    }
    if (Channel1Value == 0)
    {
      Channel1Value = lastgood1;
    } 
    else 
    {
      lastgood1 = Channel1Value;
    }

    if (Channel1Value < InitialSteer) 
    {
      Right = false;
    } 
    else
    {
      Right = true;
    }
    Steer = Channel1Value - InitialSteer;
    RightMotor = Thrust - Steer;
    LeftMotor = Thrust + Steer;

    if (RightMotor > 3200) 
    {
      RightMotor = 3200;
    }
    if (LeftMotor > 3200) 
    {
      LeftMotor = 3200;
    }
    Thrust = map(Channel2Value, 3645, 5565, -(MotorLimit), MotorLimit); 
    Thrust = constrain (Thrust, -(MotorLimit), MotorLimit); //just in case

    LeftMotor = constrain (LeftMotor, -3200, 3200);
    RightMotor = constrain (RightMotor, -3200, 3200);
    setMotorSpeedleft(LeftMotor);
    setMotorSpeedright(RightMotor);
    Serial.print ("Channel 1: ");  // if you turn on your serial monitor you can see the readings.
    Serial.println (Channel1Value);
    Serial.print("Channel 2: ");
    Serial.println (Channel2Value);
    Serial.println(LeftMotor);
    Serial.println(RightMotor);
    Serial.println("");
    Serial.println("");
    Serial.println("");
    Serial.println("");

    delay(50);
  }
else 
{
motorStopBoth();
}

}

this post made me laugh due to the robot going into berskerker mode when you turn off the controller haha

you could add a short pulse to one of the transmitted signals every x amount of ms and then filter this from the input

then just check if any of your inputs have gone high in like 500 ms and if not turn that beast off since someone could be getting run over haha

Haha, yeah there was a time when one of my tx wires came loose and it went full speed of a tree. it shook the tree when it hit.

is this what you were talking about?

Yes. Some white space would help, too.

I'm not sure what you mean here

You have code in the if speedLeft is negative block that is identical to the code in the speedLeft is positive block. That code should be removed from the if blocks, and put before the if/else statement. What happens with the code now, if you discover an error in the common code? It needs to be fixed in two places (the if block and the else block). That situation leads to mistakes, when a code change is made in one place but not the other.