Make a led flash with two buttons

Hey,
I have a new problem ::slight_smile:
My aim is to make a led flash by pushing a fisrt button (here b2) and to turn the led off by pushing an other button (b). I want to do it by using the function millis(),
Actually, when I use this code, my led does not react with the buttons,
Can someone help me ?

Here is my code :

const byte bPin = 2;
const byte b2Pin = 3;
const int led = 8;
bool bState;
bool b2State;
bool ledState;
long time;

void setup() {
  // put your setup code here, to run once:
  pinMode(led, OUTPUT);
  pinMode(bPin, INPUT);
  pinMode(b2Pin, INPUT);
  ledState = 1;
  digitalWrite(led, ledState);
  bState = HIGH;
  bState = HIGH;


}

void loop() {
  // put your main code here, to run repeatedly:
  bState = digitalRead(bPin);
  b2State = digitalRead(b2Pin);
  time = millis();
  bool i = 0;


  if (b2State == false) {
    i = 0;
  }
  if (i = 0) {
    while (millis - time < 250) {
      digitalWrite(led, ledState);
      time = millis();
    }
    if (millis() - time > 250) {
      ledState = !ledState;
      digitalWrite(led, ledState);
    }
  }

  if (bState == false) {
    i = !i;
    digitalWrite(led, HIGH);
  }
}

if (i = 0)oopsie

You're doing the timing with a while loop. So execution stays in the while loop until it is done. All you have done is recreated the delay function. You might as well use delay with this code.

Go study blink without delay. Instead of being stuck in a while loop it just checks the time once in an if and moves on if it's not time.

or just use a library like this:

#include "SimplePress.h"

const byte buttonPin0 = 2;
const byte buttonPin1 = 3;

SimplePress button0(buttonPin0, 500);         // half second capture for multi-presses; uses default 200ms debounce
                                              // a long press is defined as greater than the capture time (500ms in this example)
                                              // or...
SimplePress button1(buttonPin1, 500, 150);    // or specify custom debounce time in ms

void setup() 
{
  Serial.begin(9600);
  pinMode(13, OUTPUT);
  button0.begin();
  button1.begin();
}

void loop() 
{
  if(int pressCount = button0.pressed())
  {
    switch(pressCount)
    {
      case -1:
        Serial.println(F("long Press"));
        longFlashPin13();
        break;
      case 1:
        Serial.println(F("one Press"));
        flashPin13(1);
        break;
      case 2:
        Serial.println(F("two Presses"));
        flashPin13(2);
        break;
    }
  }
  if(int press2count = button1.pressed())
  {
    switch(press2count)
    {
      case -1:
        Serial.println(F("long Press"));
        longFlashPin13();
        break;
      case 1:
        Serial.println(F("one Press"));
        flashPin13(1);
        break;
    }
  }
}

void flashPin13(byte numTimes)
{
  for(byte i = 0; i < numTimes; i++)
  {
    digitalWrite(13, HIGH);
    delay(250);
    digitalWrite(13, LOW);
    if (i < numTimes - 1) delay(250);
  }
}

void longFlashPin13(void)
{
  digitalWrite(13, HIGH);
  delay(1000);
  digitalWrite(13, LOW);
}

here is the header file you need “SimplePress.h” (no cpp needed)

#ifndef SIMPLEPRESS_H
#define SIMPLEPRESS_H

#include <Arduino.h>

class SimplePress{
  public:
    SimplePress(int _pin, uint32_t _pressInterval, uint32_t _debouncePeriod = 200);
    bool begin();
    int8_t pressed();

  private:
    byte pressCount;
    byte lastState;
    byte pin;
    uint32_t lastMillis;
    uint32_t debouncePeriod;
    uint32_t pressInterval;
};

SimplePress::SimplePress(int _pin, uint32_t _pressInterval, uint32_t _debouncePeriod)
{
  pin = _pin;
  debouncePeriod = _debouncePeriod;
  pressInterval = _pressInterval;
}

bool SimplePress::begin()
{
  pinMode(pin, INPUT_PULLUP);
  lastState = HIGH;
  return true;
}

int8_t SimplePress::pressed()
{
  byte nowState = digitalRead(pin);
  if(nowState != lastState)
  {
    
    if(millis() - lastMillis < debouncePeriod) return false;
    if(nowState == LOW)
    {
      lastMillis = millis();
      pressCount++;
    }
    else 
    {
      if (millis() - lastMillis > pressInterval) // a long press
      {
        lastState = nowState;
        pressCount = 0;
        return -1;
      }
    }
  }
  if(pressCount != 0)
  {
    if(millis() - lastMillis > pressInterval and nowState == HIGH)
    {
      int presses = pressCount;
      pressCount = 0;
      return presses;
    }
  }
  lastState = nowState;
  return 0;
}

#endif

Oh, okay thanks that is more clear, I try it

I tried with delay but my code seams false again :

here is my loop (I did not change the rest) :

void loop() {
  // put your main code here, to run repeatedly:
  bState = digitalRead(bPin);
  b2State = digitalRead(b2Pin);
  bool i = 0;


  if (b2State == false) {
    i = 0;
  }
  if (i = 0) {
    digitalWrite(led, ledState);
    delay(250);
    ledState = !ledState;
    digitalWrite(led, ledState);
    delay(250);
  }


  if (bState == false) {
    i = !i;
    digitalWrite(led, HIGH);
  }
}

I have just found, if you are interrested that is the (not perfect but) correct version :

const byte bPin = 2;
const byte b2Pin = 3;
const int led = 8;
bool bState;
bool b2State;
bool ledState;
long time;
bool i = 1;

void setup() {
  // put your setup code here, to run once:
  pinMode(led, OUTPUT);
  pinMode(bPin, INPUT);
  pinMode(b2Pin, INPUT);
  ledState = 1;
  digitalWrite(led, ledState);
  bState = HIGH;
  bState = HIGH;


}

void loop() {
  // put your main code here, to run repeatedly:
  bState = digitalRead(bPin);
  b2State = digitalRead(b2Pin);



  if (b2State == false) {
    i = 0;
  }
  if (bState == false) {
    i = 1;
  }
  if (i == 1) {
    ledState = 1;
    digitalWrite(led, ledState);
  }

  if (i == 0) {
    digitalWrite(led, ledState);
    delay(100);
    ledState = !ledState;
    digitalWrite(led, ledState);
    delay(100);
  }
}