Millis() Timing goes way off over time.

Hello, I set this code for my arduino, It was working fine for around 36 hours until one day I came home and my relay switched on (water solenoid) and didn't turn off like it was supposed to. I had to disconnect the wire and mop up my flooded house -_-.

So my program is set using a Millis() function, and written into my own little function. this code switched a solenoid on for 5 seconds and then keep it off for an hour, repeatedly.

but I dont' understand why it would stay on like that. I may have coded in a bug that I cannot seem to fix. My plants are depending on this and so it really sucks that I can't seem to figure it out.

Can you please check it out and let me know what I have done to make it do that (stay on for longer than 5 seconds)?

unsigned char relayPin[4] = {4,5,6,7}; // Define the led's pin

void Zone(unsigned long on, unsigned long off, int i, unsigned long previousMilis[]);

int mode[4]={LOW,LOW,LOW,LOW};
unsigned long previousMillis[4]={0,0,0,0};
unsigned long Sec = 1000L;
unsigned long Min= 60*1000L;
unsigned long Hour=60*Min;
unsigned long onTime1=1*Sec; 
unsigned long offTime1=5*Min;
unsigned long currentMillis[4]={0,0,0,0};

void setup()
{
  int i;
  for(i = 0; i < 4; i++)
  {
    pinMode(relayPin[i],OUTPUT);
    digitalWrite(relayPin[i],mode[i]);
  }
}

void loop(){
  Zone(5000UL,3600000UL,0,previousMillis);
}

void Zone(unsigned long on, unsigned long off, int i, unsigned long previousMilis[]){
  currentMillis[i] = millis();
  if (currentMillis[i] - previousMillis[i]> off){
    previousMillis[i]=currentMillis[i];
    while(millis()-previousMillis[i]<=on){
      digitalWrite(relayPin[i],HIGH);
    }
    digitalWrite(relayPin[i],LOW);
  }
}
void loop(){
  Zone(5000UL,3600000UL,0,previousMillis);
}

Why? Why does loop do nothing but call another function? How come Zone() doesn't call a couple dozen more just-call-a-single-function functions?

previousMillis is a global variable. Why do you need to pass it to Zone()? If you are going to, why are you not passing currentMillis, too?

What's with the magic numbers?

    while(millis()-previousMillis[i]<=on){
      digitalWrite(relayPin[i],HIGH);
    }

Congratulations on successfully (maybe) reinventing delay(). If you are going to (try to) reinvent delay, why not just call delay()? It isn't necessary to repeatedly set the pin HIGH. It is fine to have an empty body for a statement.

Thanks for the reply!

The reason I wanted to do:

void loop(){
Zone(3000UL,1800000UL,0,previousMillis);
}

because i wanted to add things like

void loop(){
Zone(3000UL,1800000UL,0,previousMillis); //zone 1
Zone(3000UL,1800000UL,1,previousMillis); //zone 2
Zone(3000UL,1800000UL,2,previousMillis); //zone 3
Zone(3000UL,1800000UL,3,previousMillis) //zone 4
}

I can't use DELAY because i cannot multi task with delay().

So do you recommend I pass both previousMillis and currentMillis to the function?

The Numbers are (unsigned long) based on all of the tutorials and questions asked about millis, i need to add UL to make sure my program works within the hours range... ---> http://arduino.cc/it/Reference/IntegerConstants

while(millis()-previousMillis[i]<=on){
      digitalWrite(relayPin[i],HIGH);
    }

This part of the code turns on the relay for the "on" period.

Please let me know what others ways I can write this code. I am stumped because it is supposed to be pretty easy......

I am also using an 5V external power supply.

I am using the seeduino V2.12 and seeduino Relay Shield.

I can't use DELAY because i cannot multi task with delay().

You can't multitask the way you've written it either - the Zone routine doesn't return until it's done watering. However, do you need to multitask? Given the timescales you're working with, does it really matter if there is one zone's watering schedule is delayed three seconds? Or is there more to come in the sketch that is more time critical?

I can't use DELAY because i cannot multi task with delay().

Then, why did you re-invent it?

Write down the steps YOU would use to do what you are trying to get the Arduino to do, using a watch and a pad of paper to keep track of things. Once you can clearly write instructions that anyone can follow to do what you want the Arduino to do, getting the Arduino to do it is (nearly) trivial.

you are right, i don't need to multitask, but would It would be nice to apply this code into different parts of my aeroponic project.. (2 flowering area , 1 veg, and 1 clones) that i would need to adjust the time for.

