Turning on random LEDs using a pushbutton.

I have an assignment for a class I am taking to get a random LED to light (out of three) using a push button, and stay lit, then when the other button is pressed it will go out. I thought my program would work, but it won't turn on just ONE LED, it turns all of them on (I am just trying to get the random part for now).

const int button1Pin = 2;
const int button2Pin = 3;
const int ledPin =  13;
const int ledPin1 = 12;
const int ledPin2 = 8;
const int maxLedPins = 3;

int ledpins[maxLedPins] = {ledPin, ledPin1, ledPin2};

void setup()
{
  pinMode(button1Pin, INPUT);
  pinMode(button2Pin, INPUT);
  pinMode(ledPin, OUTPUT); 
  pinMode(ledPin1, OUTPUT); 
  pinMode(ledPin2, OUTPUT); 
}

void loop()
{
int button1State, button2State;
button1State = digitalRead(button1Pin);
button2State = digitalRead(button2Pin);
if (((button1State == LOW) || (button2State == LOW))
      && !                                               
      ((button1State == LOW) && (button2State == LOW)))
{
digitalWrite (ledpins[random(maxLedPins)], HIGH);
}
else
{
    digitalWrite(ledPin, LOW); 
    digitalWrite(ledPin1, LOW); 
    digitalWrite(ledPin2, LOW); 
}
}
  pinMode(button1Pin, INPUT);
  pinMode(button2Pin, INPUT);

You are not using the internal pullup resistor. So, how are your switches wired?

int button1State, button2State;
if (((button1State == LOW) || (button2State == LOW))
      && !                                               
      ((button1State == LOW) && (button2State == LOW)))

You don't think it's important to actually read the switch to determine the state?

Fixed my code in the first post. Realized it didn't copy everything.

It is possible, preferred even, to declare and initialize a variable on one line:

int button1State = digitalRead(button1Pin);
int button2State = digitalRead(button2Pin);

It is possible to print to the serial port the data that you read, so you can tell that the value matches the switch state that you expect, to rule out floating pin issues.

if (((button1State == LOW) || (button2State == LOW))
      && !                                               
      ((button1State == LOW) && (button2State == LOW)))

When you use convoluted logic like this, comments are extremely important.

Even more important though is a clear set of requirements. Why are there two switches to randomly light one of three LEDs?

If one turns a random LED on, the other should turn that LED off. Of course that means that you either have to keep track of which LED was turned on (not difficult) to turn all the LEDs off, regardless of whether an LED was on or not) (trivial).

The key is to make reading each switch, and acting on the switch state, independent. There should be nothing in the "if switch one pressed" code that knows or cares about the state of switch two. Similarly, there should be nothing in the "if switch two pressed" code that knows or cares about the state of switch one.

My assignment as it was given to me. Is that button1 (or 2) must light 1 out of the 3 LEDs and keep it lit. While the opposite button must turn all of the lit LEDs off. However, if both buttons are pressed, do nothing. This is a basic class, and I am new to arduino, as well as programming (I have done some HTML/CSS/very little C++ and some PERL).

if (((button1State == LOW) || (button2State == LOW))//If button 1 or button 2 is pressed
      && !                                                                     //And Not
      ((button1State == LOW) && (button2State == LOW)))   //Both button 1 and button 2

you push one button => a random LED turn on you push both => the three LED turn off

that's it ? If yes don't worry, it comes from your code :P

EDIT : Sorry, i didn't see your last answer -_-

My assignment as it was given to me. Is that button1 (or 2) must light 1 out of the 3 LEDs and keep it lit. While the opposite button must turn all of the lit LEDs off. However, if both buttons are pressed, do nothing.

This is a little more complicated, then. First, you need to set a boolean (call it one) to true if switch one is pressed, and to false of it isn’t. Then, set another boolean (call it two) to true if the second switch is pressed, and to false if it isn’t.

Then, you have four conditions to deal with:

if(one && two) // Both are pressed
{
   // Do NOTHING
}
else if(one) // Only switch one is pressed
{
   // Light a random LED
}
else if(two) // Only switch two is pressed
{
   // Turn all LEDs off
}
else // Neither switch is pressed
{
   // Do nothing
}

Dealing with the effect of pressing a switch should be separate from detecting that the switch is, or is not, pressed.

I edited my last post... Sorry, i didn't see your last answer -_-

Thegrim: Is that button1 (or 2) must light 1 out of the 3 LEDs and keep it lit. While the opposite button must turn all of the lit LEDs off.

The four conditions given by PaulS are good, but you need to set a third parameter to save what button was pushed first. The code seems like this :

if(one && two) // Both are pressed
{
   // Do NOTHING
}
else if(one) // Only switch one is pressed
{
   if(one was pushed first)
   {
       // Light a random LED
   }
   else
   {
       // Turn all LEDs off
   }
}
else if(two) // Only switch two is pressed
{
   if(two was pushed first) //the opposite
   {
       // Light a random LED
   }
   else
   {
       // Turn all LEDs off
   }
}
else // Neither switch is pressed
{
   // Do nothing
}

You need to save the last state changing of your buttons, then to compare each other to determinate which one was pushed first. Quite borring but working fine :P

PaulS:

My assignment as it was given to me. Is that button1 (or 2) must light 1 out of the 3 LEDs and keep it lit. While the opposite button must turn all of the lit LEDs off. However, if both buttons are pressed, do nothing.

This is a little more complicated, then.

Not really. If you turn everything off every time you detect switch 2 is active and do this processing after dealing with switch 1 then you will get the required behaviour without having to combine the state of both switches.

