Reverse Osmosis State Machine

Following on from a previous post I have now completed my Reverse Osmosis Controller. 90% of it works fine except for the last part which is the override button.

Sketch Brief (using the stage abbreviations in my sketch)

1/ OFF - idle position directed to SWAP3.
2/ SWAP3 - if FlagSW(switch) is HIGH proceed to FLAG else proceed to FSWT.
3/FSWT - FLUSH1 - FLUSH1TIME - FLUSH1END - SWAP1 - FLUSH2 - FLUSH2TIME - FLUSH2END - SWAP2 - RUN - RUNEND - this process is a FSWT(float switch) activates a booster pump and 2 types of flushes are carried out prior to the production of pure water then FSWT de-activates and the stae returns to OFF.
4/FLAG - if FlagSW(switch) is high, the boostePump & FROW(selenoid) are high and pure water is being produced. This state should not be influenced by the TankFSW(float switch).
5/FLAGTIMEON - if 30 minutes have passed and the FlagSW(switch) is still HIGH procced to FLAGOFF else proceed to OFF.
6/FLAGOFF - turn everything off proceed to FLAGTIMEOFF
7/FLAGTIMEOFF - if 10 minutes have passed and the FlagSW(switch) is HIGH procced to FLAG else proceed to OFF.

The part which does not seem to work as it shoud is from SWAP3 - FLAG - FLAGTIMEON - FLAGOFF - FLAGTIMEOFF - OFF.

Can someone please have a look at my code and advise me where am i going wrong please.

AQUA_PRO_RO_Controller.ino (5.95 KB)

The part which does not seem to work as it shoud is from SWAP3 - FLAG - FLAGTIMEON - FLAGOFF - FLAGTIMEOFF - OFF.

The code you posted does something. You need to explain what it actually does.
You want it to do something. You need to explain what.

"does not seem to work" is too lame to get help. You need to be much more specific.

Sorry for my vagueness,

i am using leds to test the outputs

When i turn on the TankFSW which is a float switch the BoosterPump LED and WFlush LED turn on. The WFlush LED is HIGH for 2 minutes then turns off and ROFlush LED is HIGH, after a futher 2 minutes the ROFlush LED turns off and FROW LED turns on and stay on until the float switch is turned off. This is working as it should be.

When i turn the FlagSW HIGH the BoosterPump LED and FROW LED dimly lights up but instead of going to the next stage which would be off it just sticks there. What should happen here is if i turn on the FlagSW to HIGH the BoosterPump pin and FROW pin should be HIGH for a period not longer than 30 minute at which point they should turn off for 10 minutes then back on (looping) until i turn FlagSW LOW.

How are the switches wired? Using the internal pullup resistors, and connecting one leg to ground and the other leg to the digital pin would be easier.

The comment says that FlagSW is an override switch. Why is the override switch pin called FlagSW?

When the switch pin is read, if it is HIGH, the state is set to FLAG. Now, I picture that triggering a servo with a little cocktail flag tape to the servo horn. I doubt that is what you envision happening. Therefore, FLAG appears to be a dumb name for the state.

    case FLAG:
    digitalWrite(BoosterPump, HIGH);
    digitalWrite(ROFlush, LOW);
    digitalWrite(ROFlush, LOW);
    digitalWrite(FROW, HIGH);

Why do you need to diddle with the ROFlush pin twice?

but instead of going to the next stage which would be off it just sticks there.

    case FLAG:
    digitalWrite(BoosterPump, HIGH);
    digitalWrite(ROFlush, LOW);
    digitalWrite(ROFlush, LOW);
    digitalWrite(FROW, HIGH);
    ts=millis();
    state = FLAGTIMEON;
    break;

Doesn't look to me like the next state is off. Looks to me like the next state is FLAGTIMEON.

On the other hand, setting the state to FLAGTIMEON doesn't make sense, since next time through loop(), a few nanoseconds later, 5 seconds will not have passed, so you set state to OFF, and then, a few nanoseconds later, after turning the stuff off that you just turned on, you set state to SWAP3, where you again test the float switch and change state again.

I think your state machine is changing state far too often.

