Sensor Voltage Timer with Relay Timing problems

Hi All

I am trying to get a soil moisture sensor to turn on and off, to prevent corrosion of my probe. (2 secs on, 1 hour off)
I am using a relay for my water pump to turn on and off based on the sensor reading, AND an unsigned long timing. (5 secs on, 1 hour off)

I do not want the pump to pump for more than a few seconds every hour, even if the moisture is low. I have had issues with sensor corrosion/electrolysis, and the entire system flooded.

So I and trying to use 2 x unsigned long timing setups. Maybe I need to shuffle some code around, or try and alternative.

Let me know if you can help me out here. I have a feeling that is could either be some small, minor error, or the thinking behind this sketch isn't any good.

The wiring:
The moisture sensor's voltage Pin is connected to Digital 11 on my Nano v.3 board
and the Moisture sensor signal cable is connected to A5.
The relay is connected to Digital 12.
and the other GND - GND, VCC-5V etc

Code is as follows:

int moisture = A5;                              //moisture sensor
int vout     = 11;                              //voltage input of moisture sensor
int relayPin = 12;                              //relay connected to water pump
unsigned long moistureReadOntime  = 2000;       //amount of time the moisture sensor is on (millis)
unsigned long moistureReadOfftime = 360000;       //amount of time the moisture sensor is off (millis)
unsigned long previousMillis1     = 0;          // will store last time moisture sensor was updated
unsigned long previousMillis2     = 0;          // will store last time relay was updated
unsigned long pumpOnTime          = 5000;       // amount of time the water pump is on (millis)
unsigned long pumpOffTime         = 360000;       // amount of time the water pump is off (millis)

int voutState = HIGH;                         // starts sensor voltage as being on
int relayState = LOW;                        // starts relay voltage as being off

void setup() {
  pinMode(moisture, INPUT);                     // sets moisture sensor as an input
  pinMode(relayPin, OUTPUT);                    // sets relay as an output
  pinMode(vout, OUTPUT);                        // sets sensor voltage as an output
  digitalWrite(vout, voutState);                // writes the current state of the sensor voltage
  digitalWrite(relayPin, relayState);           // writes the current state of the relay
}

// the loop routine runs over and over again forever:
void loop() {

  unsigned long currentMillis1 = millis(); // Stores the value of millis() in each iteration
  unsigned long currentMillis2 = millis(); // Stores the value of millis() in each iteration

  // check to see if the moisture sensor voltage is off, and if its been off for the offtime, and turns it on
  if ((voutState = LOW) && (currentMillis1 - previousMillis1) >= moistureReadOfftime)
  { voutState = HIGH;
    previousMillis1 = currentMillis1;
    digitalWrite(vout, voutState);

    // check to see if the moisture sensor voltage is on, and if its been on for the ontime, and turns it off
  }
  else if ((voutState = HIGH) && (currentMillis1 - previousMillis1) >= moistureReadOntime)
  { voutState = LOW;
    previousMillis1 = currentMillis1;
    digitalWrite(vout, voutState);
  }

  //reads the moisture sensor
  int sensorValue = analogRead(moisture);
  int value = analogRead(moisture);
  delay(100);
  // if the moisture value is above 500, turns relay off.
  if (value > 500)
  { relayState = LOW;
    digitalWrite(relayPin, relayState);
  }
  //checks if the moisture value is below 500, and if the offtime has passed, and turns the relay on if true.
  else if ((value < 500) && (currentMillis2 - previousMillis2  >= pumpOffTime))
  { relayState = HIGH;
    previousMillis2 = currentMillis2;
    digitalWrite(relayPin, relayState);
  }
  //checks if the relay is on, and if the ontime has passed, and turns the relay off if true.
  else if ((relayState = HIGH) && (currentMillis2 - previousMillis2  >= pumpOnTime))
  {
    relayState = LOW;
    previousMillis2 = currentMillis2;
    digitalWrite(relayPin, relayState);
  }
}
unsigned long previousMillis1     = 0;          // will store last time moisture sensor was updated
unsigned long previousMillis2     = 0;          // will store last time relay was updated

There is NOTHING magic about the name previousMillis. Something happened at that time. Make the variable name reflect that.

