Missing a specific Mills

I needed code to run a pump for 15 min every hour.
I tried using the Processor speed as a clock which worked after some help from the Forum...thanks again.
Somebody commented that Mills is not difficult when I said I also tried that rout. Well I like a challenge so here is my code.

The question I have. Is it possible that the program can miss a specific Mills. Sometimes when I run the program the printLn stating RESET LOOP TIMER is not in the monitor.

// constants won't change. Used here to set a pin number :


// Variables will change :
unsigned long previousMillis = 0;        // will store last time cycle was reset
unsigned long LoopMillis;        
int PumpRealayPin = 13;
// constants won't change :
const long LoopInterval = 3600000;           // interval at which Cycle will reset
const long OnInterval  = 150000;           // interval at which Pump is on

void setup() 
{
  Serial.begin(9600);
  pinMode      (PumpRealayPin , OUTPUT);
  digitalWrite (PumpRealayPin , HIGH);
}
void loop() 
{
  unsigned long RunningMillis = millis();

  if (RunningMillis - previousMillis <= LoopInterval) 
      {
        LoopMillis = (RunningMillis - previousMillis);
          if (LoopMillis < OnInterval)
            {
              digitalWrite (PumpRealayPin , HIGH);
              Serial.print(RunningMillis/1000);
              Serial.println("  Pump is On");
            }
          else 
        {
        digitalWrite(PumpRealayPin , LOW);
        //Serial.print(RunningMillis/1000);
          //    Serial.println("  Pump is OFF");
        } 
      }
  else  
    {
       previousMillis = RunningMillis;
        Serial.println("  **** Reset Loop Timer ******  ");
    } 
}
// constants won't change :
const long LoopInterval = 3600000;           // interval at which Cycle will reset
const long OnInterval  = 150000;           // interval at which Pump is on

The correct datatype is unsigned long.

And use unsigned long constants:

const unsigned long LoopInterval = 3600000UL;

Note UL for unsigned long.

I would do it using a 'latch' and testing for the intervals separately like this

// Variables will change :
bool pumpon;
unsigned long previousMillis = 0;        // will store last time cycle was reset
const int PumpRealayPin = 13;
// constants won't change :
const unsigned long OffInterval = 2700000;           // interval at which the pump is off
const unsigned long OnInterval  = 900000;           // interval at which Pump is on

void setup() 
{
  Serial.begin(9600);
  pinMode      (PumpRealayPin , OUTPUT);
  digitalWrite (PumpRealayPin , HIGH);
  pumpon=true;
  Serial.println("  Pump is On");
}

void loop() 
{
  unsigned long RunningMillis = millis();
  if ((pumpon)  && (RunningMillis - previousMillis>OnInterval)) { // turn pump off
    digitalWrite(PumpRealayPin , LOW);
    pumpon=false;
    Serial.println("  Pump is OFF");
    previousMillis=RunningMillis;
  }
  if ((!pumpon) && (RunningMillis - previousMillis>OffInterval)) { // turn pump on
    digitalWrite (PumpRealayPin , HIGH);
    pumpon=true;
    Serial.println("  Pump is On");
    previousMillis=RunningMillis;
  }    
}

Also you only had the pump turned on for 2.5 minutes. And does the pin really have to be 'HIGH' for the pump to be low or is it active 'LOW" ? (is more common.)

MarkT:
And use unsigned long constants:

const unsigned long LoopInterval = 3600000UL;

Note UL for unsigned long.

Why's it necessary to spell out "unsigned long" and put UL at the end though, MarkT?

Because C is a pretty poorly designed language? The RHS is just an expression, it has no idea
its part of a const variable declaration, and The Arduino AVR compiler defaults all integer values to
16 bit.

The need to specify the size as part of the constant denotation is one of the many rough edges of C/C++,

Consider the use of a "state machine" to do timing like this.

//motor control states
enum MCStates 
{
    STATE_TURN_ON,
    STATE_TURN_OFF
};

MCStates 
    statePumpControl;


// constants won't change. Used here to set a pin number :
const int PumpRelayPin = 9;
const int pinHeartbeat = 13;

// Variables will change :
unsigned long timeMotorRun;        // will store last time cycle was reset
unsigned long timeLoop;        
unsigned long timeNow;

// constants won't change :
//15-mins is 60*15*1000 = 900000mS
const unsigned long LoopInterval = 3600000ul;   //interval at which Cycle will reset
const unsigned long OnInterval  = 900000ul;     // interval at which Pump is on


