Error says i need more } before "else", can't get it right

system · January 15, 2014, 7:26pm

Im making a very simple program to make a motor spin when i push a button and depending on 2 other buttons to figure out witch way to go. I'll be using an arduino nano and a L293D IC.

What i want the code to do:When i press the button knapp it's going to debounce the input, the check if it was really pressed and then go on and see which of the "kontroll" buttons is pressed and then spin the motor.

How does it look, is the code going to do what i want it to do? see attached pic for schematic and error.

"knapp" = button
"kontroll" = check where it is

int motorPin1 = 10;
int motorPin2 = 9;
int knappPin = 8;
int kontrollPin1 = 7;
int kontrollPin2 = 6;
boolean kontroll1 = HIGH;
boolean kontroll2 = HIGH;
boolean knapp = HIGH;


int val;
int val2;



void setup()
{
  pinMode(motorPin1, OUTPUT);
  pinMode(motorPin2, OUTPUT);
  pinMode(knappPin, INPUT_PULLUP);
  pinMode(kontrollPin1, INPUT_PULLUP);
  pinMode(kontrollPin2, INPUT_PULLUP);
  knapp = digitalRead(knappPin);
}






void loop(){
  kontroll1 = digitalRead(kontrollPin1);
  kontroll2 = digitalRead(kontrollPin2);
  val = digitalRead(knappPin);
  delay(10);
  val2 = digitalRead(knappPin);
  if (val == val2) {
    if (val != knapp) {
      if (val == LOW) {
        if (kontroll1 == LOW) {
          while (digitalRead(kontrollPin1) == LOW) {
            digitalWrite(motorPin1, HIGH);}
          else {
            digitalWrite(motorPin1, LOW);}
        if (kontroll2 == LOW) {
          while (digitalRead(kontrollPin2) == LOW) {
            digitalWrite(motorPin2, HIGH);}
          else {
            digitalWrite(motorPin2 , LOW);}}
        
      }
    }      
  }         
}

HazardsMind · January 15, 2014, 7:39pm

You need a closing bracket } for your if statement "kontroll1 == LOW". It would help you and us greatly if you formatted your code. Press CTRL+T to format your code.

system · January 15, 2014, 7:42pm

Ok, added 2 } ant pressed CTRL+T and it went ballistic. Here is the new code, i also realised a small error on my part.
I can see now something is clearly wrong but i dont know how to fix it.

int motorPin1 = 10;
int motorPin2 = 9;
int knappPin = 8;
int kontrollPin1 = 7;
int kontrollPin2 = 6;
boolean kontroll1 = HIGH;
boolean kontroll2 = HIGH;
boolean knapp = HIGH;


int val;
int val2;



void setup()
{
  pinMode(motorPin1, OUTPUT);
  pinMode(motorPin2, OUTPUT);
  pinMode(knappPin, INPUT_PULLUP);
  pinMode(kontrollPin1, INPUT_PULLUP);
  pinMode(kontrollPin2, INPUT_PULLUP);
  knapp = digitalRead(knappPin);
}






void loop(){
  kontroll1 = digitalRead(kontrollPin1);
  kontroll2 = digitalRead(kontrollPin2);
  val = digitalRead(knappPin);
  delay(10);
  val2 = digitalRead(knappPin);
  if (val == val2) {
    if (val != knapp) {
      if (val == LOW) {
        if (kontroll1 == LOW) {
          while (digitalRead(kontrollPin2) != LOW) {
            digitalWrite(motorPin1, HIGH);
          }
else {
  digitalWrite(motorPin1, LOW);
}
if (kontroll2 == LOW) {
  while (digitalRead(kontrollPin1) != LOW) {
    digitalWrite(motorPin2, HIGH);
  }
else {
  digitalWrite(motorPin2 , LOW);
}
}

}
}      
}         
}
}

robtillaart · January 15, 2014, 7:46pm

this is why I use this style, you can far more easily see the matching { }

if (condition)
{
   // do something
}
else
{
  // something else
}

this resulted in (with some added {} )

const int motorPin1 = 10;
const int motorPin2 = 9;
const int knappPin = 8;

const int kontrollPin1 = 7;
const int kontrollPin2 = 6;

boolean kontroll1 = HIGH;
boolean kontroll2 = HIGH;
boolean knapp = HIGH;

int val;
int val2;

void setup()
{
  pinMode(motorPin1, OUTPUT);
  pinMode(motorPin2, OUTPUT);
  pinMode(knappPin, INPUT_PULLUP);
  pinMode(kontrollPin1, INPUT_PULLUP);
  pinMode(kontrollPin2, INPUT_PULLUP);

  knapp = digitalRead(knappPin);
}


void loop()
{
  kontroll1 = digitalRead(kontrollPin1);
  kontroll2 = digitalRead(kontrollPin2);

  val = digitalRead(knappPin);
  delay(10);
  val2 = digitalRead(knappPin);

  if (val == val2) 
  {
    if (val != knapp) 
    {
      if (val == LOW) 
      {
        if (kontroll1 == LOW) 
        {
          while (digitalRead(kontrollPin1) == LOW) 
          {
            digitalWrite(motorPin1, HIGH);
          }
        }
        else 
        {
          digitalWrite(motorPin1, LOW);
        }
        if (kontroll2 == LOW) 
        {
          while (digitalRead(kontrollPin2) == LOW) 
          {
            digitalWrite(motorPin2, HIGH);
          }
        }
        else 
        {
          digitalWrite(motorPin2 , LOW);
        }
      }
    }
  }         
}

