If condition not work properly...

I’m doing a climbing robot for my Robotics and Automation project…
So I using only 3 servos and 1 switch, the following is the programming code that I currently using.

The function of switch is, when the pinMode is HIGH, so the robot will climb forward on rope with 45 degree slanted, then when pinMode is LOW, the robot will go down back (reverse).

In this case I using ‘if’ as condition for HIGH and LOW, the first ‘if’ (HIGH) don’t have any problem with it, but proceed with second ‘if’ (LOW), the first condition is interrupting along with the second condition…

For example, when the switch is HIGH, it will proceed with 1st condition which the robot will move forward. But when I press switch to LOW, it proceed with 2nd condition which is reverse but mixing with the forward… which sometimes it move forward 2 times, then reverse 3 times, then forward 1 times… :slightly_frowning_face: :slightly_frowning_face:

Is there any problem with my program code? :blush: :blush:

#include <Servo.h>

Servo Front;
Servo Rear;
Servo Body;

const int ReverseSwitch = 12;

int angle = 0;
int buttonState = 0;

void setup()
{
  Front.attach(8);
  Rear.attach(9);
  Body.attach(10);

  pinMode(ReverseSwitch, INPUT);

  front_gripping();
  rear_gripping();
}

void loop()
{ 
  buttonState = digitalRead(ReverseSwitch);

  if(buttonState == HIGH){
  forward();}

  else if(buttonState == LOW){
  reverse();}
 
  else{}
}

void front_gripping(){
  for(angle = 50; angle > 0; angle--){                                
    Front.write(angle);}}

void front_release(){
  for(angle = 0; angle < 50; angle++){                                  
    Front.write(angle);}}

void rear_gripping(){
  for(angle = 50; angle > 0; angle--){                                
    Rear.write(angle);}}

void rear_release(){
  for(angle = 0; angle < 50; angle++){                                  
    Rear.write(angle);}}

void body_expand(){
  for(angle = 30; angle < 110; angle++){                                  
    Body.write(angle);}}

void body_retract(){
  for(angle = 110; angle > 30; angle--){                                
    Body.write(angle);}}

void forward(){
  front_release();
  delay(200);
  body_expand();
  delay(800);
  front_gripping();
  delay(500);
  rear_release();
  delay(200);
  body_retract();
  delay(800);
  rear_gripping();
  delay(500);
  front_gripping();}
  
void reverse(){
  rear_release();
  delay(300);
  body_expand();
  delay(800);
  rear_gripping();
  delay(500);
  front_release();
  delay(300);
  body_retract();
  delay(800);
  front_gripping();
  delay(500);}

Is the switch you are using On-Off-On ? Or just On-Off? What I mean is your robot always going up or down......... there is no resting option?

Regards,

Graham

Frst lets clean up that 'if' to be nice and simple:

  buttonState = digitalRead(ReverseSwitch);

  if(buttonState == HIGH){
  forward();}

  else if(buttonState == LOW){
  reverse();}
 
  else{}
  if (digitalRead (ReverseSwitch))
    forward () ;
  else
    reverse () ;

Note that digitalRead returns only LOW or HIGH, and that an empty else block is
pointless and redundant. The variable isn't needed, we just look at the pin and
choose one of two options. Now the code is easier to read and its clear what it
does.

Your issue must lie elsewhere - note that we don't have to worry about debouncing
here since both forward() and reverse() take a long time to complete.

Its not immediately clear what you are trying to do. You have a switch which
has only two states, but I think you are wanting three states - forward, reverse and
do-nothing. With one digital input you can't have three states...

I like the simple structure of your code. very obvious what is happening.

I think you need a short delay after every servo.write() rather than at the end of the call to the function that contains servo.write()

I'm not saying that would solve the problem you describe.

Is your switch some form of toggle switch (rather than a momentary switch)? If so I don't immediately see why your code does not work.

Can you temporarily insert a Serial.println(buttonState); and watch the result on the Serial Monitor.
And maybe put Serial.println("FWD"); at the top of the forward() function and equivalent for the reverse() function.

What happens if you temporarily add return; at the top of the forward() function so it does nothing.

...R

Do you have a pullup or pulldown resistor on the switch pin, to guarantee its state when it's not pressed?

Well, the first thing to say is that you have more code here than necessary

void loop()
{ 
  buttonState = digitalRead(ReverseSwitch);

  if(buttonState == HIGH){
    forward();
  }

  else if(buttonState == LOW){
    reverse();
  }

  else{
  }
}

buttonState can only be HIGH or LOW so you only need to test it once like this

void loop()
{ 
  buttonState = digitalRead(ReverseSwitch);

  if(buttonState == HIGH)
  {
    forward();
  }
  else
  {
    reverse();
  }
}

This will not cause the symptoms you describe but keeping things simple is always a good idea. How is the switch activated ? Could the contacts be bouncing and producing a series of false forward/reverse signals when it is pressed ?

Yep I was about to offer the simplified loop but I suspect it's not the problem. I do notice, however that your reversing switch is on pin 0. Since this is also used for Serial RX it's possible that there's some issue being introduced to your circuit here.

Also I notice you're not using the internal pullups. Are you perhaps leaving your input line floating when the switch is not in it's "on" position?

