(SOLVED) Push button to Blink LED

I'm really new to this, and I've done lots of research and examined many sketches, but I just can't figure out what I'm doing wrong. I have a push button. Pressed once, I want my led to blink a certain duration on and off. I have one line of code which blinks the way I want, without delay, and works just fine on it's own. But that code won't work when the button is pushed. The led just lights solid and does not blink and do my code. What am I missing. And this code is being used on an Adafruit Trinket. Sorry so new.

// Sketch to turn on & off blinking LED when push button is pressed


//Constants
const int buttonPin = 0;   
const int PIN = 1;    

//Variables
int buttonState = 0;
int flag=0;

void setup() {

  //Input or output?
  pinMode(PIN, OUTPUT);      
  pinMode(buttonPin, INPUT_PULLUP);   
  digitalWrite(PIN, LOW);
}


void loop(){

  //Read button state 
  buttonState = digitalRead(buttonPin);

  //If button pressed...
  if (buttonState == LOW) { 
    //turn led on & blink
    if ( flag == 0){
      digitalWrite(PIN, (millis() / 800) %4); // blinks LED roughly 2 sec. on, 1 sec. off
      flag=1; //change flag variable
    }

    //button pressed again, turn led off
    else if ( flag == 1){
      digitalWrite(PIN, LOW);
      flag=0; //change flag variable again 
    }    
  }
}
1 Like

The first time you press the button, flag is 0 so you do the digitalWrite and set flag to 1. The next time through the loop, the button is still pressed and flag is 1 so you skip the digitalWrite and then turn the LED off. The next time through, you turn it back on.

You are doing this so fast, you can't see it actually blinking.

Your logic is not correct.

This line:

digitalWrite(PIN, (millis() / 800) %4); // blinks LED roughly 2 sec. on, 1 sec. off

makes absolutely no sense whatsoever.

Read this.

The LED is probably blinking, but thousands of times a second. You are simply cycling through your if loop while the button is low.

SteveMann:
This line:

digitalWrite(PIN, (millis() / 800) %4); // blinks LED roughly 2 sec. on, 1 sec. off

makes absolutely no sense whatsoever.

Read this.

The LED is probably blinking, but thousands of times a second. You are simply cycling through your if loop while the button is low.

OK.

I know how to blink with delay, but because of other coding that will be going on, I can't have it interrupted from delays.. And I can do blinking without delay using millis, which also doesn't work when in the above code. This single line I found on a tutorial site, and works great on it's own and is super simple. Blinks the LED just fine. But I understand at least what you're saying about cycling through so fast.

Can it be done with a case switch? The whole issue is trying to blink without delay, but also at a different on interval than the off interval. Not just a simple blink.

Pressed once, I want my led to blink a certain duration on and off.

The whole issue is trying to blink without delay, but also at a different on interval than the off interval. Not just a simple blink.

Please clarify before we go into coding advice. Does the "off" interval include time waiting for the next keypress? Blink once? Twice? Continuously? Walk us through it step by step. Don't leave out any details.

aarg:
Please clarify before we go into coding advice. Does the "off" interval include time waiting for the next keypress? Blink once? Twice? Continuously? Walk us through it step by step. Don't leave out any details.

It's an alarm that is being activated. So when the button is pressed, the LED begins to blink, 2 seconds on, 1 second off (roughly) and continues to blink until button is pressed again, turning it off. I could do it easily with a latching button, but I need to use a momentary button, which is where the problem is coming in to play.

Well, here's the blinking part...

const int ledPin =  LED_BUILTIN;// the number of the LED pin
void setup() {
  // set the digital pin as output:
  pinMode(ledPin, OUTPUT);
}
void loop() {
  digitalWrite(ledPin, (millis()>>10) & 3);
}

Use millis()>>9 if you want to double the speed.

aarg:
Well, here's the blinking part...

const int ledPin =  LED_BUILTIN;// the number of the LED pin

void setup() {
  // set the digital pin as output:
  pinMode(ledPin, OUTPUT);
}
void loop() {
  digitalWrite(ledPin, (millis()>>10) & 3);
}


