Problem with relay

Projects Programming
1

I am using three solenoids, activated by TIP 120 transistors, and one pump activated by a 5V relay (I had a 4-channel module laying around but I only need one channel of it). I am using millis timer function instead of delay. The solenoids activate and are timed just fine. The pump does not activate except with Solenoid #3 the pump activates but the timing does not work, it turns off immediately when a sensor value drops below a set point. The code for the solenoids and the relay to activate the pump is the same. I don't understand why the relay is not activating, can anyone help please?

#include <LiquidCrystal.h>

LiquidCrystal lcd(12, 11, 5, 4, 3, 2);
int up = 7;
int down = 8;
int alt = 6;
int mode = 9;
int moisture1 = A0;
int moisture2 = A1;
int moisture3 = A2;
int valve1 = 0;
int valve2 = 10;
int valve3 = 13;
int pump = 1;
int pumpstate = HIGH;
int valvestate1 = LOW;
int valvestate2 = LOW;
int valvestate3 = LOW;

int lvl1 = 0;
int lvl2 = 0;
int lvl3 = 0;
int vary = 0;
int cycle = 1;

int u1;                     // variable for reading the pin status
int u2;                    // variable for reading the delayed status
int uState;               // variable to hold the button state
int d1;
int d2;
int dState;
int a1;
int a2;
int aState;
int m1;
int m2;
int mState;

unsigned long settime1 = 0;
unsigned long settime2 = 0;
unsigned long settime3 = 0;
unsigned long timer1 = 0;
unsigned long timer2 = 0;
unsigned long timer3 = 0;

void setup() {

  pinMode(up, INPUT);
  pinMode(down, INPUT);
  pinMode(alt, INPUT);
  pinMode(mode, INPUT);
  pinMode(moisture1, INPUT);
  pinMode(moisture2, INPUT);
  pinMode(moisture3, INPUT);
  pinMode(valve1, OUTPUT);
  pinMode(valve2, OUTPUT);
  pinMode(valve3, OUTPUT);
  pinMode(pump, OUTPUT);
  lcd.begin(16, 2);

  uState=digitalRead(up);
  dState=digitalRead(down);
  aState=digitalRead(alt);
  mState=digitalRead(mode);

}

