Counting up and down and integer with 2 buttons

Morning guys,

I'm having a little trouble counting up (++) and down (--) and integer using two buttons.

I'm looking to have the buttons count up an integer I've called "baudRate". baudRate starts on "0" and each time one button is pushed the integer becomes 1,2,3,4... and these related to baud rates e.g. 9600,19200,38400,57600... I want the code to also read the state of the second button, and each time this button is pressed we cycle back through the baudRate 57600,38400,19200,9600.

I've managed to get the buttons to cycle up, but the problem is that nothing happens when the second buttons is pressed.

I'm using a Mega 2560 and buttons with internal pullup resistors from the board, not physical pullup resistors.

I'm also de-bouncing the buttons.

Thanks in advance.

Below is my code:

const int K8 = 7;
const int K7 = 6;
//----------------
const int K6 = 5;
const int K5 = 4;
//----------------
const int K4 = 3;
const int K3 = 2;


unsigned long lastDebounceTime = 0;  // the last time the output pin was toggled
unsigned long debounceDelay = 50;    // the debounce time; increase if the output flickers

// Variables will change:
int K8buttonState;             // the current reading from the input pin
int K8lastButtonState = LOW;   // the previous reading from the input pin
int K7buttonState;             // the current reading from the input pin
int K7lastButtonState = LOW;   // the previous reading from the input pin

int baudRate = 0;   // counter for the number of button presses


void setup() {
  pinMode(K8, INPUT_PULLUP);
  pinMode(K7, INPUT_PULLUP);
  pinMode(K6, INPUT_PULLUP);
  pinMode(K5, INPUT_PULLUP);
  pinMode(K4, INPUT_PULLUP);
  pinMode(K3, INPUT_PULLUP);
  
  Serial.begin(9600);
}

void loop() {

// read the state of the switch into a local variable:
   int K8reading = digitalRead(K8);
   int K7reading = digitalRead(K7);

      if (K8reading != K8lastButtonState) 
   {
      lastDebounceTime = millis();
   }
      if ((millis() - lastDebounceTime) > debounceDelay) 
   {
      if (K8reading != K8buttonState) 
   {
      K8buttonState = K8reading;

      if (K8buttonState == HIGH) 
   {  
        baudRate++;
        
      if (baudRate == 1)
   {
      Serial.println("9600");
   }
      if (baudRate == 2)
   {
      Serial.println("19200");
   }
     if (baudRate == 3)
   {
      Serial.println("38400");
   }
     if (baudRate == 4)
   {
      Serial.println("57600");
   }
     if (baudRate == 5)
   {
      Serial.println("115200");
   }
      else if (K7buttonState == HIGH)
      {
      baudRate--;
      }  
   }
         
   }
   }

   K8lastButtonState = K8reading;
   K7lastButtonState = K7reading;
}

With internal pullup resistors, digitalRead() returns LOW when the switch is pressed, and HIGH when it is not.

Why are you debouncing the first pin, but not the second one? Why are you detecting state changes on the first pin, but not the second one?

Thanks Paul,

If I debounce the second pin, the state changes are not detected on either pin. So for the time being I've left it out of the code.

Does my else if look correct in the flow of the argument?

You need to format your code properly so that all the curly braces line up. Press control+T in the arduino editor to do this for you. Once you have done that, you will notice that the curly braces are in the wrong place:

if (baudRate == 5)
   {
      Serial.println("115200");
   }
      else if (K7buttonState == HIGH)

Should be:

if (baudRate == 5)
   {
      Serial.println("115200");
   }
  }
  else if (K7buttonState == HIGH)

otherwise the K7 check will only happen if K8 was pushed as well, and the baudrate is not equal to 5.

Also, check out the "switch" statement, it will make your code much more readable.

Thanks TRex,

I didn't know about the CTRL+L command, so firstly thank you for that!

I noticed the difference between your code and mine was the extra curly brace before the else if. I've placed this in but it throws up an error that my final line of code "K8lastButtonState does not name a type".

I assume you removed one of the extra closing curly brackets near the bottom to compensate for the additional one added as per my suggestion, and more importantly, that you removed the RIGHT one? :slight_smile:

I hadn't... But have now, and it compiles correctly.

It still doesn't count down if button K7 is pressed though.

Would it be the position of the else if that's causing it not to be triggered? Or something else obvious that you can spot?

I don't see that the state of the second switch has ANYTHING to do with the state of the first switch.

