light control unit code problems

there is a subtle difference i just got misses to check as well lol
prob not the difference you hoping for tho lol

Sorry, I don't understand your last post.

ok so where do you think i have gone wrong as i thought i had all the semicolons in the right place obviously i havent?

Look at your if statements (I can't because you haven't posted your code).
Is there a semicolon there?
Do you think it should be there?
If so, leave it there, otherwise, remove it.

i would say leve it there but i don't have much experience of this

// Example 02: Lighting Timer  26/10/2011

#define A_light 13     // Light A on pin 13
#define B_light 12     // Light B on pin 12
#define C_light 11     // Light C on pin 11
#define D_light 10     // Light D on pin 10
#define time_button 7  // Button on pin 7
#define A_light_button  1// Light A button
#define B_light_button  2// Light B button
#define C_light_button  3// Light C button
#define D_light_button  4// Light D button

int timebutton = 0;     
int buttonA = 0;      
int buttonB = 0;         // val will be used to store the
int buttonC = 0;         // state of the input pin
int buttonD = 0;

void setup() {
  pinMode(A_light, OUTPUT);
  pinMode(B_light, OUTPUT);       //Setting the digital
  pinMode(C_light, OUTPUT);       // pin's as output
  pinMode(D_light, OUTPUT);
  pinMode(time_button, INPUT);   
  pinMode(A_light_button, INPUT);
  pinMode(B_light_button, INPUT);
  pinMode(C_light_button, INPUT); // Setting as an input
  pinMode(D_light_button, INPUT);
}

void loop ()
{
timebutton = digitalRead(time_button); //read input value and store it
buttonA = digitalRead(A_light_button); //read input value and store it
buttonB = digitalRead(B_light_button); //read input value and store it
buttonC = digitalRead(C_light_button); //read input value and store it
buttonD = digitalRead(D_light_button); //read input value and store it

delay(1000); //turn all off for 1 second
if (buttonA == HIGH); { // if button A is on do the following
  digitalWrite(A_light, HIGH);  //turn A light on
  delay(9000UL); // wait for 15 mins
  if (timebutton == HIGH) {
    delay(900000UL); // delay for an additional 15 mins if HIGH
  }   }
  digitalWrite(A_light, LOW); // turn A light off
  delay(1000);  // turn all off for 1 second
if (buttonB == HIGH); { // if button B is on do the following
  digitalWrite(B_light, HIGH);  //turn B light on
  delay(9000UL); //waint for 15mins
  if (timebutton == HIGH) {
    delay(900000UL); // delay for an additional 15 mins if HIGH
  } }
  digitalWrite(B_light, LOW); //turn B ligt off
  delay(1000); // turn all off for 1 second
if (buttonC == HIGH); { // if button C is on do the following
  digitalWrite(C_light, HIGH);  // turn C light on
  delay(9000UL); // waint for 15 mins
  if (timebutton == HIGH) {
    delay(900000UL); // delay for an additional 15 mins if HIGH
  } }
  digitalWrite(C_light, LOW); // turn C light off
  delay(1000); //turn all off for 1 second
if (buttonD == HIGH); { // if button D is on do the following
  digitalWrite(D_light, HIGH);  // turn D light on
  delay(9000UL); // waint for 15 mins
  if (timebutton == HIGH) {
    delay(900000UL); // delay for an additional 15 mins if HIGH
  } }
  digitalWrite(D_light, LOW); // turn D light off
  
}

Take a look at the Arduino reference for if: http://arduino.cc/en/Reference/If. Compare yours, with particular reference to semicolons, to the ones you will see there. There is a very important difference.

I don't know what "leve" means.
Ok look at "if (buttonA == HIGH);"
What do you think I'm going on about?
(Hint: semicolon)

i ment leave

ok so i guess i dont need them then

delay(1000); //turn all off for 1 second

delay() doesn't turn anything on or off.

Do you suppose automobiles are assembled by piling all the parts around the edge of the room, and put together as they are encountered? No. The engine is put together. The transmission is put together.

You should be writing code the same way. Write a sketch that does nothing more than a Serial print when a switch is pressed. When that works, expand on that to print another message when the switch is released. When that works, expand on that to print only once when the switch transitions from released to pressed, and once when the switch transitions from pressed to released.

