Simple Relay timer 10 minutes... new to coding.

It's a really easy code but I struggled...
Comments welcomed... I'm new to coding.
Any shortcuts for what I've written or faults?

Thanks!

/*

CONTROLLING A RELAY USING AN INPUT SIGNAL
WITH A 10 MINUTE TIMER (START, STOP/RESET)

 Based on:
 http://www.arduino.cc/en/Tutorial/BlinkWithoutDelay
 */

// constants won't change. Used here to 
// set pin numbers:
const int pumpPin =  13;      // the number of the PUMP pin
const int pulseInput = 11;                // the number of the PULSE pin


// Variables will change:
int pumpState = LOW;             // pumpState used to set the PUMP
long startMillis = 0;        // will store last time PUMP was updated


// the follow variables is a long because the time, measured in miliseconds,
// will quickly become a bigger number than can be stored in an int.
long timer = 1000*60*10;           // timer of 10 minutes

void setup() {
  // set the digital pin as output:
  pinMode(pumpPin, OUTPUT); 
  digitalWrite(pumpPin, LOW);
  pinMode(pulseInput, INPUT);   
}

void loop()
{
 
  // Checking for the pulse
 if (digitalRead(pulseInput) == HIGH){
   if (pumpState == LOW)
      pumpState = HIGH;
    else
      pumpState = LOW;
 }
 
 // Update, update, update...
 unsigned long currentMillis = millis(); 
 
// Load startMillis
if (pumpState == HIGH & startMillis == 0)
startMillis = currentMillis;

// If PumpState HIGH and Timer less than 10 minutes
if (pumpState == HIGH & currentMillis - startMillis < timer){
   // Turn ON pump
   digitalWrite(pumpPin, HIGH);
 
}
  // Else turn OFF pump, reset startMillis and pumpState
 else {
   digitalWrite(pumpPin, LOW);
   startMillis = 0;
   pumpState = LOW;
 
 }
 
} // End of Loop
1 Like
long timer = 1000*60*10;           // timer of 10 minutes

All 3 literals are ints. The multiplication will be performed using int methods, regardless of the fact that you wish to store the result in a long and regardless of the fact that the multiplication will overflow. One or more of the literals needs UL tacked on the end.

Time is ALWAYS unsigned. Time does not flow backwards, except in Hollyweird.

The value being defined is an interval, not a timer. The name sucks.

if (pumpState == HIGH & startMillis == 0)
startMillis = currentMillis;

Proper indenting might not matter to you or to the compiler. It does to me. Tools + Auto Format is you friend. Not using it will never make you mine.

Damn Paul thanks for the Auto Format... it was killing me to go TAB TAB TAB... XD

ok time will always be unsigned from now on.. I'll look that one up cause really I have no idea yet what's the difference. :fearful:

and yes!!! name sux... is not a timer... but an interval, I'll change that! :stuck_out_tongue:

about the multiplication.. I'll have too look that one too cause I have no idea whats the difference when multiplying INTs vs LONGs :fearful:

/* 
 
 CONTROLLING A RELAY USING AN INPUT SIGNAL
 WITH A 10 MINUTE TIMER (START, STOP/RESET)

 Based on:
 http://www.arduino.cc/en/Tutorial/BlinkWithoutDelay
 */

// constants won't change. Used here to 
// set pin numbers:
const int pumpPin =  13;      // the number of the PUMP pin
const int pulseInput = 11;                // the number of the PULSE pin


// Variables will change:
int pumpState = LOW;             // pumpState used to set the PUMP
unsigned long startMillis = 0;        // will store last time PUMP was updated


// the follow variables is a long because the time, measured in miliseconds,
// will quickly become a bigger number than can be stored in an int.
unsigned long interval = 600000;           // interval of 10 minutes

void setup() {
  // set the digital pin as output:
  pinMode(pumpPin, OUTPUT); 
  digitalWrite(pumpPin, LOW);
  pinMode(pulseInput, INPUT);   
}

