Delay not working when pressing a button

graylight64 · May 30, 2022, 3:10pm

Currently writing code that begins with an if statement in the loop function that is meant to add to a count every time the button is pressed so that each time the button is pressed, the LED lights behave differently. The if statement basically is meant to add to the integer "count" then a delay
added for a second to allow the button to close again before checking if the button is pressed. to check this is working, I'm printing the value of count every time it changes.

However, this delay is not working how I intend it to. the button is adding to the count multiple times every time I press the button. Is there something missing in this if statement? every example I've seen is the same as how I've done it.

The screenshot of the circuit diagram below, the code below too. I will add that there is an external input signal in pin 3 (normally 2 but circuito.io would let me swap the pins)

The part I'm having issue with:

const byte buttonPin = 4; 
byte count = 0; // counts the amount of times the button has been pressed

void setup() {
digitalWrite(buttonPin, HIGH);
}

void loop() {
  if (digitalRead(buttonPin) == HIGH) {
    count++; //increase the counter once the button is released
    Serial.println(count);
  } 
}

The entire code:

const byte redLEDPin = 5; // set red led output to 5 (PWM pin, not sure if that is goingto beb necessary since its a logical output )
const byte greenLEDPin = 6; // set green led to 6 (PWM pin) 
const byte buttonPin = 2; 


byte count = 0; // counts the amount of times the button has been pressed


volatile int redLEDState; // initial state of led, let us know if the circuit is on 
volatile int greenLEDState;



void setup() {
  // put your setup code here, to run once:
  Serial.begin(9600);
  pinMode(redLEDPin, OUTPUT);
  pinMode(greenLEDPin, OUTPUT);
  pinMode(buttonPin, INPUT_PULLUP);
  pinMode(2, INPUT);


  redLEDState = HIGH;
  greenLEDState = LOW;
  digitalWrite(redLEDPin, redLEDState);
  digitalWrite(greenLEDPin, greenLEDState);

  digitalWrite(buttonPin, HIGH);
  
  attachInterrupt(digitalPinToInterrupt(3), PulseISR, CHANGE);
  noInterrupts();
}


void loop() {

  if (digitalRead(buttonPin) == HIGH) {
    count++; //increase the counter once the button is released
    Serial.println(count);
  } 

  
  // MODES 

  // MODE 1: INITAL PHASE 
  if (count == 0) { 
  noInterrupts();
  redLEDState = 1;
  greenLEDState = 1;
  digitalWrite(redLEDPin, redLEDState);
  digitalWrite(greenLEDPin, greenLEDState);
  delay(1000);
  redLEDState = 0;
  greenLEDState = 0;
  digitalWrite(redLEDPin, redLEDState);
  digitalWrite(greenLEDPin, greenLEDState);
  delay(1000);
  }
  

 // MODE 2: constant red mode    
  if (count == 1) { 
   redLEDState = HIGH;
   greenLEDState = LOW;
   digitalWrite(redLEDPin,redLEDState);
   digitalWrite(greenLEDPin,greenLEDState);
  }

  // MODE 3: constant green mode 
  if (count == 2) {   
   redLEDState = LOW;
   greenLEDState = HIGH;
   digitalWrite(redLEDPin,redLEDState);
   digitalWrite(greenLEDPin,greenLEDState);
  }


  // MODE 4: aLternating LEDS mode  
  if (count == 3) {
   redLEDState = HIGH;
   greenLEDState = LOW;
   digitalWrite(redLEDPin,redLEDState);
   digitalWrite(greenLEDPin,greenLEDState);
   interrupts();
  }


 // RESET 
   if (count == 4) {
    count = 0;
  }
}

void PulseISR() {
    if (redLEDState == HIGH) {
    redLEDState = LOW;
    greenLEDState = HIGH;
    digitalWrite(redLEDPin, redLEDState);
    digitalWrite(greenLEDPin, greenLEDState);
  }
  else if (greenLEDState == HIGH) {
    redLEDState = HIGH;
    greenLEDState = LOW;
    digitalWrite(redLEDPin, redLEDState);
    digitalWrite(greenLEDPin, greenLEDState);
  }
  else {
    Serial.print("error");
  }
}

Grumpy_Mike · May 30, 2022, 3:18pm

@graylight64 you posted in the wrong section, read the header and guess why. So moved it here.

What you are doing wrong is you are counting all the time on the state of the input button, not when the button becomes pressed or becomes unpressed.

You need to have a variable to keep track of the last state of this button, updated at the end of your loop and test when the current state is not equal to the last state.

There is an example of this technique in the IDE examples menu called state change detection.