Create a function, checkForUp(), in which you place ALL of the code for dealing with the up switch and NONE of the code for dealing with the down switch.

Then, create a function, checkForDown(), in which you place all of the code for dealing with the down switch.

Call those two functions from loop().

Paul,

For fear of looking stupid, can I ask you take a look at that please?

I’m now not receiving able to get anything to print when the buttons are pushed…

const int K8 = 7;
const int K7 = 6;
//----------------
const int K6 = 5;
const int K5 = 4;
//----------------
const int K4 = 3;
const int K3 = 2;

unsigned long lastDebounceTime = 0;  // the last time the output pin was toggled
unsigned long debounceDelay = 50;    // the debounce time; increase if the output flickers

// Variables will change:
int K8buttonState;             // the current reading from the input pin
int K8lastButtonState = LOW;   // the previous reading from the input pin
int K7buttonState;             // the current reading from the input pin
int K7lastButtonState = LOW;   // the previous reading from the input pin

int baudUp = 0;   // counter for the number of button presses

void setup() {
  pinMode(K8, INPUT_PULLUP);
  pinMode(K7, INPUT_PULLUP);
  pinMode(K6, INPUT_PULLUP);
  pinMode(K5, INPUT_PULLUP);
  pinMode(K4, INPUT_PULLUP);
  pinMode(K3, INPUT_PULLUP);

  Serial.begin(9600);
}

