Millis() not working as expected

Projects Programming
1

Good day everyone,

Just want to ask some help regarding my program using millis() function.

My set-up is composed of 3 relays, controlled by arduino uno.
The 1st relay should first on for 10mins then off for 40mins then repeat
The 2nd relay should first off for 35mins then on for 15mins then repeat
The 3rd relay should first on for 10mins then off for 90mins then repeat

I have successfully uploaded the program, however, the problem is that 1st and 3rd relay is always on and 2nd relay is always off. It never changes its state.

#define RELAY1 6 //inlet 
#define RELAY2 7 //exhaust 
#define RELAY3 8 //mist maker 

long previousMillis1 = 0;
const long interval1 = 21600000; //interval at which LED will switch off
const long interval2 = 7200000; //interval at whihc LED will switch on 

long previousMillis2 = 0;
const long interval3 = 900000;  //interval at which exhaust will off 900000
const long interval4 = 2100000;  //interval at which exhaust will on 2100000

long previousMillis3 = 0;
const long interval5 = 600000;  //interval at which inlet will off 600000
const long interval6 = 2400000;  //interval at which inlet will on 2400000

long previousMillis4 = 0;
const long interval7 = 600000;  //interval at which mist will off 600000
const long interval8 = 5400000;  //interval at which mist will on 5400000

void setup() {
pinMode(RELAY1, OUTPUT);
pinMode(RELAY2, OUTPUT);
pinMode(RELAY3, OUTPUT);
}

void Fan1(){            //inlet fan is 10min on 40mins off 
  static bool state = 1 ; 
    digitalWrite(RELAY1, LOW);
  unsigned long currentMillis3 = millis(); 
      if(currentMillis3 - previousMillis3 > interval6 && state == 0){
        state = 1;
        previousMillis3 = currentMillis3;
          digitalWrite(RELAY1, LOW);
      }
      else if(currentMillis3-previousMillis3 > interval5 && state ==1){
        state = 0;
        previousMillis3 = currentMillis3; 
          digitalWrite(RELAY1, HIGH);
      }
}

void Fan2(){            //exhaust fan is 35mins off 15mins on
  static bool state = 0; 
    digitalWrite(RELAY2, HIGH);
  unsigned long currentMillis2 = millis(); 
      if(currentMillis2 - previousMillis2 > interval4  && state == 0){
        state = 1;
        previousMillis2 = currentMillis2;
         digitalWrite(RELAY2, LOW);
      }
      else if(currentMillis2 - previousMillis2 > interval3 && state == 1){
        state = 0; 
        previousMillis2 = currentMillis2; 
        digitalWrite(RELAY2, HIGH);
      }
}

void MistMaker(){       //mist is 10mins on 90mins off
  static bool state = 1;
    digitalWrite(RELAY3, LOW);
  unsigned long currentMillis4 = millis(); 
      if(currentMillis4 - previousMillis4 > interval8 && state ==0){
        state = 1;
        previousMillis4 = currentMillis4;
          digitalWrite(RELAY3, LOW);
      }
      else if(currentMillis4-previousMillis4 > interval7 && state ==1){
        state = 0;
        previousMillis4 = currentMillis4; 
          digitalWrite(RELAY3, HIGH);
      }
  }

void loop() {
Fan1();
Fan2();
MistMaker();
}

TestWhite.ino (3.41 KB)

2

ALWAYS use unsigned long for variables associated with millis().

The problem is not with millis(), it is with the program logic. The first thing each fan() function does is set the relay output to HIGH or LOW, and those functions are called thousands, to tens of thousands of times per second by loop(). So, it does not matter what else happens within each fan routine.

3

long previousMillis1 = 0;
Change to:
unsigned long previousMillis1 = 0;

Read these
https://forum.arduino.cc/index.php?topic=503368.0

http://forum.arduino.cc/index.php?topic=223286.0

4

MistMaker();

That is a great name for a function. It tells you exactly what it does.

interval7 is not a great name. Without looking at the comnent at the top, can you tell me what it is for? Maybe today you can, but not next time you look at editing this code.

Give all your functions and variables useful names. Sometimes you will have just Fan1 and Fan2 but usually you can give them useful names too. Or you need an array of Fans[]

5

Please check that the following diagram describes the timings of the events of your post. If so, it can be easily represented by pseudo codes and then into Arduino codes. RL1, RL2, amd RL3 refers to the Relays -- 1, 2, and 3; 8, 9, and 10 refers to the connector pins of the UNO. ON and OFF refers tothe ON-period and OFF-period of the Relays.

Pseudo Codes:

101 ----> PORTB
while ((millis() - presentMillis) != 10*60*1000)
    ;
---------------------------------------------------
6