You need to save the last state changing of your buttons, then to compare each other to determinate which one was pushed first.

If that is a requirement. I didn't read that into the project description, but we didn't see all of it.

I'd recommend skipping this requirement for now. It is easy enough to get the order at any time (based on when each becomes pressed), and add to the if blocks code to deal with the order. But, do that as version 2.

Yes, it should work in each sens. 1 first then 2 or 2 first then 1. Building your programme step by step is a wise idea :) It will be easier to add other requirements later if your base code is strong.

Awesome, so far it will turn on either ALL LEDs (3) and keep them on, and it will turn them off as well. However, I can not seem to get the random to work properly.

const int button1Pin = 2;
const int button2Pin = 3;
const int ledPin =  13;
const int ledPin1 = 12;
const int ledPin2 = 8;
const int maxLedPins = 3;

int ledpins[maxLedPins] = {ledPin, ledPin1, ledPin2};

void setup()
{
  pinMode(button1Pin, INPUT);
  pinMode(button2Pin, INPUT);
  pinMode(ledPin, OUTPUT); 
  pinMode(ledPin1, OUTPUT); 
  pinMode(ledPin2, OUTPUT); 
}

void loop()
{
int button1State, button2State;
button1State = digitalRead(button1Pin);
button2State = digitalRead(button2Pin);
if  ((button1State == LOW) && (button2State == LOW)) // If both buttons are pressed turn all LEDs OFF
{
    digitalWrite(ledPin, LOW); 
    digitalWrite(ledPin1, LOW); 
    digitalWrite(ledPin2, LOW); 
}
else if (button1State == LOW) // If button1 is pressed turn on a random LED
{
digitalWrite (ledpins[random(maxLedPins)], HIGH);
}
else if (button2State == LOW) // If button2 is pressed turn off all LEDs
{
    digitalWrite(ledPin, LOW); 
    digitalWrite(ledPin1, LOW); 
    digitalWrite(ledPin2, LOW); 
}
}

The random function works fine, but Arduino is very, very and very fast ! You push either button, but the information send to arduino (position LOW) maybe take 100ms... so the loop goes so fast that the random function is called several times before you stop pushing the button. Does it seem to be your problem ?

Official documentation is "sometime"s helpfull (you see what I mean PaulS ? :P) : http://arduino.cc/en/Tutorial/Debounce (how to debounce a button input)

The problem is that when I push button1 to light a random LED, it lights all 3 at the same time. My random itself is not working. Also, since this is only our 2nd assignment, I do not feel that it has anything to do with debounce. More likely I have an error with my usage of random.

Ok, I gonna test your code with my arduino, I come back :P

Do you use randomSeed() to re-initialize the number generator ?

Your use of the random function appears fine to me except you don't seed it anywhere.

You could try a very crude debounce by adding a delay(250) right after both buttons are read. If that helps you may have found your problem. I suspect debounce myself. Reading buttons is trickier than one might expect.

ETA: Float may also be an issue. Try setting the switch pins with pinMode(button1Pin, INPUT_PULLUP);

It was debounce + not using the random command properly. Got it to work.

const int button1Pin = 2;
const int button2Pin = 3;
const int ledPin =  13;
const int ledPin1 = 12;
const int ledPin2 = 8;
const int maxLedPins = 3;

long randNumber;
int ledpins[maxLedPins] = {ledPin, ledPin1, ledPin2};

void setup()
{
  pinMode(button1Pin, INPUT);
  pinMode(button2Pin, INPUT);
  pinMode(ledPin, OUTPUT); 
  pinMode(ledPin1, OUTPUT); 
  pinMode(ledPin2, OUTPUT); 
}

void loop()
{
randNumber = random(300);
Serial.println(randNumber);
int button1State, button2State;
button1State = digitalRead(button1Pin);
delay(175);
button2State = digitalRead(button2Pin);
if  ((button1State == LOW) && (button2State == LOW))
{
    digitalWrite(ledPin, LOW); 
    digitalWrite(ledPin1, LOW); 
    digitalWrite(ledPin2, LOW); 
}
else if (button1State == LOW)
{
digitalWrite (ledpins[random(maxLedPins)], HIGH);
}
else if (button2State == LOW)
{
    digitalWrite(ledPin, LOW); 
    digitalWrite(ledPin1, LOW); 
    digitalWrite(ledPin2, LOW); 
}

}

Darwoon: so the loop goes so fast that the random function is called several times before you stop pushing the button.

I was right (Jimmy60 also) ! I just fix it with adding a delay of 1 second. The led turn on one by one. But without delay, as it is so fast, you see the three led turning on at the same time. That's why you need a debounce (and not a delay, because while your script is freezing, it doesn't pick up the push).

To-do list :P : debounce, pull-up resistance, re-initialize random number generator, put a delay of 1ms (it's better always put the minimal delay). Then it's gonna work better ! Before adding the third requirements... not an easy task you have to do hum ? :)

The issue is not so much debounce as it is also one of state-change. I think if you hold your on-button down all three leds will eventually light (which is probably not what you want according to your original statement of requirements). If you agree, the fix is to turn on a random led only once when the button goes from off to on ( rather than every time through the loop when the button is on).

Cheers, John

johncc: The issue is not so much debounce

The issue is clearly debounce ! How many times should I have to repeat ? I fixed it with my Arduino... and it's works perfectly.

johncc: when the button goes from off to on

You're talking about debounce, but you don't realize it :P