Add a push button, help~~

Ok Some things here not quite right

  1. It is irrelevant how many times the button has been pressed you just need to know if it is an odd press or an even press. Therefore you don't need to turn off the LEDs at all.

  2. using a delay for debounce will take up time and mess up the state machine so you have to use the millis counter to get that delay, so the decision box "Has the button state changed" should have "and so long has passed since the last press".

  3. you don't need to check for a second time out if the LEDs are going to flash alternately.

  4. You should be looking to set or clear a boolean flag here and use it to bypass setting the LEDs to on.

Thanks @Grumpy_Mike for checking the drawing! Just to make sure we have a common understanding: The drawing shows the code as it comes from the TO , so I fully agree with your comments, this is what the TO should care for!

One of your findings (no. 1) has been partially my fault! I guess that the use of the modulo function and a different thread I was involved in, misled me to assume the TO would like to change Leds ever forth button press... Sorry for that :wink:

@andywwf : As your code is at the moment it actually runs through the if clause

  • Every loop immediately after start as long as no button was pressed (as buttonPushCounter is 0) and 0 % 3 == 0 is true!
  • It will stop running through the clause when you hit the button the first time!
  • After that it will go though the clause every third time (buttonPushCounter = [3, 6, 9,...]

I will give you @andywwf some further hints how you may proceed in a separate post soon.

Here are some hints:

  • Reduce the complexity of your sketch by solving the issues separately and in a sequence; especially as a beginner that is highly recommended!

You could work like this:

  • Start with a new sketch that handles the button press only! Make it work as you want (use Serial.println() only to check what it does!)
  • Make a separate sketch that only handles the time interval; use Serial.println() to check the results.
  • Make a separate sketch that lets the Leds blink, first in parallel, then think how to make it alternating
  • Then put the code to handle the button, the time intervals and the Leds in separate functions in one sketch. This allows you to keep loop() as small as possible and each task much easier to understand and debug.

Dear ec2021 & Grumpy_Mike,

@ec2021 @Grumpy_Mike
I am much appreciated for your kind instruction.

I learned so much from you, not the code only, and teach me how to consider the program before coding.

They leaved valuable experience to me, I will absorb your words seriously.

Once again, million thanks!!

Andy

Thanks! Would be nice to hear from you again and do not bother to post if you experience further problems. As long as you seriously try to make progress, there will always be supporters here!

I was going to just post this code and say 'this is how I would do it' but I can see you're keen to learn so I've gone to town with the comments for you too. (It's much shorter without the comments :smiley: )

If there's anything you don't understand, please ask:

Note: Notice the change to the button pin, and the change to INPUT_PULLUP for the button, meaning no need for an external resisitor. Just connect to a pin, and to ground. This also changes the input to active LOW, meaning the pin goes LOW when the button is pressed.

#define blueLed 3    //Police light A
#define greenLed 2   //Police light B
#define buttonPin 4 // You had this ALSO defined as 2 on a previous post
#define flashInterval 150 // sets the LED toggle interval (so 150ms mean lights will flash on every 300ms)
#define debounceLength 10 // defines the length of debounce needed for the button

uint32_t currentTime; // a global 'shared' variable for holding the current time
static bool lightsOn = false; // a bool for holding if the lights are currently on or off.

void setup() {
  pinMode(blueLed, OUTPUT);  
  pinMode(greenLed, OUTPUT);  
  pinMode(buttonPin, INPUT_PULLUP);
  Serial.begin(9600);
}
  
void loop() {
  currentTime = millis(); // update the current time
  toggleLights(); // checks for button presses and toggles if the lights are on / off
  if (lightsOn)
    flashLights(); // if the toggle for lights is on, this will flash them at the flashInterval defined above.
}


void toggleLights() {
  // a variable to capture the current time when a button is pressed. Used in debouncing.
  static uint32_t capturedTimeButton;
  // a variable to store the last previous state of the button, last time through loop.
  static bool lastButtonState = HIGH;

  bool currentButtonState = digitalRead(buttonPin); // read the button pin
  if (currentButtonState != lastButtonState) { // if the button state has changed...
   if (!currentButtonState) // and the current state is 'low/false' (i.e the button pin is being grounded) 
    capturedTimeButton = currentTime; // then we capture last time we recorded a grounding on the button pin

    // if it's at least 10ms (defined as debounceLength above) after the last time we recorded a button pin grounding
    if (currentTime - capturedTimeButton >= debounceLength) { 
      if (!lightsOn) // if the lights are currently NOT active...
        lightsOn = true; // we make them active
      else { // if they ARE currently active 
        lightsOn = false; // we turn the lights toggle off
        digitalWrite(blueLed, LOW); // and actually turns the LED pins off
        digitalWrite(greenLed, LOW);
        }
    }
  }
  // then we update the 'lastButtonsState' to match the current state so we can check 'if the button state has changed'
  // on the next iteration through the loop.
  lastButtonState = currentButtonState; 
}

void flashLights() {
  // a variable for storing the last time the LEDs were toggled
  static uint32_t capturedTimeLights;

    // if it's at least 150ms (defined above as flashInterval) after the last time we captured the time for the lights
    if ((currentTime - capturedTimeLights) >= flashInterval) {
      // then we get a new time capture for the lights
      capturedTimeLights = currentTime;
      // if blueLed is off, we set it on, If it is on, we set it off.
      digitalWrite(blueLed, !digitalRead(blueLed));
      // what ever blueLed is doing, we want greenLed to do the opposite
      digitalWrite(greenLed, !digitalRead(blueLed));
  }
}

See it in action here: https://wokwi.com/arduino/projects/320703604884243028

That said....

Using a button library can often make things simpler too (plenty available for Arduino).
Here's how I would do it using a button library:

I am :smiley: (just reading back over the thread)

You should have more respect for yourself.

Hey, I could use the practice.

Dear st3v3n92,

Thank you very much.
I have tried and it is work as my thought, so wonderful.

I will study it later, because I want to complete my sketch2 first, and turn back to sketch1.

Anyway, many thanks.

Wish good health to everyone!!

Dear @ec2021 & @Grumpy_Mike,

According your above instructions, I have separated to process sketches.

Separate sketch A of stateChangeDetection:

For the button default OFF, I have solved.

Separate sketch B of blinkWithoutDelay:

For the alternation blink, I have also solved, all of blink under control.

But when I combined with one sketch, the alternating blink doesn’t work (Pin 8, 9, 10, 11), they are blinking with one frequency.

Estimated something wrong of sign “{“ and “}”, I tried to put them to other positions, the result worse than before.

Would you please let me know am I estimated right?
Thankyou!!

// constants won't change. Used here to set a pin number:
#define ledPin1 8      // police light A
#define ledPin2 9      // police light B
#define ledPin3 10     // Airplane light A
#define ledPin4 11     // Airplane light B
#define ledPin5 12     // Keep continus bright


#define WW_ON 30
#define WW_OFF 180
#define WW_ON2 60
#define WW_OFF2 300



const int  buttonPin = 2;   // the pin that the pushbutton is attached to

// Variables will change:
int ledState1 = 0; // ledState used to set the LED
int ledState2 = 0;
int ledState3 = 0;
int ledState4 = 0;

// Variables will change:
int buttonPushCounter = -2;   // counter for the number of button presses //because if (buttonPushCounter % 3 == 0), so -2 is default LEDs OFF
int buttonState = 0;         // current state of the button
int lastButtonState = 0;     // previous state of the button

// Generally, you should use "unsigned long" for variables that hold time
// The value will quickly become too large for an int to store
unsigned long previousMillis = 0;        // will store last time LED was updated
unsigned long previousMillis2 = 0;

unsigned long FLASH_START_TIME = 0;
unsigned long FLASH_START_TIME2 = 0;
unsigned long FLASH_START_TIME3 = 0;
unsigned long FLASH_START_TIME4 = 0;
 
// constants won't change:
const long interval = 500; // interval at which to blink (milliseconds)
const long interval2 = 100;

void setup() {
  // set the digital pin as output:
  pinMode(ledPin1, OUTPUT);
  pinMode(ledPin2, OUTPUT);
  pinMode(ledPin3, OUTPUT);
  pinMode(ledPin4, OUTPUT);
  pinMode(ledPin5, OUTPUT);
  
  pinMode(buttonPin, INPUT);  // initialize the button pin as a input:
  Serial.begin(9600);         // initialize serial communication:
}

void loop() 
{
  // read the pushbutton input pin:
  buttonState = digitalRead(buttonPin);

  // compare the buttonState to its previous state
  if (buttonState != lastButtonState) {
    // if the state has changed, increment the counter
    if (buttonState == HIGH) {
      // if the current state is HIGH then the button went from off to on:
      buttonPushCounter++;
      Serial.println("on");
      Serial.print("number of button pushes: ");
      Serial.println(buttonPushCounter);
    } else {
      // if the current state is LOW then the button went from on to off:
      Serial.println("off");
    }
    // Delay a little bit to avoid bouncing
    delay(50);
  }
  // save the current state as the last state, for next time through the loop
  lastButtonState = buttonState;

  
  // here is where you'd put code that needs to be running all the time.

  // check to see if it's time to blink the LED; that is, if the difference
  // between the current time and last time you blinked the LED is bigger than
  // the interval at which you want to blink the LED.
  unsigned long currentMillis = millis();

  // turns on the LED every four button pushes by checking the modulo of the
  // button push counter. the modulo function gives you the remainder of the
  // division of two numbers:
  if (buttonPushCounter % 3 == 0) {
//.......................................................................... LED1
if(millis()-FLASH_START_TIME<=WW_ON2)
   digitalWrite(ledPin1,HIGH);
  else{
      if(millis()-FLASH_START_TIME<=WW_ON2*2)
         digitalWrite(ledPin1,LOW);
        else{
            if(millis()-FLASH_START_TIME<=WW_ON2*3)
               digitalWrite(ledPin1,HIGH);
              else{
                  if (millis()-FLASH_START_TIME<=WW_ON2*4)
                      digitalWrite(ledPin1,LOW);
                     else{
                         if(millis()-FLASH_START_TIME<=WW_ON2*5)
                            digitalWrite(ledPin1,HIGH);
                           else{
                               if(millis()-FLASH_START_TIME<=(WW_ON2*5+WW_OFF2))
                                  digitalWrite(ledPin1, LOW);
                                 else{
                                     FLASH_START_TIME=millis();
                                 }
                           }
                     }
              }
        }
  }

//.......................................................................... LED2
static long FLASH_START_TIME3 = 300;
if(((long)millis()-FLASH_START_TIME3)<=WW_ON2)
   digitalWrite(ledPin2,HIGH);
  else{
      if(millis()-FLASH_START_TIME3<=WW_ON2*2)
         digitalWrite(ledPin2,LOW);
        else{
            if(millis()-FLASH_START_TIME3<=WW_ON2*3)
               digitalWrite(ledPin2,HIGH);
              else{
                  if (millis()-FLASH_START_TIME3<=WW_ON2*4)
                      digitalWrite(ledPin2,LOW);
                     else{
                         if(millis()-FLASH_START_TIME3<=WW_ON2*5)
                            digitalWrite(ledPin2,HIGH);
                           else{
                               if(millis()-FLASH_START_TIME3<=(WW_ON2*5+WW_OFF2))
                                  digitalWrite(ledPin2,LOW);
                                 else{
                                     FLASH_START_TIME3=millis();
                                 }
                           }
                     }
              }
        }
  }
  
//.......................................................................... LED3
if(millis()-FLASH_START_TIME2<=WW_ON)
   digitalWrite(ledPin3,HIGH);
  else{
      if(millis()-FLASH_START_TIME2<=WW_ON*2)
         digitalWrite(ledPin3,LOW);
        else{
            if(millis()-FLASH_START_TIME2<=WW_ON*3)
               digitalWrite(ledPin3,HIGH);
              else{
                  if (millis()-FLASH_START_TIME2<=WW_ON*4)
                      digitalWrite(ledPin3,LOW);
                     else{
                         if(millis()-FLASH_START_TIME2<=WW_ON*5)
                            digitalWrite(ledPin3,HIGH);
                           else{
                               if(millis()-FLASH_START_TIME2<=(WW_ON*5+WW_OFF))
                                  digitalWrite(ledPin3,LOW);
                                 else{
                                     FLASH_START_TIME2=millis();
                                 }
                           }
                     }
              }
        }
  }

//.......................................................................... LED4
static long FLASH_START_TIME4=180;
if(((long)millis()-FLASH_START_TIME4)<=WW_ON)
   digitalWrite(ledPin4,HIGH);
  else{
      if(millis()-FLASH_START_TIME4<=WW_ON*2)
         digitalWrite(ledPin4,LOW);
        else{
            if(millis()-FLASH_START_TIME4<=WW_ON*3)
               digitalWrite(ledPin4,HIGH);
              else{
                  if (millis()-FLASH_START_TIME4<=WW_ON*4)
                      digitalWrite(ledPin4,LOW);
                     else{
                         if(millis()-FLASH_START_TIME4<=WW_ON*5)
                            digitalWrite(ledPin4,HIGH);
                           else{
                               if(millis()-FLASH_START_TIME4<=(WW_ON*5+WW_OFF))
                                  digitalWrite(ledPin4,LOW);
                                 else{
                                     FLASH_START_TIME4=millis();
                                 }
                           }
                     }
              }
        }
  }
 //.......................................................................... LED5
    digitalWrite(ledPin5,HIGH);
    
    
     
    // set the LED with the ledState of the variable:
    digitalWrite(ledPin1, ledState1);
    digitalWrite(ledPin2, ledState2);
    digitalWrite(ledPin3, ledState3);
    digitalWrite(ledPin4, ledState4);
    
    
  }else {
    digitalWrite(ledPin1, LOW);
    digitalWrite(ledPin2, LOW);
    digitalWrite(ledPin3, LOW);
    digitalWrite(ledPin4, LOW);
    digitalWrite(ledPin5, LOW);
   }
}

I guess you have had a hard time to figure out where the problem is in your sketch, didn't you?
Reason is that you have made it difficult for you (but also even experienced developers) to understand what the program shall achieve ... :innocent:

  • Too many connected if/else clauses are hard to follow
  • Too much code inside loop()
    ** Too much code repetition
  • Complex calculation of interval times
  • You solved the button issue with a nice trick starting with -2, but there is a way to count from 0!

I am not sure if I understood all your objectives clear enough:

  • Do you want your program to count to 3 or to four to blink? In the comments there is something like "every forth press" but modulo 3 will get 0 every third press
  • Do you want each of the two (police or airplane) lights to toggle?
  • Do you want to have a different blink intervalls for police compared to airplane?

In the appended sketch I have changed your input as follows

  • Every forth button press the police and airplane Leds will start to blink
  • Police leds will toggle (if one is on the other is off)
  • Police leds will blink with intervalPolice = 500 msec
  • Airplane leds will toggle (if one is on the other is off)
  • Airplane leds will blink with intervalAirplane = 100 msec
// constants won't change. Used here to set a pin number:
#define ledPin1 8      // police light A
#define ledPin2 9      // police light B
#define ledPin3 10     // Airplane light A
#define ledPin4 11     // Airplane light B
#define ledPin5 12     // Keep continuous bright
#define buttonPin 2    // the pin that the pushbutton is attached to

// Variables will change:
int buttonPushCounter = 0;   // counter for the number of button presses;
int buttonState       = 0;         // current state of the button
int lastButtonState   = 0;     // previous state of the button

unsigned long previousMillisPolice = 0;        // will store last time LED was updated
unsigned long previousMillisAirplane = 0;
unsigned long currentMillis;

unsigned long intervalAirplane   = 100;
unsigned long intervalPolice     = 500;

void SwitchLED(int pin, int toThisState) {
  digitalWrite(pin, toThisState);
}

void SwitchPoliceAndAirplaneLedsOff() {
  SwitchLED(ledPin1, LOW);
  SwitchLED(ledPin2, LOW);
  SwitchLED(ledPin3, LOW);
  SwitchLED(ledPin4, LOW);
}

void TogglePolice() {
  SwitchLED(ledPin3, !digitalRead(ledPin3)); // Pin3 shall toggle
  SwitchLED(ledPin4, !digitalRead(ledPin3)); // Pin4 shall have the opposite state of Pin3
}

void ToggleAirplane() {
  SwitchLED(ledPin1, !digitalRead(ledPin1)); // Pin1 shall toggle
  SwitchLED(ledPin2, !digitalRead(ledPin1)); // Pin2 shall have the opposite state of Pin1
}

void HandleButton() {
  buttonState = digitalRead(buttonPin);   // read the pushbutton input pin
  if (buttonState != lastButtonState) {   // compare the buttonState to its previous state
    if (buttonState == HIGH) {            // if the state has changed to HIGH, increment the counter
      buttonPushCounter++;
      Serial.println("on");
      Serial.print("number of button pushes: ");
      Serial.println(buttonPushCounter);
    } else {
      // if the current state is LOW then the button went from on to off:
      Serial.println("off");
    }
    // Delay a little bit to avoid bouncing
    delay(50);
  }
  // save the current state as the last state, for next time through the loop
  lastButtonState = buttonState;
}

void LetPoliceBlink() {
  if (currentMillis - previousMillisPolice > intervalPolice) {
    previousMillisPolice = currentMillis;
    TogglePolice();
  }
}

void LetAirplaneBlink() {
  if (currentMillis - previousMillisAirplane > intervalAirplane) {
    previousMillisAirplane = currentMillis;
    ToggleAirplane();
  }
}


void setup() {
  // set the digital pin as output:
  pinMode(ledPin1, OUTPUT);
  pinMode(ledPin2, OUTPUT);
  pinMode(ledPin3, OUTPUT);
  pinMode(ledPin4, OUTPUT);
  pinMode(ledPin5, OUTPUT);
  digitalWrite(ledPin5, HIGH); // Led 5 shall be on all the time ?!?

  SwitchPoliceAndAirplaneLedsOff();

  pinMode(buttonPin, INPUT);  // initialize the button pin as a input:
  Serial.begin(9600);         // initialize serial communication:
}

void loop() {
  HandleButton();
  if (buttonPushCounter > 0 && buttonPushCounter % 4 == 0) {
    currentMillis = millis();
    LetPoliceBlink();
    LetAirplaneBlink();
  } else {
    SwitchPoliceAndAirplaneLedsOff();
  }
}


Dear ec2021,

First at all, I am sorry for made you confused, this is my fault not explanation clear.

I don’t need convert mode by button counter, just all of ON by pressed 2 times, then all of OFF by next pressed only.

I have modified alternating blink from 1 times instead of 2 times deliberately, because I would like to improve myself.
(alternating blink 1 time, I broke through already.)

My sketch script as below:
Push Button:
Press 2 times -> ON = for all LEDs
Press 1 times -> OFF = for all LEDs

Police light A/B: (in SLOW frequency)
A blinks 2 times -> B blink 2 times -> …….. loop
Blink 2 times each then alternate to others

Airplane light A/B: (in FAST frequency)
A blinks 2 times -> B blink 2 times -> …….. loop
Blink 2 times each then alternate to others

ledPin5 :
Just keep bright, nothing special.

Nowaday, I knew my sketch is not clever even foolish.
An “OUTER if/else” included too many “INNER if/else” inside, it is so difficult to find the problem if error.
I have separated the sketch for test the blink function, it is work when it alone.
But out of control when combined with button, so I estimated something wrong of sign “{“ and “}”, I tried to put them to other positions, the result worse than before.

But if I broke through this problem, I will have a basic skill for handle other type of blinks in forthcoming program.

@ec2021 @Grumpy_Mike @st3v3n92
I understood that you are not responsible to answer & help me, all of replies depended good intention, I am really much appreciated and respect of that.
Thank you very much!

Best Regards,
Andy

PS.
I find something when I tried to repair the problems.

When I modified below:
int buttonPushCounter = 0 //default All LEDs ON when power connect.

The result is:

POWER connected

All of LEDs are blinks on schedule as perfect.

Press Button 1 time for all LEDs OFF.

Press Button 2 time for all LEDs ON again.
They are still blink but alternating function disappeared.

Hi,

Why 2 times ON annd 1 time OFF?

Why not configure your software as a simple toggle.
1 press ON.
1 press OFF.

Why 2 presses?

Add another button to select mode?
Keep it simple.

Can you please post a circuit diagram of your project?

Tom.... :grinning: :+1: :coffee: :australia:
PS, If ON then next press OFF, If OFF then next press ON, simple logic.

I like yours better because my toggle function violates the Single Responsibility Principle. I was being lazy I guess :smiley:

Dear TomGeorge,

Thank you for your reply.

I am a freshman of Arduino.

This is my first program I refer to Arduino template “blinkWithoutDelay” and “stateChangeDetection”, and combined them with my designed blink style.

(Of course there are too many nice guys teach me a lot, so I can ask some simple question now.)

“if (buttonPushCounter % 4 == 0)”

This code command comes from stateChangeDetection, then I slightly adjust it to 3 for ON/OFF. (2 ON / 1 OFF)

I am afraid I can’t find a problem when error, so that I keep this code to avoid too many change on structure, change as little as possible.

Sorry that I don’t know how to draw a circuit diagram to you.

Each LED connected 220Ω resistance on the anode pin
Button connected Arduino on broad 5V, other button pin connected 10KΩ resistance to pin 2.

English is not my mother language, hope I can explain clear.

Best Regards,

Andy

Hi, @andywwf

I hope you have your switch wired like this.

Tom... :grinning: :+1: :coffee: :australia:

Dear st3v3n92,

I like yours too, simply and work.

But I can't understand your code at this moment, I have to improve myself.

When I completed current project, I will back to learn your code which you have been sent to me.

Thanks,

Andy

What is the difference between of Pin 2 and Pin4?