There is absolutely no need to use interrupts at all.

Are you sure this code is yours? Because I have seen this wrong structure several times this last week.

LarryD · May 30, 2022, 3:24pm

It is recommended you wire switches as S3 in the schematic below.

DrDiettrich · May 30, 2022, 3:25pm

It cannot work with interrupts offl

groundFungus · May 30, 2022, 4:10pm

State change and switch wiring for active low inputs

graylight64 · May 30, 2022, 4:36pm

Definitely my code, they say great minds think alike but maybe the less great ones occasionally do too.

I've rewritten the code with what I could understand of what you've said. The code works as intended now, but still requires that delay in there. Is this what you meant or is there something I'm missing?

const byte buttonPin = 3;
byte count = 0; 

int buttonState = LOW; 
int lastButtonState = LOW; 


void setup() {
  // put your setup code here, to run once:
  Serial.begin(9600);
  pinMode(buttonPin, INPUT_PULLUP); 
  digitalWrite(buttonPin, buttonState);
}

void loop() {
  buttonState = digitalRead(buttonPin);

  if (buttonState != lastButtonState){
   if (buttonState == HIGH) {
    count++; 
    Serial.println(count);
    delay(1000);
   }
  lastButtonState = buttonState;
  }
}

In terms of the interrupt, one of the modes for the LEDs is alternating the LEDs to a pulse width signal input into the interrupt pin that is fluctuating in frequency. The only way I've been able to effectively find when the signal is changing from low to high state is with the ISR. The original code did not include interrupt() and noInterrupts(), and I may be using these wrong as i have a pretty barebones idea of how to use them, but I don't think the code will be abe to do what I need it to do without the pulseISR function. I'll insert the original code if that makes what I'm trying to apply the ISR to a little easier to understand:


void setup() {
  // put your setup code here, to run once:
Serial.begin(9600);
  pinMode(redLEDPin, OUTPUT);
  pinMode(greenLEDPin, OUTPUT);
  pinMode(2, INPUT);


  redLEDState = HIGH;
  greenLEDState = LOW;
  digitalWrite(redLEDPin, redLEDState);
  digitalWrite(greenLEDPin, greenLEDState);
  
  attachInterrupt(digitalPinToInterrupt(2), PulseISR, CHANGE);
}

void loop() {
Serial.println(digitalRead(2));
}

void PulseISR() {
    if (redLEDState == HIGH) {
    redLEDState = LOW;
    greenLEDState = HIGH;
    digitalWrite(redLEDPin, redLEDState);
    digitalWrite(greenLEDPin, greenLEDState);
  }
  else if (greenLEDState == HIGH) {
    redLEDState = HIGH;
    greenLEDState = LOW;
    digitalWrite(redLEDPin, redLEDState);
    digitalWrite(greenLEDPin, greenLEDState);
  }
  else {
    Serial.print("error");
  }
}

graylight64 · May 30, 2022, 4:41pm

Also missed your comment about IDE example, I've found it and am going to try it out

LarryD · May 30, 2022, 5:35pm

Do you know what a State Machine is ?

//********************************************^************************************************
//  FileName.ino
//
//  Version   YY/MM/DD     Comments
//  =======   ========     ========================================================
//  1.00      YY/MM/DD     Running code
//
//
//
//********************************************^************************************************


#define LEDon                                HIGH
#define LEDoff                               LOW

#define ENABLED                              true
#define DISABLED                             false

#define PUSHED                               HIGH
#define RELEASED                             LOW

enum Machine1 {counter0 = 0 , counter1, counter2, counter3};
Machine1 mSate = counter0;

//********************************************^************************************************

const byte heartbeatLED                    = 13;

const byte redLEDPin                       = 5;
const byte greenLEDPin                     = 6;

const byte buttonPin                       = 2;

byte count                                 = 0;
byte lastButtonState                       = RELEASED;


//Timing stuff
unsigned long heartbeatMillis;
unsigned long switchMillis;
unsigned long commonMillis;

const unsigned long heartBeatInterval      = 500ul;      //1/2 second
const unsigned long sampleInterval         = 50ul;       //50 milliseconds

unsigned long commonInterval; 

//                                       s e t u p ( )
//********************************************^************************************************
//
void setup()
{
  Serial.begin(115200);                // <-----<<<<<  Change Baud Rate

  pinMode(heartbeatLED , OUTPUT);
  pinMode(redLEDPin, OUTPUT);
  pinMode(greenLEDPin, OUTPUT);

  pinMode(buttonPin, INPUT);

} //END of   setup()


