Binary counter, button problem

Using Arduino Project Guidance
#1

I am working with Arduino UNO and the “funshield” attachment, and I am trying to make a binary counter that adds with one button and subtracts with another (I managed this), the thing is, I cannot get it to work when I am holding down the buttons.
Here is my rough, but working, code:

#include "funshield.h"
//setting up
constexpr int ledPins[] { led1_pin, led2_pin, led3_pin, led4_pin };
constexpr int ledPinsCount = sizeof(ledPins) / sizeof(ledPins[0]);
constexpr int activationDelay = 1000;
constexpr int periodicDelay = 300;

unsigned lastTime = 0;
int counter = 0;
bool wasDown = false;
bool wasDown2 = false;

//Binary counter
void BN(int number, const int leds[], int ledsCount)
{
  for (int i = ledsCount-1; i >= 0; --i){
    bool currentBit = number & 1;
    digitalWrite(leds[i], currentBit ? ON : OFF);
    number = number >> 1;
  }
}

void setup(){
  for (int i = 0; i < ledPinsCount; ++i){
    pinMode(ledPins[i], OUTPUT);
    digitalWrite(ledPins[i], OFF);
  }
 pinMode(button1_pin, INPUT);
 pinMode(button2_pin, INPUT);
 lastTime = millis();
}

void loop()
{ //Checks if button is down
  bool isDown = digitalRead(button1_pin) == ON;
  //NOT WORKING
  //if (wasDown == isDown) {
    //unsigned long time = millis();
    //if (time >= lastTime + activationDelay){
      //counter = (counter + 1) % 16;
     //BN(counter);
      //lastTime += activationDelay;
    //}
    //wasDown = isDown;
  //}
  //JUST ADDS AUTOMATICALLY 
  
  //if wasn't down but now is, do BN +
 if (wasDown != isDown){
  if (isDown){
    counter = (counter + 1) % 16;
    BN(counter, ledPins, ledPinsCount);
  }
  wasDown = isDown;
 }

  //if wasn't down but now is, do BN -
 bool isDown2 = digitalRead(button2_pin) == ON;
 if (wasDown2 != isDown2){
  if (isDown2){
    counter = (counter - 1) % 16;
    BN(counter, ledPins, ledPinsCount);
  }
  wasDown2 = isDown2;
 }
}

I have what I think should check if “button was held == button is held” and then run the desired code, but it just starts running the code automatically.
I have no idea why it starts adding as soon as my Arduino turns on, because when it just turned on, isDown and wasDown should NOT be equal, but they are somehow?

EDIT:
I have also tried checking the state of “button1_pin” with things like

if (isDown = ON)
if(isDown = OFF)

but if I did either:

if (isDown = ON && wasDown = ON)
if (isDown = OFF && wasDown = OFF)

it just did the same, and started adding/substracting on startup.
Which is understandable with is OFF and was OFF, but not the other one.

#2

Check out the state change detection example in the IDE (File->examples->02.Digital->StateChangeDetection) and you will learn how to detect when a button changes state and then you can act accordingly.

You also need to make your variable ‘lastTime’ unsigned long

1 Like
#3

Do you have pullup or pulldown resistors (10k) on your button pins so they are not floating when the buttons are not pressed?

1 Like
#4

Hi,
Can you please post a circuit diagram?
A pen(cil) and paper drawing will do, please include component labels and pin labels.

Thanks… Tom… :grinning: :+1: :coffee: :australia:

1 Like
#5

Its a Pull-Up resistor 10 kΩ.

#6

I have the Arduino UNO and the Arduino Multi-function SHIELD.
image
I am using the first two buttons, from the left, and the 4 led pins on the right.

#7

That is assignment, comparison is ‘==’

1 Like
#8

Oh oops, yeah I fixed that in the program immediately, forgot it was here too.

#9

I did, and it actually helped, thank you.
Did not help enough though, as I managed to find a different problem now.

