Reading buttons to count!

Using Arduino Programming Questions
#1

ok, here’s the excerpt of my code, with the initial settings, and then the execution, but the buttons just don’t make the count move.

now i know the code is mostly correct, but as i was tinkering yesterday, i shorted out the usb and the working saved code was lost.

the simple jist is;

2 buttons, 1 for up, 1 for down, and when you press the up/down button, it checks to see if the button has been pressed in the past 50ms, and if it hasn’t it increases the integer (count) by 1.

this count figure is then ported to the seven segment display.

if i put a number in the variable for the count, it is displayed as it should
if i put a for (int j =0; j < 255; j++) then the seven segment(j); counts as it should…

the buttons are cheap and basic, and work

i just can’t see where this is failing.

const int upbuttonPin = 2;       // the number of the up button pin
const int downbuttonPin = 3;     // the number of the down button pin

// debounce buttons
long uplastDebounceTime = 0;     // the last time the output pin was toggled
long updebounceDelay = 50;       // the debounce time; increase if the output flickers
long downlastDebounceTime = 0;   // the last time the output pin was toggled
long downdebounceDelay = 50;     // the debounce time; increase if the output flickers

// initialise the button states
int upbuttonState;               // the current reading from the up input pin
int uplastbuttonState = LOW;     // the previous reading from the up input pin
int downbuttonState;             // the current reading from the down input pin
int downlastbuttonState = LOW;   // the previous reading from the down input pin

void loop() 
{
  
    // UP UP UP UP 
 int upreading = digitalRead(upbuttonPin);  // read the state of the up button
  if (upreading !=uplastbuttonState) {      // put the button state into the buttonstate int
     uplastDebounceTime = millis();         // check how long the debounce time is
  } 
  if ((millis() - uplastDebounceTime) > updebounceDelay) {    // if the time is ok then continue
    if (upreading !=upbuttonState) {        // check the reading to the state, if different
      upbuttonState = upreading;            // set the state
      if (upbuttonState == HIGH) {          // if the button  state is high
       int count = count++;                  // also increase the count by 1
    }
  }
  uplastbuttonState = upreading;            // save the button state to the reading
  }  
  
  // DOWN DOWN DOWN DOWN 
 int downreading = digitalRead(downbuttonPin);  // read the state of the down button
 if (downreading !=downlastbuttonState) {       // put the button state into the buttonstate int
    downlastDebounceTime = millis();            // check how long the debounce time is
  } 
 if ((millis() - downlastDebounceTime) > downdebounceDelay) {    // if the time is ok then continue
    if (downreading != downbuttonState) {      // check the reading to the state, if different
      downbuttonState = downreading;           // set the state
      if (downbuttonState == HIGH) {           // if the button  state is high
       int count = count--;                     // also drop the count by 1
      }
    }
  downlastbuttonState = downreading;           // save the button state to the reading
 }

  {
      SevenSegDisplay(count);                  // Run the display routine
  }

}
#2

You've declared count inside the innermost of statement, so it will only be available inside that if statement. Your curly braces also need some work to make it easier to read

#3

:fearful:

if i declare an integer inside an if statement too deep, then it won't affect the integer outside the if statement?

how many levels can the if / integer work?

#4

sorry, that doesn't make any sense.

i've copied the instructions from the arduino reference section, and the code is laid out exactly as mine is;

void loop() {
  // read the state of the switch into a local variable:
  int reading = digitalRead(buttonPin);

  // check to see if you just pressed the button 
  // (i.e. the input went from LOW to HIGH),  and you've waited 
  // long enough since the last press to ignore any noise:  

  // If the switch changed, due to noise or pressing:
  if (reading != lastButtonState) {
    // reset the debouncing timer
    lastDebounceTime = millis();
  } 
  
  if ((millis() - lastDebounceTime) > debounceDelay) {
    // whatever the reading is at, it's been there for longer
    // than the debounce delay, so take it as the actual current state:

    // if the button state has changed:
    if (reading != buttonState) {
      buttonState = reading;

      // only toggle the LED if the new button state is HIGH
      if (buttonState == HIGH) {
        ledState = !ledState;
      }
    }
  }
  
  // set the LED:
  digitalWrite(ledPin, ledState);

  // save the reading.  Next time through the loop,
  // it'll be the lastButtonState:
  lastButtonState = reading;
}

