Unexpected behavior ? what am I missing?

Good day,
I am posting here because I a stumped.
I have created this sketch that generates a random, 0 or 1. If 1, then light the red LED. If 0, then turn off the red LED. Do that for green and blue LED as well.
Then delay for a random amount of time, between .25 and 1 second.

But, here is where I get confused. Towards the end of the sketch, it checks if all random numbers are 0, if true then set the delay variable to 0, for no delay.

I want this check done, so that the there is always at least one color of the LED lit.

But once is a while, the LED is completely dark.

Any ideas on what I am missing?

Thanks in advance for your help.
Dave

Here is my code

/*

  • Random RGB

*/

int L1 = 9; // Red LED connected to pin 9
int L2 = 10; // Blue LED connected to pin 10
int L3 = 11; // Green LED connected to pin 11
int R1 = 0; // random number 1
int R2 = 0; // random number 2
int R3 = 0; // random number 3
int D = 0; // delay value

void setup() // run once, when the program starts
{
pinMode(L1, OUTPUT); // sets the digital pin as output
pinMode(L2, OUTPUT); // sets the digital pin as output
pinMode(L3, OUTPUT); // sets the digital pin as output
randomSeed (analogRead (0)); // random seed

}

void loop()

{
//generate random delay time -between .25 and 1 second

D = random (250,1001);

// === Red LED ===

R1 = random (2); //generate random value of 0 or 1

if (R1 == 1) // if 1, LED on, if 0 LED off
{
digitalWrite(L1, HIGH);
}

if (R1 == 0)
{
digitalWrite(L1, LOW);
}

// === Blue LED ===

R2 = random (2); // generate random value of 0 or 1

if (R2 == 1) // if 1 LED on, if 0 LED off
{
digitalWrite(L2, HIGH); // LED on
}

if (R1 == 0)
{
digitalWrite(L2, LOW); // LED off
}

// === Green LED ===

R3 = random (2); // generate random value of 0 or 1

if (R3 == 1) // if 1 LED on, if 0 LED off
{
digitalWrite(L3, HIGH); // LED on
}

if (R1 == 0)
{
digitalWrite(L3, LOW); // LED off
}

// Check if all random numbers are 0
// If true, set delay to 0

if (R1 == 0 && R2 == 0 && R3 == 0)
{

D = 0;

}

// === Delay ===

delay(D);

}

Do you still experience the problem if you change the end of your loop() to be like:

//  Check if all random numbers are 0 
//  If true, set delay to 0 
 
 if (!(R1 == 0 && R2 == 0 && R3 == 0)) 
   delay(D);

I don't know exactly how the Arduino delay function works, but in some libraries (e.g. <util/delay.h>) delay(0) is equivalent to delaying for the maximum amount of time the function can achieve.

  • Ben

Thinking about it a little more, maybe the better approach would be for you to do something like:

void loop()                     
{
 do
 {
   R1 = random (2);        //generate random value of 0 or 1
   R2 = random (2);
   R3 = random (2);
 while (R1 == 0 && R2 == 0 && R3 == 0);

 // === Red LED ===
 if (R1 == 0)
 {
   digitalWrite(L1, LOW);
 }
 else
 {
   digitalWrite(L1, HIGH);
 }

 // === Blue LED ===
 if (R2 == 0)
 {
   digitalWrite(L2, LOW);
 }
 else
 {
   digitalWrite(L2, HIGH);
 }

 // === Green LED ===
 if (R3 == 0)
 {
   digitalWrite(L3, LOW);
 }
 else
 {
   digitalWrite(L3, HIGH);
 }

 // === Delay ===
 //generate random delay time -between .25 and 1 second
 D = random (250,1001);
 delay(D);
}

This way your code never even makes the LED completely dark. It'd still be interesting to see why your previous code didn't work quite right, though.

  • Ben

Hey Ben,

Thanks for you quick replies. Also, thanks for introducing me to a new command, Do?.While. I recently learned of the Else part of the IF but wanted to get my code working before I made any further changes.

I have discovered what my error is. I feel pretty dumb?.I did not change the variable names (Rn) after I copied and pasted. Changing these variable names corrected the issue.

Again, thanks for you quick reply.

Dave

I have discovered what my error is. I feel pretty dumb?.I did not change the variable names (Rn) after I copied and pasted. Changing these variable names corrected the issue.

Ah, now I see it. I totally missed that when scanning through your code. Things like this are why I strongly recommend using "else" in these cases rather than two independent but mutually exclusive "if" clauses. I'm very glad you were able to get to the root of your problem as it's never satisfying to get something working without first understanding why the old version was failing.

  • Ben

A small point here ... in so much as "random(2)" can produce on the values 0 and 1 why not eliminate a lot of code by simply writing this instead:

 R1 = random(2);
 digitalWrite(L1, R1);

 R2 = random(2);
 digitalWrite(L2, R2);

 R3 = random(2);
 digitalWrite(L3, R3);

 if ((R1 + R2 + R3) > 0)
       delay(random(250,001));

just a thought ... cheers ... BBR

Brianbr,

That code is much more efficient, but I have a question.
I thought a digital write had to be key word, either HIGH or LOW. I understand that these are digital outputs, but is writing a 1 a valid value? Isn't the Arduino software looking for "HIGH" or "LOW"?

Thanks,
Dave

HIGH and LOW are #defines in wiring.h, so the compiler replaces them by 1 and 0 when you compile:

#define HIGH 0x1
#define LOW 0x0

The same is true for INPUT and OUTPUT.

As far as I know, the Arduino software is not compiling your code, the GCC C/C++ compiler is.

  • Ben

HIGH and LOW are #defines in wiring.h, so the compiler replaces them by 1 and 0 when you compile:

#define HIGH 0x1
#define LOW 0x0

The same is true for INPUT and OUTPUT.

As far as I know, the Arduino software is not compiling your code, the GCC C/C++ compiler is.

  • Ben

Quite correct. The Ardino IDE is, essentially, a "pre-processor" which takes the various Arduino 'defines' and command directives and builds a .cpp file which then gets fed to the gcc compiler.

cheers ... BBR