if /else controller

Hi All

i have written my first program to control a gear box using if/else statemtents, i used the 123d site to write it and test in virtual, worked well, so built the circuit in physical form, and it ran fine, then i thought i would be clever and put a oscilloscope on the outputs..... turns out all but one output is running at about 9khz, so by elimination i figured out that the first if statement is true, it turns the output on and moves on in the program, and then as the 2nd if statement is now false promptly turns the output off, i need to have various combinations of the outputs to control gearbox soleniods, i assume there is a statement or syntax i need to use to prevent this, or do i literally put a block/stop in the program till it is false and allow it to continue, the inputs cannot be all on at same time due to the input switching sensor. thinks the above makes sense, please be gentle took me 3 weeks to get here!!

Cheers

Pedro

/*
 GEAR    Sol A    Sol B

 4TH      OFF   OFF
 3RD      OFF   ON
 2ND      ON    ON    
 1ST      ON    OFF


 */
void setup () {
  pinMode(3, INPUT_PULLUP); //SWITCH 1ST GEAR
  pinMode(4, INPUT_PULLUP);//SWITCH 2ND GEAR
  pinMode(5, INPUT_PULLUP);// SWITCH 3RD GEAR
  pinMode(6, INPUT_PULLUP);//LOW RANGE/HIGH RANGE


  pinMode(7, OUTPUT);  //LOW/HIGH RANGE INDICATE
  pinMode(8 , OUTPUT); // red  (SOL A)
  pinMode(9 , OUTPUT); //green  (SOL B)
  pinMode(10 , OUTPUT); //BLUE  (1ST INDICATE LED)
  pinMode(11 , OUTPUT); //BLUE   (2ND INDICATE LED)
  pinMode(12 , OUTPUT); //BLUE   (3RD INDICATE LED)
  pinMode(13 , OUTPUT); //BLUE   (4TH INDICATE LED)
}

void loop () {
  if (digitalRead(3)== LOW && digitalRead( 6) == LOW) // 1ST LOW
  {
    digitalWrite(8, HIGH); // TURN ON SOL A
    digitalWrite(10, HIGH); // TURN ON 1ST INDICATE
  }
  else
  { 
    digitalWrite(8, LOW); // TURN OFF SOL A
    digitalWrite(10, LOW); // turn OFF 1ST INDICATE


    if (digitalRead(4)== LOW && digitalRead(6) == LOW)// 2ND LOW

    { 
      digitalWrite(8, HIGH); // TURN ON SOL A
      digitalWrite(9, HIGH); // TURN ON SOL B
      digitalWrite(11, HIGH); // TURN ON 2ND INDICATE
    }
    else

    {
      digitalWrite(8, LOW); // TURN OFF  SOL A
      digitalWrite(9, LOW); // TURN OFF SOL B 
      digitalWrite(11, LOW); // TURN OFF 2ND INDICATE
    }

    if (digitalRead(5)== LOW && digitalRead(6) == LOW) //3RD LOW 
    {   
      digitalWrite(9, HIGH); // TURN ON SOL B
      digitalWrite(12, HIGH); // TURN ON 3RD INDICATE
    }
    else
    {
      digitalWrite(9, LOW); // TURN OFF SOL B
      digitalWrite(12, LOW); // TURN OFF 3RD INDICATE


    }
    if (digitalRead(3)== LOW && digitalRead(6) == HIGH)// 2ND HIGH 

    { 
      digitalWrite(8, HIGH); // TURN ON SOL A
      digitalWrite(9, HIGH); // TURN ON SOL B
      digitalWrite(11, HIGH); // TURN ON 2ND INDICATE
    }
    else

    {
      digitalWrite(8, LOW); // TURN OFF  SOL A
      digitalWrite(9, LOW); // TURN OFF SOL B 
      digitalWrite(11, LOW); // TURN OFF 2ND INDICATE
    }

    if (digitalRead(4)== LOW && digitalRead(6) == HIGH) //3RD HIGH 
    {   
      digitalWrite(9, HIGH); // TURN ON SOL B
      digitalWrite(12, HIGH); // TURN ON 3RD INDICATE
    }
    else
    {
     digitalWrite(9, LOW); // TURN OFF SOL B
      digitalWrite(12, LOW); // TURN OFF 3RD INDICATE
    }

    if (digitalRead(5)== LOW && digitalRead(6) == HIGH) //4TH HIGH 
    {   
      digitalWrite(13, HIGH); // TURN ON 4TH INDICATE
    }
    else
    {
     digitalWrite(13, LOW); // TURN OFF 4TH INDICATE
    }
    if (digitalRead(6) == LOW) //4TH HIGH 
    {   
      digitalWrite(7, HIGH); // TURN ON HIGH/LOW INDICATE
    }
    else
    {
      digitalWrite(7, LOW); // TURN OFF HIGH/LOW INDICATE
    }
  }   

}

Please use [ code ] tags.

I'm not sure what you're trying to do. Just turn on a light to show what gear you're in? You are in the best position to work out the logical flow for that.

One way to structure your program is to separate the inputs, the calculations and the outputs. Read all of the input switches together at the top. Do some calculations, for example, work out what number gear it's in. Then the output section can do something with that to turn the correct lights on and off. Use functions for this.

void loop() {
  readInputs();
  doCalculations();
  setOutputs();
}

Hi

thanks for the reply, will work my way thru that, and yes there are indicator lights for what gear, but sola solb are soleniod for gear box selection, and sorry about the [ ] was not aware.

cheers

I'm not sure I 100% follow your program's logic but I think you made a fairly simple mistake with your curly braces.

After your first else block, there's no closing brace, so just about the whole of the rest of your sketch is inside the first if/else statement.

Here's what I mean - I added the brace and a couple of comments.

/*
 GEAR    Sol A    Sol B

 4TH      OFF   OFF
 3RD      OFF   ON
 2ND      ON    ON    
 1ST      ON    OFF


 */
void setup () {
  pinMode(3, INPUT_PULLUP); //SWITCH 1ST GEAR
  pinMode(4, INPUT_PULLUP);//SWITCH 2ND GEAR
  pinMode(5, INPUT_PULLUP);// SWITCH 3RD GEAR
  pinMode(6, INPUT_PULLUP);//LOW RANGE/HIGH RANGE


  pinMode(7, OUTPUT);  //LOW/HIGH RANGE INDICATE
  pinMode(8 , OUTPUT); // red  (SOL A)
  pinMode(9 , OUTPUT); //green  (SOL B)
  pinMode(10 , OUTPUT); //BLUE  (1ST INDICATE LED)
  pinMode(11 , OUTPUT); //BLUE   (2ND INDICATE LED)
  pinMode(12 , OUTPUT); //BLUE   (3RD INDICATE LED)
  pinMode(13 , OUTPUT); //BLUE   (4TH INDICATE LED)
}

void loop () {
  if (digitalRead(3)== LOW && digitalRead( 6) == LOW) // 1ST LOW
  {
    digitalWrite(8, HIGH); // TURN ON SOL A
    digitalWrite(10, HIGH); // TURN ON 1ST INDICATE
  }
  else
  { 
    digitalWrite(8, LOW); // TURN OFF SOL A
    digitalWrite(10, LOW); // turn OFF 1ST INDICATE
  }  //  *** this was missing I think ***

    if (digitalRead(4)== LOW && digitalRead(6) == LOW)// 2ND LOW

    { 
      digitalWrite(8, HIGH); // TURN ON SOL A
      digitalWrite(9, HIGH); // TURN ON SOL B
      digitalWrite(11, HIGH); // TURN ON 2ND INDICATE
    }
    else

    {
      digitalWrite(8, LOW); // TURN OFF  SOL A
      digitalWrite(9, LOW); // TURN OFF SOL B 
      digitalWrite(11, LOW); // TURN OFF 2ND INDICATE
    }

    if (digitalRead(5)== LOW && digitalRead(6) == LOW) //3RD LOW 
    {   
      digitalWrite(9, HIGH); // TURN ON SOL B
      digitalWrite(12, HIGH); // TURN ON 3RD INDICATE
    }
    else
    {
      digitalWrite(9, LOW); // TURN OFF SOL B
      digitalWrite(12, LOW); // TURN OFF 3RD INDICATE


    }
    if (digitalRead(3)== LOW && digitalRead(6) == HIGH)// 2ND HIGH 

    { 
      digitalWrite(8, HIGH); // TURN ON SOL A
      digitalWrite(9, HIGH); // TURN ON SOL B
      digitalWrite(11, HIGH); // TURN ON 2ND INDICATE
    }
    else

    {
      digitalWrite(8, LOW); // TURN OFF  SOL A
      digitalWrite(9, LOW); // TURN OFF SOL B 
      digitalWrite(11, LOW); // TURN OFF 2ND INDICATE
    }

    if (digitalRead(4)== LOW && digitalRead(6) == HIGH) //3RD HIGH 
    {   
      digitalWrite(9, HIGH); // TURN ON SOL B
      digitalWrite(12, HIGH); // TURN ON 3RD INDICATE
    }
    else
    {
     digitalWrite(9, LOW); // TURN OFF SOL B
      digitalWrite(12, LOW); // TURN OFF 3RD INDICATE
    }

    if (digitalRead(5)== LOW && digitalRead(6) == HIGH) //4TH HIGH 
    {   
      digitalWrite(13, HIGH); // TURN ON 4TH INDICATE
    }
    else
    {
     digitalWrite(13, LOW); // TURN OFF 4TH INDICATE
    }
    if (digitalRead(6) == LOW) //4TH HIGH 
    {   
      digitalWrite(7, HIGH); // TURN ON HIGH/LOW INDICATE
    }
    else
    {
      digitalWrite(7, LOW); // TURN OFF HIGH/LOW INDICATE
    }
 // }  *** If that was missing, you don't need this one ***

}