integer, inside an if, inside an if, inside an if being set, closed and then used.

#5

The operative word is declare.

Declaring a variable:

int count;

Declaring a variable and assigning it a value:

int count = 5;

Assigning a value to a variable:

count = 5;

See the difference?

#6

So something similar to this,

/* 
Andrew Mascolo
Simple count up and down, + button press or hold feature
*/

#include <Wire.h>
#include <LiquidCrystal_I2C.h>

LiquidCrystal_I2C lcd(0x20,16,2);

const byte buttonPin1 = 2; // Up button
const byte buttonPin2 = 3; // Down Button
//const int ledPin =  11;

int buttonState1 = 0;
int lastReading1 = 0;
int buttonState2 = 0;
int lastReading2 = 0;
int lastcount =0;
long onTime1=0;
long onTime2=0;
int count = 0;

void setup() { 
  //pinMode(ledPin, OUTPUT);      
  pinMode(buttonPin1, INPUT);
  pinMode(buttonPin2, INPUT);

  lcd.init();                      // initialize the lcd 
  lcd.backlight();
  Serial.begin(9600);  
}

void loop(){ 
  buttonState1 = digitalRead(buttonPin1);
  buttonState2 = digitalRead(buttonPin2);
  if (buttonState1 == HIGH && lastReading1 == LOW) {
    onTime1 = millis();
    count++;
  }
  if (buttonState2 == HIGH && lastReading2 == LOW) {
    onTime2 = millis();
    count--;
  }

  //held
  if (buttonState1 == HIGH && lastReading1 == HIGH) { 
    if ((millis() - onTime1) > 500 ) {
      delay(200);
      count++;
      lastReading1 = LOW;
    } 
  }
  if (buttonState2 == HIGH && lastReading2 == HIGH) { 
    if ((millis() - onTime2) > 500 ) {
      delay(200);
      count--;
      lastReading2 = LOW;
    } 
  }
  Serial.println(count);
  lcd.setCursor(0,0);
  //lcd.clear();
  if(lastcount < 0 && count >= 0) lcd.clear();
  if(lastcount >=10 && count < 10) lcd.clear();
  if(lastcount >=100 && count < 100) lcd.clear();
  if(lastcount >=1000 && count < 1000) lcd.clear();
  lcd.print(count);
  //lcd.clear();
  lastcount = count;
  lastReading1 = buttonState1;
  lastReading2 = buttonState2;
}
#7

i've copied the instructions from the arduino reference section, and the code is laid out exactly as mine is;

So? That doesn't mean that you can't do better. Meaningful names that go together.

For instance:

  if (reading != lastButtonState) {

makes a lot more sense as:

  if(currState != lastState)
  {

Putting the { on a new line, and making the matching } line up with it makes the code easier to read, for most of us.

#8

i only meant that the structure was the same as the arduino tutorial, and i was told that

You've declared count inside the innermost of statement, so it will only be available inside that if statement.

that's what didn't make sense.

i can get the code working with the arduino tutorial, but not when i port it across to two buttons.

i think i must have something mismatched in my code, or that i'm requesting the mills() count multiple times.

#9

In C and C++ variables are very strictly typed and very tightly scoped. Your issue, as pointed out by Arrch is one of scope.

I highly recommend the tutorial at cplusplus.com. At least read the section on variables and data types.

http://cplusplus.com/doc/tutorial/

#10

that cplusplus site is great! thanks for the link!

i have solved the issue! and it was an issue created by myself…

on trying to set limits, i had a void called that had what to do if the number went over 999 (set to 999) and what to do if the number went less than 0 (set to 0).

i inadvertently set the count to 0 everytime it looped… it didn’t matter if the count changed, as soon as it called that subroutine, it set count back to 0.

…you live and learn!

code is now working, with all debouncing put into place, and limits set!

thanks for all your help!

#11

on trying to set limits, i had a void called that

No, you didn't. You had a function whose return type was void.

#12