Please review my sketch, think I got this right but not sure

Would appreciate any feedback on my sketch. I am multi-tasking today smoking some Boston Butts, watching olympic golf and writing a sketch. I am still waiting on some parts so cant test yet. I do a lot of coding in VBA which is completely different from the loop environment in Arduino. So not positive I got this correct.

Most of this description is repeated in the comments in the code. However, to sum it up, this is will control my reverse osmosis de-ionizing resin water purification unit. There will be three solenoids in the system, one on RODI input line, one on the output line and one on the flow restrictor bypass line to auto flush the membrane. The output of the RODI unit empties into a barrel with two float switches. The lower switch will initiate the fill mode and the higher switch terminate the fill mode.

When not filling all solenoids are closed (they are NC), when fill mode initiates (caused by the lower float closing) the input and output solenoids are opened and remain open until the upper float closes. On fill mode initiation the bypass solenoid will open for 20 seconds and do so each hour while in fill mode.

The solenoids will have a separate power source and be switched on by a jbtek 4 channel relay. Three relays for the solenoids and the fourth will run an outlet for a small pump that will be added later.

It did compile successfully but I am just not sure of the functionality.

Thanks again for taking a look. I appreciate any feedback!

Edit: forgot to mention I am using a mega, overkill but what I have on hand.

/*Reverse Osmosis De-Ionizing Water System Controller
 * 
 * Water container with HighLevel (shutoff) float valve
 * and LowLevel (initiate) float valve.
 * Three solenoids on RODI unit all 4pack relay driven 12V:
 * BypassSol (bypasses flow restricter for flush)
 * OutputSol (opens/closes output line)
 * InputSol (opens/closes input water line)
 * Outlet: fourth relay controls 120V Outlet
 * 
 * Theory of operation:
 * Default state FillMode off (all solenoids closed)
 * If LowLevl closes then FillMode on
 * If FillMode on then InputSol and OutputSol Open
 * When FillMode on open BypassSol for 20 seconds
 * While FillMode on open BypassSol for 20 secondes every hour
 * If HighLevel closes FillMode off
 * 
 */

const int BypassSol = 6;
const int OutputSol = 7;
const int InputSol = 8;
const int Outlet = 9;
const int HighLevel = 5;
const int LowLevel = 3;

boolean FillMode = false;

unsigned long HourTimer = 0;
unsigned long FlushTime = 0;
const long HourInterval = 3600000;
const long FlushInterval = 20000;

void setup() {
  // Initiate serial coms
Serial.begin(9600);
pinMode(BypassSol,OUTPUT);
pinMode(OutputSol,OUTPUT);
pinMode(InputSol,OUTPUT);
pinMode(Outlet,OUTPUT);
pinMode(HighLevel,INPUT_PULLUP);
pinMode(LowLevel,INPUT_PULLUP);

}

