Float switch with overide button programming issues

Hi All,

i am making float switch controlled machine for my roll fleece filter.
This will have

1/ A DC motor
2/ Button switch
3/ Float switch
4/ Alarm
5/ Possibly a reset switch (i will decide later)

The basic function will be as follows:

1/ While both button and float switches are off = DC motor is OFF
2/ If the button switch is on and float switch is off = DC motor is ON (overide status)
3/ If the button switch is on and the float switch is on = DC motor is ON (overide status)
4/ If the button switch is off and the float switch is on for < 10secs = DC motor is ON
5/ If the button switch is off and the float switch is on for > 10secs = DC motor is OFF and Alarm is ON

i have written the code and and the issue i have is that when the button switch is off and float switch is on for >10 secs it does not switch to the Alarm. It stays stuck on the On mode.

Any help anyone please

//PIN ASSIGNMENTS//
const int ClariSea = 2;         //ClariSea filter roll
const int LedGreen = 3;       //GreenLed pin
const int LedRed = 4;          //RedLed pin
const int AlarmPin = 5;          //Alarm pin
const int button1 = A1;           //Overide button
const int FloatSwitch = A2;   //Clarisea float switch
const int Reset = A3;             //Reset button

//TIME CONSTANTS & VARIABLES//
unsigned long TimeOn = 10000UL;    //10 seconds
static unsigned long ts;

void setup()
{
  pinMode(ClariSea, OUTPUT);
  pinMode(LedGreen, OUTPUT);
  pinMode(LedRed, OUTPUT);
  pinMode(AlarmPin, OUTPUT);
  pinMode(button1, INPUT);
  pinMode(FloatSwitch, INPUT);
  pinMode(Reset, INPUT);
  digitalWrite(ClariSea, LOW);
  digitalWrite(LedGreen, LOW);
  digitalWrite(LedRed, LOW);
  digitalWrite(AlarmPin, HIGH);    //Initial bleep when first turned on.
  delay(500);
  digitalWrite(AlarmPin, LOW);
  Off();
}

void Off()
{
  analogWrite(ClariSea, LOW);
  analogWrite(LedGreen, LOW);
  analogWrite(LedRed, LOW);
  analogWrite(AlarmPin, LOW);
}

void On()
{
  digitalWrite(ClariSea, HIGH);
  digitalWrite(LedGreen, HIGH);
  digitalWrite(LedRed, LOW);
  digitalWrite(AlarmPin, LOW);
}

void Alarm()
{
  digitalWrite(ClariSea, LOW);
  digitalWrite(LedGreen, LOW);
  digitalWrite(LedRed, HIGH);
  digitalWrite(AlarmPin, HIGH);
}

void loop()
{
  ts = millis();

  while(analogRead(button1) == LOW && analogRead(FloatSwitch) == LOW)      //Off status
  {
    Off();
  }
  while(analogRead(button1) == HIGH && analogRead(FloatSwitch) == LOW)     //Overide status
  {
    On();
  }
  while(digitalRead(button1) == HIGH && digitalRead(FloatSwitch) == HIGH)        //Overide status
  {
   On();
  }
  while((millis() - ts) <= TimeOn && analogRead(button1) == LOW && analogRead(FloatSwitch) == HIGH)      //On status
  {
    On();
  }
    while((millis() - ts) >= TimeOn && analogRead(button1) == LOW && analogRead(FloatSwitch) == HIGH)      //Alarm status
  {
    Alarm();
  }
}

I'm just going though the code as I see it. So the real problem might follow later on.

const int ClariSea = 2;         //ClariSea filter roll
const int LedGreen = 3;       //GreenLed pin
const int LedRed = 4;          //RedLed pin
const int AlarmPin = 5;          //Alarm pin

Good job using const! But why are the int's? Byte will do. Always pick the smallest variable that suits your need.

And why does the AlarmPin has a logical name (aka, pin in the name) and the rest does not?

ClariSea is the DC motor?

const int button1 = A1;           //Overide button

Where is button2? Aka, why isn't it called OverridePin? (And written with a capital to match the style).

unsigned long TimeOn = 10000UL;    //10 seconds

No need for the UL. And because it's not going to change make it const. You already wrote it with a capital to match that :wink:

static unsigned long ts;

