Fade without delay?

Hello, I am somewhat new to arduino and I have run into a problem that seems like it would be easier to solve than it actually is…

What I want to create is an LED that fades in and out with different rates depending on the value of the potentiometer, however, if the potentiometers’ value is 0, the LED should be shut off, if the value is maxed out the LED should be lit constantly.

Seemed simple enough so I tried to code it but ran into some problems with delay() causing bugs, I tried to use millis() instead but couldn’t figure it out, I also googled and searched the forum without any luck…

Here is my code, how can I make it work without delay()?

int led = 3;
int pot = A0;
int potval;
int fadeValue;

void setup() {
  Serial.begin(9600);
  pinMode(led, OUTPUT);
  pinMode(pot, INPUT);
}

void loop() {
  potval = map(analogRead(pot), 0, 1023, 0, 22);
  Serial.print("potval: ");
  Serial.println(potval);

  if (potval == 0) {
    // if potval == 0, turn the led off
    digitalWrite(led, 0);
  } else if (potval >= 21) {
    // if potval is >= 21, keep the led on
    digitalWrite(led, 255);
  }

  if (potval > 0 && potval < 21) {
    for (fadeValue = 0 ; fadeValue <= 255; fadeValue += potval) {
      analogWrite(led, fadeValue);
      delay(40);
    }

    for (fadeValue = 255 ; fadeValue >= 0; fadeValue -= potval) {
      analogWrite(led, fadeValue);
      delay(40);
    }
  }
}

Thanks in advance.

You need to tell us how it is currently behaving. But I think I see the dreaded for loops, that is always a bad sign. Also delay() mixed with millis(), very bad.

aarg:
You need to tell us how it is currently behaving. But I think I see the dreaded for loops, that is always a bad sign.

Why are the for loops dreaded?

The problems I'm facing with using delay() is that if I turn the LED on again after being shut off the first fade cycle will be very slow because the potentiometer value wont be able to get higher than 1.

Same thing goes for when I try to shut the LED off, if I try to turn if off from an already low value it will be slow to turn off.

hyveltjuven:
Why are the for loops dreaded?

Because they prevent you from monitoring the analog pin while you are performing the tasks inside the loop. That is why it is slow.

aarg:
Because they prevent you from monitoring the analog pin while you are performing the tasks inside the loop. That is why it is slow.

So it isn't the delay() causing the problems? How can I solve this?

So it isn't the delay() causing the problems?

It is the delay() inside a blocking for loop.

Think about how you would make the LED fade using just the fact that loop() loops.

PaulS:
It is the delay() inside a blocking for loop.

Think about how you would make the LED fade using just the fact that loop() loops.

Sorry to bump the thread again but I still haven’t figured it out, but I think I’m on better track.

I had a look at the Fade example code and have gotten rid of my for loops and instead I’m using the fact that void loop() loops as you said, however, I now cannot make the led fade in and out properly.

Now instead it just fades up and then turns off in a loop, most likely because the brightness value isn’t actually declining when reaching 255, how can I fix this?

My code now looks like this:

int led = 3;     // the pin that the LED is attached to
int pot = A0;
int potval;
int brightness;    // how bright the LED is

void setup() {
  Serial.begin(9600);
  pinMode(pot, INPUT);
  pinMode(led, OUTPUT);

}

void loop() {
  Serial.print("potval: ");
  Serial.print(potval);
  Serial.print("\t brightness: ");
  Serial.println(brightness);

  potval = map(analogRead(pot), 0, 1023, 0, 22);

  analogWrite(led, brightness);

  if (potval == 0) {
    analogWrite(led, 0);
  } else if (potval >= 21) {
    analogWrite(led, 255);
  }

    brightness = brightness + potval;

  if (brightness >= 255) {
    potval = -potval; 
  } else if (brightness <= 0){
    potval = +potval;
  }

  delay(30);
}

most likely because the brightness value isn’t actually declining when reaching 255, how can I fix this?

Most likely? You print out brightness, on each pass through loop. Does it, indeed, fail to decrease once it reaches 255?

    brightness = brightness + potval;

  if (brightness >= 255) {
    potval = -potval; 
  } else if (brightness <= 0){
    potval = +potval;
  }