When that works, you know your switches are wired correctly, and that you understand how to read them and how if statements work.

You know none of that now. Assuming the switches work has you looking at the software for problems that may be hardware issues.

There are no statements to enable the internal pullup resistors, so you need external pullup or pulldown resistors. Do you have them? How ARE the switches wired?

hi there thanks for that information i will bare that in mind for other projects. the project is working how i wanted it to now but by all means say if you see something you dont like

// Example 02: Lighting Timer  26/10/2011

#define A_light 13     // Light A on pin 13
#define B_light 12     // Light B on pin 12
#define C_light 11     // Light C on pin 11
#define D_light 10     // Light D on pin 10
#define time_button 7  // Button on pin 7
#define A_light_button  1// Light A button
#define B_light_button  2// Light B button
#define C_light_button  3// Light C button
#define D_light_button  4// Light D button

int timebutton = 0;     
int buttonA = 0;      
int buttonB = 0;         // val will be used to store the
int buttonC = 0;         // state of the input pin
int buttonD = 0;

void setup() {
  pinMode(A_light, OUTPUT);
  pinMode(B_light, OUTPUT);       //Setting the digital
  pinMode(C_light, OUTPUT);       // pin's as output
  pinMode(D_light, OUTPUT);
  pinMode(time_button, INPUT);   
  pinMode(A_light_button, INPUT);
  pinMode(B_light_button, INPUT);
  pinMode(C_light_button, INPUT); // Setting as an input
  pinMode(D_light_button, INPUT);
}

void loop ()
{
timebutton = digitalRead(time_button); //read input value and store it
buttonA = digitalRead(A_light_button); //read input value and store it
buttonB = digitalRead(B_light_button); //read input value and store it
buttonC = digitalRead(C_light_button); //read input value and store it
buttonD = digitalRead(D_light_button); //read input value and store it


if (buttonA == HIGH) { // if button A is on do the following
  digitalWrite(A_light, HIGH);  //turn A light on
  delay(900000UL); // wait for 15 mins
  if (timebutton == HIGH) {
    delay(900000UL); // delay for an additional 15 mins if HIGH
  }
    digitalWrite(A_light, LOW); // turn A light off
    delay(1000); //turn all off for 1 second
  } 
if (buttonB == HIGH) { // if button B is on do the following
  digitalWrite(B_light, HIGH);  //turn B light on
  delay(900000UL); //waint for 15mins
  if (timebutton == HIGH) {
    delay(900000UL); // delay for an additional 15 mins if HIGH
  }
    digitalWrite(B_light, LOW); //turn B ligt off
    delay(1000); //turn all off for 1 second
  } 
if (buttonC == HIGH) { // if button C is on do the following
  digitalWrite(C_light, HIGH);  // turn C light on
  delay(900000UL); // waint for 15 mins
  if (timebutton == HIGH) {
    delay(900000UL); // delay for an additional 15 mins if HIGH
  }
    digitalWrite(C_light, LOW); // turn C light off
    delay(1000); //turn all off for 1 second
  } 
if (buttonD == HIGH) { // if button D is on do the following
  digitalWrite(D_light, HIGH);  // turn D light on
  delay(900000UL); // waint for 15 mins
  if (timebutton == HIGH) {
    delay(900000UL); // delay for an additional 15 mins if HIGH
  }
  digitalWrite(D_light, LOW); // turn D light off
  delay(1000); //turn all off for 1 second
  } 
  
}

and this how i was wiring the switches

by all means say if you see something you dont like

OK. Since you invited...

Some of your variable names could stand improvement. In 3 months, will you remember what buttonC is?
Names like pinForLightC, stateOfC, pinForSwitchC, etc. are meaningful. Its easy to see which ones are meant to be constant and which ones are meant to contain changing values. Which category is buttonC in?

There is a convention that #define names be all upper case, to distinguish them from variables, whether const or not.

Making the delay time a variable means that the same value can be used in may places, but only needs to be changed in one place, if the need to change it occurs. In your code, if the need to make the on time 20 minutes, instead of 15, arose, you would have to change the value in 8 places.

Finally, the use of arrays and loops and functions would cut your sketch size considerably. You are doing the same thing 4 times, except with different pins. Create a function that tests the switch state and performs, or not, the delay(s). Call that function 4 times, with different switch and LED pin numbers. Then, call it in a loop, with the pin numbers in arrays. Watch how much smaller, and easier to understand, your code gets.