GMP:
My set-up is composed of 3 relays, controlled by arduino uno.
The 1st relay should first on for 10mins then off for 40mins then repeat
The 2nd relay should first off for 35mins then on for 15mins then repeat
The 3rd relay should first on for 10mins then off for 90mins then repeat

Are the three relays doing this simultaneously, or in sequence?

Your "then repeat" seems to mean that they are all operating simultaneously, each on their own cycle. To address this, I would define a class:

class Relay {
  const byte pin;
  const long onMs;
  const long offMs;
  boolean isOn;
  long startMs;
 
public:
  Relay(byte _pin, boolean _isOn, long _onMs, long _offMs) :
    pin(_pin),
    onMs(_onMs),
    offMs(_offMs)
  {
    isOn = _isOn; // initial state
  }

  void setup() {
    pinMode(pin, OUTPUT);
    digitalWrite(pin, isOn?HIGH:LOW);
    startMs = millis();
  }

  void loop() {
    if(millis() - startMillis >= (isOn?onMs:offMs)) {
      isOn = !isOn;
      digitalWrite(pin, isOn?HIGH:LOW);
      startMs = millis();
    }
  }
};

You would then declare three instances of this class:

Relay r1(5, true, 10L*60L*100L, 40L*60L*1000L);
Relay r2(6, false, 15L*60L*100L, 35L*60L*1000L);
Relay r3(7, true, 10L*60L*100L, 90L*60L*1000L);

Finally, you would invoke the setup and loop of each instance in your main setup and loop:

void setup() {
  r1.setup();
  r2.setup();
  r3.setup();
}

void loop() {
  r1.loop();
  r2.loop();
  r3.loop();
}

This will give you three relays on pins 5,6,7 each going independently through their individual cycle. For bonus kudos, instead of calling them r1, r2, and r3 call them what they are. If one of them controls a heater, call it 'heaterRelay' or something.

For more in this vein, check the link in my signature.

-- EDIT --

Oh, I see that relays 1 and 2 have a cycle with the same length. If they must operate in sync, then you would not use the method here, because of the possibility of drift. One way to fix this would be to pass millis() to the loop() rather than letting each loop() method take it's own timing.

7

GolamMostafa:
Pseudo Codes:

101 ----> PORTB

while ((millis() - presentMillis) != 10601000) ;

I know it’s pseudo code but will lead to disappointment if you forget that they compiler for an arduino like the UNO will evaluate the constant math expressions at compile time using 16 bit integral maths. So if you believe 10*60*1000 will lead to 600000 in the compiled code, you are wrong. The preprocesor should be informed to carry this as unsigned long math with one of the number carrying the [b]ul[/b] suffix: 10*60*1000[color=green][b]ul[/b][/color]

8

J-M-L:
I know it’s pseudo code but will lead to disappointment if you forget that...

...what you said and that millis does not always increase by one. :o

9

Indeed!

10

I know it's pseudo code but will lead to disappointment if you forget that they compiler for an arduino like the UNO will evaluate the constant math expressions at compile time using 16 bit integral maths.

The following codes are executed at 1/60th speed for the diagram of Post#4; the events are found to have occurred as expected. Yes! If the programmer forgets to append ul for unsigned long, the timings are totally messed up. I have noted down your comment.

unsigned long presentMillis;
#define N 1
void setup() 
{
  pinMode(8, OUTPUT);
  pinMode(9, OUTPUT);
  pinMode(10, OUTPUT);

}

void loop() 
{
  presentMillis = millis();
  PORTB = 0b101;
  while((millis() - presentMillis) != 10*N*1000ul)
    ;
  //-------------------------------------------------

  PORTB = 0b000;
  while((millis() - presentMillis) != 35*N*1000ul)
    ;
  //-------------------------------------------------

  PORTB = 0b010;
  while((millis() - presentMillis) != 50*N*1000ul)
    ;
  //-------------------------------------------------

  PORTB = 0b001;
  while((millis() - presentMillis) != 60*N*1000ul)
    ;
  //-------------------------------------------------

  PORTB = 0b000;
  while((millis() - presentMillis) != 85*N*1000ul)
    ;
  //-------------------------------------------------

  PORTB = 0b010;
  while((millis() - presentMillis) != 100*N*1000ul)
    ;
  //-------------------------------------------------
 
}
11

Would still recommend to not use != but < while((millis() - presentMillis) < 10*N*1000ul); and this type of code can be simplified by just using delay() since you are doing an active wait anyway

12

@PaulMurrayCbr

Will you please, place the codes of your Post#5 in an integrated fashion (all together) so that we can make a try that the codes generate timing functions as described in Post#4. Your working codes will open an opportunity to study class based program for people like us who have migrated from assembly platform to C/C++ platform.

OR

Please, present full-fledged class based codes (similar to Post#5) for the generation of the timing functions of Post#4.