Use millis()>>9 if you want to double the speed.

Thank you. That works great. Now if I can just get it to work with a push button.

aarg:
Well, here's the blinking part...

const int ledPin =  LED_BUILTIN;// the number of the LED pin

void setup() {
 // set the digital pin as output:
 pinMode(ledPin, OUTPUT);
}
void loop() {
 digitalWrite(ledPin, (millis()>>10) & 3);
}



Use millis()>>9 if you want to double the speed.

I learned something today.
The line: millis() >> 10) & 3 equates to 0,1, 2 and 3.
So, in the line:

digitalWrite(ledPin, (millis() >> 10) & 3);

the LED is on while the evaluation = 0, and off all other times (because 0 is LOW and not-zero is HIGH).
I would have never thought of this method of blinking an LED, but I can see some advantages to using this.

BUT... This method of blinking will only work if the loop() runs unblocked.

Any method of blinking that depends on millis() will stall if blocking code is invoked. An example of something that doesn't, is a timer interrupt driven scheme. But that does get into another set of pitfalls. The lucky thing is that the human eye is not all that fast at seeing changes as a computer can display them. By a long shot. So blink latency doesn't have to be all that fast.

SteveMann:
BUT... This method of blinking will only work if the loop() runs unblocked.

You say that as if it's a bad thing.

loop() should always run unblocked :wink:

finola_marsaili:
You say that as if it's a bad thing.

loop() should always run unblocked :wink:

That is what I was saying.

OP, I think this code does what you want.
(I borrowed liberally from the Arduino Tutorials).

// Sketch to toggle a blinking LED. Push-on, push off.

#define ledOn LOW
#define ledOff HIGH

const int ledPin =  LED_BUILTIN;
const int buttonPin = D0;   // Wemos D1 GPIO 16


int ledState = ledOff;         // the current state of the LED
int previousState = LOW;       // the previous buttonState from the button pin

unsigned long buttonTime = 0;                  // The last time the output pin was toggled
unsigned long debounceTime = 400;              // debounceTime time (I am using a noisy button)


void setup() {
  pinMode(ledPin, OUTPUT);
  pinMode(buttonPin, INPUT_PULLUP);
  digitalWrite(ledPin, ledState);
}


void loop() {
  int buttonState = digitalRead(buttonPin);
  if (buttonState != previousState && millis() - buttonTime > debounceTime) {
    ledState = !ledState;
    buttonTime = millis();
  }

  if (ledState == ledOn) {
    digitalWrite(ledPin, (millis() >> 9) & 3);  //Blink
  } else
    digitalWrite(ledPin, ledOff);

  previousState = buttonState;
}
1 Like

SteveMann:
OP, I think this code does what you want.
(I borrowed liberally from the Arduino Tutorials).

// Sketch to toggle a blinking LED. Push-on, push off.

#define ledOn LOW
#define ledOff HIGH

const int ledPin =  LED_BUILTIN;
const int buttonPin = D0;  // Wemos D1 GPIO 16

int ledState = ledOff;        // the current state of the LED
int previousState = LOW;      // the previous buttonState from the button pin

unsigned long buttonTime = 0;                  // The last time the output pin was toggled
unsigned long debounceTime = 400;              // debounceTime time (I am using a noisy button)

void setup() {
  pinMode(ledPin, OUTPUT);
  pinMode(buttonPin, INPUT_PULLUP);
  digitalWrite(ledPin, ledState);
}

void loop() {
  int buttonState = digitalRead(buttonPin);
  if (buttonState != previousState && millis() - buttonTime > debounceTime) {
    ledState = !ledState;
    buttonTime = millis();
  }

if (ledState == ledOn) {
    digitalWrite(ledPin, (millis() >> 9) & 3);  //Blink
  } else
    digitalWrite(ledPin, ledOff);

previousState = buttonState;
}

Thank you. That absolutely works. I'm going to really go over this to understand it compared to what I had written. Thank you again.

