Pages: [1]   Go Down
Author Topic: Unexpected behavior – what am I missing?  (Read 945 times)
0 Members and 1 Guest are viewing this topic.
0
Offline Offline
Newbie
*
Karma: 0
Posts: 5
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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);

}
Logged

Las Vegas, NV
Offline Offline
God Member
*****
Karma: 0
Posts: 507
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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

Code:
//  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
Logged


Las Vegas, NV
Offline Offline
God Member
*****
Karma: 0
Posts: 507
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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

Code:
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
Logged


0
Offline Offline
Newbie
*
Karma: 0
Posts: 5
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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
Logged

Las Vegas, NV
Offline Offline
God Member
*****
Karma: 0
Posts: 507
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
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
Logged


Underhill Center, Vermont, USA
Offline Offline
Jr. Member
**
Karma: 0
Posts: 71
Arduino rocks
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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:

Code:
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
Logged

0
Offline Offline
Newbie
*
Karma: 0
Posts: 5
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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
Logged

Las Vegas, NV
Offline Offline
God Member
*****
Karma: 0
Posts: 507
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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
Logged


Underhill Center, Vermont, USA
Offline Offline
Jr. Member
**
Karma: 0
Posts: 71
Arduino rocks
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
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
Logged

Pages: [1]   Go Up
Jump to: