script for relay won't work

i have made a script for a arduino that has a switch (pin 8) connected and 2 relays(pin 2 and 3)
i want that when i press the button relay1 will go on.
i want that when i press the button again relay1 goes off and relay2 will go on.
i want that when i press the button again they both need to go on.
and the last time i press it i want every thing to go off.

i added delays of 0,5 sec in case in hold the button i doesn’t go crazy.
my code works but it directly starts, and noting happens when i press the button.

what did i done wrong?

Timon

const int relay1 = 2;
const int relay2 = 3;
const int bell =  8;


int bellState = 0;  
int relay1State = 0;
int relay2State = 0;

void setup() {

  pinMode(relay1, OUTPUT);      
  pinMode(relay2, OUTPUT);
  pinMode(bell, INPUT);     
}

void loop(){
  // read the state of the pushbutton value:
  bellState = digitalRead(bell);
  relay1State = digitalRead(relay1);
  relay2State = digitalRead(relay2);

 
  if (bellState == HIGH);
     (relay1State == LOW);
     (relay2State == LOW); {     
        
    digitalWrite(relay1, HIGH);
    delay(0500);

  if (bellState == HIGH);
     (relay1State == HIGH);
     (relay2State == LOW); {  
    digitalWrite(relay1, LOW);
    digitalWrite(relay2, HIGH);
    delay(0500)

  ;if (bellState == HIGH);
      (relay1State == LOW);
      (relay2State == HIGH); {
    digitalWrite(relay1, HIGH);
    delay(0500)

  ;if (bellState == HIGH);
      (relay1State == HIGH);
      (relay2State == HIGH); { 
    digitalWrite(relay1, LOW);
    digitalWrite(relay2, LOW);
    delay(0500) 
 ; } 
  {
  
  }
}}}}
if (bellState == HIGH);

BZZZT.
etc.

delay(0500)

Why octal?

What do you mean with "BZZZT. etc." It is called bellState because the button is a old Wireless doorbell.

What is octal? It is the only type of delay I know.

Thanks for replying
Hope that I will find the answer.

Timon

The "BZZT" was for the semicolon.

Octal is base 8. Why specify a delay in base 8?

I am a native speaker and i am a starter in Python. I don't know what base 8 is and if there is something better. And what do I need to do with the semicolon? Can you be more clear in what I need to change?

Thanks for the replies,

Timon

I don't know what base 8 is and if there is something better.

Decimal is more usual.
If you want 500 milliseconds delay, use decimal (500).
If you want 320 milliseconds delay, use octal (0500)

And what do I need to do with the semicolon?

You need to lose it.

If you look at any of the supplied examples, nowhere will you find semicolon usage like that.

thanks for the delay.
but if i take away the semicolon, this happens:

part were i took them away