void loop() {

  int K8reading = digitalRead(K8);

  if (K8reading != K8lastButtonState) {
    lastDebounceTime = millis();
  }

  if ((millis() - lastDebounceTime) > debounceDelay) {

    if (K8reading != K8buttonState) {
      K8buttonState = K8reading;

      if (K8buttonState == HIGH) {
        baudUp++;
        if (baudUp == 1) {
          Serial.println("9600");
        }
        if (baudUp == 2) {
          Serial.println("19200");
        }
        if (baudUp == 3) {
          Serial.println("38400");
        }
        if (baudUp == 4) {
          Serial.println("57600");
        }
        if (baudUp == 5) {
          Serial.println("115200");
        }
        delay(200);
      }
    }

    K8lastButtonState = K8reading;
  }
  int K7reading = digitalRead(K7);
  if (K7buttonState == HIGH) {
    baudUp--;
    K7lastButtonState = K7reading;

  }
}
  int K7reading = digitalRead(K7);
  if (K7buttonState == HIGH) {
    baudUp--;
    K7lastButtonState = K7reading;

You have, but not actually providing a value, initialized K7buttonState to LOW. You never assign a different value to it, so you can not reasonably expect that if statement to ever evaluate to true.

What do your Serial.print() statements tell you is happening?

Why have you not done as I suggested, and created two functions to deal with the up and down switches? Why do your variable names refer to K7 and K8, instead of up and down?

They're named K7 and K8, because I'm using an 8 button module and that's the name of the buttons. Ultimately, I will be using K1-K8, so I've left the naming as is for now.

As for creating two functions with the Void Loop () function. I'm unsure as to what you're asking me to do, as I thought I had done that... Could you please explain further?

hi,

I’m using arduino to run the lights in my house, using pushbutton-type light switches.
At first, I started out with a complex state machine, including debounce, longpress, and double-tap. In the test setup, this worked like a charm, and i set about installing, only to find out that my cabling produces around 1% of false positives… :frowning:

After this, I took the approach of polling all switches ‘n’ times and determine the state according to this. It’s much simpler, and you don’t have to count millis etc for debouncing.

//this part of the loop will detect the current state

        for(int j = 0; j <= MIN_PRESS_POLLS; j++) {
                for (int i = 0; i < SWITCH_COUNT; i++) {
                        if (Switch[i].enabled) {
                                if (digitalRead(Switch[i].port)) {
                                        Switch[i].hiState++;
                                } else {
                                        Switch[i].loState++;
                                }
                        }
                }
        }

//this part of the loop will take action. 
        for (int i = 0; i < SWITCH_COUNT; i++) {
                if (Switch[i].enabled) {
                        if (Switch[i].loState > Switch[i].hiState) {
                                if (!Switch[i].status) {
                                        syslog(SYSLOG_NOTI, "switch pressed:" + String(i,DEC));
                                        toggle_group(Switch[i].shortPress);
                                        Switch[i].status = 1;
                                        Switch[i].lastStateChange = millis();
                                } else if (Switch[i].status == 1 && millis() - Switch[i].lastStateChange > LONG_PRESS_DURATION) {
                                        syslog(SYSLOG_NOTI, "switch longpressed:" + String(i,DEC));
                                        toggle_group(Switch[i].longPress);
                                        Switch[i].status = 2;
                                        Switch[i].lastStateChange = millis();
                                }
                        } else {
                                Switch[i].status = 0;
                                Switch[i].lastStateChange = millis();
                        }
                        Switch[i].hiState = Switch[i].loState = 0;
                }
        }

As you see, I do count millis, but only for detecting longpress, which you don’t need to do. The double-tap was removed from the system due to a very low WAF :slight_smile:

For your reference, I’ll post the switch struct as well, and comment it;

struct switch_t {
        byte port;       //the digital port of the arduino
        byte status;    //keeps the current status of the switch 0=idle, 1=short detected, 2=long
        bool enabled;  //dont poll if 0
        short hiState;  //counters to keep (if you want to poll long, make the var an int)
        short loState;  //counters to keep (if you want to poll long, make the var an int)
        byte shortPress;  //in my application, this refers to a group of relays
        byte longPress;  //in my application, this refers to a group of relays
        unsigned long int lastStateChange;  //to detect longpress
};

Hope this helps.

this should compile for you…

//how many polls will be enough for one signal
#define MIN_PRESS_POLLS 100

//you mention 8 switches, so we go to this, but later on, we only define 7 and 8
#define SWITCH_COUNT 8

//definition of the switch struct
struct switch_t {
        int port;
        int status;
        bool enabled;
        int hiState;
        int loState;
};

//initializing for all your switches
switch_t Switch[SWITCH_COUNT];

//global var baudUp as you have it
int baudUp = 1;

void setup() {
        Serial.begin(9600);
        print_settings();

        //switch 6 is K7 (as this is a 0-based array)
        Switch[6].port  = 6;
        Switch[6].enabled = 1;

        //switch 7 is K8 (as this is a 0-based array)
        Switch[7].port  = 7;
        Switch[7].enabled = 1;

        //as none of the other array members have 'enabled' set to 1, they will not be initialized or polled.

        //set the pinmode for enabled ports
        for(int i = 0; i < SWITCH_COUNT; i++) {
                if( Switch[i].enabled ) {
                        pinMode(Switch[i].port, INPUT_PULLUP);
                }
        }

}


void loop() {
        //loop through all switches for the minimum duration of the press (poll cycles)
        for(int j = 0; j <= MIN_PRESS_POLLS; j++) {
                for (int i = 0; i < SWITCH_COUNT; i++) {
                        if (Switch[i].enabled) {
                                if (digitalRead(Switch[i].port)) {
                                        Switch[i].hiState++;
                                } else {
                                        Switch[i].loState++;
                                }
                        }
                }
        }

        //test the switches to see if we see a switch pressed
        for (int i = 0; i < SWITCH_COUNT; i++) {
                if (Switch[i].enabled) {
                        //remember pullups, you have LOW as pressed
                        if (Switch[i].loState > Switch[i].hiState) {
                                if (!Switch[i].status) {
                                        //the function process_switch will be called if we see it pressed. note: only 1x per press, this can be changed if you want
                                        process_switch(i);
                                        Switch[i].status = 1;
                                }
                                } else {
                                Switch[i].status = 0;
                        }
                        Switch[i].hiState = Switch[i].loState = 0;
                }
        }
}


void process_switch(int i) {
        //K7 was pressed
        if( i == 6 ) {
                baudUp--;
                //K8 was pressed
                } else if( i == 7 ) {
                baudUp++;
        }

        //some consistency checking, you could also cycle through here, if you want
        if( baudUp <= 0 ) {
                baudUp = 1;
        } else if( baudUp >= 6 ) {
                baudUp = 5;
        }

        print_settings();

}


void print_settings() {
        Serial.print("baudUp:");
        Serial.print(baudUp);

        if (baudUp == 1) {
                Serial.println("9600");
        } else if (baudUp == 2) {
                Serial.println("19200");
        } else if (baudUp == 3) {
                Serial.println("38400");
        } else if (baudUp == 4) {
                Serial.println("57600");
        } else if (baudUp == 5) {
                Serial.println("115200");
        }
}

I can't even begin to tell you how greatful I am that you spent the time to do that! It works perfectly, thank you ever so much!!

That should be enough for me to now compile the rest of the code with the other 6 buttons!!

I hope this helps someone else now.

GREAT WORK bitflogger!!