Help with "until" loop

[ato_arduino]

Thanks everyone for the helpful info so far. As for the logic behind the code I would like the FlushSolenoid's 20 second occurrence to turn on given two conditions: 1) Anytime the ATOsolenoid is HIGH (turned on) or 2) anytime the ATOsolenoid has been running for more than 1 hour. For some background this FlushSolenoid will be used to start a black-flush in the system and ideally will run its 20 second cycle anytime the main ATOsolenoid starts and then once every hour when the ATOsolenoid is on. Here is what I have attempted for the code regarding this operation:

#define EXE_INTERVAL_1 5000
#define EXE_INTERVAL_2 10000

void Loop()
  //Flush Solenoid
  unsigned long lastExecutedMillis_1 = 0;  // vairable to save the last executed time for code block 1
  unsigned long lastExecutedMillis_2 = 0;  // vairable to save the last executed time for code block 2
  unsigned long currentMillis = millis();
  if (digitalRead(ATOsolenoid == HIGH)) {
        digitalWrite(FlushSolenoid, HIGH);
      } 
  if (currentMillis - lastExecutedMillis_1 >= EXE_INTERVAL_1) {
    lastExecutedMillis_1 = currentMillis;  // save the last executed time
    digitalWrite(FlushSolenoid, HIGH);
  }
  if (currentMillis - lastExecutedMillis_2 >= EXE_INTERVAL_2) {
    lastExecutedMillis_2 = currentMillis;  // save the last executed time
    digitalWrite(FlushSolenoid, LOW);
  }
}

The flush solenoid does turn on for 10 seconds when executing the code, but does not reset back to on (HIGH) when I trigger the ATOSolenoid to turn on by manipulating the eyes. It also does not seem to reset/loop every 5 seconds as I thought it would (just using 5 seconds as a test interval - this will ultimately be the 1 hour when I get the code operational). Thanks again

[pert]

I have merged your topics due to them having too much overlap on the same subject matter @ato_arduino.

In the future, please only create one topic for each distinct subject matter and be careful not to cause them to converge into parallel discussions.

The reason is that generating multiple threads on the same subject matter can waste the time of the people trying to help. Someone might spend a lot of time investigating and writing a detailed answer on one topic, without knowing that someone else already did the same in the other topic.

Thanks in advance for your cooperation.

[alto777]

@ato_arduino

turn on by manipulating the eyes

Please post a complete sketch that you have been using but haven't showed us.

The last sketch does nothing with the eyes, so it is hard to see how manipulation of the eyes will test your logic.

If the flush solenoid is waiting for an hour to go by, should any cessation of the other solenoid cancel that, and reset the flush mechanism so it will trigger again once the other solenoid is eye-triggered?

Even if one hour has not gone by?

Examine @gcjr's handling of the on/off for the flush - your use of two if statements messes up the timers as you don't know if you are coming (timing twenty seconds) or going (timing one hour).

The switch/case statement only does one of the two timed sections. It times out the on time, then switches to timing out the off time. Rinse and repeat.

Add some conditions on that to start and stop it and you'll be done.

a7

[noiasca]

@ato_arduino

so far

can you please give feedback of the previous help you received?

I've posted an example in #16 ... have you checked it?
Furthermore I came up with a state diagram - what do you think about it - does it represent what you need? If not - what should be different?

@gcjr has posted a full code in #18 - does it work like you need it? If not - what should be different?

[alto777]

@ato_arduino here I see it as two processes, with only the thin link that the second process is informed by the state of the first process:

On the left is the simple two state hysteresis control of the fluid level by the ATO.

On the right is a timer/controller for the Flish. It is idle until the ATO kicked it into action, where it repeats the on/off cycle until the ATO signal goes LOW.

I say again that a bit of logic to perform, or not perform, @gcjr's switch/case is all you need add.

a7

[ato_arduino]

@alto777

Here is the complete sketch that I had been attempting:

int ATOsolenoid = 3;    //This is the output pin on the Arduino we are using
int FlushSolenoid = 4;  //This is the output pin on the Arduino we are using
int lowEye = 7;
int highEye = 8;
#define EXE_INTERVAL_1 5000
#define EXE_INTERVAL_2 10000

void setup() {
  // put your setup code here, to run once:
  Serial.begin(9600);
  pinMode(ATOsolenoid, OUTPUT);    //Sets the pin as an output
  pinMode(FlushSolenoid, OUTPUT);  //Sets the pin as an output
  pinMode(lowEye, INPUT);
  pinMode(highEye, INPUT);
}