void loop() {
  digitalWrite(pump, pumpstate);
  digitalWrite(valve1, valvestate1);
  digitalWrite(valve2, valvestate2);
  digitalWrite(valve3, valvestate3);
  int moistureval1 = analogRead(moisture1);
  moistureval1 = map(moistureval1, 0, 1023, 0, 100);
  int moistureval2 = analogRead(moisture2);
  moistureval2 = map(moistureval2, 0, 1023, 0, 100);
  int moistureval3 = analogRead(moisture3);
  moistureval3 = map(moistureval3, 0, 1023, 0, 100);

  settime1 = constrain(settime1, 0, 300);
  lvl1 = constrain(lvl1, 0, 100);
  settime2 = constrain(settime2, 0, 300);
  lvl2 = constrain(lvl2, 0, 100);
  settime3 = constrain(settime3, 0, 300);
  lvl3 = constrain(lvl3, 0, 100);

  if ((moistureval1 > 5)&&(moistureval1 < lvl1)&&(pumpstate == HIGH)){
    pumpstate = LOW;
    valvestate1 = HIGH;
    timer1 = millis();
  }
  if ((millis() - timer1) >= settime1*1000){
    pumpstate = HIGH;
    valvestate1 = LOW;
  } 


  if ((moistureval2 > 5)&&(moistureval2 < lvl2)&&(pumpstate == HIGH)){
    pumpstate = LOW;
    valvestate2 = HIGH;
    timer2 = millis();
  }
  if ((millis() - timer2) >= settime2*1000){
    pumpstate = HIGH;
    valvestate2 = LOW;
  }


  if ((moistureval3 > 5)&&(moistureval3 < lvl3)&&(pumpstate == HIGH)){
    pumpstate = LOW;
    valvestate3 = HIGH;
    timer3 = millis();
  }
  if ((millis() - timer3) >= settime3*1000){
    pumpstate = HIGH;
    valvestate3 = LOW;
  } 


  m1=digitalRead(mode);
  delay(10);
  m2=digitalRead(mode);
  if (m1 == m2) {
    if (m1 != mState){
      if (m1 == LOW) {
        if (cycle == 1){
          cycle = 2;
        } 
        else {
          if (cycle == 2){
            cycle = 3;
          } 
          else {
            if (cycle == 3){
              cycle = 1;
            }
          }
        }
      }
    }
  }
  mState = m1;


  a1=digitalRead(alt);
  delay(10);
  a2=digitalRead(alt);
  if (a1 == a2) {
    if (a1 != aState){
      if (a1 == LOW) {
        if (vary == 0){
          vary = 1;
        } 
        else {
          if (vary == 1){
            vary = 0;
          }
        }
      }
    }
  }
  aState = a1;

  if (cycle == 1){
    lcd.setCursor(0, 0);
    lcd.print("Sensor         %");
    lcd.setCursor(8, 0);
    lcd.print("1");
    lcd.setCursor(12, 0);
    lcd.print(moistureval1);

    //up button controls time
    if (cycle == 1 && vary ==0) {
      lcd.setCursor(0, 1);
      lcd.print("Set On Time    s");
      lcd.setCursor(12, 1);
      lcd.print(settime1);
      u1=digitalRead(up); 
      delay(10);
      u2=digitalRead(up);
      if (u1 == u2) {
        if (u1 != uState) {
          if (u1 == LOW) {
            settime1 += 15;
          }
        }
      }
      uState = u1;

      //down button controls time
      d1=digitalRead(down); 
      delay(10);
      d2=digitalRead(down);
      if (d1 == d2) {
        if (d1 != dState) {
          if (d1 == LOW) {
            settime1 -= 15;
          }
        }
      }
      dState = d1;
    }


    if (cycle == 1 && vary ==1) {
      //up button controls lvl
      lcd.setCursor(0, 1);
      lcd.print("Set On Lvl     %");
      lcd.setCursor(12, 1);
      lcd.print(lvl1);
      u1=digitalRead(up); 
      delay(10);
      u2=digitalRead(up);
      if (u1 == u2) {
        if (u1 != uState) {
          if (u1 == LOW) {
            lvl1 += 5;
          }
        }
      }
      uState = u1;

      //down button controls lvl
      d1=digitalRead(down); 
      delay(10);
      d2=digitalRead(down);
      if (d1 == d2) {
        if (d1 != dState) {
          if (d1 == LOW) {
            lvl1 -= 5;
          }
        }
      }
      dState = d1;
    }
  }
  if (cycle == 2){
    lcd.setCursor(0, 0);
    lcd.print("Sensor         %");
    lcd.setCursor(8, 0);
    lcd.print("2");
    lcd.setCursor(12, 0);
    lcd.print(moistureval2);

    //up button controls time
    if (cycle == 2 && vary ==0){
      lcd.setCursor(0, 1);
      lcd.print("Set On Time    s");
      lcd.setCursor(12, 1);
      lcd.print(settime2);
      u1=digitalRead(up); 
      delay(10);
      u2=digitalRead(up);
      if (u1 == u2) {
        if (u1 != uState) {
          if (u1 == LOW) {
            settime2 += 15;
          }
        }
      }
      uState = u1;

      //down button controls time
      d1=digitalRead(down); 
      delay(10);
      d2=digitalRead(down);
      if (d1 == d2) {
        if (d1 != dState) {
          if (d1 == LOW) {
            settime2 -= 15;
          }
        }
      }
      dState = d1;

    }
    if (cycle == 2 && vary ==1) {
      //up button controls lvl
      lcd.setCursor(0, 1);
      lcd.print("Set On Lvl     %");
      lcd.setCursor(12, 1);
      lcd.print(lvl2);
      u1=digitalRead(up); 
      delay(10);
      u2=digitalRead(up);
      if (u1 == u2) {
        if (u1 != uState) {
          if (u1 == LOW) {
            lvl2 += 5;
          }
        }
      }
      uState = u1;

      //down button controls lvl
      d1=digitalRead(down); 
      delay(10);
      d2=digitalRead(down);
      if (d1 == d2) {
        if (d1 != dState) {
          if (d1 == LOW) {
            lvl2 -= 5;
          }
        }
      }
      dState = d1;

    }
  }
  if (cycle == 3){
    lcd.setCursor(0, 0);
    lcd.print("Sensor         %");
    lcd.setCursor(8, 0);
    lcd.print("3");
    lcd.setCursor(12, 0);
    lcd.print(moistureval3);

    //up button controls time
    if (cycle == 3 && vary ==0) {
      lcd.setCursor(0, 1);
      lcd.print("Set On Time    s");
      lcd.setCursor(12, 1);
      lcd.print(settime3);
      u1=digitalRead(up); 
      delay(10);
      u2=digitalRead(up);
      if (u1 == u2) {
        if (u1 != uState) {
          if (u1 == LOW) {
            settime3 += 15;
          }
        }
      }
      uState = u1;

      //down button controls time
      d1=digitalRead(down); 
      delay(10);
      d2=digitalRead(down);
      if (d1 == d2) {
        if (d1 != dState) {
          if (d1 == LOW) {
            settime3 -= 15;
          }
        }
      }
      dState = d1;

    }
    if (cycle == 3 && vary ==1) {
      //up button controls lvl
      lcd.setCursor(0, 1);
      lcd.print("Set On Lvl     %");
      lcd.setCursor(12, 1);
      lcd.print(lvl3);
      u1=digitalRead(up); 
      delay(10);
      u2=digitalRead(up);
      if (u1 == u2) {
        if (u1 != uState) {
          if (u1 == LOW) {
            lvl3 += 5;
          }
        }
      }
      uState = u1;

      //down button controls lvl
      d1=digitalRead(down); 
      delay(10);
      d2=digitalRead(down);
      if (d1 == d2) {
        if (d1 != dState) {
          if (d1 == LOW) {
            lvl3 -= 5;
          }
        }
      }
      dState = d1;
    }
  }
}

Moderator edit: name of company selling counterfeit Arduino products removed

2

You turn the pump off when any of the solenoids get turned off, and the code to do that (for each solenoid) will execute each time through loop() since e.g. timer1 will be a long time in the past when solenoid 1 is off.

If I have understood what you're trying to achieve, it would be better to work out the required solenoid states using the sensor input values and timer logic you already have, but move the pump controls into a separate piece of code that turns the pump on if any solenoid is on, and turns it off if no solenoids are on.

Incidentally, the code for sensor reading, mapping, comparison logic, solenoid output etc could be reduced in size by a factor of three by putting the relevant pin numbers into arrays and using common code to deal with all of them. This would also make maintenance much simpler.

3

You have just two functions, setup() and loop(). Very few of the global variables you have defined are shared by both functions. Those that are not should not be global.

void loop() {
  digitalWrite(pump, pumpstate);
  digitalWrite(valve1, valvestate1);
  digitalWrite(valve2, valvestate2);
  digitalWrite(valve3, valvestate3);

Why are these called on every pass through loop()? The functions should only be called when a change is needed.

  settime1 = constrain(settime1, 0, 300);
  lvl1 = constrain(lvl1, 0, 100);
  settime2 = constrain(settime2, 0, 300);
  lvl2 = constrain(lvl2, 0, 100);
  settime3 = constrain(settime3, 0, 300);
  lvl3 = constrain(lvl3, 0, 100);

What, exactly, are you constraining? None of the variables have not been assigned new values, so it seems pointless to constrain them here. Constrain them, if needed, when they ARE assigned new values.

  if ((moistureval3 > 5)&&(moistureval3 < lvl3)&&(pumpstate == HIGH)){

Consistent useofwhite space isagood thing.

unsigned long timer1 = 0;
unsigned long timer2 = 0;
unsigned long timer3 = 0;

Lousy names. millis() is not a timer. It's a clock. Meaningful names would make the code easier to read.

Personally, I gave up, because there is so much useless code and too few comments to help decipher what the code is doing. There are too many things being kept track of, so that actions can happen later, when the action should happen NOW, not next time loop() starts. Doing things when they should be done eliminates the need to keep track of whether the action should happen. That reduces the amount of code needed. Sometimes greatly (as I suspect is the case here).

4

Thank you for your time and advice, what you are suggesting makes sense. I am not quite sure how to implement that but I will do a lot more reading and try to get this code cleaned up.
Thanks again.