# Can someone help me re-write this in Milllis()

this approach would world perfectly assuming delay don't overflow. can someone show me how this would be written using millis. i'm afraid to have a slight bug in the code that could cost me thousands of dollars in damage. thanks

``````int Pump =  4; // the number of the pump pin
int Pump2 = 5; // the number of the pump2 pin
int Pump3 = 6; // the number of the pump3 pin

void setup()
{
pinMode(Pump, OUTPUT);  // pump 1
pinMode(Pump2, OUTPUT); // pump 2
pinMode(Pump3, OUTPUT); // pump 3
}

void loop() {
digitalWrite(Pump, LOW);  // TURN ON
delay(480000); //WAIT 8 MINUTES
digitalWrite(Pump, HIGH);  // TURN OFF
delay(720000); //wait 12 minutes
digitalWrite(Pump2, LOW);  // TURN ON
delay(480000); //WAIT 8 MINUTES
digitalWrite(Pump2, HIGH);  // TURN OFF
delay(720000); //wait 12 minutes
digitalWrite(Pump3, LOW);  // TURN ON
delay(480000); //WAIT 8 MINUTES
digitalWrite(Pump3, HIGH);  // TURN OFF
delay(720000); //wait 12 minutes
}
``````

So, what did you try?

Btw, I would start with a single pump. After that, extend it to multiple using arrays :)

I had a single pump working with basically blink without delay. The problem im facing is I wouldn't ever want any other pump to run while a pump is on. So the important thing is that only 1 pump runs at a time and that it has atleast 12 minutes of off time before the next pump comes on

Can do that, just show us your 1 pump solution and an attempt to make it work for two pumps. Think about the hin about arrays ;)

1 pump solution, modified blink without delay

``````int Pump =  4; // the number of the pump pin
int Pump2 = 5; // the number of the pump2 pin
int Pump3 = 5; // the number of the pump3 pin

int PumpState = LOW;     // PumpState used to set the Pump
int Pump2State = LOW;    // Pump2State used to set the Pump 2
int Pump3State = LOW;    // Pump3State used to set the Pump 3

unsigned long PumpPrevMillis = 0;        // will store last time pump was updated
unsigned long PumpOnTime = 2000;           // milliseconds of on-time
unsigned long PumpOffTime = 1000;          // milliseconds of off-time

unsigned long Pump2PrevMillis = 0;        // will store last time pump was updated
unsigned long Pump2OnTime = 2000;           // milliseconds of on-time
unsigned long Pump2OffTime = 1000;          // milliseconds of off-time

unsigned long Pump3PrevMillis = 0;        // will store last time pump was updated
unsigned long Pump3OnTime = 2000;           // milliseconds of on-time
unsigned long Pump3OffTime = 1000;          // milliseconds of off-time

void setup()
{
pinMode(Pump, OUTPUT);  // pump 1
pinMode(Pump2, OUTPUT); // pump 2
pinMode(Pump3, OUTPUT); // pump 3
}

void loop()
{
unsigned long PumpCurrentMillis = millis();

///////////////////////PUMP 1/////////////////////

if ((PumpState == HIGH) && (PumpCurrentMillis - PumpPrevMillis >= PumpOnTime ))
{
PumpState = LOW;  // Turn it off
PumpPrevMillis = PumpCurrentMillis;
digitalWrite(Pump, PumpState);
}
else if ((PumpState == LOW) && (PumpCurrentMillis - PumpPrevMillis >= PumpOffTime))
{
PumpState = HIGH;  // turn it on
PumpPrevMillis = PumpCurrentMillis;
digitalWrite(Pump, PumpState);
}

}
``````

I think that it would help if you wrote out in English what sequence of events you want to achieve, including the timings.

It sounds like a state machine would fit the bill with only the code for each state running at a time.

well basically as soon as i plugin this arduino it will have 3 pumps attached to it and right away it will turn on pump 1 for 8 minutes while while pumps 2 and 3 remain off. after the 8 minutes of on time it needs to shut off and remain off for 12 minutes. then we move on to pump 2 where the exact same thing happens, then pump 3 then back to pump 1. all on for 8 and off for 12 1 at a time. id like this to repeat indefinitely, thanks

notsolowki: i'm afraid to have a slight bug in the code that could cost me thousands of dollars in damage.

If that’s the case, I’m not sure I’d trust the application to a hobbyist-quality micro controller board and sketchy firmware. Would you trust your artificial pacemaker to an Arduino board? How about your autopilot?

okay well will the way its written with delay in the first post work or will it overflow? it dont need to be in millis, i sure dont trust myself to write it in millis.

I'm a believer in state machines which in this world means millis() and switch...case, but in this scenario PROVIDED the sketch doesn't now, nor will ever, need to do anything that those delays prevent, I'd go with the code in the opening post.

notsolowki: assuming delay don't overflow.

okay well will the way its written with delay in the first post work or will it overflow?

Says here that delay() is unsigned long, so as long as no delay is itself more than 49-and-a-bit days, that's ok.

delay() doesn't overflow like millis() does, in the sense that millis() starts as soon as the Arduino wakes up and counts forever, overflowing every 7 weeks. Each delay() is a new one, so they can be 7 weeks long, and yours are mere hours.

notsolowki:
well basically as soon as i plugin this arduino it will have 3 pumps attached to it and right away it will turn on pump 1 for 8 minutes while while pumps 2 and 3 remain off. after the 8 minutes of on time it needs to shut off and remain off for 12 minutes. then we move on to pump 2 where the exact same thing happens, then pump 3 then back to pump 1. all on for 8 and off for 12 1 at a time. id like this to repeat indefinitely, thanks