#10

This is backward, if wired correctly, button is down (pressed) pin will be LOW (OFF).

1 Like
#11

Its NOT, and it is driving me crazy. The library we are supposed to use “funshield.h”, has “LOW = ON” and “HIGH = OFF”, this made looking online while I was completely new, very hard.
Because everything worked backwards.

#12

Alright so, my main problem now. That I figured out which stopped me from being able to do any working button hold, was the fact that my timing variables, which I simply wrote down and then never checked again, were “1 second” and 0.3 second" respectively.
Making any button press, shorter than 1 second is really hard, so it was just luck if it worked or not.

My current problem is here, this is my loop():

  isDown = digitalRead(button1_pin);
  unsigned long time = millis();  
  if (isDown == ON) {
    if (time > lastTime + activationDelay){
      counter = (counter + 1) % 16;
      BN(counter, ledPins, ledPinsCount);
      lastTime += activationDelay;
    }
  }

if (wasDown != isDown){
  if (isDown == ON){
    counter = (counter + 1) % 16;
    BN(counter, ledPins, ledPinsCount);
  }
  wasDown = isDown;
 }

The first one is supposed to run only if I hold down the button for “activationDelay” amount of time, and it does this and works fine when I hold down the button.
The second one works fine without the first one, but now when I press, I can press 2+ times.
I know it can sometimes press multiple times by accident, which is normal, but without the hold down code, 2+ times never happens.
Can someone please help me figure out where is the mistake here?

#13

I’m not exactly sure what you mean by press 2+ times??? Your code will run every ‘activationDelay’ milliseconds as long as the button is held down. Is that what you want? Or do you just want it to run once? It is always better to post your entire sketch after modifications…

#include "funshield.h"
//setting up
constexpr int ledPins[] { led1_pin, led2_pin, led3_pin, led4_pin };
constexpr int ledPinsCount = sizeof(ledPins) / sizeof(ledPins[0]);
constexpr int activationDelay = 1000;
constexpr int periodicDelay = 300;

unsigned long startTime;
int counter = 0;
int prevButtonState;

//Binary counter
void BN(int number, const int leds[], int ledsCount)
{
  for (int i = ledsCount - 1; i >= 0; --i) {
    bool currentBit = number & 1;
    digitalWrite(leds[i], currentBit ? ON : OFF);
    number = number >> 1;
  }
}

void setup() {
  for (int i = 0; i < ledPinsCount; ++i) {
    pinMode(ledPins[i], OUTPUT);
    digitalWrite(ledPins[i], OFF);
  }
  pinMode(button1_pin, INPUT);
  pinMode(button2_pin, INPUT);
}

void loop()
{
  unsigned long time = millis();

  //Checks if button is down
  int currentButtonState = digitalRead(button1_pin);

  if ( currentButtonState != prevButtonState ) {
    // state has changed, note the time
    startTime = time;
  }
  else {
    // state is the same so check for how long
    // maybe this is ON or OFF???
    if (currentButtonState == ON) {
      // button pressed so check the time
      if (time - startTime >= activationDelay) {
        counter = (counter + 1) % 16;
        BN(counter, ledPins, ledPinsCount);
        startTimeTime += activationDelay;
      }
    }
  }
  prevButtonState = currentButtonState;
}
#14

No, this is very common in low level electronics

#15

Thank you for the code, so here is mine “I ran out of idea, please work” disaster code:

#include "funshield.h"
//setting up
constexpr int ledPins[] { led1_pin, led2_pin, led3_pin, led4_pin };
constexpr int ledPinsCount = sizeof(ledPins) / sizeof(ledPins[0]);

constexpr int activationDelay = 1000;
constexpr int periodicDelay = 300;

unsigned long lastTime = 0;
unsigned long lastTime2 = 0;
bool firsttime = true;
bool firsttime2 = true;
int counter = 0;
bool wasDown = false;
bool wasDown2 = false;
int isDown = 0;
int isDown2 =0;

