Controlling a pump. Program runs but getting unexpected results

Building a controller to add a cup of acid a day to my pool. I have a peristaltic pump that will deliver that amount in an hour. This pool also has a hot tub. I do not want to add acid if the hot tub is on. To prevent this, I have a flow switch installed in a pool return line. The pool normally runs 5 hours a day.

How system should work: When the flow switch is closed (low), the acid pump will turn on. The pump will turn off if the allotted time time has passed or if the flow switch opens (high). Once the pump turns on, it will be prevented from turning on until the next day. This way I do not need a clock. If the pool is on and it has been long enough (say, 20 hours), the system will start. If I turn the pool off and back on a hour later, the system will not turn on the acid pump.

For testing, the program is set to run the pump for 5 seconds and then be disabled for 2 minutes.

What is going wrong: I will let the system sit for 2+ minutes, I then close the flow switch. I want it to immediately turn the pump on. Instead I get a minute + delay, then the system will run as expected.

/* Pool Acid Injection 

This program will turn on a low flow acid injection pump when the flow switch is closed and an allotted time (delay) has passed.

The flow switch will only be closed when the pool is running. The flow switch will open when the spa is on. This will prevent the
acid pump from running during spa opperation that could cause an unsafe condition.

The system will run a specific time then be disabled until the next day.
 */

const int flowSwitch = 2;         // flow switch from pool controler
const int pump =  5;              // the number of the pump output

int pumpState = HIGH;      
int flowSwitchState = 0;          // reads flow switch


long delayTime = 120000;          // how long to wait after acid pump runs before it can run again
long pumpOnTime = 5000;           // how long acid pump will stay on
long flowSettle = 2000;           // lets the flow switch settle (debounce)
unsigned long delayTimeAdder = 0; // resets time so program can restart delay period after acid pump has run
unsigned long setRunTime = 0;     // resets time so acid pump run time can reset
byte pumpIndicator = 0;           // this is set to 1 when the acid pump has been turned on
byte delayIndicator = 0;          // this is set to 1 after the delay time has been met

void setup() {
  
  pinMode(pump, OUTPUT);
  pinMode(flowSwitch, INPUT_PULLUP); 
     
}
void loop()
{ 
  unsigned long currentMillis = millis();  
  flowSwitchState = digitalRead(flowSwitch);
  
  if (currentMillis > delayTime + delayTimeAdder) // will keep pump from running until delay has past
    {  
    delayIndicator = 1; 
    }
  if (delayIndicator == 1 && pumpIndicator == 0 && flowSwitchState == LOW) 
    { 
     setRunTime = currentMillis;                 // sets base time for run time when pump turns on
     pumpIndicator = 1;                          // this stops the set run time from being continuously set to the current time
     delay (flowSettle);                         // this will let the flow switch stabalize (debounce)
     }
  if (delayIndicator == 1 && (currentMillis < (setRunTime + pumpOnTime + flowSettle)) && flowSwitchState == LOW) 
    { 
     pumpState = LOW;                            // turns on acid pump if delay has passed, flow switch is made and acid pump run time has not expired
    }
  if (delayIndicator == 1 && (currentMillis > (setRunTime + pumpOnTime)) || delayIndicator == 1 && 
      (currentMillis < (setRunTime + pumpOnTime)) && flowSwitchState == HIGH)     
    {  pumpState = HIGH;                         // turns off acid pump when run time elapses or if flow switch opens
        delayTimeAdder = currentMillis;          // resets time for delay
        delayIndicator = 0;                      // resets delay indicator
        pumpIndicator = 0;                       // resets acid pump running indicator
    }   
  digitalWrite(pump, pumpState);
 }

Instead of    (currentMillis < (setRunTime + pumpOnTime))     you should use

   (currentMillis - setRunTime < pumpOnTime)

While these two statements are mathematically identical, they are not identical with unsigned-long arithmetic. This will only make a difference once the milliseconds rolls over back to zero.

ALL of your variables that are used in these time calculations should be declared as unsigned long.

const int flowSwitch = 2;         // flow switch from pool controler
const int pump =  5;              // the number of the pump output

These are pin numbers, right? Why doesn't the name reflect that?

How IS the switch wired?

    delayIndicator = 1;

