Am building environment control for my orchids

Hi,
Am just starting to build an environment control system for my orchids and am very new to programming arduino having trouble with setting two switches controlling one led. One to turn it on and the other to turn it off in a cycle fashion. Code is below...

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

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

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);
Serial.begin(9600);
}
void loop(){
// read the state of the pushbutton value:
buttonState = digitalRead(buttonPin);
buttonState2 = digitalRead(buttonPin2);

// check if the pushbutton is pressed.
// if it is, the buttonState is HIGH:
if (buttonState == HIGH) {
// turn LED on:
digitalWrite(ledPin, HIGH);
Serial.println("lights ON");
delay(60000);
}
// check if the pushbutton is pressed.
// if it is, the buttonState is LOW:
if (buttonState2 == HIGH) {
// turn LED off:
digitalWrite(ledPin, LOW);
Serial.println("lights OFF");
delay(60000);
}

}

1st problem I see is that you have 2 switches... 4 possible states... and only 2 conditions.

2nd problem... you act on states sequentially... meaning... you can have conditional interactions between actions that are taken.

You need to build your conditionals so only 1 condition is the final result... and then take actions on that FINAL result.

For example: In your current code... since there are states you have no conditions for... you should have a DEFAULT action.

In other words... You need to be thinking this way... IF condition1 is true... do action X, if condition2 is true... do action Y, ELSE DO a default action... such as reset every thing to ZERO.

So the first change I would make to create an joined set of IF blocks and add a final ELSE statement...

Here is my preferred way... and it's not always shown as an example: Change VARIABLES in if statement blocks... Don't change any pin states until ALL your IF statements about what LEDpin status should be are complete.

Also... it's best to debug OUTSIDE of those condition statements, that way only 1 debug statement is needed... the final result.

Oh... and those LONG delays are not helping your debugging...

http://www.arduino.cc/en/Reference/Else

Here is how I would code this:

I'm not saying it's the best way... just my own personal quirky way.

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

// variables will change:
int buttonState1 = LOW;         // variable for reading the pushbutton status
int buttonState2 = LOW;
int lampstate   = LOW;          // Current Status of lamp
int laststate  = LOW;           // What it was LAST TIME

void setup() {
 // initialize the LED pin as an output:
 pinMode(ledPin, OUTPUT);      
 // initialize the pushbutton pin as an input:
 pinMode(buttonPin1, INPUT);
 pinMode(buttonPin2, INPUT);
 Serial.begin(9600);
}
void loop(){
  laststate = lampstate;  // Keeping track of status
  
 // read the state of the pushbutton value:
 buttonState1 = digitalRead(buttonPin1);
 buttonState2 = digitalRead(buttonPin2);

 // check if the pushbutton is pressed.
 // if it is, the buttonState is HIGH:
 if (buttonState1 == HIGH) {    
     lampstate = HIGH;   
     }

 // if it is, the buttonState is LOW:
 if (buttonState2 == HIGH) {    
     // turn LED off:    
     lampstate = LOW;
     }
 
 // Do stuff
 if (lampstate != laststate) {    // If there was no change since last time...
    if (lampstate == HIGH){       // do nothing
        digitalWrite(ledPin, HIGH);
        Serial.println("Lights ON");
    }else {
        digitalWrite(ledPin, LOW);
        Serial.println("Lights OFF");
    }
 }
 
}

NOTE: I took out the delays...

I presume you wanted to control ON duration with that... you can do that in a separate call IF the state gets set to ON.

thanks heaps, the long delays are for a set of timers that I am hijacking that use the alarm as output, this beeps for a minute. How would I get scipt to just read the output just once, not each time it beeps?

Use the same method I used to monitor SWITCH state in the code.

IE; Set a variable on first beep... if you see more but you already set the beep state to on... you can ignore them... just like I ignored switch presses. When done with BEEPING... clear it back to zero. You will only track the first one... if you do it right.