//Binary counter
void BN(int number, const int leds[], int ledsCount)
{
  for (int i = ledsCount-1; i >= 0; --i){
    bool currentBit = number & 1;
    digitalWrite(leds[i], currentBit ? ON : OFF);
    number = number >> 1;
  }
}

void setup(){
  for (int i = 0; i < ledPinsCount; ++i){
    pinMode(ledPins[i], OUTPUT);
    digitalWrite(ledPins[i], OFF);
  }
 pinMode(button1_pin, INPUT);
 pinMode(button2_pin, INPUT);

  lastTime = millis();
  lastTime2 = millis();
}

void loop()

{ //Checks if button is down
  isDown2 = digitalRead(button2_pin);
  isDown = digitalRead(button1_pin);
  unsigned long time = millis();
  //Holding the button for the first time
  if (isDown == ON && firsttime == true) {
    if (time > lastTime + activationDelay){
      counter = (counter + 1) % 16;
      BN(counter, ledPins, ledPinsCount);
      firsttime = false;
      lastTime += activationDelay;
    }
  }
  //Holding the button for 2+ loops
  if (isDown == ON && firsttime == false) {    
    if (time > lastTime + periodicDelay){
      counter = (counter + 1) % 16;
      BN(counter, ledPins, ledPinsCount);
      lastTime += periodicDelay;
    }
  }
  //stopped holding the button  
  if (isDown == OFF && firsttime == false) {
    firsttime = true;
    }
  
  //Holding the other button for the first time
  if (isDown2 == ON && firsttime2 == true) {
    if (time > lastTime2 + activationDelay){
      counter = (counter - 1) % 16;
      BN(counter, ledPins, ledPinsCount);
      firsttime2 = false;
      lastTime2 += activationDelay;
    }
  }
  //Holding the other button for 2+ loops
  if (isDown2 == ON && firsttime2 == false) {
    if (time > lastTime2 + periodicDelay){
      counter = (counter - 1) % 16;
      BN(counter, ledPins, ledPinsCount);
      lastTime2 += periodicDelay;
    }
  }
  //stopped holding the button  
    if (isDown2 == OFF && firsttime == false) {
    firsttime2 = true;
    }
  
  //if wasn't down but now is, I clicked first button so do BN +
 if (wasDown != isDown){
  if (isDown == ON){
    counter = (counter + 1) % 16;
    BN(counter, ledPins, ledPinsCount);
  }
  wasDown = isDown;
 }
  //if wasn't down but now is, I clicked second button so do BN -
 if (wasDown2 != isDown2){
  if (isDown2 == ON){
    counter = (counter - 1) % 16;
    BN(counter, ledPins, ledPinsCount);
  }
  wasDown2 = isDown2;
 } 
}

So what I want it to do are these things:

  1. Clicking button1 adds 1 to the binary counter and clicking button2 subtracts 1.
  2. Holding button1 for one second, adds 1 to the binary counter, then adds 1 every 0.3 seconds until you stop holding. Same for button2, but subtraction.
  3. Adding and subtraction can happen at the same time, so holding down both buttons could make the lights go “back and forth”

I can do 1. (as the first code I posted did) and the current code does 2. (I think), but not 1 anymore. As even if I make “activationDelay = 3000”, clicking sometimes (but only sometimes, why?) leads to the counter just…not working (ex: 1->3->9->14->15)
I have no idea why that is, as I have tried many things already, as you have seen with the other versions.

#16

Sample theory for one button. Two timers needed.

While the button is released both timers are held reset, ie. timer accumulated value = millis()

If button pressed accumulate millis counts on timer1 ‘til one thousand counts

When timer1 = 1K counts use state change detection to increment the binary counter (so it only registers one count).

At the same time, timer1 being done enables timer2, a 1/3 second timer, to run continuously.

When timer2 times out register a count the same way as for timer1 and then reset timer2.

