Arduino skipping for loop?

Basically, I have this sketch that is suppose to randomly change the color of a multicolor color, but do it in a smooth manor… If that makes any sense.

Anyway, first off, here is my code.

const int redPin   = 9;
const int bluePin  = 10;
const int greenPin = 11;
int redval = 255;
int greenval = 255;
int blueval = 255;
int oldredval;
int oldgreenval;
int oldblueval;


void fadeToTarget(int to, int from, int targetPin)
{
  if (to>from)
  {
    for (int flevel = from; to==flevel; flevel--)
    {
      analogWrite(targetPin, flevel);
      delay(1000);
    }
  }
  else if (to < from)
  {
    for (int flevel = from; to==flevel; flevel++)
    {
      analogWrite(targetPin, flevel);
      delay(1000);
    }
  }
  else 
  {
    Serial.println("exc");
  }

}

void setup()
{
  Serial.begin(9600);
  pinMode(bluePin, OUTPUT);
  pinMode(redPin, OUTPUT);
  pinMode(greenPin, OUTPUT);
}

void loop()
{
  oldblueval = blueval;
  blueval = random(0, 255);
  fadeToTarget(blueval, oldblueval, bluePin);
  delay(20);

  oldredval = redval;
  redval = random(0, 255);
  fadeToTarget(redval, oldredval, redPin);
  delay(20);

  oldgreenval = greenval;
  greenval = random(0, 255);
  fadeToTarget(redval, oldgreenval, greenPin);
  delay(20);
}

The problem is that it skips over the 2 for loops altogether. It simply never executes the code within. I verified this with a Serial.println

Any ideas?

are you sure the if statements are passing true

  if (to>from)
  {
    for (int flevel = from; to==flevel; flevel--)

If the target (to) is greater than the starting point (from), shouldn’t you be incrementing flevel?

      delay(1000);

In the extreme case, it will take over 4 minutes to reach the new level (255 seconds). While this may be what you want for the final version, during testing you will probably be better served with a much shorter delay.

  else if (to < from)
  {
    for (int flevel = from; to==flevel; flevel++)

If the target (to) is less than the starting point (from) shouldn’t you be decrementing flevel?

The code you’ve presented will fade each LED individually (blue fades first and completely to the new value then red fades completely to the new value then green fades completely to the new value). Is this what you want?

The reason that the for() loops are never executed is because of
your comparison. for() loops can execute zero times if, from the
start, the comparison is false.

void fadeToTarget(int to, int from, int targetPin)
{
  if (to>from)
  {
    for (int flevel = from; to==flevel; flevel--)

At the start of the loop, flevel=from and we know from the if()
that ‘to>from’ thusly ‘to==flevel’ or ‘to==from’, which is false
and the for() loop never executes. The middle portion of the
for ( ; here; ) statement – needs to be true, from the start, until
the loop is to end. This is NOT a comparison to determine when
to stop. I suspect that you want something like

 for (int flevel=from; flevel<to; flevel++)

The same goes for the for() loop in the else if() code block although
I’ll leave it for ‘chris…’ to figure out the proper way to for that second
for() loop.

Naturally, you should heed 'Coding Badly’s advice as well about the
direction of your ‘flevel’ variable in the loops.

In the extreme case, it will take over 4 minutes to reach the new level (255 seconds).

He's using int (16-bit) as a control loop variable, so it will actually take more than 18 hours (65536 seconds) to complete the loop.

BenF: While it is true that to and from are capable of storing 16 bits of data the actual values are constrained to the range 0 through 255…

blueval = random(0, 255);

Why do you use “==” instead of “>”, “>=” or “<”, “<=” in your for cycles ?

Wow, lots of replies... Thanks guys!

The long delay was only added because I wanted to be sure what was going on. At one point, I had it sending out to serial, so if I saw a 1/2 second delay, I knew it was working.

I do want the LEDs to fade individually.

Anyway, I got it working, boy I love multicolored LEDs!

By the way, here is the finished code.

// Pin assignments
const int redPin = 9;
const int bluePin = 10;
const int greenPin = 11;

// Delay times
const int color2Color = 20;
const int shade2Shade = 4;

// We need to set the inital values, since in the first iteration
// they will be copied over to the oldvalue.
int redval = 255;
int greenval = 255;
int blueval = 255;

int oldredval;
int oldgreenval;
int oldblueval;

// Fades the target LED gradually from a value to a value.
void fadeToTarget(int to, int from, int targetPin)
{
if (to>from)
{
for (int flevel = from; to>flevel; flevel++)
{
analogWrite(targetPin, flevel);
delay(shade2Shade);
}
}
else if (to<from)
{
for (int flevel = from; to<flevel; flevel–)
{
analogWrite(targetPin, flevel);
delay(shade2Shade);
}
}
}

void setup()
{
pinMode(bluePin, OUTPUT);
pinMode(redPin, OUTPUT);
pinMode(greenPin, OUTPUT);
}

void loop()
{
// Blue
oldblueval = blueval;
blueval = random(0, 255);
fadeToTarget(blueval, oldblueval, bluePin);
delay(color2Color);

// Red
oldredval = redval;
redval = random(0, 255);
fadeToTarget(redval, oldredval, redPin);
delay(color2Color);

// Green
oldgreenval = greenval;
greenval = random(0, 255);
fadeToTarget(redval, oldgreenval, greenPin);
delay(color2Color);
}

BenF: While it is true that to and from are capable of storing 16 bits of data the actual values are constrained to the range 0 through 255...

blueval = random(0, 255);

You may, then, wish to use the 'byte' datatype instead, which is 8-bit, i.e. capable of storing 2^8 -- or 256 -- values.

blueval = random(0, 255

sp. “the actual values are constrained to the range 0 through 254

:wink:

Whoops, sorry guys. Released that I've just resurrected this from 7th Dec.