Two switches, one light, conditional?

I would like to build a system where there are two rocker switches on two pins. There is a light connected that comes on when the first switch is set to HIGH - But when the second switch is set to HIGH also, the light will go off. This means that when the first switch is pressed again it will turn the light on, despite it's position now being LOW. Can this be done? I have written code that I thought would work, but it does nothing. Both switches and light have been tested individually, it's when the second switch condition is implemented that it all breaks.

const int buttonPin2 = 2;     // the number of the pushbutton pin
const int buttonPin = 3;     // the number of the pushbutton pin
const int ledPin =  13;      // the number of the LED pin

// variables will change:
int buttonState = 0;
int buttonState2 = 0;// variable for reading the pushbutton status

void setup() {
  // initialize the LED pin as an output:
  pinMode(ledPin, OUTPUT);      
  // initialize the pushbutton pin as an input:
  pinMode(buttonPin, INPUT);     
  pinMode(buttonPin2, INPUT);  
}

void loop(){
  // read the state of the pushbutton value:
  buttonState = digitalRead(buttonPin);
  buttonState2 = digitalRead(buttonPin2);


  while (buttonState == HIGH) {
    if (buttonState2 == HIGH) { 
    digitalWrite(ledPin, HIGH);  
  } 
  else {
    digitalWrite(ledPin, LOW); 
  }
  }
  
    while (buttonState == LOW) {     
    if (buttonState2 == HIGH) { 
    digitalWrite(ledPin, LOW);  
  } 
  else {
    digitalWrite(ledPin, HIGH); 
  }
  }
}

This was built using the button example and I've added 'while' for the first button in either HIGH or LOW to control the second buttons HIGH or LOW.

Thanks for any guidance you might be able to provide.

TL;DR Switch#1 turns light on. Switch#2 turns light off. Switch#1 then needs to switch light back on with it's next flick.

Once your code enters the while loops it can never exit since you never check the state of switches inside while loop.

Ooooh.. Okay, i've just dropped in this

  while (buttonState == HIGH) {
[b]buttonState = digitalRead(buttonPin);
buttonState2 = digitalRead(buttonPin2);[/b]
    if (buttonState2 == HIGH) { 
    digitalWrite(ledPin, HIGH);  
  } 
  else {
    digitalWrite(ledPin, LOW); 
  }
  }

And it seems to work! Thanks for the advice.

If anyone still has a moment to help, I'm now having trouble implementing a sound buzzer that goes off when the light is switched on. I've attempted to use millis() for this but as with every previous effort, I just can't get them to work. Delay() will not be useful here as the buzzer should sound for just a second or two, and then stay off indefinitely. Here's what my code looks like at the moment :

const int buttonPin2 = 2;     // the number of the pushbutton pin
const int buttonPin = 3;     // the number of the pushbutton pin
const int ledPin =  13;      // the number of the LED pin
const int soundPin = 12;

// variables will change:
int buttonState = 0;
int buttonState2 = 0;// variable for reading the pushbutton status

long previousMillis = 0;
long interval = 1000;

void setup() {
  // initialize the LED pin as an output:
  pinMode(ledPin, OUTPUT);   
  pinMode(soundPin, OUTPUT); 
  // initialize the pushbutton pin as an input:
  pinMode(buttonPin, INPUT);     
  pinMode(buttonPin2, INPUT);  
}

void loop(){
  // read the state of the pushbutton value:
  buttonState = digitalRead(buttonPin);
  buttonState2 = digitalRead(buttonPin2);

  while (buttonState == HIGH) {
    
  buttonState = digitalRead(buttonPin);
  buttonState2 = digitalRead(buttonPin2);
  
    if (buttonState2 == HIGH) { 
      
    digitalWrite(ledPin, HIGH); 
    digitalWrite(soundPin, HIGH);
    
   if(currentMillis - previousMillis > interval) {
    previousMillis = currentMillis;    
    digitalWrite(soundPin, LOW);
   }
    
  }
  else {
    digitalWrite(ledPin, LOW); 
  }
  }
  
  
    while (buttonState == LOW) {   
      buttonState = digitalRead(buttonPin);
  buttonState2 = digitalRead(buttonPin2);  
    if (buttonState2 == HIGH) { 
    digitalWrite(ledPin, LOW);  
  } 
  else {
    digitalWrite(ledPin, HIGH); 
  }
  }
}