in the future i would like to adjust all of my timing to do this:

void loop(){
  Zone(3000UL,1800000UL,0,previousMillis); //zone 1 //3 seconds on, 30 minutes off
  Zone(1000UL,900000UL,1,previousMillis); //zone 2 //1 second on, 15 minutes off)
  Zone(15000UL, 300000UL,2,previousMillis); //zone 3   //1.5 seconds on, 5 minutes off
  Zone(300000UL,1800000UL,3,previousMillis) //zone 4  //5 minutes on, 30 minutes off.
}

Wouldn't I need to multi-task then? since the timing are all different, and i want them to work independently from one another.

Please let me know what i can do other than use delay().... it wouldn't help me at all by using the delay() functions...

It must be something that i am overlooking.

Thanks again guys for helping!

One way to do it would be to keep another array of booleans to tell you whether the solenoid is on or off. Pass it to Zone with the others. If the solenoid is on, check if it's time to turn it off and vice versa. Dump the while loop, Voila, multitasking.

PaulS:

I can't use DELAY because i cannot multi task with delay().

Then, why did you re-invent it?

Write down the steps YOU would use to do what you are trying to get the Arduino to do, using a watch and a pad of paper to keep track of things. Once you can clearly write instructions that anyone can follow to do what you want the Arduino to do, getting the Arduino to do it is (nearly) trivial.

I didn't think I was reinventing delay(), i just can't use it for my purpose.

void Zone(unsigned long on, unsigned long off, int i, unsigned long previousMilis[]){  //PASSES ON TIME, OFF TIME AND PREVIOUS MILLIS to the function
  currentMillis[i] = millis();             //set the time since arduino started(millis) to the currentMillis
  if (currentMillis[i] - previousMillis[i]> off){   //test the time by subtracting currentMillis and previousMillis. if time is greater than OFFTIME then do:
    previousMillis[i]=currentMillis[i];               //set currentMillis to previousMillis to keep track of time
    while(millis()-previousMillis[i]<=on){         //While current time (millis()) minus Previous time is less than ON TIME
      digitalWrite(relayPin[i],HIGH);               //Turn ON the relay(array)
    }
    digitalWrite(relayPin[i],LOW);                 //after the while is done, it will turn the relay OFF
  }
}

I am simply trying to turn on and off up to 4 relays...
Here is what I am trying to design myself instead of paying $60 and it only has one relay: http://www.amazon.com/C-A-P-ART-DNe-Adjustable-Cycle-Timer/dp/B00286QNDM

This code actually works fine for 36 hours until it goes bad and stays in the ON mode for over an hour.

wildbill:
One way to do it would be to keep another array of booleans to tell you whether the solenoid is on or off. Pass it to Zone with the others. If the solenoid is on, check if it's time to turn it off and vice versa. Dump the while loop, Voila, multitasking.

hmmm.. can you please explain this in more detail, i may be too nooby to understand how you are explaining it

This is definitely one of those threads where I am too stunned for words.... anybody got popcorn? I wanna watch.

I think you should check the Timing section of the Playground. There are many libraries that allow you to setup different functions to be called at different intervals. Check out my SimpleTimer lib or the more advanced Time (which supports different "time sources", even i2c RTCs, with ease).

http://arduino.cc/playground/Main/LibraryList#Timing

    while(millis()-previousMillis[i]<=on){         //While current time (millis()) minus Previous time is less than ON TIME
      digitalWrite(relayPin[i],HIGH);               //Turn ON the relay(array)
    }

This while loop blocks until the current minus the start time is greater than the on time desired. If that isn't the EXACT same thing delay does, I'll eat my hat.

