I can't get delay function to work!!!

dave0000011111 · January 27, 2016, 9:09pm

I wrote a simple code that controls an actuator. Everything works fine except the delay. Please help!!

/*
Brake

Engages parking brake when button pressed

*/

int BUTTON = 2;
int BRAKE = 4;
int RELAY1 = 6;
int RELAY2 = 7;

void setup()
{
// initialize outputs
pinMode(BUTTON, INPUT);
pinMode(BRAKE, INPUT);
pinMode(RELAY1, OUTPUT);
pinMode(RELAY2, OUTPUT);
digitalWrite(RELAY1,LOW);
digitalWrite(RELAY2,LOW);

}
// the loop function runs over and over again forever
void loop()
{
if (digitalRead(BUTTON)== LOW &&(digitalRead(BRAKE)) == HIGH)
digitalWrite(RELAY1,LOW);
delay(1000);
digitalWrite(RELAY1,HIGH);
else (digitalWrite(RELAY1,HIGH));
if (digitalRead(BUTTON)== HIGH &&(digitalRead(BRAKE)) == HIGH)
digitalWrite(RELAY2,LOW);
delay(1000);
digitalWrite(RELAY2,HIGH);
else (digitalWrite(RELAY2,HIGH));
}

system · January 27, 2016, 9:11pm

void loop()
{
  if (digitalRead(BUTTON)== LOW &&(digitalRead(BRAKE)) == HIGH)
    digitalWrite(RELAY1,LOW);
    delay(1000);
    digitalWrite(RELAY1,HIGH);
   else (digitalWrite(RELAY1,HIGH));
  if (digitalRead(BUTTON)== HIGH &&(digitalRead(BRAKE)) == HIGH)
    digitalWrite(RELAY2,LOW);
    delay(1000);
    digitalWrite(RELAY2,HIGH);
   else (digitalWrite(RELAY2,HIGH));
}

Didn't you get an "else without previous if" error?

dave0000011111 · January 27, 2016, 9:15pm

Now I am.. I played with it and screwed it up I guess.. I'll fix that part now.. thanks for the catch!!

dave0000011111 · January 27, 2016, 9:20pm

Fixed it

int BUTTON = 2;
int BRAKE = 4;
int RELAY1 = 6;
int RELAY2 = 7;

void setup()
{
// initialize outputs
pinMode(BUTTON, INPUT);
pinMode(BRAKE, INPUT);
pinMode(RELAY1, OUTPUT);
pinMode(RELAY2, OUTPUT);
digitalWrite(RELAY1,LOW);
digitalWrite(RELAY2,LOW);

}
// the loop function runs over and over again forever
void loop()
{
if (digitalRead(BUTTON)== LOW &&(digitalRead(BRAKE)) == HIGH)
digitalWrite(RELAY1,LOW),delay(1000),digitalWrite(RELAY1,HIGH);
else (digitalWrite(RELAY1,HIGH));
if (digitalRead(BUTTON)== HIGH &&(digitalRead(BRAKE)) == HIGH)
digitalWrite(RELAY2,LOW),delay(1000),digitalWrite(RELAY2,HIGH);
else (digitalWrite(RELAY2,HIGH));
}

system · January 27, 2016, 9:24pm

Or did you?

You didn't explain what you intended,but your comments and your code format suggests there's something you don't understand about the statement following an "if".

Or code tags

UKHeliBob · January 27, 2016, 9:25pm

  if (digitalRead(BUTTON) == HIGH && (digitalRead(BRAKE)) == HIGH)
    digitalWrite(RELAY2, LOW), delay(1000), digitalWrite(RELAY2, HIGH);

Which commands will be executed when the conditions in the if evaluate to true ?

dave0000011111 · January 27, 2016, 9:32pm

if "BUTTON" HIGH and "BRAKE" LOW

Then "RELAY1" for 1sec

if "BUTTON" and "BRAKE" are both high

Then "RELAY2" for 1sec

nickgammon · January 27, 2016, 9:34pm

@dave0000011111 - Please edit your posts, select the code, and put it between [code] ... [/code] tags.

You can do that by hitting the "Code" icon above the posting area. It is the first icon, with the symbol: </>

Please use code tags.

Read this before posting a programming question

How to use this forum

system · January 27, 2016, 9:36pm

Don't use commas to separate statements; they look too much like semicolons.