I like MorganS's idea to use functions. I do not like the names suggested, or the isolating reading input from acting on them. I would suggest having a function to see if you need to shift into first (and do so if you need to), to see if you need to shift into second (and do so if you need to), etc.

That way, you really can see the structure of the program, and don't (accidentally) nest the test for the need to shift into second, third, or fourth in the body of the shift into first code.

The same inputs get used many times. The low-range switch, for example. It's checked many many times in that provided code. When dealing with mechanical things, it is usually best to get a single picture of the state of the machine all at one time. Making decisions on the current state of 'low range' along with the state of the 'first' switch 10 milliseconds ago is likely to get you into trouble. Specifically, your first if() statement may find that it's not in low range and a later if() statement may find that it's not in high range either - what range is it in when it's not low and not high?

That's why it is best to read all the inputs together, then do the calculations.

The names do seem a little silly because I don't understand the point of the original program. I usually use slightly more descriptive names. However, I have written several very large programs where the main loop is less than 10 lines long. When there's that many different kinds of inputs to read, then readInputs() is actually a good description of what's happening.

pedronz: Hi

thanks for the reply, will work my way thru that, and yes there are indicator lights for what gear, but sola solb are soleniod for gear box selection, and sorry about the [ ] was not aware.

cheers

You can still edit your opening post ;)

Type ``` [code] ``` before the code. Type ``` [/code] ``` after the code.

So it looks like

your code here

hi again,

Well i looked at the reading inputs creating variables etc and promptly confused myself, i had a brain fart about 2 am over night, i have created the program using "if" statements only, and if the statement is true turning the required outputs on or off, ie got rid of the ELSE, there very likely a better way exists, and yes the names of the inputs are long and descriptive but it made it easy to write the code. i went thru and put the line comments on each line, found it easier as a beginner to keep track of what was happening,

it has removed the pulsing outputs which was the point of my query, the last IF statement turns every thing off apart from the LOW indicator which allows us to select reverse and park , this a a manualised Automatic, if the a gear change is required, then the driver selects it, and yes, you can select 1st gear if tapped out in 4th, be a expensive gear shift!!!

the Gearbox is in a competiton 4wd, 7.0 litre V8 in something that weighs approx 1200 kg.

 /*
 GEAR    Sol A    Sol B

 4TH      OFF   OFF
 3RD      OFF   ON
 2ND      ON    ON    
 1ST      ON    OFF


 */
void setup () {
  pinMode(3, INPUT_PULLUP); //SWITCH 1ST GEAR
  pinMode(4, INPUT_PULLUP);//SWITCH 2ND GEAR
  pinMode(5, INPUT_PULLUP);// SWITCH 3RD GEAR
  pinMode(6, INPUT_PULLUP);//LOW RANGE/HIGH RANGE

  pinMode(7, OUTPUT);  //LOW RANGE INDICATE
  pinMode(8 , OUTPUT); //(SOL A)
  pinMode(9 , OUTPUT); //(SOL B)
  pinMode(10 , OUTPUT); //BLUE  (1ST INDICATE LED)
  pinMode(11 , OUTPUT); //BLUE   (2ND INDICATE LED)
  pinMode(12 , OUTPUT); //BLUE   (3RD INDICATE LED)
  pinMode(13 , OUTPUT); //BLUE   (4TH INDICATE LED)
}