ghlawrence2000:
Is the switch you are using On-Off-On ? Or just On-Off? What I mean is your robot always going up or down......... there is no resting option?

Regards,

Graham

I using On-Off. I thought, by using On-Off with 'HIGH' and 'LOW", I can separate two operations... With default while switch in off, it will always going up, only when I press it on, it will move to other operation which is going down...

That's ok, I thought I would check. Then all the other answers are fine. I would just have expected you would want a rest option.

Graham

MarkT:
Frst lets clean up that 'if' to be nice and simple:

  buttonState = digitalRead(ReverseSwitch);

if(buttonState == HIGH){
 forward();}

else if(buttonState == LOW){
 reverse();}

else{}






if (digitalRead (ReverseSwitch))
   forward () ;
 else
   reverse () ;




Note that digitalRead returns only LOW or HIGH, and that an empty else block is
pointless and redundant. The variable isn't needed, we just look at the pin and
choose one of two options. Now the code is easier to read and its clear what it
does.

Your issue must lie elsewhere - note that we don't have to worry about debouncing
here since both forward() and reverse() take a long time to complete.

Its not immediately clear what you are trying to do. You have a switch which
has only two states, but I think you are wanting three states - forward, reverse and
do-nothing. With one digital input you can't have three states...

When mention about do-nothing, yup, I think I need one more state. But for 'do-nothing', should let it blank? I mean, forward() and reverse() having its own commands, for sure do_nothing() also need its own commands, is it delay() or any...?

To Forward - Stop - Reverse, you will need an ON-OFF-ON Switch, as I asked way back… effectively 2 switches in one… commonly called a centre off toggle…

Then your code will need to be changed to implement this…

Regards,

Graham

Robin2:
I like the simple structure of your code. very obvious what is happening.

I think you need a short delay after every servo.write() rather than at the end of the call to the function that contains servo.write()

I'm not saying that would solve the problem you describe.

Is your switch some form of toggle switch (rather than a momentary switch)? If so I don't immediately see why your code does not work.

Can you temporarily insert a Serial.println(buttonState); and watch the result on the Serial Monitor.
And maybe put Serial.println("FWD"); at the top of the forward() function and equivalent for the reverse() function.

What happens if you temporarily add return; at the top of the forward() function so it does nothing.

...R

I using On-Off switch... Yes, I've done with serial.println, with the current program... I try return; at the top of forward() seems the result is a little bit a same with the problem that I had, but when I put it below forward(); the pattern like this

RVS RVS RVS RVS RVS FWD (suddenly having one FWD, and RVS appear in 5 times simultaneously...) other than that, FWD is normal when I press the button... hahaa

JimboZA:
Do you have a pullup or pulldown resistor on the switch pin, to guarantee its state when it's not pressed?

I didn't use any resistor, I know it is quite terrible without using any resistor... xD

ezraff:
I try return; at the top of forward() seems the result is a little bit a same with the problem that I had, but when I put it below forward(); the pattern like this

RVS RVS RVS RVS RVS FWD (suddenly having one FWD, and RVS appear in 5 times simultaneously…) other than that, FWD is normal when I press the button… hahaa

This may be pointing us in the correct direction. But I just need to clarify …
When you say you put return at the top of forward() do you mean, like this (which is what I intended)

void forward(){
  return;
  front_release();
  d

If, not, please show me exactly what you did.

Also, I don’t understand what you mean by “when I put it below forward()” The only thing below forward() is reverse().

If the problem persists when you have the return as shown above then the entire problem lies in your reverse() function.

…R

UKHeliBob:
Well, the first thing to say is that you have more code here than necessary

I have no doubt that the original code can be tidied up.

But when was the last time a newcomer posted code that you could instantly understand ?

...R

JimboZA:
Do you have a pullup or pulldown resistor on the switch pin, to guarantee its state when it's not pressed?

Before this I didn't put any pullup or pulldown... after I tried inserting INPUT_PULLUP, the if condition seems working well... I refer it from http://arduino.cc/en/Tutorial/InputPullupSerial
Below I simplified the code, using println("FWD") and println("RVS") to differentiate with addition of digitalWrite just to indicate the LED whether the switch I pressed is on or off... :astonished: :astonished: :astonished: :astonished:

void setup(){
  Serial.begin(9600);
  pinMode(2, INPUT_PULLUP);
  pinMode(10, OUTPUT);
}

void loop(){
  int sensorVal = digitalRead(2);

  if (sensorVal == HIGH) {
    digitalWrite(10, HIGH);
    Serial.println("FWD");
  } 
  else {
    digitalWrite(10, LOW);
    Serial.println("RVS");
  }
}

So from that you're saying it's fixed? The row if :astonished: :astonished: :astonished: :astonished: :astonished: made we wonder if there's still a problem?

(You may have noticed that with the pullup enabled, the logic flow of the sketch is that sensorVal == HIGH is with the switch unpressed. This is known as "active low", ie active in the sense that the button is activated, pressed, makes the signal go low. The signal is usually high, due to the pullup.)

The purpose of the pullup is to make sure that when the switch isn't pressed, you can guarantee it's in the opposite, known state. Otherwise, a pin can pick up stray signals from who-knows-where, the Man-in-the-Moon maybe, and give false indications.