Ok cleaned sketch up a bit and renamed things so they make a bit more sense but still not working as it should, in fact i think the state machine is not going beyond SWAP3 and i cant figure out why. Any solutions?

AQUA_PRO_RO_Controller.ino (5.96 KB)

in fact i think the state machine is not going beyond SWAP3

I'm not sure it is even getting into SWAP3 because the only exit from RUNEND seems to be OFF. If I follow correctly, SWAP2>RUN>RUNEND.

 case RUNEND:
    if(digitalRead(TankFSW) == LOW)    //Float switch off
    {
      delay(1);
      state = OFF;
    }
    break;

where should i change then. Basically this is what i am trying to achieve

RO set up.pdf (13.1 KB)

Your diagram does not show all the states in the switch case function and the transition conditions between them. Your diagram shows RUN>OFF and that's what you are getting.

If you want to sort out the logic you will need to be very precise in writing it down for yourself and others to follow and analyse.

Ok i have taken your advise and reworked the state diagram.
Any comments?

RO States.pdf (111 KB)

Bingo got it working. Can someone please have a look at my code at lines 233-242 & 253-262,

in my old code i had and "if" statement followed by "else". I have now changed this to an "if" statement followed by "else if".
Can someone please explain why, as thought that "else" meant that if the first condition is not met then do the next function

AQUA_PRO_RO_Controller.ino (6.48 KB)

lines 253-262

case OTIMEOFF:
    if(millis() >= ts + 10*Min && digitalRead(OverSW) == HIGH)    //10 minutes off
    {
      state = OVERIDEON;
    }else{
      if(millis() >= ts + 10*Min && digitalRead(OverSW) == LOW)
      {
        state = OFF;
      }
      break;

I do not see a terminating bracket for the else block.

I think this syntax error may occur in some other states containing the else followed by an if condition as well as the two you fixed.

else{
if{
condition
}
}//should be a terminating bracket

And where would you put the break; before or after the terminating bracket?

I believe that the break statement should be after the terminating bracket.

Edit: Additional comment on your code

Your time conditionals are all written similar to this

 if(millis() >= ts + 2*Min)

This form will not work through the millis() rollover.

The need to be of the form

 if(millis() -ts >= 2*Min)

By putting the terminating bracket as you said I get a compile error saying

Break statement not within loop or switch

in my old code i had and "if" statement followed by "else". I have now changed this to an "if" statement followed by "else if".

What was the bracket arrangement when you changed to the "else if" ?

Edit: When you add the bracket, you may have to add or delete some others.

if you indent so that you can easily see what brackets match which, it becomes apparent

case OTIMEOFF:
    if(millis() >= ts + 10*Min && digitalRead(OverSW) == HIGH)    //10 minutes off
      {
      state = OVERIDEON;
      }
   else{
       if(millis() >= ts + 10*Min && digitalRead(OverSW) == LOW)
         {
        state = OFF;
       }
      break;

this shows the IF() that is in the else() has no closing brackets, the last bracket is the closing one for the else statement

Yes I see that thanks. Also been reading up and I believe it should be
'' else if (ect)'' and not ''else { if (ect)''.
Is that right?

If the bracketing is done correctly

else if 
{
}

is equivalent to

else
{  
if{}
}

Cleaned up my sketch and sorted all the culy brackets out and works perfectly. Would like to say thank you to for helping. Below is my final sketck for anybody who might want to play with it.

AQUA_PRO_RO_Controller.ino (6.79 KB)

Proietti:
Yes I see that thanks. Also been reading up and I believe it should be
'' else if (ect)'' and not ''else { if (ect)''.
Is that right?

depends on the code.

if (switch1==high){
  digitalWrite(led1,LOW);
  }
else (
      if (switch2==High){
         digitalWrite(led2,LOW);
         }
     }

it is possible to follow an else with a new and different if() statement.

what you are describing might be

if (switch1==HIGH && switch2 ==HIGH){
   serial.println("all switches are high");
   }
 else if (  switch1 == LOW && switch2  == LOW){
       serial.println("all switches are low");
         }
  else{
       serial.println("some high, some low");
        }

it starts with the if()
and ends with else{}
and if you have more else in the middle they are
else if {}

if {}
else if {}
else if {}
else {}