Buttons on Analog pins, code problem

Hello,
I am doing a timer project, with a 4 digit 7 segment display.
To drive the display, i need all 12 pins. And the pin 13 for a buzzer.
But I can't figure out my code... >:(
I don't know where is the problem.
In my loop, i have an object declaration for that class, and a call of the select function of that class.
And the code seems to get into the first if statement and never get out, but my buttons work...

Thank you :slight_smile:

Here is the .cpp file

#include "Counter.h"

Counter::Counter() {
  pinMode(pinA, OUTPUT);
  pinMode(pinB, OUTPUT);
  pinMode(pinC, OUTPUT);
  pinMode(pinD, OUTPUT);
  pinMode(pinE, OUTPUT);
  pinMode(pinF, OUTPUT);
  pinMode(pinG, OUTPUT);
  pinMode(D1, OUTPUT);
  pinMode(D2, OUTPUT);
  pinMode(D3, OUTPUT);
  pinMode(D4, OUTPUT);
  pinMode(btnUp, INPUT);
  pinMode(btnDwn, INPUT);
  pinMode(btnStart, INPUT);
}

void Counter::select() {
    valBtnUp = digitalRead(btnUp);
    valBtnDwn = digitalRead(btnDwn);
    valBtnStart = digitalRead(btnStart);
  for (int i; valBtnStart != HIGH; i++){
    valBtnUp = digitalRead(btnUp);
    valBtnDwn = digitalRead(btnDwn);
    valBtnStart = digitalRead(btnStart);
    if (valBtnUp == HIGH && valBtnDwn != 0) {
      timeCount = timeCount + 10;
      valBtnUp = LOW;
      Serial.println(timeCount);
    }
    else if (valBtnDwn == HIGH && timeCount != 0 ) {
      timeCount - 10;
      valBtnDwn = LOW;
      Serial.println(timeCount);
    }
 
  }
  valBtnStart = LOW;
  valBtnUp = LOW;
  valBtnDwn = LOW;
  Serial.println("Started") ;
}

And here is the .h

#pragma once
#include "Arduino.h"

class Counter {
  public:

    Counter();

    enum class SegPosition {
      One,
      Two,
      Three,
      Four,
    };

    SegPosition pos;

    void o (SegPosition select);
    void one(SegPosition select);
    void two(SegPosition select);
    void three(SegPosition select);
    void four(SegPosition select);
    void five(SegPosition select);
    void six(SegPosition select);
    void seven(SegPosition select);
    void eight(SegPosition select);
    void nine(SegPosition select);

   void select();

    void run();

