One pushbutton change led lighting timing

I am very new to arduino and spend a week to figure out what happen is it...

here is my project:
First press: the led 13 light up, led 3 light up(fade in and out repeatly just like engine effect) after 4 seconds, then led 13 and 3 both off after 10 seconds. and wait for another press.
Second press: led 13 and 3 light up together and off after 4 seconds.
third press: return to first press.

here is my programming code...

int red = 13;
int button = 2;
int orange = 3;
unsigned long previous_orange = 0;
unsigned long interval_orange = 500;

int buttonpoll = 0;
int oldstate = 0;
int state = 0;



void setup() {
  pinMode(button,INPUT);
   pinMode(red,OUTPUT);
  pinMode(orange,OUTPUT);
  digitalWrite(red,LOW);
  digitalWrite(orange,LOW);
}

void checkorange(){
  for (int fadevalue2 = 10; fadevalue2 <= 255; fadevalue2 +=5){
    analogWrite(orange,fadevalue2);
    delay(1);
  }
  for (int fadevalue2 = 255; fadevalue2 >= 10; fadevalue2 -=5){
    analogWrite(orange,fadevalue2);
    delay(1);
}}  

void loop() {
     buttonpoll = digitalRead(button);
     if(buttonpoll == 1){
      delay(50);
      buttonpoll = digitalRead(button);
      if (buttonpoll == 0){
      state = oldstate + 1;
    }
     }else{
      delay(100);
     }

    switch (state){
      case 1:
      digitalWrite(red,HIGH);
      delay(4000);
      for (int count =0; count<100; count++){
      checkorange();
      }
      analogWrite(orange,LOW);
      digitalWrite(red, LOW);
      oldstate = state;
      break;
      case 2:
      digitalWrite(red,HIGH);
      for (int count =0; count<100; count++){
      checkorange();
      }
      analogWrite(orange,LOW);
      digitalWrite(red, LOW);
      oldstate = 0;
      break;
     
    }
}

the problem is it works in the first press, but it seems stuck in the loop, and no reaction if I press button again... hope someone could give some hits what to do...thanks!

The easier you make it to read and copy your code the more likely it is that you will get help

Please follow the advice given in the link below when posting code , use code tags and post the code here

If you get errors when compiling please copy them from the IDE using the "Copy error messages" button and paste the clipboard here in code tags

Thanks for adding the code tags!

The problem is you have written "blocking code" which means code containing long delay() and for-loops or while-loops containing delay(). This is why your code does not react to button presses. It cannot check if a button has been pressed if it is stuck in a long delay or a loop with many short delays inside. You have to re-write the code in a non-blocking style, using millis() function to time actions. This is one of the most frequently asked questions in this forum. There are many posts and tutorials about how to make the changes such as this one

1 Like

thank you for your reply!:pray::pray:
I will check the link later!!
actually I did doubt that my problem is my code included delay(), and you make me understand a bit how delay() affect to the button reaction!
however is there any other way to write a fade code with using millis()? cause my fade coding was referencing the example of "fade without delay"...

I'm sorry, but I don't see any resemblance! Where in your code is millis() used?

Hi,
Welcome to the forum.
Is this the sequence you are trying to make?

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

1 Like

exactly !
but wait not only 2 seconds, actually want all led stop until the next press.

oh you are right!! I think I opened a wrong example file and then apply it on fade without delay…:sneezing_face::sneezing_face:

even now I code it with millis()... still can't change to the next state :sob:
plus I don't know why the fade function being super slow under the button function(it works normally if there are no button), the fade speed seems can't be change :face_with_thermometer:
thanks anyway :confounded:

const byte orange = 3;
int red = 13;
int interval = 4000;
int stopinterval = 14000;
unsigned long previous = 0;
#define UP 0
#define DOWN 1
const int minPWM = 0;
const int maxPWM = 255;
byte fadeDirection = UP;
int fadeValue = 0;
byte fadeIncrement = 5;
unsigned long previousFadeMillis;
int fadeInterval = 10;

int button = 2;
int buttonpoll = 0;
int oldstate = 0;
int state = 0;



void setup() {
  pinMode(button,INPUT);
 analogWrite(orange, fadeValue); 
 pinMode(red,OUTPUT);
}

void doTheFade(unsigned long thisMillis) {
  if (thisMillis - previousFadeMillis >= fadeInterval) {
    if (fadeDirection == UP) {
      fadeValue = fadeValue + fadeIncrement;  
      if (fadeValue >= maxPWM) {
        fadeValue = maxPWM;
        fadeDirection = DOWN;
      }
    } else {
      fadeValue = fadeValue - fadeIncrement;
      if (fadeValue <= minPWM) {
        fadeValue = minPWM;
        fadeDirection = UP;
      }
    }
    analogWrite(orange, fadeValue);  
    previousFadeMillis = thisMillis;
  }
}