What does delayIndicator mean? Wouldn't using a boolean, with a meaningful name, like delayNeeded, make more sense?

    delayNeeded = true;

    if(delayNeeded && someOtherStuff)
    {

I abhor the practice of adding comments at the end of a statement. A comment should explain waht you are doing, not what the statement is doing.

// Turn the pump on if it is time
if(millis() - lastTime > interval)
{

All of your comments state the obvious, but completely fail to describe the why or the overall purpose of the code.

MorganS:
Instead of   (currentMillis < (setRunTime + pumpOnTime))    you should use

   (currentMillis - setRunTime < pumpOnTime)

While these two statements are mathematically identical, they are not identical with unsigned-long arithmetic. This will only make a difference once the milliseconds rolls over back to zero.

ALL of your variables that are used in these time calculations should be declared as unsigned long.

Thanks for the input, will make the changes you suggest. I was wondering what would happen the time rolled over, I guess these changes will keep it from going wonky?

PaulS:

const int flowSwitch = 2;         // flow switch from pool controler

const int pump =  5;              // the number of the pump output



These are pin numbers, right? Why doesn't the name reflect that?

How IS the switch wired?

I will add the pin numbers, thanks for the suggestion.

The switch connects pin 2 to the Arduino ground when closed.

    delayIndicator = 1;

What does delayIndicator mean? Wouldn't using a boolean, with a meaningful name, like delayNeeded, make more sense?

Yea, couldn't think of a better name at the time. Basically it is like a flag that is set and gets cleared later. I used it to control when a value should be set to current millis.

    delayNeeded = true;

if(delayNeeded && someOtherStuff)
   {




I abhor the practice of adding comments at the end of a statement. A comment should explain waht you are doing, not what the statement is doing.



// Turn the pump on if it is time
if(millis() - lastTime > interval)
{




All of your comments state the obvious, but completely fail to describe the why or the overall purpose of the code.

I'll work on cleaning it up. This is my first sketch that I put together that is not just a rework of existing code. Still learning convention.

I think you would do better to make the code simpler even if it is longer

safety is always the first argument

is the spa not running.........yes/no (no is else)

yes: next question

no: spa safety =0
spa timer prevmillis=currentmillis; (if the spa is running reset timer)

has the spa been off continuous for 5 seconds currentmillis-prevmillis>safetyTimer

yes: spar has been off for 5 seconds
spaSafety=1

no: loop will repeat until flowSwitch has not been sensed high for 5 seconds

as soon as the flow switch goes high the safety flag is removed (rather have a acid pump stop early)

new question

is the acid pump required (based on time since last cycle was run)
this is the harder part based on time with no rtc (real time clock). Also other things to consider if spa is running do you postpone this cycle or skip it. When does time start again if the spa was being used?

acidCycleRequest=1

else acidCycleRequest=0

next question

is acid pump on its 5 second run cycle (this cycle can be running all the time as its just a flag so 5 seconds on ????? off. timer/counter++ may be simple way to do this)

pumpRequest=1

else pumpRequest=0

if spaSafety==1 && acidCycleRequest==1 && pumpRequest=1

run pump

else

stop pump

few things that may help

you can set prevmillis = millis() in set up that way you don't get a accidental timer trip on start up

try not to use the millis argument in a complicated if statement. have the timer trip a flag as that makes trouble shooting easy

basic millis() timer that rolls over and works

unsigned long currentMillis = millis();
  if(currentMillis - previousMillis > interval) {
   //do something
previousMillis = currentMillis;}

gpop1:
new question

is the acid pump required (based on time since last cycle was run)
this is the harder part based on time with no rtc (real time clock). Also other things to consider if spa is running do you postpone this cycle or skip it. When does time start again if the spa was being used?

Thanks for your info, rewriting the code based on your suggestions. I only have 1 input for this system, the pool flow switch. If the spa is on or the pool pump is off the acid pump should not run. If the acid pump is turned on (should run a hour) but shuts off for any reason (flow switch goes high, power outage) I want the acid pump to "rest" for 20 hours then start looking for flow again. Yes I will miss adding acid to the pool that day, but I will have to test and adjust anyway.

next question

is acid pump on its 5 second run cycle (this cycle can be running all the time as its just a flag so 5 seconds on ????? off. timer/counter++ may be simple way to do this)

Not sure what your asking here, hope I answered with my above comment. The 5 seconds is just for test reasons for the program. Should work like this: 20 hours of acid pump rest, then look for the flow switch to be high (for 5 seconds min as a debounce) after that acid pump runs for an hour then goes to sleep for 20 hours etc.

sry my bad I read

"For testing, the program is set to run the pump for 5 seconds and then be disabled for 2 minutes."

and thought that during the hour the pump was allowed to run you was pulsing the acid pump on for 5 seconds every 2 mins.