void setup() 
{
    Serial.begin(9600);
    pinMode( PumpRelayPin , OUTPUT );
    digitalWrite ( PumpRelayPin , HIGH );
    pinMode( pinHeartbeat , OUTPUT );
    digitalWrite ( pinHeartbeat , LOW );
    timeMotorRun = millis();
    timeLoop = timeMotorRun;
    statePumpControl = STATE_TURN_OFF;
    
}//setup

void DoMotorControlTiming( void )
{    
    switch( statePumpControl )
    {
        case    STATE_TURN_ON:  
            timeNow = millis();
            if( (timeNow - timeLoop) < LoopInterval )
                return;

            //turn on the relay
            digitalWrite ( PumpRelayPin , HIGH );
            Serial.println("  Pump is On ");

            //set the loop time to "now" so we measure an hour from this point (not the point the pump turns off)
            timeLoop = timeNow;
            timeMotorRun = timeNow;
            
            statePumpControl = STATE_TURN_OFF;
            
        break;

        case    STATE_TURN_OFF:
            timeNow = millis();
            if( (timeNow - timeMotorRun) < OnInterval )
                return;

            //turn off the relay
            digitalWrite(PumpRelayPin , LOW);
            Serial.println("  Pump is Off ");
            
            statePumpControl = STATE_TURN_ON;
            
        break;
           
    }//switch
    
}//DoMotorControlTiming

void DoHeartBeatBlink( void )
{
    static bool
        bledState = 0;
    static unsigned long
        timeBlink = millis();
    unsigned long
        timeNow;

    timeNow = millis();
    if( (timeNow - timeBlink) >= 250 )
    {
        bledState ^= 1;
        digitalWrite( pinHeartbeat, (bledState)?HIGH:LOW );
        timeBlink = timeNow;
        
    }//if
    
}//DoHeartBeatBlink

void loop() 
{
    DoMotorControlTiming();
    DoHeartBeatBlink();
     
}//loop

One thing I like to do in state machine code such as BlackFin just posted, is to include a millis()-based delay()-less "heartbeat" blink on L13; especially in this case of an hour almost with no visible sign of life.

I just pretty much stick the BlinkWithoutDelay code in a function, and call it from loop():

void loop() 
{
    DoMotorControlTiming();
    DoHeartbeat();
    
}//loop
 if (RunningMillis - previousMillis <= LoopInterval) 

{
...
     }
 else  
   {
      previousMillis = RunningMillis;
       Serial.println("  **** Reset Loop Timer ******  ");
   }

Kinda backwards from the usual pattern but it looks like it should work. How do you know it missed one?

It is possible for a 16MHz Arduino to miss a whole millisecond. That is why you always use greater-than or less-than, never equals. It is rare, like one in a thousand, which means it occurs every second.

arduin_ologist:
One thing I like to do in state machine code such as BlackFin just posted, is to include a millis()-based delay()-less "heartbeat" blink on L13; especially in this case of an hour almost with no visible sign of life.

I just pretty much stick the BlinkWithoutDelay code in a function, and call it from loop():

void loop() 

{
    DoMotorControlTiming();
    DoHeartbeat();
   
}//loop

Great idea. I edited my post to show this.

The OP had assigned his relay pin to pin 13 (the on-board LED on a 2560...) so I moved his to pin 9 and used 13 instead for the still-alive heartbeat.

  1. I purposefully changed the MotorOn time to 2.5 min to monitor the program. That is also why I commented out the Motor Off Print Line, Was just too much in the serial monitor to go through, especially running it for 24 hours. If the sketch fail all my seedlings are lost.

  2. Why do I say I miss a Millis, is because I ran the sketch for 24+ hours and then looked at the Serial monitor, there were only 5 times the pint Line that stated the Loop Time was reset was printed.

  3. You always set me a new challenge..I have never used a Case Motor, something I will have to go and research. :o

Thanks for the advice for improvements, for now I will just add all the Unsigned Long and UL coding and run it overnight again.

See if my version does what you need, bring on the criticism :stuck_out_tongue:

unsigned long previousMillis = 0; 
int PumpRealayPin = 13;
const unsigned long LoopInterval = 36000; // interval at which Cycle will reset
const unsigned long OnInterval  = 9000;     // interval at which Pump is on
bool pumpState = false;
bool lastPumpState = false;