PaulS:

    while(millis()-previousMillis[i]<=on){   //While current time (millis()) minus Previous time is less than ON TIME

digitalWrite(relayPin[i],HIGH);  //Turn ON the relay(array)}



This while loop blocks until the current minus the start time is greater than the on time desired. If that isn't the EXACT same thing delay does, I'll eat my hat.

Remember to post it on YouTube! :slight_smile:

delay(on) ;  // wait time on,
digitalWrite(pin,HIGH);  // then turn on pin
digitalWrite(pin,HIGH);  // Turn on pin
delay(on) ;  // then wait timeon,
while(millis()-previousMillis[i]<=on) // whilst waiting time on
digitalWrite(pin,HIGH); ;  // keep (repeatedly) turning pin on

OK, I am splitting hairs here - thought it was worth a shot for the hat eating :slight_smile:

PaulS:

    while(millis()-previousMillis[i]<=on){         //While current time (millis()) minus Previous time is less than ON TIME

digitalWrite(relayPin[i],HIGH);               //Turn ON the relay(array)
    }



This while loop blocks until the current minus the start time is greater than the on time desired. If that isn't the EXACT same thing delay does, I'll eat my hat.

What if i need to delay for longer than 1-5 minutes at a time? doesn't delay stops my code, for the certain amount of time? within that 1-5 minutes of delay(), i could be turning on a bunch of relays.

unless I am confused about the delay().

I think you should check the Timing section of the Playground. There are many libraries that allow you to setup different functions to be called at different intervals. Check out my SimpleTimer lib or the more advanced Time (which supports different "time sources", even i2c RTCs, with ease).

Arduino Playground - LibraryList

Thanks, I will try it out. I was wondering, for the SimpleTimer function you mentioned. How would i be able to use it in functions? can't seem to initialize it correctly to use for the function i made. or will i need to call it all within the void loop()?
Also, how would i be able to blink at different rates (1sec ON, 3min OFF) for your code:

 timer.setInterval(1000, repeatMe);

since it only has the 1000, and it will blink at a constant rate.

What if i need to delay for longer than 1-5 minutes at a time? doesn't delay stops my code, for the certain amount of time? within that 1-5 minutes of delay(), i could be turning on a bunch of relays.

unless I am confused about the delay().

Yes, delay() stops your code for the specified duration. But, so does your code.

The thing you need to do it to get rid of the while loop(s). I'd recommend getting rid of the Zone function, too. Move the code into loop(). On each pass through loop, you check to see if it is time to turn a pin on, or not. You check to see if it is time to turn a pin off, or not. If it is time, turn the pin on, or off, and note the time that you did that. This becomes the basis for deciding exactly when to do the next thing.

If you were running around the yard turning sprinklers on and off, you wouldn't stand next to a valve for the whole time it is not, staring at your watch to see if it had been on long enough,. You'd turn on the ones that needed to be on (you should have an array of them), and then go sit in the sun and drink beer until it is time to turn them off (at least, that's what I'd do). Not all of them need to be turned on or off at the same time, so you have to check your watch periodically, so that you turn them off at about the right time. But, while the sprinklers are running, you can sit back and drink beer and enjoy the sun, or watch TV, or work on the honey-do list.

With just 4 sprinklers, you wouldn't need a piece of paper to write down the times, but, if you were managing a golf course with 1000 zones, you probably would.

Millis is the function that replaces your watch. The variables, or arrays, replace the paper. Nothing replaces the thinking that needs to occur, but the Arduino can easily decide if enough time has passed to turn a zone on or off, AND it won't drink all your beer while you are playing with the sprinklers.

SimpleTimer is not a function, but a class.

Variable intervals... interesting question. I tried a solution with this example.

#include <SimpleTimer.h>

const short ledPin = 13;

// the timer object
SimpleTimer timer;

void printUptime() {
    Serial.print("Uptime (s): ");
    Serial.println(millis() / 1000);
}

void blinkLed() {
    static boolean ledOn = true;

    if (ledOn) {
        digitalWrite(ledPin, HIGH);
        timer.setTimeout(50, blinkLed);
        //Serial.println(millis());
    }
    else {
        digitalWrite(ledPin, LOW);
        timer.setTimeout(1500, blinkLed);
        //Serial.println(millis());
    }
    
    ledOn = !ledOn;
}

void setup() {
    Serial.begin(9600);
    timer.setInterval(5000, printUptime);
    timer.setTimeout(500, blinkLed);
}

void loop() {
    timer.run();
}

I've been away from serious C++ for too long, what happens when you pass an unsigned long to unsigned long []?

void loop(){
  Zone(5000UL,3600000UL,0,previousMillis);
}

void Zone(unsigned long on, unsigned long off, int i, unsigned long previousMilis[]){
  currentMillis[i] = millis();
  if (currentMillis[i] - previousMillis[i]> off){
    previousMillis[i]=currentMillis[i];
    while(millis()-previousMillis[i]<=on){
      digitalWrite(relayPin[i],HIGH);
    }
    digitalWrite(relayPin[i],LOW);
  }
}

and does it matter if you don't actually use it in the function?

I've been away from serious C++ for too long, what happens when you pass an unsigned long to unsigned long []?

The previousMillis variable is an array. The call is correct.

and does it matter if you don't actually use it in the function?

It's referenced twice in the function.

Okay so he did pass an address to something that expects an address, like I wrote; it has been too long.
Still nowhere in the function is previousMilis[] used.