//                                       l o o p ( )
//********************************************^************************************************
//
void loop()
{
  //*********************************
  //is it time to toggle the heartbeatLED ?
  if (millis() - heartbeatMillis >= heartBeatInterval)
  {
    //restart this TIMER
    heartbeatMillis = millis();

    //toggle the heartbeatLED
    digitalWrite(heartbeatLED, !digitalRead(heartbeatLED));

  }

  //*********************************
  //is it time to check the switches ?
  if (millis() - switchMillis >= sampleInterval)
  {
    //restart this TIMER
    switchMillis = millis();

    checkSwitches();

  }

  //*********************************
  //service the State Machine
  stateMachine();


  //*********************************
  //other non blocking code
  //*********************************

} //END of   loop()


//                                s t a t e M a c h i n e ( )
//********************************************^************************************************
//
void stateMachine()
{
  switch (mSate)
  {
    //***************************
    case counter0:
      {
        Serial.print("I am in Machine State ");
        Serial.println(mSate);

        digitalWrite(redLEDPin, LEDoff);
        digitalWrite(greenLEDPin, LEDoff);        
      }
      break;

    //***************************
    case counter1:
      {
        Serial.print("I am in Machine State ");
        Serial.println(mSate);

        digitalWrite(redLEDPin, LEDon);
        digitalWrite(greenLEDPin, LEDoff);    
      }
      break;

    //***************************
    case counter2:
      {
        Serial.print("I am in Machine State ");
        Serial.println(mSate);

        digitalWrite(redLEDPin, LEDoff);
        digitalWrite(greenLEDPin, LEDon);            
      }
      break;

    //***************************
    case counter3:
      {
        Serial.print("I am in Machine State ");
        Serial.println(mSate);

        digitalWrite(redLEDPin, LEDon);
        digitalWrite(greenLEDPin, LEDon);            
      }
      break;

  } //END of   switch(sMachine)

} //END of    stateMachine()


//                              c h e c k S w i t c h e s ( )
//********************************************^************************************************
//
void checkSwitches()
{
  byte currentState;

  //*********************************                                        b u t t o n P i n
  currentState = digitalRead(buttonPin);

  //has this switch changed state ?
  if (lastButtonState != currentState)
  {
    //update to the new switch state
    lastButtonState = currentState;

    //******************
    //has the switch been pushed ?
    if (currentState == PUSHED)
    {
      count++;
      if (count == 4)
      {
        count = 0;
      }

      mSate = count;
    }

    //******************
    //switch must have been released
    else
    {
      //do somthing

    }

  } //END of    if (lastButtonState != currentState)


  //*********************************
  //future switches go here
  //*********************************

} //END of   checkSwitches()


//********************************************^************************************************
//                            E N D   o f   s k e t c h   c o d e
//********************************************^************************************************

do_not_sleep · May 30, 2022, 5:42pm

also i would through in a micro second delay in there for debouncing just to cover all the bases im sure you know that already

anon57585045 · May 30, 2022, 5:47pm

That would be a few orders of magnitude too short.

do_not_sleep · May 30, 2022, 5:47pm

"everything hurts or everything helps,but EVERYTHING counts"

anon57585045 · May 30, 2022, 5:51pm

It's true, the push button switch has to be de-bounced. The symptoms were mentioned right at the beginning.

cncnotes · May 30, 2022, 5:57pm

Is this the only part of the code that is not working as you intended?
If that is the case, button bounce is the likely culprit, as mentioned by @do_not_sleep, though some buttons can bounce for up to 20 ms. Do please look up software de-bouncing.

Grumpy_Mike · May 31, 2022, 4:14am

What switches between these two modes? I see nothing in the code that does that.

How fast is the PWM signal going to be?
Any faster than 30 times a second and it will look like both LEDs are on at the same time.

Am I right in thinking there is only one input pin and you want that pin to do two different jobs

generate an interrupt to swap over what light is on.
count the number of times it has been pressed.

You can't do that with one pin because every time it changes state both things will happen.
The delay in the counting part stops the counting for 1 second but an interrupt is still generated.

As I said before there is no need to use interrupts.

system · November 27, 2022, 4:15am

This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.

Topic		Replies	Views
Query Programming Questions	5	773	May 5, 2021
Using the if statement with a time delay Programming Questions	14	15762	May 5, 2021
Having trouble working out delay with button Programming Questions	3	740	May 5, 2021
Delay Problem. Can't understanding where to put delay in if else statements. Programming Questions	13	998	May 5, 2021
NEWBIE here. 1 button to control 2 leds differently. Project Guidance	3	926	May 5, 2021

Delay not working when pressing a button

Related topics