void loop() {
  digitalRead(lowEye);
  digitalRead(highEye);
  digitalRead(ATOsolenoid);
  bool waterIsLow = digitalRead(lowEye) == LOW;
  bool waterIsHigh = digitalRead(highEye) == HIGH;
  if (waterIsLow) {
    digitalWrite(ATOsolenoid, HIGH);
  }
  if (waterIsHigh) {
    digitalWrite(ATOsolenoid, LOW);
  }

  //Flush Solenoid
  unsigned long lastExecutedMillis_1 = 0;  // vairable to save the last executed time for code block 1
  unsigned long lastExecutedMillis_2 = 0;  // vairable to save the last executed time for code block 2
  unsigned long currentMillis = millis();
  if (digitalRead(ATOsolenoid == HIGH)) {
        digitalWrite(FlushSolenoid, HIGH);
      } 
  if (currentMillis - lastExecutedMillis_1 >= EXE_INTERVAL_1) {
    lastExecutedMillis_1 = currentMillis;  // save the last executed time
    digitalWrite(FlushSolenoid, HIGH);
  }
  if (currentMillis - lastExecutedMillis_2 >= EXE_INTERVAL_2) {
    lastExecutedMillis_2 = currentMillis;  // save the last executed time
    digitalWrite(FlushSolenoid, LOW);
  }
}

Your diagram perfectly represents the ideal outcome of this - I will attempt @gcjr's switch/case here shortly and report back. I can't thank you enough for all of the help on this!

[alto777]

@ato_arduino I can't test this from under the umbrella, but this is what might work. It is @gcjr's Flush timer machine, conditioned upon the ATOsolenoid:

// start or continue hourly 20 second flushing

  if (digitalRead(ATOsolenoid) == HIGH) {


    switch (state) {
    case Idle:
        if (msec - msec0 >= MsecIdle)    {
            msec0 = msec;
            state = Flush;
            digitalWrite(FlushSolenoid, HIGH);  //Switch Solenoid ON
        }
        break;
    case Flush:
        if (msec - msec0 >= MsecFlush )  {
            msec0 = msec;
            state = Idle;
            digitalWrite(FlushSolenoid, LOW);   //Switch Solenoid OFF
        }
        break;
    }


  }
  else {  // ATOsolenoid is LOW here, amIRight?

// kill the flush and reset waiting for ATOsolenoid pin to go HIGH again
    state = Idle;
    digitalWrite(FlushSolenoid, LOW);   //Switch Solenoid OFF

  }

BTW I've been noticing these lines of code:

  digitalRead(lowEye);
  digitalRead(highEye);
  digitalRead(ATOsolenoid);

Totally correct code that does… nothing. They read a pin, and then fail to do anything with the value that got read. It may well be the compiler just tosses them overboard.

HTH

OK, I can already see that the "kill and reset" logic may be flawed, I'll see if anyone has fixed it before I get to the lab.

a7

[alto777]

OK we managed but still cannot test.

This places the check on ATOsolenoid into the switch/case logic:

// start or continue hourly 20 second flushing
    switch (state) {
    case Idle :  // start flsushing
      if (digitalRead(ATOsolenoid) == HIGH) {
        state = Flush;
        msec0 = msec;
        digitalWrite(FlushSolenoid, HIGH);  //Switch Solenoid ON  
      }
      break;

    case Flush :  // flush for 20 seconds 
      if (msec - msec0 >= MsecFlush)    {
        msec0 = msec;
        state = Waiting;
        digitalWrite(FlushSolenoid, LOW);  //Switch Solenoid ON
      }
      break;
        
    case Waiting :  // wait an hour, or reset the flushing machine if ATOsolenoid says
      if (msec - msec0 >= MsecIdle)  {
        msec0 = msec;
        state = Flush;
        digitalWrite(FlushSolenoid, LOW);   //Switch Solenoid OFF (time to)
      }
      else if (digitalRead(ATOsolenoid) == LOW) {
        state = Idle;
        digitalWrite(FlushSolenoid, LOW);   //Switch Solenoid OFF (ATOsolenoid dropped)
      }  
      break;
    }
  }

We added a state, just add it to the enum that holds the other states. So it is Idle, Flushing or Waiting out an hour for the next flush.

enum {Idle, Flush, Waiting};
int  state = Idle;

Which now I see that second machine is like @noiasca's, except it uses the solenoid state rather than the sensors for deciding to, or not to, flush and wait repeatedly.

a7

[alto777]

OK there were enough typos and thinkos that testing turned up that I wanted to publish this wokwi simulation.

The water level is simulated by the slide fader.

Dial up the level beyond HIGH and see the valve turn off and the periodic flushing stop.

Dial down the level below LOW and see the valse turn on and the periodic flushing start.

Play with it here:

UA "until" loop

Code hiding here:

// https://wokwi.com/projects/390745796014539777
// https://forum.arduino.cc/t/help-with-until-loop/1228025