void loop() {
  unsigned long currentMillis = millis();
  // 
  if (FillMode == false) {//if not in fillmode
    if (LowLevel == LOW) {//if lowlevel float closes turn fillmode on
      FillMode = !FillMode;
      digitalWrite(BypassSol, HIGH);//turn bypass on for 20 secs to flush
      delay (20000);   //20 sec delay no issue for overflow
      digitalWrite(BypassSol, LOW);
      HourTimer = currentMillis;
    }else{
      //while not in fillmode all solenoids closed.
      digitalWrite(OutputSol, LOW);
      digitalWrite(BypassSol, LOW);
      digitalWrite(InputSol, LOW);
      digitalWrite(Outlet, LOW);
    }
  }else{//fillmode on
    //while fill mode is on the following three solenoids are always open
    digitalWrite(OutputSol, HIGH);
    digitalWrite(InputSol, HIGH);
    digitalWrite(Outlet, HIGH);
    if (HighLevel == LOW){//if high level float closes turn fill mode off
      FillMode = !FillMode;
    }else{
      if (currentMillis - HourTimer >= HourInterval){
        digitalWrite(BypassSol, HIGH);//turn bypass on for 20 secs to flush
        delay (20000);   //20 sec delay no issue for overflow
        digitalWrite(BypassSol, LOW);
        HourTimer = currentMillis;
      }
    }  
  }
}
if (LowLevel == LOW) {//if lowlevel float closes turn fillmode on

Maybe you need to actually read the pin? :-if(digitalRead(LowLevel)==LOW) {//if lowlevel float closes turn fillmode on
The same elsewhere in your code. (HighLevel)

Ooops! Thanks for that!

Got that fixed, any opinions on whether or not this will functionally do what I intend?

/*Reverse Osmosis De-Ionizing Water System Controller
 * 
 * Water container with HighLevel (shutoff) float valve
 * and LowLevel (initiate) float valve.
 * Three solenoids on RODI unit all 4pack relay driven 12V:
 * BypassSol (bypasses flow restricter for flush)
 * OutputSol (opens/closes output line)
 * InputSol (opens/closes input water line)
 * Outlet: fourth relay controls 120V Outlet
 * 
 * Theory of operation:
 * Default state FillMode off (all solenoids closed)
 * If LowLevl closes then FillMode on
 * If FillMode on then InputSol and OutputSol Open
 * When FillMode on open BypassSol for 20 seconds
 * While FillMode on open BypassSol for 20 secondes every hour
 * If HighLevel closes FillMode off
 * 
 */

const int BypassSol = 6;
const int OutputSol = 7;
const int InputSol = 8;
const int Outlet = 9;
const int HighLevel = 5;
const int LowLevel = 3;

boolean FillMode = false;

unsigned long HourTimer = 0;
unsigned long FlushTime = 0;
const long HourInterval = 3600000;
const long FlushInterval = 20000;

void setup() {
  // Initiate serial coms
Serial.begin(9600);
pinMode(BypassSol,OUTPUT);
pinMode(OutputSol,OUTPUT);
pinMode(InputSol,OUTPUT);
pinMode(Outlet,OUTPUT);
pinMode(HighLevel,INPUT_PULLUP);
pinMode(LowLevel,INPUT_PULLUP);

}

void loop() {
  unsigned long currentMillis = millis();
  // 
  if (FillMode == false) {//if not in fillmode
    if (digitalRead(LowLevel) == LOW) {//if lowlevel float closes turn fillmode on
      FillMode = !FillMode;
      digitalWrite(BypassSol, HIGH);//turn bypass on for 20 secs to flush
      delay (20000);   //20 sec delay no issue for overflow
      digitalWrite(BypassSol, LOW);
      HourTimer = currentMillis;
    }else{
      //while not in fillmode all solenoids closed.
      digitalWrite(OutputSol, LOW);
      digitalWrite(BypassSol, LOW);
      digitalWrite(InputSol, LOW);
      digitalWrite(Outlet, LOW);
    }
  }else{//fillmode on
    //while fill mode is on the following three solenoids are always open
    digitalWrite(OutputSol, HIGH);
    digitalWrite(InputSol, HIGH);
    digitalWrite(Outlet, HIGH);
    if (digitalRead(HighLevel) == LOW){//if high level float closes turn fill mode off
      FillMode = !FillMode;
    }else{
      if (currentMillis - HourTimer >= HourInterval){
        digitalWrite(BypassSol, HIGH);//turn bypass on for 20 secs to flush
        delay (20000);   //20 sec delay no issue for overflow
        digitalWrite(BypassSol, LOW);
        HourTimer = currentMillis;
      }
    }  
  }
}

It looks like it'll do what you want.

You could get rid of that 'Serial.begin(9600)' though, since you don't use serial comms. And it would be more readable if you just set 'FillMode' to either 'true' or 'false', rather than toggling.

While it doesn't affect actual operation, in C++, it's more conventional to start your variable names with a lower case character, and keep upper case first characters for classes. (Known as 'camelCase'.)

Disclaimer: It's very late - I'm pretty tired and might have missed something, (wouldn't be the first time). The best thing is the test of fire - connect it up, hit the power and see what happens. :slight_smile:

Let us know how it goes.

Steve,

Thanks for your reply. I appreciate the feedback and advice! The last of my parts arrive tomorrow, hopefully I will be able to get it built over the weekend and tested. I put the serial in as I was debating on putting a few print lines in for testing. I don't think I will now though so I will delete it. When I code in VBA I normally preface with lowercase the variable type and then capitalize the first letter of each word of the defining name. I.e. for an integer intMyNumber. As I was only dealing with integers here I just skipped that part out of pure laziness. And also part because I wrote most of it with pen and paper before I got the drive to get up and get the computer... Thanks again for your help and I will be sure to report back with results after I get it put together.