That must be the most terrible variable name of this sketch. (Whish I could say "of today" but I can't...) And why is it static?

 pinMode(button1, INPUT);
  pinMode(FloatSwitch, INPUT);
  pinMode(Reset, INPUT);

So I assume external pull down resistors? Any reason why not to use the internal pull ups and save resistors?

setup(){
  //...
  Off();
}

That is very very double. You first set all outputs low just to call off() again...

while(analogRead(button1) == LOW && analogRead(FloatSwitch) == LOW)

Don't while, use if()! The loop is already looping.

ts = millis();
//...
while((millis() - ts) >= TimeOn && analogRead(button1) == LOW && analogRead(FloatSwitch) == HIGH)      //Alarm status

So ts and millis() probably are not very different then...

Your problem is in the last two. And you need to save the time the moment you switch the motor from off to on (and only that moment).

What is the problem in the last two that you mention. Also yes i will be using pull down resistors.
Why would using if instead of while be better. I thought by using the while statement the loop stops until that particular statement is no longer true and if keeps looping round?

7 (counter) questions, one answer is a pretty bad score to get help ::slight_smile:

Also yes i will be using pull down resistors.

septillion:
Any reason why not to use the internal pull ups and save resistors?

I thought by using the while statement the loop stops until that particular statement is no longer true and if keeps looping round?

Yes, and that's the problem. You never ever want that :wink: Why do you think loop() is called loop() :wink: Let it loop! In this case, you have no real control over when ts is assigned with millis().

PS What happened with you username?

Ok I tried it with the if statement but does not work gets stuck in the ON position.
Don't know what happened to my user name!!!??

If you made changes to you code, post it again :slight_smile:

This is the code with if statements which does not work. it gets stuck on the ON mode. It should after 10 seconds go onto the ALARM mode

//PIN ASSIGNMENTS//
const int ClariSea = 2;         //ClariSea filter roll
const int LedGreen = 3;       //GreenLed pin
const int LedRed = 4;          //RedLed pin
const int AlarmPin = 5;          //Alarm pin
const int button1 = 12;           //Overide button
const int FloatSwitch = 11;   //Clarisea float switch
const int Reset = 10;             //Reset button

//TIME CONSTANTS & VARIABLES//
const long TimeOn = 10000UL;    //10 seconds
unsigned long Pts = 0;

void setup()
{
  pinMode(ClariSea, OUTPUT);
  pinMode(LedGreen, OUTPUT);
  pinMode(LedRed, OUTPUT);
  pinMode(AlarmPin, OUTPUT);
  pinMode(button1, INPUT);
  pinMode(FloatSwitch, INPUT);
  pinMode(Reset, INPUT);
  digitalWrite(ClariSea, LOW);
  digitalWrite(LedGreen, LOW);
  digitalWrite(LedRed, LOW);
  digitalWrite(AlarmPin, HIGH);    //Initial bleep when first turned on.
  delay(500);
  digitalWrite(AlarmPin, LOW);
}

void Off()
{
  digitalWrite(ClariSea, LOW);
  digitalWrite(LedGreen, LOW);
  digitalWrite(LedRed, LOW);
  digitalWrite(AlarmPin, LOW);
}

void On()
{
  digitalWrite(ClariSea, HIGH);
  digitalWrite(LedGreen, HIGH);
  digitalWrite(LedRed, LOW);
  digitalWrite(AlarmPin, LOW);
}

void Alarm()
{
  digitalWrite(ClariSea, LOW);
  digitalWrite(LedGreen, LOW);
  digitalWrite(LedRed, HIGH);
  digitalWrite(AlarmPin, HIGH);
}

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

  if(digitalRead(button1) == LOW && digitalRead(FloatSwitch) == LOW)      //Off status
  {
    Off();
  }
  else if(digitalRead(button1) == HIGH && digitalRead(FloatSwitch) == LOW || digitalRead(button1) == HIGH && digitalRead(FloatSwitch) == HIGH)       //Overide status
  {
    On();
  }
  else if(Cts - Pts <= TimeOn && digitalRead(button1) == LOW && digitalRead(FloatSwitch) == HIGH)      //On status
  {
    Pts = Cts;
    On();
  }
  else if(Cts - Pts >= TimeOn && digitalRead(button1) == LOW && digitalRead(FloatSwitch) == HIGH)      //Alarm status
 {
   Pts = Cts;
   Alarm();
  }
}

Wow, you even made it worst... Cts and Pts, really?