void loop () {
  if (digitalRead(3)== LOW && digitalRead( 6) == LOW) // 1ST LOW
  {
    digitalWrite(8, HIGH); // TURN ON SOL A
    digitalWrite(10, HIGH); // TURN ON 1ST INDICATE
    digitalWrite(9, LOW); // TURN OFF SOLB
    digitalWrite(11, LOW);// TURN OFF 2ND INDICATE
    digitalWrite(12, LOW); //TURN OFF 3RD INDICATE
    digitalWrite(13, LOW);// TURN OFF 4TH INDICATE

  }

  if (digitalRead(4)== LOW && digitalRead(6) == LOW)// 2ND LOW
  { 
    digitalWrite(8, HIGH); // TURN ON SOL A
    digitalWrite(9, HIGH); // TURN ON SOL B
    digitalWrite(11, HIGH); // TURN ON 2ND INDICATE
    digitalWrite(10, LOW);  //TURN OFF 1ST INDICATE
    digitalWrite(12, LOW); //TURN OFF 3RD INDICATE
    digitalWrite(13, LOW); //TURN OFF 4TH INDICATE
  }

  if (digitalRead(5)== LOW && digitalRead(6) == LOW) //3RD LOW 
  { 
    digitalWrite(9, HIGH); // TURN ON SOL B
    digitalWrite(12, HIGH); // TURN ON 3RD INDICATE
    digitalWrite(11, LOW); // TURN OFF 2ND INDICATE
    digitalWrite(8, LOW); //TURN OFF SOL A
    digitalWrite(10, LOW);//TURN OFF 1ST INDICATE
    digitalWrite(13, LOW);//TURN OFF 4TH INDICATE
  }

  if (digitalRead(3)== LOW && digitalRead(6) == HIGH)// 2ND HIGH 
  { 
    digitalWrite(8, HIGH); // TURN ON SOL A
    digitalWrite(9, HIGH); // TURN ON SOL B
    digitalWrite(11, HIGH); // TURN ON 2ND INDICATE
    digitalWrite(10, LOW);//TURN OFF 1ST INDICATE
    digitalWrite(12, LOW);//TURN OFF 3RD INDICATE
    digitalWrite(13, LOW);//TURN OFF 4TH INDICATE
  }

  if (digitalRead(4)== LOW && digitalRead(6) == HIGH) //3RD HIGH 
  {   
    digitalWrite(9, HIGH); // TURN ON SOL B
    digitalWrite(12, HIGH); // TURN ON 3RD INDICATE
    digitalWrite(8, LOW);// TURN OFF SOL A
    digitalWrite(10, LOW);  // TURN OFF 1ST INDICATE
    digitalWrite(11, LOW); // TURN OFF 2ND INDICATE
    digitalWrite(13, LOW);// TURN OFF 4TH INDICATE

  }
  if (digitalRead(5)== LOW && digitalRead(6) == HIGH) //4TH HIGH 
  {   
    digitalWrite(13, HIGH); // TURN ON 4TH INDICATE
    digitalWrite(8, LOW); // TURN OFF SOL A INDICATE
    digitalWrite(9, LOW);// TURN OFF SOL B INDICATE
    digitalWrite(10, LOW);// TURN OFF 1ST INDICATE
    digitalWrite(11, LOW);// TURN OFF 2ND INDICATE
    digitalWrite(12, LOW);// TURN OFF 3RD INDICATE
  }

  if (digitalRead(3) == HIGH && digitalRead(4) == HIGH && digitalRead(5) == HIGH&& digitalRead(7) == HIGH)
  {
    digitalWrite(8, LOW); // TURN FF SOL A
    digitalWrite(10, LOW); // TURN OFF 1ST INDICATE
    digitalWrite(9, LOW); // TURN OFF SOLB
    digitalWrite(11, LOW);// TURN OFF 2ND INDICATE
    digitalWrite(12, LOW); //TURN OFF 3RD INDICATE
    digitalWrite(13, LOW);// TURN OFF 4TH INDICATE
  }   
  if (digitalRead(6) == LOW)// LOW RANGE SELECT
  {
    digitalWrite(7, HIGH); //TURN ON LOW INDICATE 
  }
  else
  {
    digitalWrite(7, LOW);//TURN OFF LOW INDICATE
  }
}

and yes the names of the inputs are long and descriptive but it made it easy to write the code

You mean the names like 8, 9, and 10?

I really don't understand that statement, since, in the code you last posted, there are NO names other than those of functions that you did not write.