void loop() {

  unsigned long currentMillis

The only excuse for putting the { on the same line as the function declaration/statement is to save real estate. You look pretty stupid doing that AND including a blank line.

  unsigned long currentMillis1 = millis(); // Stores the value of millis() in each iteration
  unsigned long currentMillis2 = millis(); // Stores the value of millis() in each iteration

That's pretty silly. You do NOT need two different variables to hold now.

  if ((voutState = LOW) && (currentMillis1 - previousMillis1) >= moistureReadOfftime)

Assigning a value to a variable in an if statement is rarely correct.

  int sensorValue = analogRead(moisture);
  int value = analogRead(moisture);
  delay(100);

Why are you reading from the same pin nanoseconds apart, and storing the value in two variables that appear completely unrelated?

Why are you then sticking your head in the sand?

  if (value > 500)
  { relayState = LOW;
    digitalWrite(relayPin, relayState);
  }
  //checks if the moisture value is below 500, and if the offtime has passed, and turns the relay on if true.
  else if ((value < 500) && (currentMillis2 - previousMillis2  >= pumpOffTime))
  { relayState = HIGH;
    previousMillis2 = currentMillis2;
    digitalWrite(relayPin, relayState);
  }

NOTHING follows the { on the same line. EVER.

If the moisture sensor value is not over 500, it must be less than or equal to 500, doesn't it? Why are you doing nothing when the value is exactly 500? What makes that value special?

You didn't state what Problem you are having so we can't provide specific answers.

In Addition to what Paul already said, lines such as:

if ((voutState = LOW) && (currentMillis1 - previousMillis1) >= moistureReadOfftime)

are ambiguous. They often lead to bugs that are extremely hard to find because the Computer calculates something totally unrelated to what the programmer intended.

Parenthesis are always evaluated first so for the purpose of discussion, let's oversimplify that code to demonstrate the ambiguity.

if ((A) && (B) >= moistureReadOfftime)

Is this checking to see if A && B (which evaluates to either true or false, 0 or 1) is >= moistureReadOfftime? That will nearly never be the case.

Or, is it first evaluating if B >= moistureReadOfftime and && that with A?

It would be better to write that line like this and eliminate ambiguity altogether.

if ((voutState = LOW) && ((currentMillis1 - previousMillis1) >= moistureReadOfftime))

If you get yourself into the Habit of never coding ambiguously, you will save yourself tons of Debugging time and a lot of hair loss.

Thanks JaBa an PaulS.

The issue is I am having is that the relay is coming on permanently, and the moisture sensor and the timing have no effect on it. It remains on.

I apologies for my lack of coding layout etiquette. I'm new to this

Is the "{" placement causing errors in my code, or is it more a layout thing?

The only excuse for putting the { on the same line as the function declaration/statement is to save real estate. You look pretty stupid doing that AND including a blank line.

Code: [Select]
unsigned long currentMillis1 = millis(); // Stores the value of millis() in each iteration
unsigned long currentMillis2 = millis(); // Stores the value of millis() in each iteration

That's pretty silly. You do NOT need two different variables to hold now.

I don't really understands this. Again, being new to this, I was under the impression I would need 2 separate 'timings' running. One for the sensor voltage, and one for the relay.

I have made the changes to the ambiguous (if) lines, and changed the values to <= and >= respectively, along with removing the duplicate analgoread
It still doesn't seem to change the problem.
I'm trying with shorter times to test the code.

int moisture = A5;                              //moisture sensor
int vout     = 11;                              //voltage input of moisture sensor
int relayPin = 12;                              //relay connected to water pump
unsigned long moistureReadOntime  = 1000;       //amount of time the moisture sensor is on (millis)
unsigned long moistureReadOfftime = 4000;       //amount of time the moisture sensor is off (millis)
unsigned long previousMillis1     = 0;          // will store last time moisture sensor was updated
unsigned long previousMillis2     = 0;          // will store last time relay was updated
unsigned long pumpOnTime          = 5000;       // amount of time the water pump is on (millis)
unsigned long pumpOffTime         = 5000;       // amount of time the water pump is off (millis)

int voutState = HIGH;                         // starts sensor voltage as being on
int relayState = LOW;                        // starts relay voltage as being off

void setup()
{
  pinMode(moisture, INPUT);                     // sets moisture sensor as an input
  pinMode(relayPin, OUTPUT);                    // sets relay as an output
  pinMode(vout, OUTPUT);                        // sets sensor voltage as an output
  digitalWrite(vout, voutState);                // writes the current state of the sensor voltage
  digitalWrite(relayPin, relayState);           // writes the current state of the relay
}

// the loop routine runs over and over again forever:
void loop()
{
  unsigned long currentMillis1 = millis(); // Stores the value of millis() in each iteration
  unsigned long currentMillis2 = millis(); // Stores the value of millis() in each iteration

  // check to see if the moisture sensor voltage is off, and if its been off for the offtime, and turns it on
  if ((voutState = LOW) && ((currentMillis1 - previousMillis1) >= moistureReadOfftime))
  { 
    voutState = HIGH;
    previousMillis1 = currentMillis1;
    digitalWrite(vout, voutState);
  }

  // check to see if the moisture sensor voltage is on, and if its been on for the ontime, and turns it off
  else if ((voutState = HIGH) && ((currentMillis1 - previousMillis1) >= moistureReadOntime))
  { 
    voutState = LOW;
    previousMillis1 = currentMillis1;
    digitalWrite(vout, voutState);
  }

  //reads the moisture sensor
  int value = analogRead(moisture);
  delay(100);
  // if the moisture value is above 500, turns relay off.
  if (value >= 500)
  {
    relayState = LOW;
    digitalWrite(relayPin, relayState);
  }
  //checks if the moisture value is below 500, and if the offtime has passed, and turns the relay on if true.
  else if ((value <= 500) && ((currentMillis2 - previousMillis2) >= pumpOffTime))
  {
    relayState = HIGH;
    previousMillis2 = currentMillis2;
    digitalWrite(relayPin, relayState);
  }
  //checks if the relay is on, and if the ontime has passed, and turns the relay off if true.
  else if ((relayState = HIGH) && ((currentMillis2 - previousMillis2) >= pumpOnTime))
  {
    relayState = LOW;
    previousMillis2 = currentMillis2;
    digitalWrite(relayPin, relayState);
  }
}

I seem to have fixed it.

It has a very slight bug, but I can live with that. It performs the task at hand.
I think some of the problems came from == & =.

Anyways, code as:

int moisture = A5;                              //moisture sensor
int vout     = 11;                              //voltage input of moisture sensor
int voutState = HIGH;                           // starts sensor voltage as being on
unsigned long vouttimer           = 0;          // will store last time moisture sensor was updated
unsigned long moistureReadOntime  = 5000;       //amount of time the moisture sensor is on (millis)
unsigned long moistureReadOfftime = 360000;       //amount of time the moisture sensor is off (millis)


int relayPin = 12;                              //relay connected to water pump
int relayState = LOW;                           // starts relay voltage as being off
unsigned long pumptimer           = 0;          // will store last time relay was updated
unsigned long pumpOnTime          = 5000;       // amount of time the water pump is on (millis)
unsigned long pumpOffTime         = 360000;       // amount of time the water pump is off (millis)




void setup()
{
  pinMode(moisture, INPUT);                     // sets moisture sensor as an input
  pinMode(relayPin, OUTPUT);                    // sets relay as an output
  pinMode(vout, OUTPUT);                        // sets sensor voltage as an output
  Serial.begin(9600);                           // initialize the serial monitoring
}

// the loop routine runs over and over again forever:
void loop()
{
  unsigned long currentMillis = millis(); // Stores the value of millis() in each iteration

  //reads the moisture sensor
  int value = analogRead(moisture);       // Reads the sensor
  // check to see if the moisture sensor voltage is off, and if its been off for the offtime, and turns it on
  if ((voutState == LOW) && ((currentMillis - vouttimer) >= moistureReadOfftime))
  {
    voutState = HIGH;
    vouttimer = currentMillis;
    digitalWrite(vout, voutState);
  }

  // check to see if the moisture sensor voltage is on, and if its been on for the ontime, and turns it off
  else if ((voutState == HIGH) && ((currentMillis - vouttimer) >= moistureReadOntime))
  {
    voutState = LOW;
    vouttimer = currentMillis;
    digitalWrite(vout, voutState);
  }

  // if the moisture value is above 500, turns relay off.
  if (value >= 500)
  {
    relayState = LOW;
    digitalWrite(relayPin, relayState);
  }

  if (voutState == LOW)
  {
    relayState = LOW;
    digitalWrite(relayPin, relayState);
  }

  //checks if the moisture value is below 500, and if the offtime has passed, and turns the relay on if true.
  else if ((value <= 500) && ((currentMillis - pumptimer) >= pumpOffTime))
  {
    relayState = HIGH;
    pumptimer = currentMillis;
    digitalWrite(relayPin, relayState);
  }
  //checks if the relay is on, and if the ontime has passed, and turns the relay off if true.
  else if ((relayState == HIGH) && ((currentMillis - pumptimer) >= pumpOnTime))
  {
    relayState = LOW;
    pumptimer = currentMillis;
    digitalWrite(relayPin, relayState);
  }


  Serial.println(value);
 
}

Is it possible to "debounce" the sensor readings and the serial print? without a delay?