void loop() {
     buttonpoll = digitalRead(button);
     if(buttonpoll == 1){
      delay(50);
      buttonpoll = digitalRead(button);
      if (buttonpoll == 0){
      state = oldstate + 1;
    }
     }else{
      delay(100);
     }

    switch (state){
      case 1:
      unsigned long currentMillis = millis();
      digitalWrite(red,HIGH);
      if(currentMillis - previous >= interval){
      doTheFade(currentMillis);}
      if(currentMillis - previous >= stopinterval){
      digitalWrite(red,LOW);
      digitalWrite(orange,LOW);
      }
      oldstate = state;
      break;
      case 2:
      digitalWrite(red,HIGH);
      digitalWrite(orange,HIGH);
      oldstate = 0;
      break;
    }
}

You have several issues here.

  1. You never reset 'previous'. Therefore once 14 seconds passes, you will always be setting red and orange LEDs to LOW if you are in state 1.
  2. In state 1 when 14 seconds passes you will attempting to fade AND set red and orange LEDs to LOW
  3. You have your fade interval set to 10ms; however, because of the way you poll the button you will either delay 50ms or 100ms therefore you cannot possibly fade at an interval of 10ms. The interval will be no less than 50ms.
1 Like

thank you for your reply ! indeed, I want the led won't glow after 14 seconds. So I suppose I don't have to save the previous ?
As you pointed out my problem, seems it will
be solved if I change the way of the poll?

Here is a version of the code I corrected. It assumes that the button is connected to ground and using the internal pullup resistor. Also I added state 0 to reset the LEDs. I tested it and it works.

#define UP 0
#define DOWN 1
const int orange = 3;
const int button = 2;
const int red = 13;
const int interval = 4000;
const int stopinterval = 14000;
const int minPWM = 0;
const int maxPWM = 255;

unsigned long previous = 0;
unsigned long previousFadeMillis;

const byte fadeIncrement = 5;
const int fadeInterval = 10;
byte fadeDirection = UP;
int fadeValue = 0;

int oldstate = 0;
int state = 0;



void setup() {
  pinMode(button, INPUT_PULLUP);
  analogWrite(orange, fadeValue);
  pinMode(red, OUTPUT);
}

void doTheFade(unsigned long thisMillis) {
  if (thisMillis - previousFadeMillis >= fadeInterval) {
    if (fadeDirection == UP) {
      fadeValue = fadeValue + fadeIncrement;
      if (fadeValue >= maxPWM) {
        fadeValue = maxPWM;
        fadeDirection = DOWN;
      }
    } else {
      fadeValue = fadeValue - fadeIncrement;
      if (fadeValue <= minPWM) {
        fadeValue = minPWM;
        fadeDirection = UP;
      }
    }
    analogWrite(orange, fadeValue);
    previousFadeMillis = thisMillis;
  }
}

void loop() {
  static int prevButtonState = digitalRead(button);
  int buttonState = digitalRead(button);
  unsigned long currentMillis;
  
  // If button has been pressed then increment the state
  if (buttonState != prevButtonState) {
    delay(50);
    if (buttonState == LOW)
    {
      state++;
      state %= 3;
      previous = millis();
    }
    prevButtonState = buttonState;
  }

  switch (state) {
    case 0:
      digitalWrite(red, LOW);
      digitalWrite(orange, LOW);
      break;
      
    case 1:
      currentMillis = millis();
      if (currentMillis - previous >= stopinterval) {
        digitalWrite(red, LOW);
        digitalWrite(orange, LOW);
      }
      else if (currentMillis - previous >= interval) {
        doTheFade(currentMillis);
      }
      else {
        digitalWrite(red, HIGH);
      }
      break;
      
    case 2:
      digitalWrite(red, HIGH);
      digitalWrite(orange, HIGH);
      break;
  }
}
1 Like

According to this code, only if the button is HIGH and 50ms later, it has changed to LOW, will the state be updated. You should take a look at the "StateChange" example sketch to discover a better way to do this.

Also, you should use HIGH and LOW when comparing to the result of digitalRead(), not 1 or 0. Today, using 1 and 0 will work, but in the future, it may not always work.

Before you post your code again, and each time you post your code, please perform an Auto-Format from the Tools menu in the IDE.

1 Like

That's great, Todd, I'm sure @marveldchero will be very happy. But realise that you have given him a fish. Tomorrow, will be be able to catch his own fish?

1 Like

He made a good try with a few fundamental errors and I previously outlined his specific issues. I've seen much worse on this forum, usually in the form "give me code to...". But hey, maybe I'm too kind.

Hey, that's allowed, now and again!

But not too often. The forum has a reputation to uphold! Otherwise the "gimme da codez!" hoards will overwhelm us like in a bad sci-fi horror movie!

1 Like

omg! don't know how to say thank you :sob: :sob:
this is what I need for my learning...failure and some example...
your code included skills that I didn't know, I will try my best to understand it!!
I hope it will not be annoying if I ask more questions about those logic since I really do want to consume them :dizzy_face:
thanks again :pray: both of you!