From examples i've pulled in

long previousMillis = 0;
long interval = 1000;

and

   if(currentMillis - previousMillis > interval) {
    previousMillis = currentMillis;    
    digitalWrite(soundPin, LOW);

This second part runs just after the buzzer is switched on, but it is not turning off, even if the switch is switched turned off.

Thanks again for any help.

Maybe it's time for you to rethink and simplify the code. After all, there is more than one way to skin a cat. Start by making a truth table including all possible switch states and corresponding light state. There are 4:

0 0   0
1 0   1
1 1   0
0 1   1

As you can see the only time the light is on is when switches are in opposite states. That makes your light logic quite simple:

if (buttonState  != buttonState2)
{
  digitalWrite(ledPin, HIGH);
}
else
{
  digitalWrite(ledPin, LOW);
}

For the buzzer, you'll want to use a flag - a variable that stores the old state and compares it to the new one, then when the conditions are met (the change from the off state to on) you go about sounding the buzzer.

EDIT: The code you posted will not compile. You never declare currentMillis. But even then you never check the time with millis();

EDIT2: One more thing, a very important one. You call your variables buttonState and buttonState2 even though they refer to switches. While it is perhaps not a problem in this case, once you start writing bigger programs this will lead to confusion.

Just as a matter of interest, do you have resistors on the inputs as either pullups or pulldowns- you didn't mention any? You coded pinMode(pin, INPUT) rather than pinMode(pin, INPUT_PULLUP) so it's clear you're not using the internal pullups. But if you don't have external pullup or down, you're running the risk of false readings, since there is no guarantee of the pin's state when the switch is open.

http://arduino.cc/en/Tutorial/InputPullupSerial

Shpaget: Thanks again, but what is a flag - I can't find it here http://arduino.cc/en/Reference/HomePage ? How can I store something that is from a digitalWrite pin? I've just tried creating a variable the same as buttonState(2) and then creating an if statement for when ledPin is HIGH, but it did nothing. This has gone over my head.

JimboZA: My switches are hooked up the same as in the button example. 10k resistor on the return.

How can I store something that is from a digitalWrite pin?

A flag is just a variable, usually a bool (short for Boolean) which has only two states: so it flags it's status in the sense that a flag (a real one as in say a football lines~~man~~person's hand) is either up or down.

And since a digitalRead brings back a high or low, that fact can be stored in a bool, as follows:

bool myflag;
byte myPin = 5; //say

void setup()
{
Serial.begin(9600);
//stuff
}

void loop()
{
myFlag= digitalRead(myPin);
Serial.print(myFlag);
//more stuff
}

Flag is a variable that holds a value. You set the value however you want, not through digitalWrite. The idea is that you use it to detect the change of state and not the state itself.

So, when you detect that it is time to power up the light, you check if the flag is LOW. If it is, you change its value to HIGH and sound the buzzer. Then you turn on the light. When the code goes all the way through the loop and comes back to the same spot it will again check if the light should be on (and it will be) and it will check if the flag is LOW or HIGH. It will be HIGH so the code will ignore the part dealing with sounding the buzzer.

Same thing with turning the light off. If the flag is HIGH when the time is to turn off the light, change the value of flag back to LOW, only this time you don't need to deal with the buzzer. Then turn the light off.

ukmjb: My switches are hooked up the same as in the button example. 10k resistor on the return.

This one I assume? http://arduino.cc/en/Tutorial/Button

Cool- as long as you have one or the other. For future reference, remember there's a built-in pullUP which you enable with pinMode(pin, INPUT_PULLUP), saves fannying about with an extra resistor.