You should not be doing this if potVal is 0 or greater than or equal to 21. It needs to be in an else block.

You assign a new value to potval on each pass through loop(), you know, prior to adjusting the sign of potval. You also change the value of brightness BEFORE adjusting the sign. Bad news, there.

PaulS:
Most likely? You print out brightness, on each pass through loop. Does it, indeed, fail to decrease once it reaches 255?

    brightness = brightness + potval;

if (brightness >= 255) {
    potval = -potval;
  } else if (brightness <= 0){
    potval = +potval;
  }



You should not be doing this if potVal is 0 or greater than or equal to 21. It needs to be in an else block.

You assign a new value to potval on each pass through loop(), you know, prior to adjusting the sign of potval. You also change the value of brightness BEFORE adjusting the sign. Bad news, there.

Yes, brightness does fail to decrease once it reaches 255, potval does however turn to negative digits as it should.

I’m afraid I’m not really understanding what changes you suggest I should do to my code, sorry, I’m still new to this…

I’m afraid I’m not really understanding what changes you suggest I should do to my code, sorry, I’m still new to this…

First, this:

  if (potval == 0) {
    analogWrite(led, 0);
  } else if (potval >= 21) {
    analogWrite(led, 255);
  }

    brightness = brightness + potval;

  if (brightness >= 255) {
    potval = -potval; 
  } else if (brightness <= 0){
    potval = +potval;
  }

should be:

  if (potval == 0)
  {
    analogWrite(led, 0);
  }
  else if (potval >= 21)
  {
    analogWrite(led, 255);
  }
  else
  {
    brightness = brightness + potval;

    if (brightness >= 255)
    {
       potval = -potval; 
    }
    else if (brightness <= 0)
    {
       potval = +potval;
    }
  }

Second, potval is set to a positive value on every pass through through loop. Use the value, and then fix the sign:

    brightness = brightness + potval;

    if (brightness >= 255)
    {
       potval = -potval; 
    }
    else if (brightness <= 0)
    {
       potval = +potval;
    }

You need to fix the sign first, and then use the value:

    if (brightness >= 255)
    {
       potval = -potval; 
    }
    else if (brightness <= 0)
    {
       potval = +potval;
    }

    brightness = brightness + potval;

PaulS:
First, this:
should be:

Second, potval is set to a positive value on every pass through through loop. Use the value, and then fix the sign:
You need to fix the sign first, and then use the value:

Alright, I replaced the code with the one you wrote and now potval switches between negative to positive on every loop when brightness reaches 255. It doesn't stay negative until brightness reaches 0 as it should.

btw, thank you so much, I really appreciate you helping me through seemingly obvious things... :slight_smile:

You need to add a new variable that is either +1 or -1. Call it sign.

sign will be +1 initially, and brightness will be 0. When brightness gets to, or goes over, 255, change sign to -1. When brightness gets to, or goes below, 0, change sign to +1.

Then adjust brightness by sign * potval.

Arduino millis() returns an unsigned long value = milliseconds since start up, up to 49.7 days.

Every millisecond, that value increases by 1.

To use millis(), subtract an earlier time from a later time to get the difference up to 49.7 days.

If a pin changes from HIGH to LOW and I read millis() into a variable named tStart, when the pin changes back to HIGH the interval = ( millis() - tStart ).

I put a check in loop()

if ( desiredWait > 0 )
{
if ( millis() - tStart >= desiredWait )
{
// time is up! do something!

desiredWait = 0; // makes this a 1-shot. you set up tStart and desiredWait from another routine
// to make it repeat, set up tStart and desiredWait here instead.
}
}

What you have with that delay(30) could be in that if() as a repeater (resets tStart) with desiredWait == 30 and your led control in the do something part.

A for-next loop is a loop with a counter/index and end check.

You can run your own counter inside of loop() and use it to index through an array one element per pass through loop() instead of processing the whole array in one pass through loop(). What this does is to let any other tasks you want to run along with your led will get a chance to act on their time or signal when it arrives instead of waiting for the whole array to process, which it will either way.

Learn to write this way and you will have an easier time writing, changing and adding to your code.

Every millisecond, that value increases by 1.

Except for when it increments by 2.

Yes, once in a great while it will increment by 2.