[SOLVED] if else if works sort of

Below is code I have heavily modified from the Button sketch for model railway signal operation. It works kind of. 

There are two signals each with two lights. Only one light is meant to be lit on each signal at any one time.

In the first three conditions I have faulty outputs. The last two are OK. (as noted in sketch)

I would be most thank full for a nudge in the right direction.

Daryl.

PS, I sincerely hope I have not contravened any protocol by asking this question in this forum.


/*

  Model Railway Signals

  Button sketch originally created 2005
  by DojoDave <http://www.0j0.org>
  modified 30 Aug 2011
  by Tom Igoe This example code is in the public domain.
  http://www.arduino.cc/en/Tutorial/Button

  This circuit:
  Modified Button sketch
  created 2018
  by Daryl Roe

  Arduino UNO

  LED's attached to pins 9, 10, 11 and 12 to ground via appropriate resistors.
  Distant signal has Yellow over Green and Home signal has Red over Green.

  3 current sensors attached to pins 2, 3 and 4.
  1 switch (on/off) from turnout to pin 5.
  The sensors give HIGH as unoccupied and LOW as occupied. HIGH and LOW read as > 4.2v and < 1v  respectively at pins.
  The turnout gives LOW as straight and HIGH as diverging.

  HELP. The first 3 conditions are giving wrong readings and I have no idea why :-(
*/

// constants won't change. They're used here to
// set pin numbers:
const int mainPin = 2;     // the number of the main sensor pin
const int turnoutPin = 5;  // the number of the turnout switch pin
const int homePin = 3;     // the number of the home sensor pin
const int loopPin = 4;     // the number of the loop sensor pin
const int DGPin =  9;      // the number of the distant green LED pin
const int DYPin = 10;      // the number of the distant yellow LED pin
const int HRPin = 12;      // the number of the home red LED pin
const int HGPin = 11;      // the number of the home green LED pin
// variables will change:
int mainState = 0;         // variables for reading the main sensor status
int turnoutState = 0;      //etc
int homeState = 0;         //etc
int loopState = 0;         //etc

void setup()
{
  // initialize the LED pins as outputs:
  pinMode(DGPin, OUTPUT);  //Distant green
  pinMode(DYPin, OUTPUT);  //Distant yellow
  pinMode(HRPin, OUTPUT);  //Home red
  pinMode(HGPin, OUTPUT);  //home green
  // initialize the sensor pins as inputs:
  pinMode(mainPin, INPUT);
  pinMode(turnoutPin, INPUT);
  pinMode(homePin, INPUT);
  pinMode(loopPin, INPUT);
}
void loop()
{
  // read the state of the sensor values:
  mainState = digitalRead(mainPin);
  turnoutState = digitalRead(turnoutPin);
  homeState = digitalRead(homePin);
  loopState = digitalRead(loopPin);

  // check the various sensor states
  // if the states are as follows:
  if ((mainState == LOW) && (turnoutState == LOW) && (homeState == HIGH)) //first condition. Main occupied, t/out straight, home unoccupied.
  {
    // turn appropriate LED's on or off:
    digitalWrite(DGPin, HIGH);      //ok
    digitalWrite(DYPin, LOW);       //high
    digitalWrite(HGPin, HIGH);      //ok
    digitalWrite(HRPin, LOW);       //high

  }
  else if  ((mainState == LOW) && (turnoutState == LOW) && (homeState == LOW))  //second condition. Main occupied, t/out straight, home occupied.
  {
    digitalWrite(DGPin, LOW);       //ok
    digitalWrite(DYPin, HIGH);      //ok
    digitalWrite(HGPin, LOW);       //high
    digitalWrite(HRPin, HIGH);      //ok
  }
  else if  ((mainState == LOW) && (turnoutState == HIGH) && (loopState == HIGH))  //third condition, Main occupied, t/out diverging, loop unoccupied.
  {
    digitalWrite(DGPin, LOW);       //ok
    digitalWrite(DYPin, HIGH);      //ok
    digitalWrite(HGPin, HIGH);      //ok
    digitalWrite(HRPin, LOW);       //high
  }
  else if ((mainState == LOW) && (turnoutState == HIGH) && (loopState == LOW))  //fourth condition. Main occupied, t/out diverging, loop occupied.
  {
    digitalWrite(DGPin, LOW);       //ok
    digitalWrite(DYPin, HIGH);      //ok
    digitalWrite(HGPin, LOW);       //ok
    digitalWrite(HRPin, HIGH);      //ok
  }
  else (mainState == HIGH);   //else condition. No trains.
  {
    digitalWrite(DGPin, LOW);       //ok
    digitalWrite(DYPin, HIGH);      //ok
    digitalWrite(HGPin, LOW);       //ok
    digitalWrite(HRPin, HIGH);      //ok
  }
  return (loop);
}