So now, the thing is when I incorporate that code into the rest of my working code, it doesn't work at all. I have a feeling it's because of my LED Strip breath effect code, though I'm not positive. It uses a "for" statement, and researching through forums and such, it seems that a "for" statement can hold up a loop. I also got the breath code online, and it works really well.

Below is my full code for my working system. It's for a model I'm building, in which there are multiple flashing LEDs (done with class Flasher) , which are shining through fiber optics for multiple flight consoles, a breathing LED code for some wall beams lit with adafruit neopixel strips, and then the new code (from post above) to activate the alarm system of the ship. So the flashing and breathing code work together just fine together and the new code for the alarm activation works fine on it's own. It's just together is the issue.

// JUPITER 2 SKETCH for Adafruit Trinket Pro 3.3v


#include <Adafruit_NeoPixel.h>


#define PIN 3
#define signLED 4
#define ledOn LOW
#define ledOff HIGH

const int alarmLED =  1;
const int buttonPin = 0;   

int ledState1 = ledOff;         // the current state of the LED
int previousState = LOW;       // the previous buttonState from the button pin

unsigned long buttonTime = 0;                  // The last time the output pin was toggled
unsigned long debounceTime = 400;              // debounceTime time (I am using a noisy button)


// Parameter 1 = number of pixels in strip
// Parameter 2 = pin number
// Parameter 3 = pixel type flags, add together as needed:
//   NEO_KHZ800  800 KHz bitstream (most NeoPixel products w/WS2812 LEDs)
//   NEO_GRB     Pixels are wired for GRB bitstream (most NeoPixel products)
Adafruit_NeoPixel strip = Adafruit_NeoPixel(3, PIN, NEO_GRB + NEO_KHZ800);


class Flasher
{
  // Class Member Variables for BLINKING LEDs for CONSOLE FIBERS
  // These are initialized at startup
  int ledPin;      // the number of the LED pin
  long OnTime;     // milliseconds of on-time
  long OffTime;    // milliseconds of off-time

  // These maintain the current state
  int ledState;                 // ledState used to set the LED
  unsigned long previousMillis;   // will store last time LED was updated

  // Constructor - creates a Flasher 
  // and initializes the member variables and state
  public:
  Flasher(int pin, long on, long off)
  {
  ledPin = pin;
  pinMode(ledPin, OUTPUT);     
    
  OnTime = on;
  OffTime = off;
  
  ledState = LOW; 
  previousMillis = 0;
  }

  void Update() // For flashing LEDs
  {
    // check to see if it's time to change the state of the LED
    unsigned long currentMillis = millis();
     
    if((ledState == HIGH) && (currentMillis - previousMillis >= OnTime))
    {
      ledState = LOW;  // Turn it off
      previousMillis = currentMillis;  // Remember the time
      digitalWrite(ledPin, ledState);  // Update the actual LED
    }
    else if ((ledState == LOW) && (currentMillis - previousMillis >= OffTime))
    {
      ledState = HIGH;  // turn it on
      previousMillis = currentMillis;   // Remember the time
      digitalWrite(ledPin, ledState);   // Update the actual LED
    }
  }
};

// Flashers for Main console & Freezing tubes
Flasher led1(9, 4500, 1800); // (pin#, on time, off time) 
Flasher led2(10, 3500, 2000); // (pin#, on time, off time)
Flasher led3(11, 3000, 1000); // (pin#, on time, off time)

// Flashers for communication bay
Flasher led4(12, 6000, 1000); // (pin#, on time, off time)
Flasher led5(13, 4000, 500); // (pin#, on time, off time)

void setup() {
  
  strip.begin(); // start LED strip for pulasating 
  strip.setBrightness(100);  // Lower brightness and save eyeballs!
  strip.show(); // Initialize all pixels to 'off'

  pinMode(signLED, OUTPUT);
  pinMode(alarmLED, OUTPUT);
  digitalWrite(alarmLED, ledState1);
  digitalWrite(signLED, LOW);
  
}