void loop()
{

  // Checking for the pulse
  if (digitalRead(pulseInput) == HIGH){
    if (pumpState == LOW)
      pumpState = HIGH;
    else
      pumpState = LOW;
  }
  
  // Update, update, update...
  unsigned long currentMillis = millis(); 

  // Load startMillis
  if (pumpState == HIGH & startMillis == 0)
    startMillis = currentMillis;

  // If PumpState HIGH and Timer less than 10 minutes
  if (pumpState == HIGH & currentMillis - startMillis < interval){
    // Turn ON pump
    digitalWrite(pumpPin, LOW);

  }
  // Else turn OFF pump, reset startMillis and pumpState
  else {
    digitalWrite(pumpPin, HIGH);
    startMillis = 0;
    pumpState = LOW;
    Serial.println("END");
  }

} // End of Loop
unsigned long interval = 600000;           // interval of 10 minutes

That's harder to change to 12 minutes, for instance

unsigned long interval = 1000UL * 60 * 12; // 12 minute interval

Next, you'll need to look at the state change detection example. You want to turn the pump on when the switch BECOMES pressed, not IS pressed, right? Then, separately, if the pump is on, and it has been on for the defined interval, you want to turn it off, right?

Something like this:

int currState;
int prevState = LOW;
unsigned long pumpOnTime;

void loop()
{
   currState = digitalRead(pulseInput);
   if(currState != prevState)
   {
        // The switch became pressed or it became released
        if(currState == HIGH)
        {
            // It became pressed...
           // Turn the pump on and record the time
           pumpOnTime = millis();
           digitalWrite(pumpPin, LOW);
       }
   }
   prevState = currState;

   if(pumpOnTime > 0) // The pump is on
   {
       if(millis() - pumpOnTime > interval) // It's time to turn it off
       {
           // Turn the pump off and clear the on time
           pumpOnTime = 0;
           digitalWrite(pumpPin, HIGH);
      }
   }
}

Damn thanks.. feeling support of the Forum here.

Looks much more clean that last part of the code I'll implement that, it looks much better...

and thanks for the advice going 1000UL to be able to multiply on Long.

Little big steps on my learning curve. XD

I'll post the changes.

Thanks!

Thanks for the help. This is how it's looking so far.
It's all working.

Used an analog input for 3.3 signal and mapped it 0 and 1.

Also the pumps should be able to stop anytime.

int currState;
int prevState = LOW;
unsigned long pumpOnTime;
int pulseInput = 5;
int pumpPin = 12;
unsigned long interval;

void setup()
{
  pinMode(pumpPin, OUTPUT);
  digitalWrite(pumpPin, HIGH);
  interval = 1000UL*60*10;
}

void loop()
{
  currState = analogRead(pulseInput);
  currState = map(currState, 0, 600, 0, 1);

    if(currState != prevState)
  {

    // The switch became pressed or it became released
    if(currState == HIGH)
    {
      // It became pressed...
      // check if the pump is on to shut it off
      if (pumpOnTime > 0) {
        prevState = currState;
        goto OFF; //Jump to OFF: tag.

      }
      // Turn the pump on and record the time
      pumpOnTime = millis();
      digitalWrite(pumpPin, LOW);
    }
    if (pumpOnTime == 0) Serial.println ("IDLE");
  }

  prevState = currState;

  if(pumpOnTime > 0) // The pump is on
  {
    Serial.println (millis() - pumpOnTime);
    if(millis() - pumpOnTime > interval) // It's time to turn it off
    {
      // Turn the pump off and clear the on time
OFF:
      pumpOnTime = 0;
      digitalWrite(pumpPin, HIGH);
    }
  }

}
  currState = analogRead(pulseInput);
  currState = map(currState, 0, 600, 0, 1);

So, any value from 0 to 599 becomes 0, and anything over 599 becomes 1.

currState = 0;
if(analogRead(pulseInput) >= 600)
   currState = 1;

would be more efficient.

        goto OFF; //Jump to OFF: tag.

No. There is no reason to resort to gotos.