Duplicate all the above for the subtraction button.

These techniques are illustrated in the timer counter state change toggle FSM tutorial

1 Like
#17

In fact, it is the normal way to do things. :grin: For a number of reasons.

1 Like
#18

This is an adaptation of the state change detection example

#include "funshield.h"
//setting up
constexpr int ledPins[] { led1_pin, led2_pin, led3_pin, led4_pin };
constexpr int ledPinsCount = sizeof(ledPins) / sizeof(ledPins[0]);

const unsigned long activationDelay = 1000;
const unsigned long periodicDelay = 300;

unsigned long startTime;
unsigned long startTime2;

unsigned long interval;
unsigned long interval2;

int counter = 0;
int wasDown;
int wasDown2;
int isDown = 0;
int isDown2 = 0;

//Binary counter
void BN(int number, const int leds[], int ledsCount)
{
  for (int i = ledsCount - 1; i >= 0; --i) {
    bool currentBit = number & 1;
    digitalWrite(leds[i], currentBit ? ON : OFF);
    number = number >> 1;
  }
}

void setup() {
  for (int i = 0; i < ledPinsCount; ++i) {
    pinMode(ledPins[i], OUTPUT);
    digitalWrite(ledPins[i], OFF);
  }
  pinMode(button1_pin, INPUT);
  pinMode(button2_pin, INPUT);

  lastTime = millis();
  lastTime2 = millis();
}

void loop()
{
  
  isDown2 = digitalRead(button2_pin);
  isDown = digitalRead(button1_pin);
  unsigned long time = millis();

  // check button1
  if ( isDown != wasDown ) {
    // state has changed
    if ( isDown == ON ) {
      // initial press, start timer and set interval
      startTime = millis();
      interval = activationDelay;
    }
    else {
      // button released, nothing to do here
    }
    delay(25);  // debounce a bit
  }
  else {
    // same state as last time through the loop, reset timer if not pressed
    if ( isDown == OFF ) {
      startTime = millis();
    }
  }
  wasDown = isDown;

  // check time to do work
  if (time - startTime >= interval) {
    counter = (counter + 1) % 16;
    BN(counter, ledPins, ledPinsCount);
    startTime = time;
    if ( interval == activationDelay ) {
      // time to shift to periodic delay
      interval = periodicDelay;
    }
  }

  // check button2
  if ( isDown2 != wasDown2 ) {
    // state has changed
    if ( isDown2 == ON ) {
      // initial press, start timer and set interval
      startTime2 = millis();
      interval2 = activationDelay;
    }
    else {
      // button released, nothing to do here
    }
    delay(25);  // debounce a bit
  }
  else {
    // same state as last time through the loop, reset timer if not pressed
    if ( isDown2 == OFF ) {
      startTime2 = millis();
    }
  }
  wasDown2 = isDown2;

  // check time to do work
  if (time - startTime2 >= interval2) {
    counter = (counter - 1) % 16;
    BN(counter, ledPins, ledPinsCount);
    startTime2 = time;
    if ( interval2 == activationDelay ) {
      // time to shift to periodic delay
      interval2 = periodicDelay;
    }
  }
}

Note: you really need to use subtraction in your time calculations to determine elapsed time, not addition to some future time.

1 Like
#19

Thank you for trying to help, I really appreciate but I am sorry to say that this doesn’t work at all.
It looks like it should, at least from my few weeks beginner perspective, but so does my not working code, so that doesn’t really say much.

Here is a review of what happens if I try the code:
One led is immediately on
Adding or substracting makes every other led light up immediately
Every other button press makes the lit on leds flicker
Holding down the adding button does nothing, but substraction makes one light flicker a lot.

I will however use substraction along with millis from now on, it never occurred to me as our professor uses adding…so I did as well without much thought.
Substraction makes more sense to my brain though.

#20

This helped me understand that I don’t understand millis correctly, so thank you for that.