void loop() {

// ALARM ACTIVATION
int buttonState = digitalRead(buttonPin);
  if (buttonState != previousState && millis() - buttonTime > debounceTime) {
    ledState1 = !ledState1;
    buttonTime = millis();
  }

  if (ledState1 == ledOn) {
    digitalWrite(alarmLED, (millis() >> 9) & 3);  //Blink
    digitalWrite(signLED, HIGH);
  } else
    digitalWrite(alarmLED, LOW);
    digitalWrite(signLED, LOW);

  previousState = buttonState;


// PULSATING THE WALL BEAMS
int TOTAL_LEDS = 3;
float MaximumBrightness = 225;
float SpeedFactor = 0.0006; 
float StepDelay = 5; // ms for a step delay on the lights

// Make the lights breathe
for (int i = 0; i < 65535; i++) {
// Intensity will go from 10 - MaximumBrightness in a "breathing" manner
float intensity = MaximumBrightness /2.0 * (1.0 + sin(SpeedFactor * i));
strip.setBrightness(intensity);
// Now set every LED to that color
for (int ledNumber=0; ledNumber<TOTAL_LEDS; ledNumber++) {
strip.setPixelColor(ledNumber, 255, 255, 255);
}

strip.show();


//Update flashing LED for consoles
  
  led1.Update();
  led2.Update();
  led3.Update();
  led4.Update();
  led5.Update();
}
}
for (int i = 0; i < 65535; i++) {

You absolutely can not do this. An int type can only represent positive numbers up to and including 32767. Also you are correct, that it monopolizes the CPU time and nothing else can run while it's looping.

OK. It was code I got from a sample on Adafruit I think. It worked, so I stuck with it. Guess I'll have to try and figure out another way to breath the strip.

aarg:

for (int i = 0; i < 65535; i++) {

You absolutely can not do this. An int type can only represent positive numbers up to and including 32767. Also you are correct, that it monopolizes the CPU time and nothing else can run while it's looping.

OK. It was code I got from a sample on Adafruit I think. But it worked really well, so I stuck with it. Guess I'll have to try and figure out another way to breath the strip so I can run all my code together.

So I got new code to do my breathing LEDs, but I still have the same problem as before. All of my code runs fine together except the push button for blinking LED. It won't run when placed with the rest of the code, but runs on it's own. I've moved it around within the code, but can't figure out why it won't run.

So running all together and working is a breathing LED, with breathe speed controlled by a pot, and 5 various timed flashing LEDs. Just no button press for blinking LED (alarm activation).

(UPDATE)
And upon doing a little testing, the breathing and flashers work together just fine and the breathing and push to blink work together just fine. But the push to blink and flashers do not work together. Only the flashers run, but I can't activate the blinking LED. So it appears it's something to do with the flashers that's interfering with the push button activation.

// JUPITER 2 SKETCH for Adafruit Trinket Pro 3.3v


#include <Adafruit_NeoPixel.h>


#define PIN 3
#define signLED 4
#define POTPIN A6
#define ledOn LOW
#define ledOff HIGH

int potValue = 0;
int Speed = 0;

const int alarmLED =  1;
const int buttonPin = 0;   

int ledState1 = ledOff;         // the current state of the LED
int previousState = LOW;       // the previous buttonState from the button pin

unsigned long buttonTime = 0;                  // The last time the output pin was toggled
unsigned long debounceTime = 400;              // debounceTime time (I am using a noisy button)


// Parameter 1 = number of pixels in strip
// Parameter 2 = pin number
// Parameter 3 = pixel type flags, add together as needed:
//   NEO_KHZ800  800 KHz bitstream (most NeoPixel products w/WS2812 LEDs)
//   NEO_GRB     Pixels are wired for GRB bitstream (most NeoPixel products)
Adafruit_NeoPixel strip = Adafruit_NeoPixel(3, PIN, NEO_GRB + NEO_KHZ800);

const uint8_t mySegment[3] = {0,1,2};  // LED strip setup

class Flasher
{
  // Class Member Variables for BLINKING LEDs for CONSOLE FIBERS
  // These are initialized at startup
  int ledPin;      // the number of the LED pin
  long OnTime;     // milliseconds of on-time
  long OffTime;    // milliseconds of off-time

  // These maintain the current state
  int ledState;                 // ledState used to set the LED
  unsigned long previousMillis;   // will store last time LED was updated

  // Constructor - creates a Flasher 
  // and initializes the member variables and state
  public:
  Flasher(int pin, long on, long off)
  {
  ledPin = pin;
  pinMode(ledPin, OUTPUT);     
    
  OnTime = on;
  OffTime = off;
  
  ledState = LOW; 
  previousMillis = 0;
  }

  void Update() // For flashing LEDs
  {
    // check to see if it's time to change the state of the LED
    unsigned long currentMillis = millis();
     
    if((ledState == HIGH) && (currentMillis - previousMillis >= OnTime))
    {
      ledState = LOW;  // Turn it off
      previousMillis = currentMillis;  // Remember the time
      digitalWrite(ledPin, ledState);  // Update the actual LED
    }
    else if ((ledState == LOW) && (currentMillis - previousMillis >= OffTime))
    {
      ledState = HIGH;  // turn it on
      previousMillis = currentMillis;   // Remember the time
      digitalWrite(ledPin, ledState);   // Update the actual LED
    }
  }
};

// Flashers for Main console & Freezing tubes
Flasher led1(9, 4500, 1800); // (pin#, on time, off time) 
Flasher led2(10, 3500, 2000); // (pin#, on time, off time)
Flasher led3(11, 3000, 1000); // (pin#, on time, off time)

// Flashers for communication bay
Flasher led4(12, 6000, 1000); // (pin#, on time, off time)
Flasher led5(13, 4000, 500); // (pin#, on time, off time)

void setup() {
  
  strip.begin(); // start LED strip for pulasating 
  strip.setBrightness(100);  // Set LED strip brightness
  strip.show(); // Initialize all pixels to 'off'

  pinMode(signLED, OUTPUT);
  pinMode(alarmLED, OUTPUT);
  digitalWrite(alarmLED, ledState1);
  digitalWrite(signLED, LOW);
  
}

void loop() {

// ALARM ACTIVATION
int buttonState = digitalRead(buttonPin);
  if (buttonState != previousState && millis() - buttonTime > debounceTime) {
    ledState1 = !ledState1;
    buttonTime = millis();
  }

  if (ledState1 == ledOn) {
    digitalWrite(alarmLED, (millis() >> 9) & 3);  //Blink
    digitalWrite(signLED, HIGH);
  } else
    digitalWrite(alarmLED, LOW);
    digitalWrite(signLED, LOW);

  previousState = buttonState;
  


// PULSATING THE WALL BEAMS
potValue = analogRead(POTPIN);
Speed = map(potValue, 0, 1023, 5, 80);  // speed value of pulsating wall beam LEDs
  
  breatheUpdate(mySegment, (Speed), 1, 5); // (speed,steps ,lowest brightness)
}

void breatheUpdate(const uint8_t * segment, const uint32_t increment, const uint8_t step, const uint8_t lowLimit)
{
  static uint32_t lastTimeChange = 0;
  static uint8_t direction = 1;
  static uint8_t value = lowLimit;
  if(millis() - lastTimeChange > increment)
  {
    value += (direction * step);
    if (value <= lowLimit || value >= 255)
    {
      direction = direction * -1;
    }
    for(uint8_t i = 0; i < sizeof(segment) * 2; i++)
    {
      strip.setPixelColor(segment[i], strip.Color(value, value, value));
    }
    strip.show();
    lastTimeChange += increment;
  }


//Update flashing LED for consoles
  
  led1.Update();
  led2.Update();
  led3.Update();
  led4.Update();
  led5.Update();
}

This code

  } else
    digitalWrite(alarmLED, LOW);
    digitalWrite(signLED, LOW);

Is not formatted correctly (CTRL-T in the IDE). If you do that, you will notice that the second digitalWrite() function outdends. It is not associated with the 'else' clause since you did not use curly braces. This is why you should always use them for if/else.

if your breatheUpdate() function, you declare

  static uint8_t direction = 1;

so it is unsigned, but later on, you multiply it by -1. Unsigned variables can not contain negative values (They get converted to unsigned values so in this case -1 == 255).

Try changing the type of 'direction'

blh64:
This code

  } else