PullUP reverses the logic though, from the example's pullDOWN since with pullDOWN the pin is normally at 0V and switched high to 5V; with pullUP it's usually high at 5V and switch low to 0V.

My latest attempt, using Shpaget's light control suggestion and code from http://arduino.cc/en/Reference/BooleanVariables :

const int buttonPin2 = 2;     // the number of the pushbutton pin
const int buttonPin = 3;     // the number of the pushbutton pin
const int ledPin =  13;      // the number of the LED pin
const int soundPin = 12;

// variables will change:
int buttonState = 0;
int buttonState2 = 0;// variable for reading the pushbutton status

boolean ledon = false;


void setup() {
  // initialize the LED pin as an output:
  pinMode(ledPin, OUTPUT);   
  pinMode(soundPin, OUTPUT); 
  // initialize the pushbutton pin as an input:
  pinMode(buttonPin, INPUT);     
  pinMode(buttonPin2, INPUT);  
}

void loop(){

  buttonState = digitalRead(buttonPin);
  buttonState2 = digitalRead(buttonPin2);


if (digitalRead(buttonPin) == HIGH)
  {
    delay(100);                        // delay to debounce switch
    ledon = !ledon;                // toggle running variable
    digitalWrite(soundPin, ledon);      // indicate via LED
  }
  

if (buttonState  != buttonState2)
{
  digitalWrite(ledPin, HIGH);
}
else
{
  digitalWrite(ledPin, LOW);
}



}

I'm trying to turn on the buzzer when the ledPin is at HIGH, but the buzzer never activates.

I understand now what the flag should do, but not how to get the information for if the flag is 'up' or 'down' OR how to execute something based on that flag.

The logic makes sense, but I can't identify the commands or where to put them.

if (digitalRead(buttonPin) == HIGH)
  {
    delay(100);                        // delay to debounce switch
    ledon = !ledon;                // toggle running variable
    digitalWrite(soundPin, ledon);      // indicate via LED
  }

What this does is change ledon value 10 times per second as long as buttonPin is HIGH. You want the ledon value to change only once for each switch change.

Try:

if (ledon  == LOW)
  {
    delay(100);                        // delay to debounce switch
    ledon = !ledon;                // toggle running variable
    digitalWrite(soundPin, ledon);      // indicate via LED
  }

Also, move this last bit inside the if (buttonState != buttonState2) You want both conditions (different switch states and flag) to be met. Remember that you can nest as many ifs inside one another as you need.

Don't forget to place an equivalent code for changing the flag back to LOW (it would go inside else) and remember to turn the siren off.

It might be worth reading about Finite State Machines, where the key thing that helps me is to see how an Event causes a Transistion from CurrentState to NewState.

Wikipedia http://en.wikipedia.org/wiki/Finite-state_machine Robert C Martin http://www.objectmentor.com/resources/articles/umlfsm.pdf Nick Gammon http://www.gammon.com.au/forum/?id=12316 Grumpy Mike http://www.thebox.myzen.co.uk/Tutorial/State_Machine.html

I posted some documented code here. It's an implementation of the turnstile in the Robert C Martin link above.

Alright, I think this is progress, with this code the light comes on, goes off, as does the beeper, all working nicely. Now all I need is to add the delay (or equivalent) so that the beep only lasts for three seconds and then stays off until the next switch-on.

const int buttonPin2 = 2;     // the number of the pushbutton pin
const int buttonPin = 3;     // the number of the pushbutton pin
const int ledPin =  13;      // the number of the LED pin
const int soundPin = 12;

// variables will change:
int buttonState = 0;
int buttonState2 = 0;// variable for reading the pushbutton status

boolean ledon = false;

void setup() {
  // initialize the LED pin as an output:
  pinMode(ledPin, OUTPUT);   
  pinMode(soundPin, OUTPUT); 
  // initialize the pushbutton pin as an input:
  pinMode(buttonPin, INPUT);     
  pinMode(buttonPin2, INPUT);  
}

