next topic »

[on:]

I have a routine that looks for input from one of 5 buttons.  It was originally written such that on detecting a button press it waited for the button to be released before returning a flag indicating that a button had been pressed and released and the number of the button.  This was as follows :

Code:

void lookForButton()    //return btnPressed=true and btnNum if a button is pressed and released
{
  btnPressed = false;
  for (int btn=8;btn<=12;btn++)
  {
    if (digitalRead(btn) == HIGH)     // check for a button being pressed
    {
      btnPressed = true;
      btnNum = btn;                   //save number of button pressed
    } 
  }

  if (btnPressed == true)
  {
    while (digitalRead(btnNum) == HIGH)  //wait for the button to be released
    {   
      delay(100);                        //debounce
    }
  }
   
}

Being aware that I might not want the main program to stall at some time in the future I have re-written the routine as follows :

Code:

void lookForButton()   //return btnReleased=true and btnNum if a button is pressed and released
{
  boolean static btnPressed = false;        //note STATIC to preserve value for next call
  for (int btn=8;btn<=12;btn++)
  {
    if (digitalRead(btn) == HIGH)     // check for a button being pressed now
    {
      btnPressed = true;              //flag that we have found a button pressed and need to check for release
      btnNum = btn;                   //save number of button pressed (could exit the loop at this point)
    } 
  }

  if (btnPressed == false)              //no button pressed
  {
    btnReleased = false;                //not pressed so it can't have been released so flag it and return
    return;
  }

  currentBtnState = digitalRead(btnNum);    //get current state of the button
  if (currentBtnState != prevBtnState)      //button state has changed. Did it bounce or was it released ?
  {
    if ((millis() - prevBtnTime > 100) && (digitalRead(btnNum) == LOW)) //button released and debounce time has elapsed
    {

      Serial.println("got a button"); //***********************DEBUGGING CODE********************* 
      btnPressed = false;                  //reset the flag for next time that it is needed
      btnReleased = true;                  //flag release of button for action
    }
    else
    {   
      Serial.println("no valid button yet");    //***********************DEBUGGING CODE*********************
      prevBtnTime =  millis();             //button is still pressed or debounce time has not elapsed
      prevBtnState = currentBtnState;      //remember the current button state and current time
    }
  }   
}

prevBtnTime is defined as a global long initially set to 0
currentBtnState and prevButtonState are defined as global ints initially set to LOW
None of these variables are changed anywhere else in the sketch

The routine appears to work but the debug Serial.print only outputs "no valid button yet" the first time that the routine is used after uploading a sketch or resetting the Arduino so something is wrong and I would be grateful for advice as I suspect my logic is wrong somewhere.

[Reply #1 on:]

Code:

void lookForButton()   //return btnReleased=true and btnNum if a button is pressed and released

This function does not return anything. The comment is WRONG. It SETS some global values. It SHOULD return btnNum, which should NOT be a global.

Code:

  for (int btn=8;btn<=12;btn++)
  {
    if (digitalRead(btn) == HIGH)     // check for a button being pressed now
    {
      btnPressed = true;              //flag that we have found a button pressed and need to check for release
      btnNum = btn;                   //save number of button pressed (could exit the loop at this point)
    } 
  }

There should be a break in this loop. Once you found A switch pressed, there is no reason to check the others.

Code:

      prevBtnState = currentBtnState;      //remember the current button state and current time

This should happen EVERY time currentBtnState is set, not inside nested ifs.

[Reply #2 on:]

Thanks Paul, much appreciated

Point taken about the misleading comment and I had already spotted the possibility of breaking out of the loop as noted in my comment.

I have now fixed the logic of where prevBtnState = currentBtnState;  occurs

Fixed code as follows :

Code:

void lookForButton()   //sets btnReleased=true and btnNum if a button is pressed and released
{
  boolean static btnPressed = false;        //note STATIC to preserve value for next call
  for (int btn=8;btn<=12;btn++)
  {
    if (digitalRead(btn) == HIGH)     // check for a button being pressed now
    {
      btnPressed = true;              //flag that we have found a button pressed and need to check for release
      btnNum = btn;                   //save number of button pressed and exit the loop
      break;
    } 
  }
  if (btnPressed == false)              //no button pressed
  {
    btnReleased = false;                //not pressed so it can't have been released so flag it and return
    return;
  }
  prevBtnState = currentBtnState;      //remember the current button state and current time
  currentBtnState = digitalRead(btnNum);    //get current state of the button
  if (currentBtnState != prevBtnState)      //button state has changed. Did it bounce or was it released ?
  {
    if ((millis() - prevBtnTime > 100) && (digitalRead(btnNum) == LOW)) //button released and debounce time has elapsed
    {

      Serial.println("got a button"); //***********************DEBUGGING CODE********************* 
      btnPressed = false;                  //reset the flag for next time that it is needed
      btnReleased = true;                  //flag release of button for action
    }
    else
    {   
      Serial.println("no valid button yet");    //***********************DEBUGGING CODE*********************
      prevBtnTime =  millis();             //button is still pressed or debounce time has not elapsed
    }
  }   
}

Some sample output :

Code:

no valid button yet
got a button
no valid button yet
got a button
no valid button yet
no valid button yet
no valid button yet
no valid button yet
no valid button yet
got a button

[Reply #3 on:]

Some sample output :

So, what is the problem? Why are you not printing WHICH button was pressed/released? That might give you a clue as to the problem, if there is one.

Once a button is pressed, btnPressed is never set back to false.

[Reply #4 on:]

Paul

The point that I was trying to make, perhaps not clearly enough, is that the problem is fixed.  The debug output and the working of the routine with the routines(s) calling it are exactly as expected.

btnPressed is set back to false once a button is released and de-bounce time has elapsed in this portion of the routine. 

Code:

    if ((millis() - prevBtnTime > 100) && (digitalRead(btnNum) == LOW)) //button released and debounce time has elapsed
    {

      Serial.println("got a button"); //***********************DEBUGGING CODE********************* 
      btnPressed = false;                  //reset the flag for next time that it is needed
      btnReleased = true;                  //flag release of button for action
    }
    else
    {   
      Serial.println("no valid button yet");    //***********************DEBUGGING CODE*********************
      prevBtnTime =  millis();             //button is still pressed or debounce time has not elapsed
    }

This was in my original millis() version of the routine posted here and has not needed to be changed.

Once again, thanks for your help.

[Reply #5 on:]

The point that I was trying to make, perhaps not clearly enough, is that the problem is fixed.

Oh. Well, that's great.

btnPressed is set back to false once a button is released and de-bounce time has elapsed in this portion of the routine. 

So it is. I guess I missed that.

[Reply #6 on:]

It's amazing how easy it is to miss the obvious things as I know only too well.

An earlier version of the routine mistakenly had for (int btn=8;btn<=13;btn++) in the portion that reads the buttons when I really meant for (int btn=8;btn<13;btn++).  This routine is part of an alarm clock program that I have written to stretch my understanding of the Arduino. 

All was OK until I added the code to set off the alarm and turned on the LED on pin 13 when the alarm was triggered.  It took me ages (and several Serial.prints scattered throughout the program to find out what was wrong.  That reminds me.  I meant to change the code to

Code:

for (int btn = 8; btn <= 12; btn++)

to make it easier to read.  I prefer to use <= rather than < because to me the limits of the loop are more obvious.  What do others think ?

[Reply #7 on:]

I prefer to use <= rather than < because to me the limits of the loop are more obvious.  What do others think ?

I think it depends on the context. In this case, I think it is a good idea.