digitalWrite(alarmLED, LOW);
    digitalWrite(signLED, LOW);




if your breatheUpdate() function, you declare


static uint8_t direction = 1;


so it is unsigned, but later on, you multiply it by -1. Unsigned variables can not contain negative values (They get converted to unsigned values so in this case -1 == 255).

Try changing the type of 'direction'

Thanks. Asside from the breathing function though, it's the flashers that interfere with the button push function. That's what I can't figure out. Basically, this code below, they do not work together, but each on their own work just fine.

#define signLED 4
#define ledOn LOW
#define ledOff HIGH

const int alarmLED =  1;
const int buttonPin = 0;

int ledState1 = ledOff;         // the current state of the LED
int previousState = LOW;       // the previous buttonState from the button pin

unsigned long buttonTime = 0;                  // The last time the output pin was toggled
unsigned long debounceTime = 400;              // debounceTime time (I am using a noisy button)


class Flasher
{
    // Class Member Variables for BLINKING LEDs for CONSOLE FIBERS
    // These are initialized at startup
    int ledPin;      // the number of the LED pin
    long OnTime;     // milliseconds of on-time
    long OffTime;    // milliseconds of off-time

    // These maintain the current state
    int ledState;                 // ledState used to set the LED
    unsigned long previousMillis;   // will store last time LED was updated