[/code]

How do you know that both the last two conditions work ? They both switch on an an identical LED pattern.

Put some Serial.print() statements in your code so you can see what values the variables here have :

  mainState = digitalRead(mainPin);
  turnoutState = digitalRead(turnoutPin);
  homeState = digitalRead(homePin);
  loopState = digitalRead(loopPin);

and also in each if statement then you'll see what is going wrong.

edit:
mainState etc. should really be declared as bool and not int. and you are using external pull up resistors on your switches ?

I suggest you cascade your IF statements - it makes it much easier to see the relationships. Something like this

if (mainState == LOW) {
  if  (turnoutState == LOW) {
      if (homeState == HIGH) {

       }
      else if (homeState == LOW) {
// etc

Also an ELSE following a compound IF can be responding to a failure of any of the three tests and there is no means to know which one.

...R

Thank you Robin2 and 6v6gt for your replies.

I have carried out the changes as you both have suggested but the problem still exists. I connected to Serial.print() and this shows the UNO is reading the inputs as intended.

I then deleted the last 3 conditions and the LED's behaved. As I added another condition the LED's decided to play up. I have changed the sequence of events to no avail.

Now I have lost the plot and nothing works.

I'm not giving up though and determined to get it right. Back to google.

Daryl.

Ab811:
I have carried out the changes as you both have suggested but the problem still exists.

If we are to help you have to post the latest version of your program (in your next Reply) so we can see exactly what you have done.

Also please describe in detail what happens when you run that version of the program.

...R

Hi Robin2 and 6v6gt. I finaly come up with a sketch that works as below.
A lot of the problem was {}'s and ;'s.
Although the last condition starts with else if, the lights run as required.
I changed the last condition to an else but this had lights up and others faintly flickering.
Any ideas?

Yours Sincerely,
Daryl.

// set pin numbers:
const int mainPin = 2;     // the number of the main sensor pin
const int turnoutPin = 5;  // the number of the turnout switch pin
const int homePin = 3;     // the number of the home sensor pin 
const int loopPin = 4;     // the number of the loop sensor pin
const int DGPin =  9;      // the number of the distant green LED pin
const int DYPin = 10;      // the number of the distant yellow LED pin
const int HRPin = 12;      // the number of the home red LED pin
const int HGPin = 11;      // the number of the home green LED pin
// variables will change:
bool mainState = 0;         // variables for reading the main sensor status
bool turnoutState = 0;      //etc
bool homeState = 0;         //etc
bool loopState = 0;         //etc
bool mainValue = 0;

void setup()
{
  Serial.begin(9600);
  // initialize the LED pins as outputs:
  pinMode(DGPin, OUTPUT);  //Distant green = clear
  pinMode(DYPin, OUTPUT);  //Distant yellow = caution
  pinMode(HRPin, OUTPUT);  //Home red == STOP
  pinMode(HGPin, OUTPUT);  //home green
  // initialize the sensor pins as inputs:
  pinMode(mainPin, INPUT);
  pinMode(turnoutPin, INPUT);
  pinMode(homePin, INPUT);
  pinMode(loopPin, INPUT);
}
void loop()
{

  Serial.print(mainPin);
  // read the state of the sensor values:
  mainState = digitalRead(mainPin);
  Serial.print(mainState);
  delay(10);
  turnoutState = digitalRead(turnoutPin);
  Serial.println(turnoutState);
  delay(10);
  homeState = digitalRead(homePin);
  Serial.println(homeState);
  delay(10);
  loopState = digitalRead(loopPin);
  Serial.println(loopState);
  delay(10);

  // check the various sensor states
  // I will call this condition 1
  if ((mainState == HIGH) &&        //no train. This could be demeed as a default
      (turnoutState == HIGH) &&     //straight
      (homeState == HIGH))          //unoccupied
    //(loopState == DON'T CARE)
  {
    // turn appropriate LED's on or off
    digitalWrite(DGPin, LOW);     //distant green off
    digitalWrite(DYPin, HIGH);    //distant yellow on
    digitalWrite(HGPin, LOW);     //home green off
    digitalWrite(HRPin, HIGH);    //home red on
  }
  else if
  //condition 2)
  ((mainState == LOW) &&         //train on main
      (turnoutState == HIGH) &&  //turnout set straight
      (homeState == HIGH))       //no train on home
    //(loopState == DON'T CARE)
  {
    // then turn appropriate LED's on or off and train can carry on through:
    digitalWrite(DGPin, HIGH);  //on
    digitalWrite(DYPin, LOW);   //off
    digitalWrite(HGPin, HIGH);  //on
    digitalWrite(HRPin, LOW);   //off
  }
  else if
  //condition 3)
  ((mainState == LOW) &&         //train on main
      (turnoutState == HIGH) &&  //turnout set straight
      (homeState == LOW))        //occupied, train on home
    //(loopState == DON'T CARE)
  {
    // then turn appropriate LED's on or off and train must stop at home signal:
    digitalWrite(DGPin, LOW);   //of
    digitalWrite(DYPin, HIGH);  //on
    digitalWrite(HGPin, LOW);   //off
    digitalWrite(HRPin, HIGH);  //on
  }
  else if
  //condition 4)
  ((mainState == LOW) &&        //train on main
      (turnoutState == LOW) &&  //turnout set diverging
      // (homeState == DON"T CARE)
      (loopState == HIGH))      //no train on loop
  {
    // then turn appropriate LED's on or off and train can enter loop:
    digitalWrite(DGPin, LOW);   //off
    digitalWrite(DYPin, HIGH);  //on
    digitalWrite(HGPin, HIGH);  //on
    digitalWrite(HRPin, LOW);   //off
  }
  else if
  //condition 5)
  ((mainState == LOW) &&        //train on main
      (turnoutState == LOW) &&  //turnout set diverging
      // (homeState == DON"T CARE)
      (loopState == LOW))       //train on loop
  {
    // then turn appropriate LED's on or off and train can enter loop:
    digitalWrite(DGPin, LOW);    //off
    digitalWrite(DYPin, HIGH);   //on
    digitalWrite(HGPin, LOW);    //off
    digitalWrite(HRPin, HIGH);   //on
  }
}
//up to now the lights change as expected.

Hi Ab811,

Please edit your last post, and type
** **[code]** **
as the first line of your code, and
** **[/code]** **
after your code. It will make it lots easier for folks to help you.

Thanks,
Chris

An 'else' statement at the end (without an 'if' or any of those conditions) would be executed if none of the previous conditions were met.

Try

else {
    digitalWrite(DGPin, LOW);     
    digitalWrite(DYPin, LOW);   
    digitalWrite(HGPin, LOW);     
    digitalWrite(HRPin, LOW);    
}

and see what happens.

Thank you Chris, I hope thats a little better. My 62yo mind is a bit slow nowadays.

And thank you 6v6ft, I will try your suggestion tomorrow.

Thanks again to you both,

Daryl.

The following is NOT what I was recommending in Reply #2

  if ((mainState == HIGH) &&        //no train. This could be demeed as a default
      (turnoutState == HIGH) &&     //straight
      (homeState == HIGH))          //unoccupied

...R

Hi Robin,

"The following is NOT what I was recommending in Reply #2"

Umm, sorry about that.

Below is what I think you mean, it compiled OK but when uploaded, nothing happened.

Now, I am happy with the original outcome but for the want of learning the right way (especially from the Master) where did I go wrong?

Should the else if's after the first else if been just an if?

Daryl.

const int mainPin = 2;     // the number of the main sensor pin
const int turnoutPin = 5;  // the number of the turnout switch pin
const int homePin = 3;     // the number of the home sensor pin
const int loopPin = 4;     // the number of the loop sensor pin
const int DGPin =  9;      // the number of the distant green LED pin
const int DYPin = 10;      // the number of the distant yellow LED pin
const int HRPin = 12;      // the number of the home red LED pin
const int HGPin = 11;      // the number of the home green LED pin
// variables will change:
bool mainState = 0;         // variables for reading the main sensor status
bool turnoutState = 0;      //etc
bool homeState = 0;         //etc
bool loopState = 0;         //etc
bool mainValue = 0;

void setup()
{
  Serial.begin(9600);
  // initialize the LED pins as outputs:
  pinMode(DGPin, OUTPUT);  //Distant green = clear
  pinMode(DYPin, OUTPUT);  //Distant yellow = caution
  pinMode(HRPin, OUTPUT);  //Home red == STOP
  pinMode(HGPin, OUTPUT);  //home green
  // initialize the sensor pins as inputs:
  pinMode(mainPin, INPUT);
  pinMode(turnoutPin, INPUT);
  pinMode(homePin, INPUT);
  pinMode(loopPin, INPUT);
}
void loop()
{

  Serial.print(mainPin);
  // read the state of the sensor values:
  mainState = digitalRead(mainPin);
  Serial.print(mainState);
  delay(10);
  turnoutState = digitalRead(turnoutPin);
  Serial.println(turnoutState);
  delay(10);
  homeState = digitalRead(homePin);
  Serial.println(homeState);
  delay(10);
  loopState = digitalRead(loopPin);
  Serial.println(loopState);
  delay(10);

  if (mainState == HIGH)  {         //no train. This could be demeed as a default
    if (turnoutState == HIGH)  {    //straight
      if (homeState == HIGH)  {     //unoccupied

        digitalWrite(DGPin, LOW);     //distant green off
        digitalWrite(DYPin, HIGH);    //distant yellow on
        digitalWrite(HGPin, LOW);     //home green off
        digitalWrite(HRPin, HIGH);    //home red on
      }
      else if (mainState == LOW)  {     //train on main
        if (turnoutState == HIGH)  {    //turnout set straight
          if (homeState == HIGH)  {     //no train on home

            digitalWrite(DGPin, HIGH);    //on
            digitalWrite(DYPin, LOW);     //off
            digitalWrite(HGPin, HIGH);    //on
            digitalWrite(HRPin, LOW);     //off
          }
          else if (mainState == LOW)  {     //train on main
            if (turnoutState == HIGH)  {    //turnout set straight
              if (homeState == LOW)  {      //occupied, train on home

                digitalWrite(DGPin, LOW);   //of
                digitalWrite(DYPin, HIGH);  //on
                digitalWrite(HGPin, LOW);   //off
                digitalWrite(HRPin, HIGH); //on
              }
              else if (mainState == LOW)  {    //train on main
                if (turnoutState == LOW)  {    //turnout set diverging
                  if (loopState == HIGH)  {    //no train on loop

                    digitalWrite(DGPin, LOW);   //off
                    digitalWrite(DYPin, HIGH);  //on
                    digitalWrite(HGPin, HIGH);  //on
                    digitalWrite(HRPin, LOW);   //off
                  }
                  else
                  {
                    digitalWrite(DGPin, LOW);      //off
                    digitalWrite(DYPin, HIGH);     //on
                    digitalWrite(HGPin, LOW);      //off
                    digitalWrite(HRPin, HIGH);     //on
                  }
                }
              }
            }
          }
        }
      }
    }
  }  }

I believe it is a matter of style whether you use the "post #2" method or "post #5" method.
Clearly in this particular case, your interpretation of the post #2 method is incorrect. Maybe @Robin2 will correct your attempt to demonstrate the benefit of his method. However, my preference in this case would be for the "post #5" and especially where there is a large number of options and not all option combinations are used.

Where there is an even larger number of (binary) options, for example, lighting segments on a seven segment display, there is another technique which can be used. That is mapping the options to individual bits in say a byte which can yield compact code.

Ab811:
Below is what I think you mean, it compiled OK but when uploaded, nothing happened.

You are still very far from what I had in mind.

Think of each IF has a YES/NO choice

So, if (mainState == HIGH) ...

What are all the possible things that can happen while that is true - deal with them in that block of code so it looks like this

if (mainState == HIGH) {
   // all the things when this true
}
else {
   // all the things when it is false
}

Then if (turnoutState == HIGH) ...

What are all the possible things that can happem - and deal with them in the same way

if (turnoutState == HIGH) {
  // all the things when this true
}
else {
   // all the things when this true
}

And then when you bring these ideas together it would look like this

if (mainState == HIGH) {
    if (turnoutState == HIGH) {
        // all the things when turnoutState is HIGH
    }
    else {
        // all the things when turnoutState is LOW
    }
    // any oher things when mainState == HIGH
}
else {
    // all the things when mainState == LOW
}

By the way, when you think about the problem like this you may well conclude that a different order of priority would make more sense / would simplify the decision making

...R

Robin,

Thank you (and others) so much for letting me invade your world with "newbie" silly questions but I ensure you your replies have been very much appreciated.

I have had several looks at your last post and I'm having a hard time trying to arrange the required outcome from your perspective and or logic.

The sketch that I had eventually come up with does in fact work as required, and I am very pleased with this outcome. This particular scenario is of a train coming to a turnout, the lights giving the driver an indication of what is happening immediately ahead and will eventually be integrated into a bigger more global system.

To me this was a test case to not only see if it is possible (as all the arduino gurus have assured me), but also to find out if I had the required intelligence to carry it out. I'm actually enjoying this new venture.

I have copy/pasted your suggestions to a new sketch and will continue to play with it.

Daryl.

Hi,
Can you post your final code please?

Thanks.. Tom.. :slight_smile:

HI,
Did you look at switch.. case function?

https://www.arduino.cc/reference/en/language/structure/control-structure/switchcase/

Tom.... :slight_smile:

Hello Tom,

The sketch I finished up with.

Daryl.

/*
  Model Railway Signal Sketch

  Button sketch originaly created 2005
  by DojoDave <http://www.0j0.org>
  modified 30 Aug 2011
  by Tom Igoe This example code is in the public domain.
  http://www.arduino.cc/en/Tutorial/Button

  This sketch:
  Modified Button sketch
  2018
  by Daryl Roe

  Arduino UNO

  LED's attached to pins 9, 10, 11 and 12 to ground via appropriate resistors.
  Distant signal has Yellow over Green and Home signal has Red over Green.

  3 current sensors detecting block occupancy attached to pins 2, 3 and 4.
  1 micro switch (on/off) from turnout to pin 5.
  The sensors give HIGH as unoccupied and LOW as occupied. 
  HIGH and LOW read as > 4.95v and < 1v respectively at pins.
  The turnout gives HIGH as straight and LOW as diverging.

*/

// constants won't change. They're used here to
// set pin numbers:
const int mainPin = 2;     // the number of the main sensor pin
const int turnoutPin = 5;  // the number of the turnout switch pin
const int homePin = 3;     // the number of the home sensor pin
const int loopPin = 4;     // the number of the loop sensor pin
const int DGPin =  9;      // the number of the distant green LED pin
const int DYPin = 10;      // the number of the distant yellow LED pin
const int HRPin = 12;      // the number of the home red LED pin
const int HGPin = 11;      // the number of the home green LED pin
// variables will change:
bool mainState = 0;         // variables for reading the main sensor status
bool turnoutState = 0;      //etc
bool homeState = 0;         //etc
bool loopState = 0;         //etc
bool mainValue = 0;

void setup()
{
  Serial.begin(9600);
  // initialize the LED pins as outputs:
  pinMode(DGPin, OUTPUT);  //Distant green = clear
  pinMode(DYPin, OUTPUT);  //Distant yellow = caution
  pinMode(HRPin, OUTPUT);  //Home red = STOP
  pinMode(HGPin, OUTPUT);  //home green
  // initialize the sensor pins as inputs:
  pinMode(mainPin, INPUT);
  pinMode(turnoutPin, INPUT);
  pinMode(homePin, INPUT);
  pinMode(loopPin, INPUT);
}
void loop()
{

  Serial.print(mainPin);
  // read the state of the sensor values:
  mainState = digitalRead(mainPin);
  Serial.print(mainState);
  delay(10);
  turnoutState = digitalRead(turnoutPin);
  Serial.println(turnoutState);
  delay(10);
  homeState = digitalRead(homePin);
  Serial.println(homeState);
  delay(10);
  loopState = digitalRead(loopPin);
  Serial.println(loopState);
  delay(10);

  // check the various sensor states
  // I will call this condition 1
  if ((mainState == HIGH) &&        //no train. This could be demeed as a default
      (turnoutState == HIGH) &&     //straight
      (homeState == HIGH))          //unoccupied
    //(loopState == DON'T CARE)
  {
    // turn appropriate LED's on or off
    digitalWrite(DGPin, LOW);     //distant green off
    digitalWrite(DYPin, HIGH);    //distant yellow on
    digitalWrite(HGPin, LOW);     //home green off
    digitalWrite(HRPin, HIGH);    //home red on
  }
  else if
  //condition 2)
  ((mainState == LOW) &&         //train on main
      (turnoutState == HIGH) &&  //turnout set straight
      (homeState == HIGH))       //no train on home
    //(loopState == DON'T CARE)
  {
    // then turn appropriate LED's on or off and train can carry on through:
    digitalWrite(DGPin, HIGH);  //on
    digitalWrite(DYPin, LOW);   //off
    digitalWrite(HGPin, HIGH);  //on
    digitalWrite(HRPin, LOW);   //off
  }
  else if
  //condition 3)
  ((mainState == LOW) &&         //train on main
      (turnoutState == HIGH) &&  //turnout set straight
      (homeState == LOW))        //occupied, train on home
    //(loopState == DON'T CARE)
  {
    // then turn appropriate LED's on or off and train must stop at home signal:
    digitalWrite(DGPin, LOW);   //of
    digitalWrite(DYPin, HIGH);  //on
    digitalWrite(HGPin, LOW);   //off
    digitalWrite(HRPin, HIGH);  //on
  }
  else if
  //condition 4)
  ((mainState == LOW) &&        //train on main
      (turnoutState == LOW) &&  //turnout set diverging
      // (homeState == DON"T CARE)
      (loopState == HIGH))      //no train on loop
  {
    // then turn appropriate LED's on or off and train can enter loop:
    digitalWrite(DGPin, LOW);   //off
    digitalWrite(DYPin, HIGH);  //on
    digitalWrite(HGPin, HIGH);  //on
    digitalWrite(HRPin, LOW);   //off
  }
  else 
  {
    // then turn appropriate LED's on or off and train can enter loop:
    digitalWrite(DGPin, LOW);    //off
    digitalWrite(DYPin, HIGH);   //on
    digitalWrite(HGPin, LOW);    //off
    digitalWrite(HRPin, HIGH);   //on
  }
}

TomGeorge:
Hi,
Can you post your final code please?

Thanks… Tom… :slight_smile:

TomGeorge:
HI,
Did you look at switch.. case function?

switch...case - Arduino Reference

Tom.... :slight_smile:

Yes Tom I had, but I seemed to comprehend the if..else function a lot easier.

Perhaps you could give me a heads up :slight_smile:

Daryl.

Hi,
Can you post a table showing the different combinations on inputs you can have and the respective outputs for each combination?

If your system will allow, each combination of inputs will be given a specific numeric value.
The "switch" then selects the appropriate "case" by using those values, the case has the code to select the appropriate outputs.

I am familiar with home and distant signals, a diagram of the placement of the points (Sorry GodsWonderfullRailway fan) and signals would help.

Good to see you have your code running.

Tom... :slight_smile:

Ab811:
Yes Tom I had, but I seemed to comprehend the if..else function a lot easier.

Perhaps you could give me a heads up :slight_smile:

Daryl.

Works like this:

Pick a number, win a prize: (number)

  the number is: 0
                   win a new car

  the number is: 1
                   win a cruise vacation

  the number is: 2
                   win a pie in the face

  the number is anything else:
                   nothing
switch (number) {

  case 0:
    // code to implement winning a new car
    break;

  case 1:
    // code to implement winning a cruise vacation
    break;

  case 2:
    // code to implement getting a pie in the face
    break;

  default:
    // get nothing
    break;
}