I'm a beginner trying to carry a project which has the following scenario:
A sump with high and low water sensors which control 2 pumps. High level sensor switches 1 pump on, after 30 secs the next pump kicks in, after 30 secs an alarm sounds and both pumps stop. If at any point the low level sensor activates, any pumps running will stop as will the alarm. The pumps alternate between which start the first time to share run time.
I'm using a uno board with push buttons to act as the sensors. Everything works from my code below except after 60 secs, the alarm operates but the pumps do not stop running.
Any ideas as to where i am going wrong?
const int lowlevel = 2;
const int highlevel = 3;
const int pump1 = 13;
const int pump2 = 12;
const int alarm = 11;
int buttonState1 = 0;
int buttonState2 = 0;
int pumpSelection = 1; // Allows the 2 pumps to be cycled each time hgih water level detected to share pump run time
unsigned long onTime1; // Allows a timer to be started once high level has been detected.
void setup () {
pinMode (lowlevel, INPUT);
pinMode (highlevel, INPUT);
pinMode (pump2, OUTPUT);
pinMode (pump1, OUTPUT);
pinMode (alarm, OUTPUT);
onTime1 = millis();
}
void loop (){
buttonState1 = digitalRead(lowlevel);
buttonState2 = digitalRead (highlevel);
if (buttonState2 == HIGH) // High water level detected
if (pumpSelection == 1) { digitalWrite (pump1, HIGH); // Pump 1 to run. pumpSelection determines which pump will run.
delay(1000);
pumpSelection = !pumpSelection;
onTime1 = millis(); // starts timer to determine if 2nd pump required to operate.
}
else
{ digitalWrite (pump2, HIGH); // Pump 2 to run
delay(1000);
pumpSelection = !pumpSelection;
onTime1 = millis();
}
if(onTime1 > 0 && millis() - onTime1 > 30000) { // If after 30 seconds the low level sensor has been detected, both pumps will become energized.
digitalWrite (pump2, HIGH);
digitalWrite (pump1, HIGH);
}
if (onTime1 > 0 && millis() - onTime1 > 60000) // If after another 30 seconds the low level sensor hasnt been detected, both pumps stop and an alarm sounds.
{ digitalWrite (alarm, HIGH);
digitalWrite (pump1, LOW);
digitalWrite (pump2, LOW);
}
if (buttonState1 == HIGH) // Both pumps to be low when low level sensor detected.
{ digitalWrite (pump1, LOW);
onTime1 = 0; // stops and resets the timer when the low level sensor has been reached.
digitalWrite (pump2, LOW);
digitalWrite (alarm, LOW); // If alarm has been been sounding, it also stops when low water level is detected.
}
}
Check out ‘state machines’.
Your project is interesting because of the alternating pumps, but that is not difficult to handle.
Also, while this is a low-frequency event driven application, it is a good time to learn about millis(), and ditch the delay() calls.
First you should declare your buttonState1, buttonState2 and pumpSelection variables as bool.
This test does not open braces: if (buttonState2 == HIGH) // High water level detectedso the µC does not know the range of the test.
And lastchancename is true, you should try state machines: if you want to use more complex conditions in the future, it will be much simpler to implement them.
That is an unusual technique for using the millis timer, not the way it is usually done. Have you considered what would happen if millis() was exactly 0, which will happen every 49 days or so.
As for the code, to begin with you left the {} off the first IF statement.
Also, in the following IF statements, when the 2nd is true, the first will always be true also.
if(onTime1 > 0 && millis() - onTime1 > 30000) { // If after 30 seconds the low level sensor has been detected, both pumps will become energized.
if (onTime1 > 0 && millis() - onTime1 > 60000) // If after another 30 seconds the low level sensor hasnt been detected, both pumps stop and an alarm sounds.
An obvious question is do you have any pull down resistors in place on your inputs or are they floating at an unknown level ?
Personally I would write the program as a more explicit state machine with the states defined in a enum and given meaningful names and the code to be executed controlled by switch/case based on the current state. You could also make the pump wear levelling processing more compact by putting the pump pin numbers in an array and using pumpSelection as the index. This would remove the need for duplicate code.
david_2018:
That is an unusual technique for using the millis timer, not the way it is usually done. Have you considered what would happen if millis() was exactly 0, which will happen every 49 days or so.
His code contains these lines at the end:
if (buttonState1 == HIGH) // Both pumps to be low when low level sensor detected.
{ digitalWrite (pump1, LOW);
onTime1 = 0; // stops and resets the timer when the low level sensor has been reached.
Thanks for your replies. I am using pull down resistors for the inputs.
As david_2018 said (Also, in the following IF statements, when the 2nd is true, the first will always be true also), i can see why the pumps aren't cutting out.
I will look in to state machines and improve the code.
Here's an example of the form a statemachine might take. This compiles but I haven't tested it.
const int lowlevel = 2;
const int highlevel = 3;
const int pump1 = 13;
const int pump2 = 12;
const int alarm = 11;
void setup ()
{
pinMode (lowlevel, INPUT); //can be unpredicable unless you have pull-down resistors on these pins!
pinMode (highlevel, INPUT);
//
pinMode (pump2, OUTPUT);
pinMode (pump1, OUTPUT);
pinMode (alarm, OUTPUT);
}//setup
#define STATE_IDLE 0
#define STATE_PUMP1 1
#define STATE_PUMPS_BOTH 2
#define STATE_PUMP_ALARM 3
#define STATE_PUMPS_OFF 4
void PumpStateMachine( void )
{
static bool
bPumpSel = false;
static byte
statePump = STATE_IDLE;
static unsigned long
timePump;
unsigned long
timeNow;
timeNow = millis();
switch( statePump )
{
case STATE_IDLE:
//waiting for the high level sensor
if( digitalRead( highlevel ) == HIGH )
{
//sensor shows high level; turn on the pump
//which one depends on state of bPumpSel
//if true, pump 1, if false, pump 2
//this is toggled in STATE_PUMPS_OFF for wear-leveling
digitalWrite( (bPumpSel)?pump1:pump2, HIGH);
//grab the time now and move to state PUMP1
timePump = timeNow;
statePump = STATE_PUMP1;
}//if
break;
case STATE_PUMP1:
//if we see the low level sensor trip, move to turn the pump(s) off
if( digitalRead( lowlevel ) == HIGH )
statePump = STATE_PUMPS_OFF;
else
{
//not yet; have we exceeded 30-sec runtime on the 1st pump?
if( (millis() - timePump) >= 30000 )
{
//yes, turn on the second pump
//notice the order of the pumps is reversed here (pump2:pump1) vs (pump1:pump2)
//above; if bPumpSel=true, pump1 is selected in STATE_IDLE and pump2
//is selected here. When bPumpSel is toggled the opposite happens
digitalWrite( (bPumpSel)?pump2:pump1, HIGH);
//grab the time and move to both-pumps-on state
timePump = timeNow;
statePump = STATE_PUMPS_BOTH;
}//if
}//else
break;
case STATE_PUMPS_BOTH:
//again, a low level moves us to the OFF state
if( digitalRead( lowlevel ) == HIGH )
statePump = STATE_PUMPS_OFF;
else
{
//have we been running in this state for 30-s or more?
if( (millis() - timePump) >= 30000 )
{
//yes; turn on the alarm, turn off the pumps and move to the alarm state
digitalWrite( alarm, HIGH );
digitalWrite( pump1, LOW );
digitalWrite( pump2, LOW );
statePump = STATE_PUMP_ALARM;
}//if
}//else
break;
case STATE_PUMP_ALARM:
//hold the alarm active until the low level sensor trips; then move to OFF
if( digitalRead( lowlevel ) == HIGH )
statePump = STATE_PUMPS_OFF;
break;
case STATE_PUMPS_OFF:
//turn off the alarm and pumps...
digitalWrite( alarm, LOW );
digitalWrite( pump1, LOW );
digitalWrite( pump2, LOW );
//and toggle the state of bPumpSel; XOR with true means if it's true now it will be false
//and if false now it will be true.
bPumpSel ^= true;
//then go and wait for the high-level trip again
statePump = STATE_IDLE;
break;
}//switch
}//PumpStateMachine
void loop ()
{
PumpStateMachine();
}//loop
