I've watched the BWD example, but I think I've bitten off a bit more than I can chew with the analog slider. The problem is obviously it's checking the slider value on each loop so constantly updating the lowPumpStart
What I'm trying to make work is if pump1 has been on 30 seconds, then pump2 comes online. However, if I push the slider below 900 then they both cut off. Obviously this was just going to be an IF Pump1 has been on for 30 seconds, set pin HIGH. Any tips?
int motor1 = 1;
int motor2 = 2;
int waterheight = A2;
int lowPumpStart = 0;
int highPumpStart = 0;
int highPumpWait = 10000;
void setup()
{
pinMode(motor1, OUTPUT);
pinMode(motor2, OUTPUT);
pinMode(waterheight, INPUT);
digitalWrite(motor1, LOW);
digitalWrite(motor2, LOW);
}
void loop()
{
unsigned long currentTime = millis();
int pumpTrigger = analogRead(waterheight);
if (pumpTrigger > 900)
{
digitalWrite(motor1, HIGH);
lowPumpStart = currentTime;
}
}
Pump1 is meant to come on when the water hits a certain height (set by the analog slider going above 900)
Then if Pump1 has been on 30 seconds, Pump2 is meant to kick in to help it.
So all I was going to do was log the time Pump1 came on, and if the state was still HIGH and 30 seconds had elapsed, turn on Pump2 (NOTE: I'm using 10s in the code currently to avoid waiting the full 30 when testing). But obviously because I'm updating that time in the loop it just spams the currentTime to the global var.
Should have mentioned I'm using UnoArduSim as well.
When pump2 comes online then both pumps stay on until waterheight is below 900? I'm trying to understand how you want this to work.
A couple of general comments:
Any variables storing millis() time should be declared as unsigned long
Analog values tend to bounce a little. I would check the value within a +/- delta so that you don't have the pumps turning on and off when the value is around 900
With respect to comparing the value to 900 you need to detect when the value moves from less than 900 to greater than 900. Otherwise you have the issue you have now where you are constantly resetting the time value. This requires that you save the last value of water height and compare it to the current value every time you go through loop().
So basically measure the water height in realtime (hence using the slider)
If it reaches 900 then start Pump1, if it doesn't fall below 900 within 30 seconds then activate Pump2 (so two pumps are active).
Then when the slider is changed to below 900, turn both pumps off again
This will show you how to use state change detection to detect the water height. I don't have time to test but hopefully you see how it works:
int motor1 = 1;
int motor2 = 2;
int waterheight = A2;
unsigned long lowPumpStart = 0;
unsigned long highPumpStart = 0;
const unsigned long highPumpWait = 10000;
const int whDelta = 5;
void setup()
{
pinMode(motor1, OUTPUT);
pinMode(motor2, OUTPUT);
pinMode(waterheight, INPUT);
digitalWrite(motor1, LOW);
digitalWrite(motor2, LOW);
}
void loop()
{
static int lastPumpTrigger = 0;
int pumpTrigger = analogRead(waterheight);
unsigned long currentTime = millis();
if (abs(lastPumpTrigger - pumpTrigger) > whDelta)
{
// Use state change detection to turn motors on and off
if (lastPumpTrigger < 900 && pumpTrigger > 900)
{
// Water height has exceeded 900 so turn pump 1 on
digitalWrite(motor1, HIGH);
lowPumpStart = currentTime;
}
else if (lastPumpTrigger > 900 && pumpTrigger < 900)
{
// Water height is less than 900 so turn both pumps off
digitalWrite(motor1, LOW);
digitalWrite(motor2, LOW);
}
}
lastPumpTrigger = pumpTrigger;
// TBD: test to see if pump 1 has been on more than 30 seconds and if so turn pump 2 on.
}
What you did will work but only because HIGH and LOW are defined as follows:
#define HIGH 0x1
#define LOW 0x0
However, it is customary to use the defined values should they be changed in the future, otherwise your code will break. It is also more readable, in my opinion, and consistent.
Using digitalWrite or digitalRead with bool or boolean variables has advantages.
If you do some research, you'll notice that digitalRead takes a byte data type as value parameter and applies the byte to bool c++ internal rules. (Anything except 0 is HIGH / true)
An int variable is error prone. Give digitalWrite(13, -256); a try and see that int is the wrong type: warning: large integer implicitly truncated to unsigned type [-Woverflow]
BTW: this warning disappears if you use an int variable int i = 256 or = -256
Whilst the chances of the definitions of true, false, HIGH, LOW, etc being changed are small I prefer to use the appropriate words as it tends to make the code easier to read and understand
Code like
if (digitalRead(pin0 == 1)
or
if (started == 1)
always makes me squirm even though I know what it means. To me the state of a digital pin is either HIGH or LOW, not 1 or zero and a bool is either true or false
The defined values exist for a reason so why not use them ?
