Simple If Clause

Noob to the max, can't make a simple If clause work
what am I missing?

/*

  • DC Motor test program
    */

int relay = 3;
unsigned long Timer;

void setup() // run once, when the sketch starts
{
pinMode(relay, OUTPUT);
}

void loop() {
Timer = millis();
digitalWrite(relay, HIGH);
delay(1000);
digitalWrite(relay, LOW);
delay(1000);
if (Timer > 4000)
{
digitalWrite(relay, LOW);
}
}

Thanks, John

Define "work"

the motor continues to run in 1 sec bursts
seems after 4 sec the pin should go low and end it

thanks again, John

seems after 4 sec the pin should go low and end it

And then the loop() immediately repeats and turns the motor on again.

I thought millis() started when the program first started running.
so what is a good method to end it at a certain point?

It does start at zero.
You could restructure your sketch.

I thought millis() started when the program first started running.

So.

what is a good method to end it at a certain point?

loop() repeats forever so when you have finished it starts again.
One way to stop it is to hold it in another loop like:-
while(true) { }
BUT - no one ever writes a program like this.

this was my first program ever, and just to explore what could be done
what's a better structure to control a motor for a given period of time, or events
and the shut it down?

Thanks for your help, it's appreciated
John

Mike's "while (true){};" inside the "if" would do the trick.

The simplest way is to put it all in the setup() and leave the loop() blank.

Read your loop, line by line, and see what it does. Have a piece of paper and create a truth table.
What do you really want it to do?

Cheers,
Kari

Try this, my lines have the comments at the beginning. Remove them and upload to your board. The variable run is just a flag to decide what to do. A digital input could be read to determine if you want to run the motor again by setting run to true again. That is if milli() resets each run of loop() as implied earlier.

/*

  • DC Motor test program
    */

int relay = 3;
unsigned long Timer;
// boolean run;

void setup() // run once, when the sketch starts
{
pinMode(relay, OUTPUT);
// run = true;
}

void loop() {
// if(run){
Timer = millis();
digitalWrite(relay, HIGH);
delay(1000);
digitalWrite(relay, LOW);
delay(1000);
if (Timer > 4000)
{
digitalWrite(relay, LOW);
// run = false;
}
// }
// Add additional code here to detect an input and reset run if wanted.

}

Can you all please post code using the # icon.

I think the name of this thread should be changed to:- Simple Santa Clause - Merry Christmas

One of many possiblities -

#include <WProgram.h>

const unsigned long     ONE_SECOND      = 1000UL;
const unsigned long     FOUR_SECONDS    = (4 * ONE_SECOND);

#define HAS_TIME_PASSED(VAR_FUTURE_)    (((long)(millis() - VAR_FUTURE_)) >= 0)


const uint8_t           pinRELAY        = 3;

const int               RELAY_OFF       = LOW;
const int               RELAY_ON        = HIGH;

unsigned long           trigger_time;

void setRelay(int state)
{
    digitalWrite(pinRELAY, state);
}

void loop()
{
    if ( HAS_TIME_PASSED(trigger_time) )
    {
        // note, once triggered will continue to trigger
        setRelay(RELAY_OFF);
    }
}

void setup()
{
    pinMode(pinRELAY, OUTPUT);

    setRelay(RELAY_ON);
    trigger_time = millis() + FOUR_SECONDS;
}