Electricity Project

I'm in a physics class and would like to go above and beyond, but i need some help getting this code to work. Any suggestions? Code is below all help is appreciated. Basically what i wanted to do is control lights on and off with a pushbutton using my own code, but it is not registering and nothing is lighting up. I also have a potentiometer that will control the brightness of the PWM leds, the pot is working just not the digital pins. There are 5 buttons and 7 LEDs.
Thanks,
Andrew

#define led1 0
#define led2 1
#define led3 2
#define led4 3
#define led5 4
#define led6 5
#define led7 6

#define button1 7
#define button2 8
#define button3 9
#define button4 10
#define button5 11

int state1 = 0; //stores the current state of the led
int state2 = 0;
int state3 = 0;
int state4 = 0;
int state5 = 0;

int pot = 0;
int potval = 0;

int temp1 = 0;
int temp2 = 0;
int temp3 = 0;
int temp4 = 0;
int temp5 = 0;

void setup() 
{
  pinMode(led1, OUTPUT);  
  pinMode(led2, OUTPUT);
  pinMode(led3, OUTPUT);
  pinMode(led4, OUTPUT);
  pinMode(led5, OUTPUT);
  pinMode(led6, OUTPUT);
  pinMode(led7, OUTPUT);
  
  pinMode(button1, INPUT);
  pinMode(button2, INPUT);
  pinMode(button3, INPUT);
  pinMode(button4, INPUT);
  pinMode(button5, INPUT);
  
  Serial.begin(9600);  //for debugging
}

void loop() 
{  
  pot = analogRead(0); //get reading from pot
  potval = map(pot, 0, 1023, 0, 255); //if it appears that the whole scale is not used use a smaller or no resistor
  
  if(button1==1 || button2==1 || button3==1 || button4==1 || button5==1)  // if any state change wait 50ms for debounce
  {
    delay(50);  //debounce
    temp1 = digitalRead(button1);  //then determine which buttons were pressed
    temp2 = digitalRead(button2);
    temp3 = digitalRead(button3);
    temp4 = digitalRead(button4);
    temp5 = digitalRead(button5);
    
    if(temp1 == 1 && state1 == 0)        //if a button is pressed change the led status, if 0 change to 1, if 1 change to 0.
    {
      state1 = 1;
    }
    else
    {
      state1 = 0;
    }
    
    if(temp2 == 1 && state2 == 0)        
    {
      state2 = 1;
    }
    else
    {
      state2 = 0;
    }
    
    if(temp3 == 1 && state3 == 0)        
    {
      state3 = 1;
    }
    else
    {
      state3 = 0;
    }
    
    if(temp4 == 1 && state4 == 0)      
    {
      state4 = 1;
    }
    else
    {
      state4 = 0;
    }
    
    if(temp5 == 1 && state5 == 0)        
    {
      state5 = 1;
    }
    else
    {
      state5 = 0;
    }
  }
  
  if(state3 == 1 )  //change the status of the PWM LEDs
  {
    analogWrite(led4, potval);
  }
  else
  {
    analogWrite(led4, 0); //write the pot value or off
  }
  
  if(state5 == 1 )
  {
    analogWrite(led6, potval);//same as above
  }
  else
  {
    analogWrite(led6, 0);
  }
  
    digitalWrite(led1, state1);  //write status to digital leds
    digitalWrite(led2, state2);
    digitalWrite(led3, state3);
    digitalWrite(led5, state5);
    digitalWrite(led7, state5); //control 2 LEDs
    
    Serial.print(state1);  //to help with debugging, tell the state
    Serial.print("   ");
    Serial.print(temp1);
    Serial.print("   ");
    Serial.println(potval);
    delay(20);  //slow down the serial conn.
}

No code posted!

Code can't get more virtual than this.

I'd suggest instead of:

if(temp1 == 1 && state1 == 0)        //if a button is pressed change the led status, if 0 change to 1, if 1 change to 0.
    {
      state1 = 1;
    }
    else
    {
      state1 = 0;
    }

use

if(temp1)
    
      state1 = ~state1;

http://www.arduino.cc/playground/Code/BitMath

I'd also suggest http://arduino.cc/en/Reference/Array for your reading pleasure.

Given that:

#define button1 7
#define button2 8
#define button3 9
#define button4 10
#define button5 11

this:if(button1==1 || button2==1 || button3==1 || button4==1 || button5==1) doesn't ever seem likely, does it?

Also, not such a good idea to hang LEDs on a serial port you're trying to use for debugging.

Some guidance: start simple, and build up from that, testing and retesting at every stage.

Also, not such a good idea to hang LEDs on a serial port you're trying to use for debugging.

Don't forget that you can use the analog pins as digital pins, too (A0, A1, A2, etc.).

Thanks for the tips guys! I am learning so much lately now that I've given more time to do what i love! The small things like code efficiency which i still don't fully understand! But, In reply to AWVOL's comment, i think that you have confused the operands because the if statement says if button1 OR button2 OR... is pressed DO... , so if any button is pressed!

-feel free to correct me if i'm wrong!

Thanks guys! oh, and I agree with you on the serial port thing, im short on pins and it was just occansionly used for debugging, when it is on display i will comment out.

Andrew

In reply to AWVOL's comment, i think that you have confused the operands because the if statement says if button1 OR button2 OR... is pressed DO

I think that I have been doing this sort of thing long enough that I am not confused.

I do however, despite my advancing years, remember how the C preprocessor works.
Let's expand the line causing the confusion, and see what the compiler sees:if(7==1 || 8==1 || 9==1 || 10==1 || 11==1)
All perfectly valid C, but a condition that is very unlikely to ever fire.

im short on pins

Did you read PaulS' comments about using the analogue pins as digital I/Os?

i think that you have confused the operands because the if statement says if button1 OR button2 OR... is pressed DO... , so if any button is pressed!

To expand on what AWOL is saying, your code is NOT saying that "if button1 is pressed OR button 2 is pressed OR...". It is saying "If the pin number is 1 OR the other pin number is 1 OR ...".

You are checking the value of the pin number, not the state of the pin.
if(state1 == 1 || state2 == 1 ...
Using HIGH or LOW instead of 0 and 1 makes the code easier to understand, by the way. One need not remember what value represents HIGH or LOW.