That does not make sense. If you have 3 pumps and only one should be on at any one time and if the ON period is 8 minutes then the OFF period must be 16 minutes.

If you really want each pump to run for a period of 8 minutes one after the other then set up a simple timer using millis() that updates a variable every 8 minutes from 0 to 1 to 2 and back to 0. Depending on the value of the variable the appropriate pump will run. Something like

``````if (millis() - prevPumpStart >= millisFor8Minutes) {
prevPumpStart += millisFor8Minutes;
pumpIndex ++;
if (pumpIndex > 2) {
pumpIndex = 0;
}
for (byte n = 0; n < 3; n++) {
digitalWrite(pumpPin[n], LOW); // turn them all off
}
digitalWrite(pumpPin[pumpIndex], HIGH); // turn one on
}
``````

…R

Robin2: That does not make sense. If you have 3 pumps and only one should be on at any one time and if the ON period is 8 minutes then the OFF period must be 16 minutes.

But that's not what he wants, the way I read it: all the pumps are off for 12 minutes after the last one goes off. The 12 minutes isn't the incorrectly calculated on-time of the other 2 pumps which as you say would be 16 if one pump was always running: it's off-time for all 3.

notsolowki: only 1 pump runs at a time and that it has atleast 12 minutes of off time before the next pump comes on

And that's what OP's code in the opening post does:

pump 1 on for 8 minutes all off for 12 minutes pump 2 on for 8 minutes all off for 12 minutes pump 3 on for 8 minutes all off for 12 minutes

notsolowki: this approach would world perfectly assuming delay don't overflow. can someone show me how this would be written using millis. i'm afraid to have a slight bug in the code that could cost me thousands of dollars in damage. thanks

notsolowki: I had a single pump working with basically blink without delay. The problem im facing is I wouldn't ever want any other pump to run while a pump is on. So the important thing is that only 1 pump runs at a time and that it has atleast 12 minutes of off time before the next pump comes on

What happens if the Arduino loses power for whatever reason, such as a power outage, or someone accidentally flipping the wrong switch? How sure are you that there will be no interruptions in power?

From a common-sense point of view, it seems that the very first thing your code should do is to turn all of the pumps off and wait for 12 minutes before turning the first pump on. This way, if the Arduino somehow loses power (or gets reset) when one of the pumps is on, it will be able to recover safely.

LesserMole: But that's not what he wants, the way I read it: all the pumps are off for 12 minutes after the last one goes off.

I am certainly not going to argue that you are wrong.

I based my response on this text

and right away it will turn on pump 1 for 8 minutes while while pumps 2 and 3 remain off. after the 8 minutes of on time it needs to shut off and remain off for 12 minutes

and I assumed that IT meant the pump that had just been on for 8 minutes.

...R

Always better to explain these sequences visually on a timeline, otherwise there's always going to be ambiguity.

It looks like the OP may have been run off by all the confusion.

This request isn't easily done "using millis()" because there is a sequence involved. to do this well, and allow for some flexibility in the code, I found it a challenge without getting the toolbox out.

OP isn't really a newbie, so I'll throw him a bone:

``````constexpr uint32_t HOLD_MINUTES(uint32_t min) {
return min * 60 * 1000;
};

class PumpTimer {
using functPtr = void(*)(void);

public:
PumpTimer(int pumpPin, uint32_t runTime, uint32_t pauseTime, functPtr stoppedAction = nullptr) : pin(pumpPin), cycleMillis(runTime), offMillis(pauseTime), callback(stoppedAction) {}
void setTime(uint32_t runTime) {
cycleMillis = runTime;
}
void init(void) {
pinMode(pin, OUTPUT);
}
void start(uint32_t duration) {
cycleMillis = duration;
start();
}
void start(void) {
startMillis = millis();
state = ACTIVE_RUNNING;
digitalWrite(pin, HIGH);
}
void process(void) {
switch (state) {
case STOPPED:
break;
case ACTIVE_RUNNING:
if (millis() - startMillis > cycleMillis) {
digitalWrite(pin, LOW);
state = ACTIVE_PAUSED;
startMillis = millis();
}
break;
case ACTIVE_PAUSED:
if (millis() - startMillis > offMillis) {
state = STOPPED;
if (callback) {
callback();
}
}
break;
}
}
void stop(void) {
digitalWrite(pin, LOW);
state = STOPPED;
callback();
}
private:
enum {
STOPPED,
ACTIVE_RUNNING,
ACTIVE_PAUSED,
} state = STOPPED;
uint8_t pin;
uint32_t cycleMillis;
uint32_t offMillis;
uint32_t startMillis;
functPtr callback;
};

PumpTimer timer[] = {
/*Pin number, On Time, Pause Time and pointer to callback function */
{ 4, HOLD_MINUTES(8), HOLD_MINUTES(12), []() {
timer[1].start();
}
},
{ 5, HOLD_MINUTES(8), HOLD_MINUTES(12), []() {
timer[2].start();
}
},
{ 6, HOLD_MINUTES(8), HOLD_MINUTES(12), []() {
timer[0].start();
}
},
};

void setup() {
for (auto& t : timer) {
t.init();
}
timer[0].start();
}

void loop() {
for (auto& t : timer) {
t.process();
}
// do other non-blocking things here...
}
``````

only minimally tested...

BulldogLowell: It looks like the OP may have been run off by all the confusion.

This request isn't easily done "using millis()" because there is a sequence involved. to do this well, and allow for some flexibility in the code, I found it a challenge without getting the toolbox out.

OP isn't really a newbie, so I'll throw him a bone:

(here follows a 92-line bone)

I can't follow your "bone", and I'm sure that the OP can't either.

``````using functPtr = void(*)(void);