Congratulations on getting a functioning program created that meets your needs.

hi thanks for your reply
the reason being the name button c this will be going into a glass house with four sections hence a,b,c,d.
but yes i could see there could be more improvement on that

making sketch smaller:
ok would you quickly knock an example sketch up to show me this compared to what i have done.

thanks for your time and help.

ok would you quickly knock an example sketch up to show me this compared to what i have done.

#define A_light 13     // Light A on pin 13
#define B_light 12     // Light B on pin 12
#define C_light 11     // Light C on pin 11
#define D_light 10     // Light D on pin 10

becomes

const int lightPins[] = {13, 12, 11, 10};
#define A_light_button  1// Light A button
#define B_light_button  2// Light B button
#define C_light_button  3// Light C button
#define D_light_button  4// Light D button

becomes

const int switchPins[] = {1, 2, 3, 4};
int buttonA = 0;      
int buttonB = 0;         // val will be used to store the
int buttonC = 0;         // state of the input pin
int buttonD = 0;

becomes

int states[4];
void setup() {
  pinMode(A_light, OUTPUT);
  pinMode(B_light, OUTPUT);       //Setting the digital
  pinMode(C_light, OUTPUT);       // pin's as output
  pinMode(D_light, OUTPUT);
  pinMode(time_button, INPUT);   
  pinMode(A_light_button, INPUT);
  pinMode(B_light_button, INPUT);
  pinMode(C_light_button, INPUT); // Setting as an input
  pinMode(D_light_button, INPUT);
}

becomes

void setup()
{
  for(byte b=0; b<4; b++)
  {
    pinMode(switchPins[i], INPUT);
    pinMode(lightPins[i], OUTPUT);
  }
  pinMode(time_button, INPUT);   
}

Now, you try making the changes to loop(). Read the states in a loop. Create a function to use the state of one pin to toggle the LED on another pin. The function will look like:

void toggleWithLongDelay(int switchState, int lightPin, bool longDelay)
{
   // Do stuff here...
}

The call will look like

   toggleWithLongDelay(state[i], lightPins[i], timeButton == HIGH);

That call will be made in a loop, obviously.

Don't use 'b' as a loop variable and 'i' as an index

Don't use 'b' as a loop variable and 'i' as an index

Excellent advice!

dont augh :stuck_out_tongue: what do you think?

const int lightPins[] = {13, 12, 11, 10};
const int switchPins[] = {1, 2, 3, 4};

int states[4];

void setup()
{
for(byte b=0; b<4; b++)
{
pinMode(switchPins[a], INPUT);
pinMode(lightPins[a], OUTPUT);
}
pinMode (time_button, INPUT);
}

void toggleWithLongDelay(int switchState, int lightPin, bool longDelay)
{
  
    if (button == HIGH) { 
  digitalWrite(lightPin, HIGH);  
  delay(900000UL); 
  if (timeButton == HIGH) {
    delay(900000UL); 
  }
    digitalWrite(lightPin, LOW); 
    delay(1000); 
    }
    
    toggleWithLongDelay(state[a], lightPins[a], timeButton == HIGH);
   
}

I think don't use b as as loop count, and a as an index

The loop function is missing. The code to read the switch states is missing. The toggle... function should not call the toggle... function; the loop() function should.

geting there?

const int lightPins[] = {13, 12, 11, 10};
const int switchPins[] = {1, 2, 3, 4};
#define time_button 7

int states[5];

void setup()
{
for(byte f=0; f<5; f++)
{
pinMode(switchPins[f], INPUT);
pinMode(lightPins[f], OUTPUT);
}
pinMode (time_button, INPUT);
}

void loop ()
{  
void toggleWithLongDelay(int switchState, int lightPin, bool longDelay)

  
    if (switchPins == HIGH) { 
  digitalWrite(lightPin, HIGH);  
  delay(900000UL); 
  if (time_Button == HIGH) {
    delay(900000UL); 
  }
    digitalWrite(lightPin, LOW); 
    delay(1000); 
    }
    
    toggleWithLongDelay(state[f], lightPins[f], time_Button == HIGH);
   
}

You can't nest function declarations