fading LED loops, getting stuck in a for loop

Hi all,

I’ve assembled an array of RGB LEDs, and want to make a random color mix mood lamp. I had easy success getting a single LED to do this, without smooth transitions from color to color, and now that I have added a fading feature to transition in a less jarring way it’s not really working any longer. I just get the red lamp dimming at the correct rate, then jumping to its initial value over and over again, so I know it is the red for loop where things are sticky, but cannot see a problem. Thanks in advance!

// random fading to new color

const int RED = 3; // red LED in pin 3
const int GRE = 5; // green LED in pin 5
const int BLU = 6; // blue LED in pin 6
int r = 0;
int g = 0;
int b = 0;
int r1 = 0;
int g1 = 0;
int b1 = 0;
int i = 0;

void setup()
{
  pinMode(RED,OUTPUT); //sets digital pin as output
  pinMode(GRE,OUTPUT);
  pinMode(BLU,OUTPUT);
}

void loop()
{ 
  r = random(0,255);
  g = random(0,255);
  b = random(0,255);
  
   while (r+g+b != 255){
     r = random(0,255);
     g = random(0,255);
     b = random(0,255);
   }
   if (r1 > r){
    for(i = r1; r1 > r; i--){
     analogWrite(RED,i); //set red LED brightness
     delay(5);
    }} else {
      for (i = r1; r1 < r; i++){
        analogWrite(RED,i);
        delay (5);
      }} r1 == r;
   if (g1 > g){
     for (i = g1; g1 > g; i--){
     analogWrite(GRE,i);
     delay(5);
    }} else {
      for (i = g1; g1 < g; i++){
        analogWrite(GRE,g);
        delay(5);
    }} g1 == g;
   if (b1 > b){
     for (i= b1; b1 > b; i--){ 
     analogWrite(BLU,b);
     delay(5);
    }} else {
      for (i = b1; b1 < b; i++){
       analogWrite(BLU,b);
       delay(5);
    }} b1 == b;
    delay(500);
}

First a couple of tips. You don’t need to declare i as a global variable. Try to use better variable names than r then r1. I know its the red value but doesn’t give any indication to the purpose of each value. I’m guessing one is the current value and the other is used for the next value to fade to.

What’s the reason for your while loop, seems odd to block the program until your combined color is 255.

When you do r1 == r that’s a comparison not assignment. To assign the value of r to r1 use only 1 =.

Ok so the reason your getting stuck in the for loop is that you using r1<r for the condition. So as you never change the value of r1 it will always be TRUE unless r = 0. For the condition of your for loop you should be using i<r.

Make those changes to the green and blue parts too.

Thanks for the quick help.

I’m using the while loop in order to minimize brightness fluctuations. So, the combined output of the 3 colors is always the same intensity, while allowing for the greatest color variations. It doesn’t seem to take long (certainly less than a hundred ms or so) to fulfill that requirement. I would be very happy to learn of another, more efficient technique to have the color vary without the brightness pulsing. I’m very new to this so all tricks are a surprise.

I’m then using r and r1, because after a new random value for red intensity is determined, I want it to fade from the last used value to the new value. So r1 was supposed to be the last used value, and r was supposed to be the most recently assigned value. I see my mistake in the r1<r condition of the for loop.

Thanks again - I’ll repost if I’ve gotten it to work tomorrow.

Never thought about keeping the brightness constant, I guess your while loop does the job. Although you will probably find it rare to get pure red, green or blue but that's up to you if you really want them or if it gets close enough.

   if (r1 > r){
    for(i = r1; r1 > r; i--){
     analogWrite(RED,i); //set red LED brightness
     delay(5);
    }} else {
      for (i = r1; r1 < r; i++){
        analogWrite(RED,i);
        delay (5);
      }} r1 == r;

This mess is too hard to read. Each { belongs on a new line, in my opinion. No matter what your thoughts on that issue, I think everyone agrees that each } belongs on its own line. And NO code on the line after the }, except at the end of a do/while statement.

r1 == r; probably doesn’t do what you intended, anyway.

if (r1 > r){
    for(i = r1; r1 > r; i--){

Well, "i" varies, but "r1" and "r" will always keep the same initial relationship.

As well as tidying up like PaulS said, you also could/should change

r = random(0,255);
g = random(0,255);
b = random(0,255);
  
while (r+g+b != 255)
{
   r = random(0,255);
   g = random(0,255);
   b = random(0,255);
}

to

do
{
   r = random(0,255);
   g = random(0,255);
   b = random(0,255);
} while (r+g+b != 255);

Or even r = random(0,256); etc.

Thanks Wazzled - it's working as intended now. I didn't see any difference between my while loop and your do while, but it makes sense to me that there's a function of the language that makes the way I did it less good.

AWOL - I'm not sure I understand your random(0,256) comment. Zero to 255 is 256 values, so I don't know how to interpret the number 256. I guess I've been assuming the LED intensity is assigned an 8-bit depth, but maybe it's really arbitrary? Could you please clarify what you're expressing?

Finally, I have the first draft running smoothly, and looks pretty good just running of a 9V battery. I do notice, however, that after a while (maybe 20 mins or so), it crashes. The LEDs start blinking white, full intensity. The battery still has plenty of charge, so I don't know if there's a rare fail to my program, or some other issue that I'm ignorant of.

Thanks again for all the quick replies! Cd00d

I'm not sure I understand your random(0,256) comment.

The 2nd argument defines an upper limit. The random function will return a value that is less than the upper limit, always. If you upper limit is is 255, the highest value that will be returned is 254. Changing it it 256 ensures that, now and then, random() will actually return a value of 255.

Ah, I see - I thought it was inclusive. Is this true on the lower limit as well?

Is this true on the lower limit as well?

No. The lower limit is inclusive.

I see. Thanks, I really appreciate the help.

Cd00d: I didn't see any difference between my while loop and your do while

The only difference is removing the duplicate code to make it a bit tidier, it wont change the output of your sketch.

Glad you got it working.