int ATOsolenoid = 3;    //This is the output pin on the Arduino we are using
int FlushSolenoid = 4;  //This is the output pin on the Arduino we are using

int lowEye = 7;
int highEye = 8;

#define EXE_INTERVAL_1 5000
#define EXE_INTERVAL_2 10000

void setup() {
  Serial.begin(9600);

  pinMode(ATOsolenoid, OUTPUT);    //Sets the pin as an output
  pinMode(FlushSolenoid, OUTPUT);  //Sets the pin as an output
  pinMode(lowEye, INPUT);
  pinMode(highEye, INPUT);
}

unsigned long now;
enum notAnonymous {Idle, Flush, Waiting};
int state = Idle;

const unsigned int flushForTime = 777;    // flush on duration in milliseconds
const unsigned int pauseForTime = 2222;   // wait between flushes

void loop() {

  now = millis();   // read the time for use in this loop iteration

//  bool waterIsLow = digitalRead(lowEye) == LOW;
//  bool waterIsHigh = digitalRead(highEye) == HIGH;
// proxy water level and sensors instead
  bool waterIsLow = analogRead(A0) < 128;
  bool waterIsHigh = analogRead(A0) > 896;

// adjust valse or pump or whatever it is
  if (waterIsLow) {
    digitalWrite(ATOsolenoid, HIGH);
  }
  if (waterIsHigh) {
    digitalWrite(ATOsolenoid, LOW);
  }

// start or continue hourly 20 second flushing

  static unsigned long flushTimer;

  switch (state) {
  case Idle :  // start flushing
    if (digitalRead(ATOsolenoid) == HIGH) {
      state = Flush;
      flushTimer = now;
      digitalWrite(FlushSolenoid, HIGH);  //Switch Solenoid ON  
    }
    break;

  case Flush :  // flush for 20 seconds, or reset the flushing machine
    if (now - flushTimer >= flushForTime) {
      flushTimer = now;
      state = Waiting;
      digitalWrite(FlushSolenoid, LOW);  //Switch Solenoid OFF
    }
    else if (digitalRead(ATOsolenoid) == LOW) {
      digitalWrite(FlushSolenoid, LOW);  //Switch Solenoid OFF 
      state = Idle;     
    }
    break;
      
  case Waiting :  // wait an hour, or reset the flushing machine
    if (now - flushTimer >= pauseForTime) {
      flushTimer = now;
      state = Flush;
      digitalWrite(FlushSolenoid, HIGH);   //Switch Solenoid ON (time to)
    }
    else if (digitalRead(ATOsolenoid) == LOW) {
      state = Idle;
      digitalWrite(FlushSolenoid, LOW);   //Switch Solenoid OFF (ATOsolenoid dropped)
    }  
    break;
  }
}

This will interrupt a 20 second flushing in midstream, so to speak. All such details can be handled in the case logic.

HTH

a7

[ato_arduino]

Sorry the followup took me so long - I ended up re-wiring everything to a little more robust configuration. The code and simulation you provided were perfect, I cannot thank you enough!! You helped me solve this and I learned a lot through your examples. Thank you again @alto777!

[StefanL38]

@noiasca

"Blink without Delay" is a good start. The only add on you need is to differentiate the on time from the off time. 20 seconds vs. 60*60-20 seconds.

no it is not!

as you could have read already here:

@ato_arduino

I have read through the blink without delay tutorial but am having a tough time adapting it to this situation....

What I will continue to post is:
as long as the Blink without delay-example does not explicitly point on the very different nature of non-blocking timing

a lot of people will think about it that BWD is some kind of "delay()-replacement. And this causes extra-confusing about how it works.

[alto777]

@ato_arduino

Sorry the followup took me so long

NP. It is not unusual for someone to just disappear with the solution… leaving us to wonder if it even haolened at all.

When you get a chance, post the code you ended up with that was the result of your efforts and our help.

a7

[ato_arduino]

Here is the final code being used:

int ATOsolenoid = 3;    //This is the output pin on the Arduino we are using
int FlushSolenoid = 4;  //This is the output pin on the Arduino we are using

int lowEye = 7;
int highEye = 8;

void setup() {
  Serial.begin(9600);

  pinMode(ATOsolenoid, OUTPUT);    //Sets the pin as an output
  pinMode(FlushSolenoid, OUTPUT);  //Sets the pin as an output
  pinMode(lowEye, INPUT);
  pinMode(highEye, INPUT);
}

unsigned long now;
enum notAnonymous { Idle,
                    Flush,
                    Waiting };
int state = Idle;

const unsigned int flushForTime = 20000;   // flush for 20 seconds
const unsigned int pauseForTime = 3600000;  // wait 1 hr between flushes