    // Constructor - creates a Flasher
    // and initializes the member variables and state
  public:
    Flasher(int pin, long on, long off)
    {
      ledPin = pin;
      pinMode(ledPin, OUTPUT);

      OnTime = on;
      OffTime = off;

      ledState = LOW;
      previousMillis = 0;
    }

    void Update() // For flashing LEDs
    {
      // check to see if it's time to change the state of the LED
      unsigned long currentMillis = millis();

      if ((ledState == HIGH) && (currentMillis - previousMillis >= OnTime))
      {
        ledState = LOW;  // Turn it off
        previousMillis = currentMillis;  // Remember the time
        digitalWrite(ledPin, ledState);  // Update the actual LED
      }
      else if ((ledState == LOW) && (currentMillis - previousMillis >= OffTime))
      {
        ledState = HIGH;  // turn it on
        previousMillis = currentMillis;   // Remember the time
        digitalWrite(ledPin, ledState);   // Update the actual LED
      }
    }
};

// Flashers for Main console & Freezing tubes
Flasher led1(9, 4500, 1800); // (pin#, on time, off time)
Flasher led2(10, 3500, 2000); // (pin#, on time, off time)
Flasher led3(11, 3000, 1000); // (pin#, on time, off time)

// Flashers for communication bay
Flasher led4(12, 6000, 1000); // (pin#, on time, off time)
Flasher led5(13, 4000, 500); // (pin#, on time, off time)

void setup() {

  pinMode(signLED, OUTPUT);
  pinMode(alarmLED, OUTPUT);
  digitalWrite(alarmLED, ledState1);
  digitalWrite(signLED, LOW);

}

void loop() {

  // ALARM ACTIVATION
  int buttonState = digitalRead(buttonPin);
  if (buttonState != previousState && millis() - buttonTime > debounceTime) {
    ledState1 = !ledState1;
    buttonTime = millis();
  }

  if (ledState1 == ledOn) {
    digitalWrite(alarmLED, (millis() >> 9) & 3);  //Blink
    digitalWrite(signLED, HIGH);
  } else
    digitalWrite(alarmLED, LOW);
  digitalWrite(signLED, LOW);

  previousState = buttonState;



  //Update flashing LED for consoles

  led1.Update();
  led2.Update();
  led3.Update();
  led4.Update();
  led5.Update();
}