If Millis(); Stops working after time

Hello I have written a program to trigger 4 relays in the following;

relay 1 & 2 - on 500ms, off 1000ms, on 500ms and off 8000ms these two are equally staggered so they don’t trigger at the same time.

relay 3 & 4 - on 500ms, off 14000ms, on 500ms and off 44000ms these two trigger at the same time.

My issue is that after an amount of time my program stops working and the relays no longer work I’m not sure what the problem is. I think that the Arduino is not totally accurate to after a while my if statements no longer apply but I’m not sure?

Here’s my code below,

Wet_Rig_V3.ino (7.01 KB)

You appear to have never heard of arrays, or to have some aversion to simplifying your code considerably by using arrays.

I quit reading when I saw long variables with int in the name. If you need to be reminded of the type whenever you type a variable name, at least make the type in the name the type of the variable.

  unsigned long CuON1R1 = millis();
  unsigned long CuON1TR1 = millis();

  unsigned long CuDel1R1 = millis();
  unsigned long CuDel1TR1 = millis();

  unsigned long CuOFF1R1 = millis();
  unsigned long CuOFF1TR1 = millis();

  unsigned long CuDel2R1 = millis();
  unsigned long CuDel2TR1 = millis();

Well, actually, I did read a bit farther to see that you have 8 copies of now. No wonder you have problems.

  if ((CuON1R1 - PMON1R1 == intON1R1) && (CuON1TR1 - PMON1TR1 <= intON1TR1)) {

Expecting time variables to EXACTLY match is unrealistic.

You are trying to control four relays according to a simple schedule. Yet your code contains literally dozens of variables. What are all these variables for?
Your code is almost completely devoid of comments, which makes it difficult for us to understand how you intend it to work.

Sorry guys guess I gotta start somewhere not an expert.

essentially I want to trigger two sets of relays at the following

relay 1 & 2 - on 500ms, off 1000ms, on 500ms and off 8000ms (equates to a 10 second cycle) these two are equally staggered so they don't trigger at the same time.

relay 3 & 4 - on 500ms, off 14500ms, on 500ms and off 44500ms (equates to a 1 min cycle) these two trigger at the same time.

because I want to trigger relay 3 and 4 at a completely different time interval to 1 and 2 I cannot use the delay function so I read up on blink with no delay and this was my attempt of using that method to control relays.

and this was my attempt of using that method to control relays.

That does not preclude using arrays, comments, or functions.

It does not preclude understanding that expecting the interval between two times to be EXACTLY some value is not realistic.

Normally, one tests that now minus then is greater than or equal to the desired interval. That way, if millis() incremented by two, instead of one, at just the wrong time, you are still OK.

It does not preclude having ONE now variable.

PaulS:
That does not preclude using arrays, comments, or functions.

It does not preclude understanding that expecting the interval between two times to be EXACTLY some value is not realistic.

Normally, one tests that now minus then is greater than or equal to the desired interval. That way, if millis() incremented by two, instead of one, at just the wrong time, you are still OK.

It does not preclude having ONE now variable.

thanks PaulS I appreciate your help would it be possible for you to show me an example with the timings I said? as initially I did try if now minus then is greater or equal to the interval which worked on each if statement individually, but I could not daisy chain them to make it go on to the next if statement. that's why I did && and another statement that I new would be false so the program would move on.

Sorry realised that one was wrong this one does not have any == statements but still stops working after 40 mins or so.

Wet_Rig_V3.ino (7.01 KB)

The major thing missing from your code is Serial.print() statements that would give you a clue. Of course, arrays would mean 1/4 of the code to debug. Comments would be useful, too.

PaulS:
The major thing missing from your code is Serial.print() statements that would give you a clue. Of course, arrays would mean 1/4 of the code to debug. Comments would be useful, too.

What do you think of this? so far so good though it hasnt been running for very long.

Wet_Rig_Push_Button.ino (2.22 KB)

I think this is what you mean:

// even indexes are ON, odd indexes are OFF.
// both cycles are six elemts long, so I zero-pad the second

const int CYCLE_LEN = 6;

unsigned long cycle1_ms[CYCLE_LEN] = {500, 1000, 500, 3000, 0, 5000};
unsigned long cycle2_ms[CYCLE_LEN] = {500, 1400, 500, 44000, 0, 0};

// dude has decidd to numner his relays starting at 1. I'llk roll with it.
// we start relay 2 at the 5-millisecond "off" in cycle 1

int relay_index[5] = {0, 5, 0, 0};
unsigned long relay_start_ms[5] = {0, 0, 0, 0, 0};
int relay_pin[5] = {0, 4, 5, 6, 7};
unsigned long *relay_cycle[5] = { NULL, cycle1_ms, cycle1_ms, cycle2_ms, cycle2_ms };

void setup() {
  unsigned long ms  = millis();
  for (int i = 1; i <= 4; i++) {
    pinMode(relay_pin[i], OUTPUT);
    digitalWrite(relay_pin[i], relay_index[5] % 2 == 0 ? HIGH : LOW);
    relay_start_ms[i] = ms;
  }
  Serial.begin(9600);
}

void loop() {
  for (int i = 1; i <= 4; i++) {
    if (millis() - relay_start_ms[i] > relay_cycle[i][relay_index[i]]) {
      // use the interbal length rather than the real millis()
      relay_start_ms[i] += relay_cycle[i][relay_index[i]];

      // move to the next interval, but skip any zero-length intevals
      do {
        relay_index[i] = (relay_index[i] + 1) % CYCLE_LEN;
      } while (relay_cycle[i][relay_index[i]] == 0);

      digitalWrite(relay_pin[i], relay_index[i] % 2 == 0 ? HIGH : LOW);

      for (int tab = 1; tab < i; tab++) {
        Serial.print("        ");
      }
      Serial.print( relay_index[i] % 2 == 0 ? "HIGH" : "LOW");
      Serial.println();
    }
  }
}

Yeees - actually, that’s pretty nasty. But it might do what you want.

What do you think of this?

Not too much, really.

long Relay1Array[] = {
  500, 1000, 500, 8000
};

The { and } and were a dead giveaway that the declaration is for an array. Array does not need to be part of the name. But, the name needs something more than Relay1 for the name. What do those values represent? Intervals?

int R3pin = 4;
int R4pin = 5;

What is an R3? Why is it attached to pin 4?
What is an R4? Why is it attached to pin 5?

Relay1() and Relay2() look suspiciously similar. The names are useless because they don't define what the functions do.

Can you imagine how much more difficult it would be to program the Arduino if digitalRead() were called function1273() and analogWrite() was function54735()?

That the 3rd and 4th relays share the same on/off intervals does not mean that they should use the same array. Someday, you might want to use different intervals and you'd need to change a lot of code because you tried to save 16 bytes of memory.

stopRelay1() and stopRelay2() really ARE nearly identical. One does NOT stop a relay. The relay isn't moving, so there is nothing to stop. Write ONE function, with a reasonable name, like turnRelayOff(), that takes the pin number that the relay is connected to, and make the function turn that pin off. Call that function in 4 places.

Anonymous printing sucks. You have a stream of lines of data appearing in the serial monitor with no clue what any given number means.

PaulS:
Not too much, really.

long Relay1Array[] = {

500, 1000, 500, 8000
};



The { and } and [] were a dead giveaway that the declaration is for an array. Array does not need to be part of the name. But, the name needs something more than Relay1 for the name. What do those values represent? Intervals?



int R3pin = 4;
int R4pin = 5;



What is an R3? Why is it attached to pin 4?
What is an R4? Why is it attached to pin 5?

Relay1() and Relay2() look suspiciously similar. The names are useless because they don't define what the functions do.

Can you imagine how much more difficult it would be to program the Arduino if digitalRead() were called function1273() and analogWrite() was function54735()?

That the 3rd and 4th relays share the same on/off intervals does not mean that they should use the same array. Someday, you might want to use different intervals and you'd need to change a lot of code because you tried to save 16 bytes of memory.

stopRelay1() and stopRelay2() really ARE nearly identical. One does NOT stop a relay. The relay isn't moving, so there is nothing to stop. Write ONE function, with a reasonable name, like turnRelayOff(), that takes the pin number that the relay is connected to, and make the function turn that pin off. Call that function in 4 places.

Anonymous printing sucks. You have a stream of lines of data appearing in the serial monitor with no clue what any given number means.

R1 meant Relay 1 so on and so forth for the other R2,3 etc

but ok i see where your coming from functionally it works but that doesnt mean much if its not written logically / bad grammer i will have to work on this. But thanks for your help its good that i question myself or i wont improve :wink:

PaulMurrayCbr:

int relay_index[5] = {0, 5, 0, 0}; 

// what are the indices for those array elements?

digitalWrite(relay_pin[i], relay_index[5] % 2 == 0 ? HIGH : LOW);
// does relay_index[5] even exist?

There might be other bugs; I don't know.

Wenborn:
Sorry guys guess I gotta start somewhere not an expert.

essentially I want to trigger two sets of relays at the following

relay 1 & 2 - on 500ms, off 1000ms, on 500ms and off 8000ms (equates to a 10 second cycle) these two are equally staggered so they don't trigger at the same time.

relay 3 & 4 - on 500ms, off 14500ms, on 500ms and off 44500ms (equates to a 1 min cycle) these two trigger at the same time.

because I want to trigger relay 3 and 4 at a completely different time interval to 1 and 2 I cannot use the delay function so I read up on blink with no delay and this was my attempt of using that method to control relays.

Did you not realize that six 10-second intervals will exactly fit in a 1-minute interval?

Can anyone understand why the latest one i done stops after 30 mins or so? im lost.

Wenborn:
Can anyone understand why the latest one i done stops after 30 mins or so? im lost.

I think reply #13 might have hit it on the head.