  protected:
    int pinA = 2;
    int pinB = 3;
    int pinC = 4;
    int pinD = 5;
    int pinE = 6;
    int pinF = 7;
    int pinG = 8;
    int D1 = 9;
    int D2 = 10;
    int D3 = 11;
    int D4 = 12;
    int i = 2;
    int v = 10;
    int valBtnUp;
    int valBtnDwn;
    int valBtnStart;
    int btnUp = 15;
    int btnDwn = 16;
    int btnStart = 17;
    uint16_t timeCount = 0;
    
};
    if (valBtnUp == HIGH && valBtnDwn != 0) {

Which one is high again? Maybe it's not zero.

      timeCount - 10;

Were you planning to do anything with this value?

For that matter, were you planning to do anything with the value of i? Maybe a while() loop would be better for this task.

How are the inputs wired ?

Are there any pulldown resistors in place or are the inputs floating at an unknown voltage ?

@MorganS

Yah, I will do a do while loop it's more appropriate for that case ^^

And @UKHeliBob,

You're supposed to use pull up resistors when u use buttons on analog pins ?
I thaught you could just use them as regular Digital pins.

Yes you use them as regular digital pins. And even on regular digital pins you need a pull-up or pull-down or your pin will be floating when not pressed.

Oh,
Well thank you, i'll try that and give updates :slight_smile: :slight_smile: :slight_smile:

Hmmmm,
Well,
Putting INPUT_PULLUP istead of INPUT didn't change anything, added a do while.
So back to the starting point.
Really wired >:(

Here is the code :

.INO :

#include "Counter.h"

Counter c1;

void setup() {
  Serial.begin(115200);
}

void loop() {
  c1.select();
  delay(2000);
}

The .h :

#pragma once
#include "Arduino.h"

class Counter {
  public:

    Counter();

    enum class SegPosition {
      One,
      Two,
      Three,
      Four,
    };

    SegPosition pos;

    void o (SegPosition select);
    void one(SegPosition select);
    void two(SegPosition select);
    void three(SegPosition select);
    void four(SegPosition select);
    void five(SegPosition select);
    void six(SegPosition select);
    void seven(SegPosition select);
    void eight(SegPosition select);
    void nine(SegPosition select);

   void select();

    void run();

  protected:
    int pinA = 2;
    int pinB = 3;
    int pinC = 4;
    int pinD = 5;
    int pinE = 6;
    int pinF = 7;
    int pinG = 8;
    int D1 = 9;
    int D2 = 10;
    int D3 = 11;
    int D4 = 12;
    int i = 2;
    int v = 10;
    int valBtnUp;
    int valBtnDwn;
    int valBtnStart;
    int btnUp = 15;
    int btnDwn = 16;
    int btnStart = 17;
    uint16_t timeCount = 0;
    
};

And the .cpp :

#include "Counter.h"

Counter::Counter() {
  pinMode(pinA, OUTPUT);
  pinMode(pinB, OUTPUT);
  pinMode(pinC, OUTPUT);
  pinMode(pinD, OUTPUT);
  pinMode(pinE, OUTPUT);
  pinMode(pinF, OUTPUT);
  pinMode(pinG, OUTPUT);
  pinMode(D1, OUTPUT);
  pinMode(D2, OUTPUT);
  pinMode(D3, OUTPUT);
  pinMode(D4, OUTPUT);
  pinMode(btnUp, INPUT_PULLUP);
  pinMode(btnDwn, INPUT_PULLUP);
  pinMode(btnStart, INPUT_PULLUP);
}

void Counter::select() {
    valBtnUp = digitalRead(btnUp);
    valBtnDwn = digitalRead(btnDwn);
    valBtnStart = digitalRead(btnStart);
  do{
    valBtnUp = digitalRead(btnUp);
    valBtnDwn = digitalRead(btnDwn);
    valBtnStart = digitalRead(btnStart);
    if (valBtnUp == HIGH && valBtnDwn != 0) {
      timeCount = timeCount + 10;
      valBtnUp = LOW;
      Serial.println(timeCount);
    }
    else if (valBtnDwn == HIGH && timeCount != 0 ) {
      timeCount - 10;
      valBtnDwn = LOW;
    }
 
  }while(valBtnStart != HIGH);
  valBtnStart = LOW;
  valBtnUp = LOW;
  valBtnDwn = LOW;
  Serial.println("Started") ;
}

And here is a serial output :

55080
55090
55100
55110
55120
55130
55140
55150
55160
55170
55180
55190
55200
55210
55220
55230
55240
55250
55260
55270
55280
55290
55300
55310
55320
55330
55340
55350
55360
55370
55380
55390
55400
55410
55420
55430
55440
55450
55460
55470
55480
55490
55500
55510
55520
55530
55540
55550
55560
55570
55580
55590
55600
55610
55620
55630
55640
55650
55660
55670
55680
55690
55700
55710
55720
55730
55740
55750
55760
55770
55780
55790
55800
55810
55820
55830
55840
55850
55860
55870
55880
55890
55900
55910
55920
55930
55940
55950
55960
55970
55980
55990
56000
56010
56020
56030
56040
56050
56060
56070
56080
56090
56100
56110
56120
56130
56140
56150
56160

There's some fundamental problems with your design. The main reason for writing a class instead of classic C code is you need more than one of them. An Arduino Due has 4 serial ports, so Serial1.print() works just the same as Serial3.print(). Display and sensor libraries always use classes because you're usually going to have more than one sensor and sometimes more than one display.

So, what would happen if I try to instantiate a second counter, to count something else? First it's going to fight with the first counter for control over the display and second, you've hardcoded the buttons so it can't use different pins for the buttons. Third, the class is "blocking" meaning it doesn't let any other code run at the same time, so the second class never gets a chance to see if its buttons were pressed.

OK, maybe you are happy with the hardcoded pins and the restriction to only have one instance. But it's still blocking. Your code can do literally nothing else while its counting. So just put all that stuff in loop() and be done with it.

I would start by splitting it into two classes - one for the display and one for the counting. Maybe even three - one for buttons can be separate.

Hmm,
Mabe just put all of that in an ino file without classes.
You think it would fix that issue @MorganS?