the problem was that you invented a new statement: a marriage of the while and if then else

while (condition)
{
  // do something;
}
else
{
  // something else
}

system · January 15, 2014, 7:59pm

Im just gonna steal the code directly! Thank you so much! I have one more question. In a while loop(?), if i set digitalWrite an output as HIGH, will it go back down to LOW when the while loop is no longer active? So maybe i could remove the:

Else 
{
 digitalWrite(motorPin, LOW)
}

and again, thanks!

system · January 15, 2014, 8:04pm

if i set digitalWrite an output as HIGH, will it go back down to LOW when the while loop is no longer active?

No, it won't. It won't go LOW again until you explicitly tell it to. Which means that setting it HIGH over and over in the while body is useless. Set it once, before the while statement, and leave the body empty.

system · January 15, 2014, 9:12pm

But it will still work without damaging the Arduino? Also, how would i put the digitalWrite before the while loop?

system · January 16, 2014, 1:33pm

But it will still work without damaging the Arduino?

Yes.

Also, how would i put the digitalWrite before the while loop?

Seriously?

Change:

          while (digitalRead(kontrollPin1) == LOW) 
          {
            digitalWrite(motorPin1, HIGH);
          }

to:

          digitalWrite(motorPin1, HIGH);
          while (digitalRead(kontrollPin1) == LOW) 
          {
          }

system · January 16, 2014, 2:03pm

So the while would work like a pause in the program? so i would just take it back down to low again after the while statement?

digitalWrite(motorPin1, HIGH);
while (digitalRead(kontrollPin1) == LOW)
{
}
else
{
digitalWrite(motorPin1, LOW);
}

wildbill · January 16, 2014, 2:32pm

While doesn't have an else clause, as the compiler should be telling you. All you need is:

digitalWrite(motorPin1, HIGH);
while (digitalRead(kontrollPin1) == LOW)
{
}
digitalWrite(motorPin1, LOW);

Grumpy_Mike · January 16, 2014, 4:46pm

If you click your cursor just after the { in the IDE the matching } will be highlighted. If no match is highlighted or the wrong one is then you know you have something wrong with your code.

krupski · January 16, 2014, 7:56pm

wildiness:
Ok, added 2 } ant pressed CTRL+T and it went ballistic.

I heartily suggest using a REAL editor to work on your source code and simply use the Arduino IDE as the "compiler".

If you use Windows, I suggest Edit Pad Pro ( http://www.editpadpro.com ) as the editor. It has syntax highlighting, regular expressions, code templates and what you need now... matching bracket highlighting!

I stuck your code into EPP and immediately saw the problem... the missing brace that everyone told you about.

Check out EPP. You can download a trial for free and see if you like. I'll bet that you love it.

Good luck.

(note: I am in no way affiliated with Editpad Pro and my post is not advertising)

krupski · January 16, 2014, 7:58pm

wildbill:
While doesn't have an else clause, as the compiler should be telling you. All you need is:
digitalWrite(motorPin1, HIGH);

while (digitalRead(kontrollPin1) == LOW)
{
}
digitalWrite(motorPin1, LOW);

You don't even need the braces.

while (digitalRead(kontrollPin1) == LOW);
//-- or --
while (!digitalRead(kontrollPin1));

is just fine.

Delta_G · January 16, 2014, 10:32pm

But do take note that the while line without the braces gets a semicolon while with the braces it does not.

system · January 16, 2014, 11:05pm

Delta_G:
But do take note that the while line without the braces gets a semicolon while with the braces it does not.

And the braces give you a place to put a comment that says something like

// Yes, I know this does nothing. That's what I want here.

so that later you don't spend an hour scratching your head wondering where the code that belonged in the block was missing.

krupski · January 17, 2014, 1:54pm

PaulS:

Delta_G:
But do take note that the while line without the braces gets a semicolon while with the braces it does not.

And the braces give you a place to put a comment that says something like
// Yes, I know this does nothing. That's what I want here.
so that later you don't spend an hour scratching your head wondering where the code that belonged in the block was missing.

Well then you could do this:

while (digitalRead(kontrollPin1) == LOW) {;} // this is not a typo. There really isn't any code being run while I wait for the control pin to go high. Just so we're clear on that. It's not a coding error. OK? Are we sure? Good.

UKHeliBob · January 17, 2014, 3:57pm

while (digitalRead(kontrollPin1) == LOW) {;}

YUCK.
That looks horrible (to me), but that is a personal opinion, of course.
In my world braces are always present, even if not required, and they each go on their own line.

westfw · January 17, 2014, 5:03pm

I'm not sure whether anyone explained your initial mistake in sufficient detail. In this code segment:

        if (kontroll1 == LOW) {
          while (digitalRead(kontrollPin1) == LOW) {
            digitalWrite(motorPin1, HIGH);}
          else {
            digitalWrite(motorPin1, LOW);}

You are missing the closing brace for the "if" statement. You have:

        if () { // start if
          while () {  // start while
            } // end while

// !!! Missing } end if !!!!

          else {  // start else
            } //end else

system · May 5, 2021, 8:29pm