Do use braces { } to form compound statements.

DrAzzy · January 27, 2016, 9:41pm

I... I think that bit of code does what he wants it to?

It's very poor coding style though.

if (digitalRead(BUTTON)== LOW &&(digitalRead(BRAKE)) == HIGH)
digitalWrite(RELAY1,LOW),delay(1000),digitalWrite(RELAY1,HIGH);
else (digitalWrite(RELAY1,HIGH));
if (digitalRead(BUTTON)== HIGH &&(digitalRead(BRAKE)) == HIGH)
digitalWrite(RELAY2,LOW),delay(1000),digitalWrite(RELAY2,HIGH);
else (digitalWrite(RELAY2,HIGH));

There is no need to enclose the digitalWrite() function in parens after the else ( else (digitalWrite(RELAY1,HIGH)); ).
Get in the habit of using curly braces. They make code much more readable.
After doing 2, you can put the statements in the if blocks on their own lines, instead of relying on the comma operator to stick those function calls together.

if (digitalRead(BUTTON)== LOW &&(digitalRead(BRAKE)) == HIGH){
  digitalWrite(RELAY1,LOW);
  delay(1000);
  digitalWrite(RELAY1,HIGH);
} else {
  digitalWrite(RELAY1,HIGH);
}
if (digitalRead(BUTTON)== HIGH &&(digitalRead(BRAKE)) == HIGH) {
  digitalWrite(RELAY2,LOW);
  delay(1000);
  digitalWrite(RELAY2,HIGH);
} else {
  digitalWrite(RELAY2,HIGH);
}

Now if we look at the two code paths, you might notice that you can shorten it a bit further:

if (digitalRead(BUTTON)== LOW &&(digitalRead(BRAKE)) == HIGH){
  digitalWrite(RELAY1,LOW);
  delay(1000);
}
digitalWrite(RELAY1,HIGH);
if (digitalRead(BUTTON)== HIGH &&(digitalRead(BRAKE)) == HIGH) {
  digitalWrite(RELAY2,LOW);
  delay(1000);
}
digitalWrite(RELAY2,HIGH);

@AWOL - he has to use commas there instead of semicolons, because he has to compress it down to one statement because he doesn't have curly braces around them.

system · January 27, 2016, 9:51pm

@AWOL - he has to use commas there instead of semicolons, because he has to compress it down to one statement because he doesn't have curly braces around them.

I know that, that's why I said not to use them, because one time, you'll forget.

dave0000011111 · January 27, 2016, 10:00pm

Ok I made the changes that you guys offered. Thanks! I'm a hardware and wiring guy. Not a programmer..

So it is triggering the relays but it for 4-5 seconds.. Any ideas?

int BUTTON = 2;
int BRAKE = 4;
int RELAY1 = 6;
int RELAY2 = 7;


void setup() 
{
  // initialize outputs
  pinMode(BUTTON, INPUT);
  pinMode(BRAKE, INPUT);
  pinMode(RELAY1, OUTPUT);
  pinMode(RELAY2, OUTPUT);
  digitalWrite(RELAY1,LOW);
  digitalWrite(RELAY2,LOW);
 
  }
// the loop function runs over and over again forever
void loop() 
{
 if (digitalRead(BUTTON)== LOW &&(digitalRead(BRAKE)) == HIGH){
  digitalWrite(RELAY1,LOW);
  delay(1000);
}
digitalWrite(RELAY1,HIGH);
if (digitalRead(BUTTON)== HIGH &&(digitalRead(BRAKE)) == HIGH) {
  digitalWrite(RELAY2,LOW);
  delay(1000);
}
digitalWrite(RELAY2,HIGH);
}

system · January 27, 2016, 10:03pm

Because the conditions persist that long?
You've got external pullups?

dave0000011111 · January 27, 2016, 10:08pm

Nothing external i was sending a pulsed signal to it. But that brings another question.. If I change the input to a latched/switch how would I keep the output to the relays the set delay?

system · January 27, 2016, 10:53pm

One too many parenthesis;