void loop(){

  buttonState = digitalRead(buttonPin);
  buttonState2 = digitalRead(buttonPin2);

if (digitalRead(buttonPin) == HIGH)
  {
    delay(100);                        // delay to debounce switch
    ledon = !ledon;                // toggle running variable
    digitalWrite(soundPin, ledon);      // indicate via LED
  }

if (buttonState  != buttonState2)
{
      if (ledon  == LOW)
    {
      ledon = !ledon;                // toggle running variable
      digitalWrite(soundPin, ledon);      // indicate via LED
      
    }
  digitalWrite(ledPin, HIGH);
  

  
}


else
{
    if (ledon  == HIGH)
    {
      ledon = !ledon;                // toggle running variable
      digitalWrite(soundPin, ledon);      // indicate via LED
      
    }
  digitalWrite(ledPin, LOW);
}



}

Get rid of this.

if (digitalRead(buttonPin) == HIGH)
  {
    delay(100);                        // delay to debounce switch
    ledon = !ledon;                // toggle running variable
    digitalWrite(soundPin, ledon);      // indicate via LED
  }

You don't want to mess with the value of ledon unless there is a change in the state of the button. What this does is toggle the ledon value and turns the buzzer on and off rapidly for as long as buttonPin is HIGH. You want only one change.

For the off timer, you started to work with millis(). That's fine and you'll need to get to know how to use it eventually, so why not now, eh? I'm not sure how familiar are you with it, so forgive me if I cover the basics.

millis() gives you the time in milliseconds from the moment the microchip was turned on. It doesn't tell the time, only elapsed time. It's like a stopwatch, not a clock. Using it is the same as you use any watch when you want to do something for a period of time. In your case you want to turn off a light three seconds after it was lit up. What you do is take a look at what time it is (millis()) at the moment of turning the light on and store that time in a variable. You want to make sure you write down that time only once - at the moment of turning the light on and don't overwrite it unintentionally. We've already set up the basis for this when we introduced the flag. The place to check this time is in if (ledon == LOW) Now that you know when you turned the light on, all you need to do is occasionally take a look at the watch and see if three seconds have passed. If yes, turn the light off, if not, check again later. In this case a good place to check time and decide if you want to do something is in the main loop.

I feel I'm getting close, again thanks for continued guidance. Millis has never made sense to me, but your explanation made it clear as day. Unfortunately it's not doing what I would now expect it to.

const int buttonPin2 = 2;     // the number of the pushbutton pin
const int buttonPin = 3;     // the number of the pushbutton pin
const int ledPin =  13;      // the number of the LED pin
const int soundPin = 12;

// variables will change:
int buttonState = 0;
int buttonState2 = 0;// variable for reading the pushbutton status

boolean ledon = false;

unsigned long previousMillis = 0;
unsigned long currentMillis = 0;

long three = 3000;

void setup() {
  // initialize the LED pin as an output:
  pinMode(ledPin, OUTPUT);   
  pinMode(soundPin, OUTPUT); 
  // initialize the pushbutton pin as an input:
  pinMode(buttonPin, INPUT);     
  pinMode(buttonPin2, INPUT);  
}

void loop(){

  buttonState = digitalRead(buttonPin);
  buttonState2 = digitalRead(buttonPin2);



if (buttonState  != buttonState2)
{
      if (ledon  == LOW)
    {
      ledon = !ledon;                // toggle running variable
      digitalWrite(soundPin, ledon);      // indicate via LED
      unsigned long currentMillis = millis();
    }
  digitalWrite(ledPin, HIGH);
  

  
}

else
{
  digitalWrite(ledPin, LOW);
}

if(currentMillis - previousMillis > three) {
    previousMillis = currentMillis;   
      if (ledon = true){
        digitalWrite(soundPin, LOW);
      }
  }

}

