else statement is not working !

hello guys,
i have an else statement in my codes that is not working !
what should happen is this : i have 3 potentiometers, but the one will be needed in this post are 2, one for throttle and one for brakes, and a push button to reverse, so when the push button state is LOW the reverse is off and when the push button state is HIGH the reverse is on but the else statement is not working for that to happen so any idea why ?
i tried else and else if and if alone but nothing nevertheless the debugging is giving a good state for the button and it indicates that it works (the statement i am talking about is indicated with ////)
here are the codes :

//Transmitter
#include <VirtualWire.h>

char Array[10];
int potPin1 = A0;    // Throttle
int potPin2 = A1;    // Brakes
int potPin3 = A2;    //Streering 

//these values will be sent by the transmitter
int valThrottle = 0;  
int valBrakes = 0;
int valSteering = 0;

int transmitterPin = 10; //transmitter pin

int downShift = 2; //Left paddle shift pin 2
int upShift = 3; //Right paddle shift pin 3
int burnout = 5; //Burnout Button pin 5
int neutral = 6; //Neutral Green Button pin 6
int reverse = 7; //Reverse Button pin 7
int limiter = 8; //Pit Limiter Button pin 8
//int cam = 9; //Cam grey Button pin 9
//pin 4 is the white button

//downShift
boolean lastButton1 = LOW; 
boolean currentButton1 = LOW; 

//upShift
boolean lastButton2 = LOW; 
boolean currentButton2 = LOW; 

//burnout
boolean lastButton4 = LOW; 
boolean currentButton4 = LOW;

//neutral
boolean lastButton5 = LOW; 
boolean currentButton5 = LOW; 

//reverse
boolean lastButton6 = LOW; 
boolean currentButton6 = LOW; 
boolean reverseState = LOW;

//limiter
boolean lastButton7 = LOW; 
boolean currentButton7 = LOW; 

//cam
boolean lastButton8 = LOW; 
boolean currentButton8 = LOW; 

void setup()
{
  Serial.begin(9600);	  // Debugging only
  Serial.println("Sending"); // Debugging only

  // Initialise the IO and ISR
  //vw_set_ptt_inverted(true); // Required for DR3100
  vw_setup(2000);	 // Bits per sec
  vw_set_tx_pin(transmitterPin); //RF sending pin Arduino Mega
  
  for(int i = 2; i<10; i++){
    pinMode(i, INPUT);
  }
  
}

//debounce function
boolean debounce(boolean last, int button)
{
  boolean current = digitalRead(button);
  if(last != current)
  {
    delay(5);
    current = digitalRead(button);
  }
  return current;
}



void loop(){
  //for the downShift
  currentButton1 = debounce(lastButton1, downShift);
  if(lastButton1 == LOW && currentButton1 == HIGH)
  {
    Serial.write('-');
  }
  lastButton1 = currentButton1;
  
  
  //for the upShift
  currentButton2 = debounce(lastButton2, upShift);
  if(lastButton2 == LOW && currentButton2 == HIGH)
  {
    Serial.write('+');
  }
  lastButton2 = currentButton2;
  
  
  //for the burnout
  currentButton4 = debounce(lastButton4, burnout);
  if(lastButton4 == LOW && currentButton4 == HIGH)
  { 
    Serial.write('b');
  }  
  
  lastButton4 = currentButton4;
  
  
  //for the neutral
  currentButton5 = debounce(lastButton5, neutral);
  if(lastButton5 == LOW && currentButton5 == HIGH)
  {
    Serial.write('n');
    
  }
  lastButton5 = currentButton5;
  
  
  //for the reverse
  currentButton6 = debounce(lastButton6, reverse);
  if(lastButton6 == LOW && currentButton6 == HIGH)
  {
    ////////////////////////////////////////////////////////////
    Serial.write('r');
    reverseState = !reverseState;
    Serial.println(reverseState);
    delay(1000);
    //////////////////////////////////////////////////////////////
  }
  lastButton6 = currentButton6;
  
  
  //for the limiter
  currentButton7 = debounce(lastButton7, limiter);
  if(lastButton7 == LOW && currentButton7 == HIGH)
  {
    Serial.write('l');
    
  }
  lastButton7 = currentButton7;
  
  
  //for the cam
  /*currentButton8 = debounce(lastButton8, cam);
  if(lastButton8 == LOW && currentButton8 == HIGH)
  {
     Serial.write('c');
  }
  lastButton8 = currentButton8;*/
  
  //transmitter codes
  valThrottle = analogRead(potPin1);  // read the value from the sensor
  valThrottle = map(valThrottle, 0, 55, 0, 255); 
  valThrottle = constrain(valThrottle, 0, 255);
  
  valBrakes = analogRead(potPin2);  // read the value from the sensor

  ////////////////////////////////////////////////////////////this is the statement///// 
 if(reverseState == HIGH){
    valBrakes = map(valBrakes, 1023, 0, 0, 255);  // this pot is broken
    valBrakes = constrain(valBrakes, 0, 255);
    
  }else{
    valBrakes = map(valBrakes, 1023, 0, 0, 10);  // this pot is broken
    valBrakes = constrain(valBrakes, 0, 10);
  }
///////////////////////////////////////////////////////////////////////////////////////////////
  
  valSteering = analogRead(potPin3); // 0 to 1023
  valSteering = map(valSteering, 0, 1023, 255, 0);
  Serial.println(valSteering);
  
  sprintf(Array, "%d,%d,%d.", valBrakes,valThrottle,valSteering);
  vw_send((uint8_t*)Array, strlen(Array));
  vw_wait_tx(); 
  Serial.println(Array);
    
}
  boolean current = digitalRead(button);
  if(last != current)
  {
    delay(5);
    current = digitalRead(button); <---------------------- last ????

K5CZ:

  boolean current = digitalRead(button);

if(last != current)
  {
    delay(5);
    current = digitalRead(button); <---------------------- last ????

i didn't get your question ?
this one should be current as it is because it is reading the current state not the last

You use //////// in a couple of places in your code. I assume the first one (lines 127 .. 132) is where you intend to update reverseState based on the state of the reverse pin:

  currentButton6 = debounce(lastButton6, reverse);

However, reverse is actually the reverse pin number, not the pin state. I assume that debouncing a constant number will return the constant number, and since the number does not equal HIGH I don't see how the code in lines 127 .. 132 would ever be executed. It ought to be obvious from the serial output whether it was actually being executed and if so what reverseState was getting set to. Since you haven't posted that output, of course we have no way to tell.

In lines 163 .. 171 you will execute one or other of the clauses of the if statement but there's no direct evidence of which clause is executed so I don't see how you could conclude that there's a problem here. In any case it looks to me as if the problem is that reverseState is not being set correctly because of the previous problem.

last is boolean VS current is integer
Try to define last as integer too

I use interrupt for reading states of buttons , you may adopt this

boolean debounce(int ButtonToCheck){
      ButtonState = digitalRead(ButtonToCheck);
      if (ButtonState == ButtonLastState[ButtonToCheck]) {
        if ( ++ButtonDebounce[ButtonToCheck] > BUTTON_FLICKERING ) { 
          ButtonStabilizedState[ButtonToCheck] = ButtonState; 
          ButtonDebounce[ButtonToCheck] = 0;
          if (ButtonStabilizedState[ButtonToCheck] == HIGH){
            //do something
          }
          else{
            //do something else
          }
          else
//          ButtonStabilizedState[ButtonToCheck] = -1; //=unknown state , if you want to work with tri-state
        }; 
      }
      else{ 
        ButtonLastState[ButtonToCheck] = ButtonState;
        ButtonDebounce[ButtonToCheck] = 0;
      };
  return ButtonLastState[ButtonToCheck];
//or you may
// return ButtonStabilizedState[ButtonToCheck]
};

PeterH:
You use //////// in a couple of places in your code. I assume the first one (lines 127 .. 132) is where you intend to update reverseState based on the state of the reverse pin:

  currentButton6 = debounce(lastButton6, reverse);

However, reverse is actually the reverse pin number, not the pin state. I assume that debouncing a constant number will return the constant number, and since the number does not equal HIGH I don't see how the code in lines 127 .. 132 would ever be executed. It ought to be obvious from the serial output whether it was actually being executed and if so what reverseState was getting set to. Since you haven't posted that output, of course we have no way to tell.

In lines 163 .. 171 you will execute one or other of the clauses of the if statement but there's no direct evidence of which clause is executed so I don't see how you could conclude that there's a problem here. In any case it looks to me as if the problem is that reverseState is not being set correctly because of the previous problem.

ok then what do i have to do ?
i put it "false" before didnt work so i though about high and low

K5CZ:
last is boolean VS current is integer
Try to define last as integer too

i don't think this is incorrect because all i am doing here is defining which pin i am reading from and if u see i put button in digitalRead()

i didn't get your codes, can you explain a bit please ?
about my debouncing i always used it and always worked i don't think the debounce method is the error

firashelou:
ok then what do i have to do ?

Look at the serial output and see whether it confirms my theory.

Correct your code so that it debounces the pin state rather than the pin number.

PeterH:

firashelou:
ok then what do i have to do ?

Look at the serial output and see whether it confirms my theory.

Correct your code so that it debounces the pin state rather than the pin number.

but as i said before the debounce int button is to define which pin i am reading from because i used the function for many pins so it must be like that if your talking about int button which is reverse in our case

It would help if you provide SPECIFICs.
"is not working " is pretty vague.
Is the value returned incorrect?
Can you step tru the function?
Can you execute the "if" statement in "else" statement correctly - maybe the "else" parameters are invalid?

Vaclav:
It would help if you provide SPECIFIC.
"is not working " is pretty vague.
When a function returns a value

ok when i press the pushbutton (state 0) the brakes works, and when i press it again (state 1) the potentiometer mapping doesnt work so i can reverse the motor backward

Your debounce function may work OK, but you use delay(5); and this is not much good for smooth run.

In summary (my english is not much good):
The timer (interrupt) evaluates the status of buttons several times per second. There are used fields of button states.

ButtonLastState[] - field is filled with the latest read state of the buttons
ButtonStabilizedState[] - field is filled with the stabilized (debounced) state of the buttons
ButtonDebounce[] - field is filled with the debounce counters - the button state considered stable when the counter reaches BUTTON_FLICKERING value

In the main program you can operate only with ButtonStabilizedState(s) and you must not solve previous and current states.

Unfortunately, I have no simple sketch where I could disclose this.

what is BUTTON_FLICKERING ?

Hey guys, his debounce() is fine... look at the function. It's passed a boolean and an int . He then uses the int to read the PIN, into current, which is a local, then checks the result against the boolean.

boolean debounce(boolean last, int button)
{
  boolean current = digitalRead(button);
  if(last != current)
  {
    delay(5);
    current = digitalRead(button);
  }
  return current;
}

firashelou:
what is BUTTON_FLICKERING ?

The debounce counter must pass X times, e.g.

#define BUTTON_FLICKERING some number         //X passes of interrupt routine

Part of my sketch to explain (not working separately)

#define RED_BUTTON_FLICKERING 7         // 7 ms

volatile int RedButtonState = LOW;
volatile int RedButtonLastState = LOW;
volatile int RedButtonStabilizedState = LOW;
volatile int RedButtonDebounce = 0;
bool RedButtonInProgress = false;

//because slowest timer interrupt frequency is 60 Hz and I want 10 Hz, I must divide
#define COMPARE_MATCH_REGISTER 155 //1024 prescaler & 100 Hz = 155,25
#define INT_RATIO 10 // cca 10 Hz 

//volatile because interrupt
volatile bool timer2_interrupt_in_progress = false; //if the program execution takes longer than the interrupter frequency
volatile int interrupt_numerator = 1;               //ratio

void setup() {
  noInterrupts();                 // disable all interrupts
  //Timer2 Settings
  TCCR2A = 0;
  TCCR2B = 0;
  TCNT2  = 0;
  TIMSK2 = 0;
  TCCR2A |= (1 << WGM21);         // turn on CTC mode
  TCCR2B |= ((1<<CS22) | (0<<CS21) | (1<<CS20));
  TIMSK2 |= (1 << OCIE2A);        // enable timer compare interrupt
  OCR2A = COMPARE_MATCH_REGISTER; // compare match register
  interrupts();                   // reenable all interrupts
  //initialize timer interrupt done
};
//process timer interrupt
ISR(TIMER2_COMPA_vect)            // timer compare interrupt service routine
{
  if ( !timer2_interrupt_in_progress ){          //previous run must be done
    timer2_interrupt_in_progress = true;         //processing flag
    if ( interrupt_numerator++ == INT_RATIO ){//not every interrupt is processed
      interrupt_numerator = 1;

      RedButtonState = digitalRead(RedButtonPin);
      if (RedButtonState == RedButtonLastState) {
        if ( ++RedButtonDebounce > RED_BUTTON_FLICKERING ){
          RedButtonStabilizedState = RedButtonState;
          RedButtonDebounce = 0;
          if (RedButtonStabilizedState == HIGH){ //supress dimmer
            brightness = FADE_H_LIMIT;
            LCD_brightness = LCD_BRIGHTNESS_H_LIMIT;
          };
        };
      }
      else{
        RedButtonLastState = RedButtonState;
        RedButtonDebounce = 0;
      };

    };
    timer2_interrupt_in_progress = false;
  };
}; 

void loop(){
  if (RedButtonStabilizedState == HIGH){
    if (RedButtonInProgress == false){
      RedButtonInProgress = true;
    };
  }
//Red button is processed after Red button is pressed and then released (not immediately when is pressed)
  else{
    if(RedButtonInProgress == true){
        ProcessRedButton();
      };
      RedButtonInProgress = false;
    };
  }; 
};

lar3ry:
Hey guys, his debounce() is fine... look at the function. It's passed a boolean and an int . He then uses the int to read the PIN, into current, which is a local, then checks the result against the boolean.

exactly the pin button is used to define a button in digitalRead()
i put it as a variable because i need to use the function with many pins

firashelou:
but as i said before the debounce int button is to define which pin i am reading from because i used the function for many pins so it must be like that if your talking about int button which is reverse in our case

I missed what the debounce() function was doing - you're right, that is correct.

Does the Serial output show that reverseState is taking the value you expect?

Given this bit of code, either one clause or the other will be executed:

 if(reverseState == HIGH){
    valBrakes = map(valBrakes, 1023, 0, 0, 255);  // this pot is broken
    valBrakes = constrain(valBrakes, 0, 255);
    
  }else{
    valBrakes = map(valBrakes, 1023, 0, 0, 10);  // this pot is broken
    valBrakes = constrain(valBrakes, 0, 10);
  }

Which path do you expect to be executed when the problem occurs, and which path is actually executed? And the followup, of course, is how do you know?

Given that your pins and variables obviously do have meaningful purposes related to real-world things such as throttle, brakes, steering, why don't you give them names that reflect that rather than meaningless names like potPin2 and lastButton6? With the current names, the names are meaningless, it would not be at all obvious if you were accidentally using the wrong variable somewhere.

PeterH:

firashelou:
but as i said before the debounce int button is to define which pin i am reading from because i used the function for many pins so it must be like that if your talking about int button which is reverse in our case

I missed what the debounce() function was doing - you're right, that is correct.

Does the Serial output show that reverseState is taking the value you expect?

Given this bit of code, either one clause or the other will be executed:

 if(reverseState == HIGH){

valBrakes = map(valBrakes, 1023, 0, 0, 255);  // this pot is broken
    valBrakes = constrain(valBrakes, 0, 255);
   
  }else{
    valBrakes = map(valBrakes, 1023, 0, 0, 10);  // this pot is broken
    valBrakes = constrain(valBrakes, 0, 10);
  }




Which path do you expect to be executed when the problem occurs, and which path is actually executed? And the followup, of course, is how do you [u]know[/u]?

the one that is executed is the mapped to 10 always is 10
how do i know, because i put Serial.prinln(values of the pots) before in the script

Well, maybe your comment is right. Perhaps that pot IS broken.
Have you tried a Serial.print right after the analog read of that pot?

Got it, I think. You toggle reverseState on a LOW to HIGH transition, but unless I missed it, you never set it false again.

Edit: er... make that "You never set it true again"