void loop() {

  now = millis();  // read the time for use in this loop iteration

  bool waterIsLow = digitalRead(lowEye) == LOW;
  bool waterIsHigh = digitalRead(highEye) == HIGH;

  // adjust water level with ATOsolenoid
  if (waterIsLow) {
    digitalWrite(ATOsolenoid, HIGH);
  }
  if (waterIsHigh) {
    digitalWrite(ATOsolenoid, LOW);
  }

  // start or continue hourly 20 second flushing

  static unsigned long flushTimer;

  switch (state) {
    case Idle:  // start flushing
      if (digitalRead(ATOsolenoid) == HIGH) {
        state = Flush;
        flushTimer = now;
        digitalWrite(FlushSolenoid, HIGH);  //Switch Solenoid ON
      }
      break;

    case Flush:  // flush for 20 seconds, or reset the flushing machine
      if (now - flushTimer >= flushForTime) {
        flushTimer = now;
        state = Waiting;
        digitalWrite(FlushSolenoid, LOW);  //Switch Solenoid OFF
      } else if (digitalRead(ATOsolenoid) == LOW) {
        digitalWrite(FlushSolenoid, LOW);  //Switch Solenoid OFF
        state = Idle;
      }
      break;

    case Waiting:  // wait an hour, or reset the flushing machine
      if (now - flushTimer >= pauseForTime) {
        flushTimer = now;
        state = Flush;
        digitalWrite(FlushSolenoid, HIGH);  //Switch Solenoid ON (time to)
      } else if (digitalRead(ATOsolenoid) == LOW) {
        state = Idle;
        digitalWrite(FlushSolenoid, LOW);  //Switch Solenoid OFF (ATOsolenoid dropped)
      }
      break;
  }
}

[alto777]

@ato_arduino

the final code

In my world, final is relative, more accurately I don't seem to ever be entirely happy, or my empty life is so boring I keep thinking of features to add.

Here's a challenge for "learning time"…

Although we can see that the flushing is happening, and perhaps understand it will happen again because we see the valve solenoid, it would be nice (and impressive) to add some LEDs for a little status panel.

Use your imagination, but I'd add one LED that indicates that a 20 second flush is in progress, and another that indicates that we are in the one hours waiting period.

Indicate phase one: just LEDs that are on during those times. Too easy.

Indicate phase two: make the appropriate LED blink at a nice rate to show action is happening.

Or not. I never set my heart on it, but today is a perfect beach day and I would bet she who must not be kept waiting will text soon that she is rolling towards me and off we shall go.

a7

[ato_arduino]

@alto777

Hey Alto, hope you are doing well and still willing to lend me a hand re-visiting this project. I finally got a chance to implement everything and found a flaw in the programming I can't seem to pin down. I used the simulation you provided earlier to confirm this - when I set the flush time to 20s and the off time for 2 minutes, the flush turns back on at ~54 seconds and does not wait for the full 2 minutes. No matter what I set this flush off time to it still turns the flush solenoid on every ~54 seconds. I hope it shows what I am describing in the updated simulation below:

untilWhile Copy - Wokwi ESP32, STM32, Arduino Simulator

[alto777]

@ato_arduino

still willing to lend me a hand re-visiting this project

Of course.

@ato_arduino

[I] found a flaw in the programming I can't seem to pin down.

Gack! Well today your lucky day.

I bet it isn't ~54 seconds, but precisely 54.464 seconds.

Please make a tiny change to your simulation, and to your real code if it has the same problem I will describe.

const unsigned long flushForTime = 20000;    // flush on duration in milliseconds
const unsigned long pauseForTime = 120000;   // wait between flushes

Any variables that are involved with timing using the various millis() mechanisms should be unsigned long unless you really know what you are doing.

@alto777 plays it fast and loose and is sloppy.

Since the times I used for the durations were unsigned int, a type with but 16 bits, when you ask for 120000 it doesn't fit in that variable, and the part that does fit is 120000 modulo 65536, or just the bottom 16 bits of 120000. 54,464. About 54 seconds.

Had I increased those times, or taken into account the known fact that you were going to, I would probably have noticed and made at least the 120000 an unsigned long.

I can't test it: I am with two feet out the door to get to the Eclipse on time (airport, actually!), but I can practically guarantee this will fix you up.

Appy polly loggies for all the minutes it took you testing that, I know life is short.

I will flog myself with a damp trout as required by the internet to atone.

And I may just start taking the good advice - just use unsigned long variables for timing, even when you know you don't have to. It's a cheap price to pay.

a7

[ato_arduino]

@alto777

That worked!! Thankfully that was a very simple change - again I cannot thank you enough for your help on all of this. Enjoy your eclipse viewing, and safest of travels! Your "mistake" is more than forgivable in my eyes, this wouldn't have ever made it past a concept if it wasn't for your help!