I've given current and previousMillis variables. Saved currentMillis at the buzzer switch on flag. Then i've used code from 'BlinkWithoutDelay' to ask if currentMillis minus previousMillis is greater than 3000, if it is, save previousMillis as the new currentMillis ready for next time. Then if ledon is still true (which it should be) then tell the buzzer(soundPin) to turn off.

I've tried juggling things around, but at the moment the buzzer comes on with the light and never stops.

No problem. Others helped me, all I’m trying to do is spread the love around.

      if (ledon  == LOW)
    {
      ledon = !ledon;                // toggle running variable
      digitalWrite(soundPin, ledon);      // indicate via LED
      unsigned long currentMillis = millis();
    }

Get rid of “unsigned long”.
You already declared that variable.
currentMillis = millis(); is enough.
I would advise you use a different, more descriptive, variable name, though. Something like buzzerOnMillis or similar.
Other than that, this part is good. You now know when the buzzer started to buzz.

if(currentMillis - previousMillis > three) {
    previousMillis = currentMillis;   
      if (ledon = true){
        digitalWrite(soundPin, LOW);
      }
  }

You never change the value of previousMillis so it’s still 0. That means the code will enter this if statement if the buzzer was turned on more than three seconds after the power up. That is not what you want, is it?

You need to check if current time is more than three seconds after the time the buzzer was turned on.
if (millis() > currentMillis + three) or buzzerOnMillis if you chose to rename the variable.
Same condition can be expressed in more ways:
if (millis() - three > currentMillis)
if (currentMillis + three < millis())
if (currentMillis < millis() - three)
Pick the one you find the most intuitive.

You still need to add the part where you turn the flag back to LOW when the switches are in proper state.

"three" is a crazy name for a variable. Firstly it doesn't describe itself, like eg "buzzerOnTime" would. Second, if experience showed you that 3 was a wrong value for the time and preferred 4, then you would say three = 4 which is a bit weird....

Haha, I can tell you guys are getting really annoyed at the naming conventions - let me explain, I'm quite simple, and so once I've read something and understood it things have to stay the same, or else I look at a term and think "where did that come from?!". So if I'm borrowing code from an example, or making one up on the spot, I have to keep them until I'm confident the code works. Then rest assured I'll find & replace to make things are clearer.

I'm happy with how this is working now, here is the code that works for me. If you're still here, let me know if there are any glaring faults, but as it stands it works, buzzer for three seconds, LED comes on, everything goes off when it should.

const int buttonPin2 = 2;     // the number of the pushbutton pin
const int buttonPin = 3;     // the number of the pushbutton pin
const int ledPin =  13;      // the number of the LED pin
const int soundPin = 12;

// variables will change:
int buttonState = 0;
int buttonState2 = 0;// variable for reading the pushbutton status

boolean ledon = false;

unsigned long previousMillis = 0;
unsigned long currentMillis = 0;

long three = 3000;

void setup() {
  // initialize the LED pin as an output:
  pinMode(ledPin, OUTPUT);   
  pinMode(soundPin, OUTPUT); 
  // initialize the pushbutton pin as an input:
  pinMode(buttonPin, INPUT);     
  pinMode(buttonPin2, INPUT);  
}

void loop(){

  buttonState = digitalRead(buttonPin);
  buttonState2 = digitalRead(buttonPin2);



if (buttonState  != buttonState2)
{
      if (ledon  == LOW)
    {
      ledon = !ledon;                // toggle running variable
      digitalWrite(soundPin, ledon);      // indicate via Buzz
      currentMillis = millis();
    }
  digitalWrite(ledPin, HIGH);
  

  
}

else
{
  if (ledon  == HIGH)
    {
      ledon = !ledon;                // toggle running variable
    }
  digitalWrite(ledPin, LOW);
}

if (millis() > currentMillis + three) {
        digitalWrite(soundPin, LOW);
  }

}

Thanks again for the help from both of you, it does not look like much for two days work, but I always knew the coding would be the challenge. I'm eager to get in and install the system.

And of course, just in case I ever come back to the code later, I'm going to comment in all the critical parts.