if (bellState == HIGH) 
     (relay1State == LOW)
     (relay2State == LOW); {

error message

relay_on_on_both_on_off.ino: In function 'void loop()':
relay_on_on_both_on_off:28: error: '(relay1State == 0)' cannot be used as a function

when i first wrote the code i also didn't had so much semicolon's.
what now?

Timon

How about some && or some || ?

thanks for the fast reply.
i didn't knew that about &&
i changed it but it still goes without pressing the button.
here is the code:

const int relay1 = 2;
const int relay2 = 3;
const int bell =  8;

int bellState = 0;  
int relay1State = 0;
int relay2State = 0;

void setup() {

  pinMode(relay1, OUTPUT);      
  pinMode(relay2, OUTPUT);
  pinMode(bell, INPUT);     
}

void loop(){

  bellState = digitalRead(bell);
  relay1State = digitalRead(relay1);
  relay2State = digitalRead(relay2);

  if (bellState == HIGH && relay1State == LOW && relay2State == LOW); {
    digitalWrite(relay1, HIGH);
    delay(500);

  if (bellState == HIGH && relay1State == HIGH && relay2State == LOW); {  
    digitalWrite(relay1, LOW);
    digitalWrite(relay2, HIGH);
    delay(500);

  if (bellState == HIGH && relay1State == LOW && relay2State == HIGH); {
    digitalWrite(relay1, HIGH);
    delay(500);

  if (bellState == HIGH && relay1State == HIGH && relay2State == HIGH); { 
    digitalWrite(relay1, LOW);
    digitalWrite(relay2, LOW);
    delay(500) 
 ; } 
  {
  
  }
}}}}

Timon

Here you are i checked also , the button must be connected from pin 2 to ground

#define but 2    // Pin 2 on arduino board
#define Relay1 3 // Pin 3 on arduino board
#define Relay2 4 // Pin 4 on arduino board

void setup()
{
	pinMode(but,INPUT_PULLUP);
	pinMode(Relay1,OUTPUT);
	pinMode(Relay2,OUTPUT);

	digitalWrite(Relay1,LOW);
	digitalWrite(Relay2,LOW);
}

void loop()
{
	if (digitalRead(but) == LOW && digitalRead(Relay1) == LOW && digitalRead(Relay2) == LOW){
		digitalWrite(Relay1,HIGH);
		digitalWrite(Relay2,LOW);
		delay(500);
	}
	
	if (digitalRead(but) == LOW && digitalRead(Relay1) == HIGH && digitalRead(Relay2) == LOW){
		digitalWrite(Relay1,LOW);
		digitalWrite(Relay2,HIGH);
		delay(500);
	}
	
	if (digitalRead(but) == LOW && digitalRead(Relay1) == LOW && digitalRead(Relay2) == HIGH){
		digitalWrite(Relay1,LOW);
		digitalWrite(Relay2,LOW);
		delay(500);
	}
	
}
 if (bellState == HIGH && relay1State == LOW && relay2State == LOW); {
    digitalWrite(relay1, HIGH);
    delay(500);

  if (bellState == HIGH && relay1State == HIGH && relay2State == LOW);

No really, lose the semicolons on those conditionals.
And ask yourself how relay1State could have changed value in 500 milliseconds.

You find the solution on the time i was write for you :slight_smile: Well done :wink:

Updated (Check comments) :slight_smile:

#define but 2    // Pin 2 on arduino board
#define Relay1 3 // Pin 3 on arduino board
#define Relay2 4 // Pin 4 on arduino board

void setup()
{
	pinMode(but,INPUT_PULLUP);
	pinMode(Relay1,OUTPUT);
	pinMode(Relay2,OUTPUT);

	digitalWrite(Relay1,LOW);
	digitalWrite(Relay2,LOW);
}

void loop()
{
	// First hit relay1 on  relay 2 off
	if (digitalRead(but) == LOW && digitalRead(Relay1) == LOW && digitalRead(Relay2) == LOW){
		digitalWrite(Relay1,HIGH);
		digitalWrite(Relay2,LOW);
		while(digitalRead(but) == LOW){delay(50);} // wait here until release the button
		// or
		//delay(500);
	}
	
	// Second hit relay 1 off relay 2 on
	if (digitalRead(but) == LOW && digitalRead(Relay1) == HIGH && digitalRead(Relay2) == LOW){
		digitalWrite(Relay1,LOW);
		digitalWrite(Relay2,HIGH);
		while(digitalRead(but) == LOW){delay(50);} // wait here until release the button
		// or
		//delay(500);
	}
	
	// Third hit relay 1 on relay 2 on
	if (digitalRead(but) == LOW && digitalRead(Relay1) == LOW && digitalRead(Relay2) == HIGH){
		digitalWrite(Relay1,HIGH);
		digitalWrite(Relay2,HIGH);
		while(digitalRead(but) == LOW){delay(50);} // wait here until release the button
		// or
		//delay(500);
	}

	// forth hit relay 1 off relay 2 off
	if (digitalRead(but) == LOW && digitalRead(Relay1) == HIGH && digitalRead(Relay2) == HIGH){
		digitalWrite(Relay1,LOW);
		digitalWrite(Relay2,LOW);
		while(digitalRead(but) == LOW){delay(50);} // wait here until release the button
		// or
		//delay(500);
	}
	
}

thanks, it worked. I try to learn from my mistakes but i don't see the cause why mine script just ''ignored'' the button.
if you could tell me that, that would be awesome.

thanks for all

Timon

The semicolon says “that’s the end of this expression”
So

if (someCondition);
{
  doSomething();
}

Will first evaluate someCondition and then it will always execute doSomething(), whether or not someCondition was true or false.

timon989:
thanks, it worked. I try to learn from my mistakes but i don't see the cause why mine script just ''ignored'' the button.
if you could tell me that, that would be awesome.

thanks for all

Timon

One thing it is like friend AWOL said, look here for more explain http://arduino.cc/en/Tutorial/IfStatement

and the second on each if you should change on both relays the status other wise you will not what you expect :wink: