Reading multiple inputs on digital pins

Hello guys
Am new in arduino and i would need your help if possible please. My small project that am building has three inputs pins and one relay pin as output, The three inputs which go HIGH (5V) two at the time and one remain low (0 V). There are four conditions that must be tested. Three of the conditions are the fault conditions and one is Good condition. Now this Good condition and one of fault conditions are tested when the relay goes high. When the relay is HIGH (5 V) must test one of the two condition, if three inputs are LOW (0 V), the relay must reset or go to LOW, and all LEDs must go OFF. What am missing in my code is that i cant reset the relay go to LOW, please if you do not understand my explanation tell me so that can make more clear the part you dont understand. Thanks in advance guys. Please guide me wherever you can.

below is the code:

int relayPin = 8;
int ledPin = 7;  // goes HIGH same time with relay just to show when the relay is HIGH or LOW

int redLed = 12;  // This must go HIGH when the relay is HIGH and if its fault condition
int greenLed = 11; // This must go HIGH when the relay is HIGH and if its Good condition

int lightGreen = 10;  // This must go HIGH if the relay is LOW, is showing fault condition
int YellowLed = 9;    // this must go HIGH if the realy is LOW, is showiing other fault condition

int inPut1 = 2;    // input1
int inPut2 = 3;    // input2
int inPut3 = 4;    // input3

boolean readValue1 ;
boolean readValue2 ;
boolean readValue3 ;

boolean relayState;

void setup(){
  
  pinMode(12, OUTPUT);
  pinMode(11, OUTPUT);
  pinMode(10, OUTPUT);
  pinMode(9, OUTPUT);
  pinMode(8, OUTPUT);
  pinMode(7, OUTPUT);
  
  pinMode(2, INPUT);
  pinMode(3, INPUT);
  pinMode(4, INPUT);
  
 
}

void loop(){
  readValue1 = digitalRead(inPut1);
  readValue2 = digitalRead(inPut2);
  readValue3 = digitalRead(inPut3);
  
  if(readValue1 == true && readValue2 == true && readValue3 == false){
    delay(500);
    digitalWrite(8, HIGH);
    digitalWrite(7, HIGH);
    delay(2000);

    while( relayPin == HIGH){
      if( readValue1 == false && readValue2 == true && readValue3 == true){ // fault condition when relay is HIGH
      
        digitalWrite(12, HIGH);  
      }

      if (readValue1 == true && readValue2 == false && readValue3 == true){ //Good condition when relay is HIGH
        digitalWrite(11, HIGH);  
      }

       // Here is where my problem comes, i cant reset everything when the three inputs are LOW

      if (readValue1 == false && readValue2 == false && readValue3 == false){

         digitalWrite(8,LOW) ;
         digitalWrite(11, LOW);
         digitalWrite(12, LOW);
        
      }
    }
  }
  
   // Here everything works right if the relay not HIGH
  
  else if(readValue1 == true && readValue2 == false && readValue3 == true){ // fault condition when relay is LOW
    digitalWrite(10, HIGH) ;
    //delay(2000);
  }

  else if(readValue1 == false && readValue2 == true && readValue3 == true){  // fault condition when relay is LOW
    digitalWrite(9, HIGH);
  }
 
  else{
    digitalWrite(8, LOW); 
    digitalWrite(7, LOW); 
    digitalWrite(11, LOW);
    digitalWrite(12, LOW);
    digitalWrite(10, LOW);
    digitalWrite(9, LOW);
    
 }
}

Thanks guys in advance

The first thing I see is this

int relayPin = 8;
//then later in the code
while (relayPin == HIGH)

You need to test the state of the pin not the value of the pin which will always be HIGH

Hey UKHeliBob
Thanks for the reply, i also thought so, as the while loop not stopping. how can i test the state of the pin. am still new in programming. thanks.

how can i test the state of the pin.

    while (digitalRead(relayPin) == HIGH)

Do yourself a favour and use the pin names you have declared
So instead of

digitalWrite(8, LOW) ;

use

digitalWrite(relayPin, LOW) ;

and the same with the other pin names. It makes the code so much easier to read

Are the input pins in a known state when not taken HIGH or are they floating at an unknown, possible HIGH, state ? What takes them HIGH ?

Give the values read from the inputs meaningful names. Once again it makes the code much easier to read

Thanks again, i have change the names as you suggested
And this pice of code, but it goes now in the while loop making the relayPin and greenLed ON and OFF repeatedly. i want both greenLed and relayPin to remain HIGH, and all must reset to zero if the three input are LOW

now its only resetting if all inputs are LOW,

// while (digitalRead(relayPin) == HIGH)

the input voltages (5V) coming from external circuit (3 opto-couplers), each voltage is in series with a 10K resistor (pull up) before its connected to arduino nano digital pin. i dont think the inputs are floating.

Post your code as it is now so that we can see what you are looking at

UKHeliBob:
The first thing I see is this

int relayPin = 8;

//then later in the code
while (relayPin == HIGH)



You need to test the [u]state[/u] of the pin not the [u]value[/u] of the pin which will always be HIGH

I assume you meant never? (8 != 1)

@OP

if you are within a while loop, you need to update the variables you test against in your ifs

  readValue1 = digitalRead(inPut1);
  readValue2 = digitalRead(inPut2);
  readValue3 = digitalRead(inPut3);

otherwise you'll only see the original values

A small state machine drawing would probably help you see the various states and possible transitions you need to handle.

hey guys
sorry for late reply, was a bit busy at work
J-M-L i did that yesterday and now again am trying to figure out what am missing but with no chance

here is the code again:

int ledPin = 7;
int relayPin = 8;
int yellowLed = 9;
int lightGreenLed = 10;
int greenLed = 11;
int redLed = 12;

int inPut1 = 2;
int inPut2 = 3;
int inPut3 = 4;

boolean readValue1 ;
boolean readValue2 ;
boolean readValue3 ;

boolean relayState;

void setup(){
  
  pinMode(redLed, OUTPUT);
  pinMode(greenLed, OUTPUT);
  pinMode(lightGreenLed, OUTPUT);
  pinMode(yellowLed, OUTPUT);
  pinMode(relayPin, OUTPUT);
  pinMode(ledPin, OUTPUT);
  
  pinMode(inPut1, INPUT);
  pinMode(inPut2, INPUT);
  pinMode(inPut3, INPUT);
  
}

void loop(){
  readValue1 = digitalRead(inPut1);
  readValue2 = digitalRead(inPut2);
  readValue3 = digitalRead(inPut3);
  
  if (readValue1 == true && readValue2 == true && readValue3 == false){
    delay(1000);
    digitalWrite(8, HIGH);
    digitalWrite(7, HIGH);
    delay(2000);
  
    relayState = digitalRead(relayPin);

    readValue1 = digitalRead(inPut1);
    readValue2 = digitalRead(inPut2);
    readValue3 = digitalRead(inPut3);

    // here is where my the problem is, cant reset the relayPin to LOW (0 V) when all three inputs are LOW (0 V)
    
    while(digitalRead(relayPin) == HIGH){

      if ( readValue1 == false && readValue2 == true && readValue3 == true){  // fault condition when the relayPin is HIGH (5 V)
      
        digitalWrite(redLed, HIGH);
      }   

     if (readValue1 == true && readValue2 == false && readValue3 == true){ // right condition when the relayPin is HIGH (5 V)
        digitalWrite(greenLed, HIGH);  
        
    }
        
     if ( readValue1 == false && readValue2 == false && readValue3 == false){ // here i am trying to reset everything to LOW (0 V) if all three inputs are LOW (0 V)

        digitalWrite(greenLed, LOW);
        digitalWrite(redLed, LOW);
        digitalWrite(ledPin, LOW);
        digitalWrite(relayPin, LOW);
            
      }
      
    }
  }
  
   //here everything looks working right when the relay not HIGH
   
  else if( readValue1 == true && readValue2 == false && readValue3 == true){        // fault condition when the relayPin is LOW
    digitalWrite(lightGreenLed, HIGH) ;
  }

  else if(readValue1 == false && readValue2 == true && readValue3 == true){     // other fault condition when relayPin is LOW
    digitalWrite(yellowLed, HIGH);
  }
 
  else{
   
    digitalWrite(relayPin, LOW); 
    digitalWrite(ledPin, LOW); 
    digitalWrite(yellowLed, LOW);
    digitalWrite(redLed, LOW);
    digitalWrite(lightGreenLed, LOW);
    digitalWrite(greenLed, LOW);  
 }
  
}

In the above code the relayPin goes HIGH and greenLed as expected, but if the three are LOW (0 V), both relayPin and greenLed remain HIGH, i want them to go LOW.

thanks guys

issndu:
J-M-L i did that yesterday and now again am trying to figure out what am missing but with no chance

You did but not in the right place...

Look at this while loop for example

    while (digitalRead(relayPin) == HIGH) {

      if ( readValue1 == false && readValue2 == true && readValue3 == true) { // fault condition when the relayPin is HIGH (5 V)
        digitalWrite(redLed, HIGH);
      }

      if (readValue1 == true && readValue2 == false && readValue3 == true) { // right condition when the relayPin is HIGH (5 V)
        digitalWrite(greenLed, HIGH);
      }

      if ( readValue1 == false && readValue2 == false && readValue3 == false) { // here i am trying to reset everything to LOW (0 V) if all three inputs are LOW (0 V)
        digitalWrite(greenLed, LOW);
        digitalWrite(redLed, LOW);
        digitalWrite(ledPin, LOW);
        digitalWrite(relayPin, LOW);
      }
    }

you are testing readValue1, readValue2 and readValue3 but you never update them, so they will never change during the while()

note as well that when working with boolean, it's not necessary to write == true. A boolean is a truth value in itself and if you want to check if it's false, then use the ! (not) operator ==> Instead of      if ( readValue1 == false && readValue2 == true && readValue3 == true) { you can write      if ( !readValue1 && readValue2 && readValue3) {

Or another example, Instead ofif ( readValue1 == false && readValue2 == false && readValue3 == false) {you can just writeif ( !readValue1 && !readValue2 && !readValue3) {

last but not least, if you were to find more meaningful names for "readValueX" that would help understand the code better. What's being read, what does it represent? ==> use that for the name of the variable

There is a common problem with using variables like this:

  readValue1 = digitalRead(inPut1);
  readValue2 = digitalRead(inPut2);
  readValue3 = digitalRead(inPut3);

It often gains you nothing but more lines of code and means that subsequent conditionals
may be stale if you forget to update the variables.

If you do a test like this:

  if (readValue1 == LOW)

You are really coding "if at some point in the past, depending on the rest of the code above, was pin
inPut1 LOW (assuming I remembered to ever read the pin into the variable at all)?"

However if you put:

  if (digitalRead (inPut1) == LOW)

Then you are coding "if the pin inPut1 is now LOW..." which is normally what you meant.

Sometimes you do want to store the result of digitalRead in a variable, but when this
situation arises you'll realize to do this (StateChangeDetection example is one case of this
I think). And in this case you can use C++'s assignment expressions to good effect:

if ((readValue = digitalRead (inPut1)) == LOW)

This assigns the value from digitalRead into the variable and returns the same value to be compared to LOW.