void setup()
{
  Serial.begin(9600);
  digitalWrite (PumpRealayPin , HIGH);
  pinMode      (PumpRealayPin , OUTPUT);
 
}
void loop()
{
  digitalWrite(PumpRealayPin,millis() - previousMillis < OnInterval);
  if(millis() - previousMillis >= LoopInterval)
  {
    previousMillis += LoopInterval;
  }
  pumpState = digitalRead(PumpRealayPin); 
  if(pumpState != lastPumpState)
  {
    if(pumpState)
    {
      Serial.print(" Pump is ON  \t");
      Serial.println(millis() / 1000);
    }
    else
    {
      Serial.print(" Pump is OFF  \t");
      Serial.println(millis() / 1000);
    }
    lastPumpState = pumpState;
  }    
}

If this fast version works, add 2 zeros to the end of OnInterval and LoopInterval.

outsider:
bring on the criticism :stuck_out_tongue:

I think my version is more transparent.

I have to agree, I've not tried yours yet, I'm too busy gloating over mine> :grin:

Sketch uses 2358 bytes (7%) of program storage space. Maximum is 30720 bytes.
Global variables use 224 bytes (10%) of dynamic memory, leaving 1824 bytes for local variables.
Maximum is 2048 bytes.

Going to try both and make sure I understand it. Like new code for the same function especially if there are enough comments to make a newby understand.

For now, changing the variable type seem to have solved the problem. Ran it last night and even changed the loop time to 5 min and the pump time to 30 sec and it ran faultlessly. every "Reset Loop" was in the Serial Monitor.

Let us know if you need more comments and detailed explanation.

Why do you use a Boolean variable when you set the variable equal to the Relay Pin in any case. As both are using the Boolean Variable it MUST be the right thing to do, just do not understand it.

Is the "Case Machine" referring to the "Case Switch" function? Any pointers if I want to use this method. Only gone to the resource section to see what it is and how it works. New to me.

From the Refrence section on Switch Case function...

It says the variables that can be used is int and char, but Mills() is in Long.

HermanJFourie:
Is the "Case Machine" referring to the "Case Switch" function?

I think you mean "state machine" and "switch...case" and answer would be "yes" although state machines can be implemented other ways.

A state machine is just a fancy way of saying that we define a bunch of states, where "state" there simply means a defined sets of conditions, or circumstances. So a device could be (say, and not using your example here on purpose, to be general) Running, or Idle, or Paused, or UndergoingService, or whatever.

The switch..case paradigm is a simple way to implement that. Switch...case requires integers to define the cases though, so we would first define say Running as 1, Idle as 0 etc, then use those friendly names to be the values we check for in switch...case, as say theCurrentState.

Since theCurrentState can only have one value at a time (it's just a variable, after all), each section in the switch..case represents a unique state of our system, and in there we do what that state requires. Importantly, we only look for things that can legitimately take us out of that state. For example it may not be allowed to go from Running to UndergoingService, but only from Idle or Paused. So let's say there's a bigRedButton that takes us to UndergoingService, we simply do not do a digitalRead(bigRedButton) when we are in state Running,and will never inadvertently make that transition.

Switch..case is really just a fancy, simple way of doing a zillion "if" statements without the risk of ending so deep we get lost :wink: .


HermanJFourie:
From the Refrence section on Switch Case function...

It says the variables that can be used is int and char, but Mills() is in Long.

But millis() isn't the variable you would use to "define" one case compared to the next; they would be the integers 0 for Idle or 1 for Running, to continue my generic example.

HermanJFourie:
Why do you use a Boolean variable when you set the variable equal to the Relay Pin in any case. As both are using the Boolean Variable it MUST be the right thing to do, just do not understand it.

This question may be addressed to me. well quite simple the boolean keeps track of what 'state' the pump is in, and checks for a different condition depending on it's 'state'. If the pump is on, compare the elapsed time to the OnInterval and if it has passed, turn it off, set the 'state' - (latch) and reset the timer. now when the pump is off compare the elapsed time to the OffInterval and once that has passed etc... It is a form of a 'state' machine but since it has only 2 states can also be referred to as 'using a latch' i hope that clarifies it
.
n.b. in the if statement first the 'state' is tested so the other test doesn't get executed at all if it is false, doesn't really matter in the example but would save time (in theory..)