if (digitalRead(BUTTON)== HIGH &&(digitalRead(BRAKE)) == HIGH) {
                                 ^

PaulMurrayCbr · January 28, 2016, 1:50am

dave0000011111:
void loop() 

{
if (digitalRead(BUTTON)== LOW &&(digitalRead(BRAKE)) == HIGH)
digitalWrite(RELAY1,LOW),delay(1000),digitalWrite(RELAY1,HIGH);
else (digitalWrite(RELAY1,HIGH));
if (digitalRead(BUTTON)== HIGH &&(digitalRead(BRAKE)) == HIGH)
digitalWrite(RELAY2,LOW),delay(1000),digitalWrite(RELAY2,HIGH);
else (digitalWrite(RELAY2,HIGH));
}

Arrgh! Arrgh! Comma operator!

It works, but please don't do this. USe statement blocks.

void loop() 
{
  if (digitalRead(BUTTON)== LOW &&(digitalRead(BRAKE)) == HIGH) {
    digitalWrite(RELAY1,LOW);
    delay(1000);
    digitalWrite(RELAY1,HIGH);
  }
  else {
    digitalWrite(RELAY1,HIGH);
  }

  if (digitalRead(BUTTON)== HIGH &&(digitalRead(BRAKE)) == HIGH) {
    digitalWrite(RELAY2,LOW);
    delay(1000);
    digitalWrite(RELAY2,HIGH);
  }
  else {
    digitalWrite(RELAY2,HIGH);
  }
}

Now notice that whichever branch of the if executes, you do the same thing when you are done.
That means this can be simplified to

void loop() 
{
  if (digitalRead(BUTTON) == LOW && digitalRead(BRAKE) == HIGH) {
    digitalWrite(RELAY1,LOW);
    delay(1000);
  }
  digitalWrite(RELAY1,HIGH);

  if (digitalRead(BUTTON) == HIGH && digitalRead(BRAKE) == HIGH) {
    digitalWrite(RELAY2,LOW);
    delay(1000);
  }
  digitalWrite(RELAY2,HIGH);
}

But notice the condition? There's a common thing there:

void loop() 
{
  if (digitalRead(BRAKE) == HIGH) {
    if (digitalRead(BUTTON) == LOW) {
      digitalWrite(RELAY1,LOW);
      delay(1000);
      digitalWrite(RELAY1,HIGH);
    }
    else {
      digitalWrite(RELAY2,LOW);
      delay(1000);
      digitalWrite(RELAY2,HIGH);
    }
  }
}

Now, this doesn't do exactly the same thing as the previous code - it doesn't explicitly set your relays to HIGH even if they don't need to be. But by looking at what your code does, it will probably do the same.

In any case, we can get this down event further.

void loop() 
{
  if (digitalRead(BRAKE) == HIGH) {
    byte relay = (digitalRead(BUTTON) == LOW) ? RELAY1 : RELAY2;
    digitalWrite(relay,LOW);
    delay(1000);
    digitalWrite(relay,HIGH);
  }
}

The goal isn't to make the code shorter just for the sake of making it shorter. It's to make clearer what you are trying to do. "If the brake is on, brake relay1 or relay2 for one second, depending on how the button is set."

A common convention in arduino sketches is to do all of your reading of inputs in one spot. So the sketch becomes:

void loop() 
{
  const boolean brake = digitalRead(BRAKE) == HIGH;
  const boolean button = digitalRead(BUTTON) == HIGH;

  if (brake) {
    const byte relay = button ? RELAY2 : RELAY1;

    digitalWrite(relay,LOW);
    delay(1000);
    digitalWrite(relay,HIGH);
  }
}

PaulMurrayCbr · January 28, 2016, 1:59am

dave0000011111:
Nothing external i was sending a pulsed signal to it. But that brings another question.. If I change the input to a latched/switch how would I keep the output to the relays the set delay?

In that case, rather than triggering the relay when the input is high, you trigger it when you see the input CHANGE from low to high. This means that you have to remember the state of the input in a global variable.

// button is normally low, we need to sense when there's a rising edge

byte prevInputState = LOW;

void loop() {
  byte inputState = digitalRead(INPUT);
  if(prevInputState == LOW && inputState == HIGH) {
    // rising edge. do stuff.
  }

  prevInputState = inputState; // always unconditionally do this.
}

Topic		Replies	Views
help with the delay\time plz! Programming	18	1515	May 5, 2021
Delay in my project is not working Programming	35	2808	May 5, 2021
Not working with other condition Programming	15	537	August 25, 2021
A delay problem Programming	8	1013	May 5, 2021
Delay function stopped working without any change Programming	39	2555	November 2, 2022

I can't get delay function to work!!!

Related topics