But yeah, now we are at my last point in reply #1. You need to remember when you switched the pump from off to on. Not all the time the pump is on.

And if you want more help, please start to address the other points as well. The code is just so damn hard to read this way.

Well i re-looked at the whole thing did a bit of reading and decided to take a different approach. I have written a state machine which works perfectly. everything is doing what it is supposed to be doing. I would like to thank you for your advice especially in relation to when i start to count the millis ie. when motor is on.
With regards to my code abbreviations i am a bit dislexit and therefore short letters are more memorable to me.
Attached is my final code for info.
[code//STATE MACJHINE STATES//
const int OFF = 0;                         //Idle state(S)
const int SWITCH1 = 1;                //Read from both switches(T)
const int OVERIDEON = 2;          //Overideon state(S)
const int SWAP1 = 3;                  //Read overide switch(T)
const int ROLLON = 5;                 //Clarisea rollon(S)
const int SWITCH2 = 6;               //Determine time on(T)
const int ALARM = 7;                   //Alarm(S)
const int RESET = 8;                   //Reset switch(S)


//PIN ASSIGNMENTS//
const int ClariSea = 2;         //ClariSea filter roll
const int LedGreen = 3;       //GreenLed pin
const int LedRed = 4;          //RedLed pin
const int AlarmPin = 5;          //Alarm pin
const int button1 = 11;           //Overide button
const int FloatSwitch = 12;   //Clarisea float switch
const int Reset = 10;             //Reset button

//TIME CONSTANTS & VARIABLES//
unsigned long TimeOn = 10000UL;    //10 seconds

void setup()
{
  pinMode(ClariSea, OUTPUT);
  pinMode(LedGreen, OUTPUT);
  pinMode(LedRed, OUTPUT);
  pinMode(AlarmPin, OUTPUT);
  pinMode(button1, INPUT);
  pinMode(FloatSwitch, INPUT);
  pinMode(Reset, INPUT);
  digitalWrite(AlarmPin, HIGH);    //Initial bleep when first turned on.
  delay(500);
  digitalWrite(AlarmPin, LOW);
  //Off();
}

void Off()
{
  digitalWrite(ClariSea, LOW);
  digitalWrite(LedGreen, LOW);
  digitalWrite(LedRed, LOW);
  digitalWrite(AlarmPin, LOW);
}

void On()
{
  digitalWrite(ClariSea, HIGH);
  digitalWrite(LedGreen, HIGH);
  digitalWrite(LedRed, LOW);
  digitalWrite(AlarmPin, LOW);
}

void Alarm()
{
  digitalWrite(ClariSea, LOW);
  digitalWrite(LedGreen, LOW);
  digitalWrite(LedRed, HIGH);
  digitalWrite(AlarmPin, HIGH);
}

void loop()
{
  static int state = OFF;
  static unsigned long ts;    //Time seconds
  switch(state)
  {
    case OFF:               //Idle state if no switches are activated
    Off();
    state = SWITCH1;
    break;
   
    case SWITCH1:
    if(digitalRead(button1) == HIGH && digitalRead(FloatSwitch) == LOW)
    {
      delay(10);
      state = OVERIDEON;
    }
    else if(digitalRead(button1) == HIGH && digitalRead(FloatSwitch) == HIGH)
    {
      delay(10);
      state = OVERIDEON;
    }
    else if(digitalRead(button1) == LOW && digitalRead(FloatSwitch) == HIGH)
    {
      delay(10);
      state = ROLLON;
    }
    else
    {
      //ts=millis();
      state = OFF;
    }
    break;
    
    case OVERIDEON:
    On();
    //ts=millis();
    state = SWAP1;
    break;
    
    case SWAP1:
    if(digitalRead(button1) == LOW)
    {
      state = OFF;
    }
    break;
    
    case ROLLON:
    On();
    ts=millis();
    state = SWITCH2;
    break;
    
    case SWITCH2:
    if((millis() - ts) >= TimeOn && digitalRead(FloatSwitch) == HIGH)
    {
     state = ALARM;
    }
    else if(digitalRead(FloatSwitch) == LOW)
    {
     state = OFF;
    }
    break;
    
    case ALARM:
    Alarm();
    state = RESET;
    break;
    
    case RESET:
    if(digitalRead(Reset) == HIGH)
    {
      delay(10);
      Off();
      state = OFF;
    }
    break;
    
    default:
    state = OFF;
    break;
  }
}]