Go Down

Topic: error with "if" statment (Read 990 times) previous topic - next topic

mabvs

Sorry for the newbish question here, but I am new to C programming, as well as the arduino.

So, I am working on trying to get the Night Rider effect with 8 LEDs (dig outs 2-9). I want to control the speed with a POT. Now, I was able to do so, with calling each LED in sequence (2,3,4,5,6,7,8,9,8,7,6,5,4,3 loop). SO, I started trying to clean up the code a bit with a couple nested if statements.
Code: [Select]
int sensorPin = A0;
int ledPin = 2;
int sensorValue = 0;
int reverse = 0;

void setup() {
  pinMode(2, OUTPUT);
  pinMode(3, OUTPUT);
  pinMode(4, OUTPUT);
  pinMode(5, OUTPUT);
  pinMode(6, OUTPUT);
  pinMode(7, OUTPUT);
  pinMode(8, OUTPUT);
  pinMode(9, OUTPUT);   

}

void loop() {
  if   (reverse = 0); { // count up
       digitalWrite(ledPin, HIGH);
      sensorValue = analogRead(sensorPin);
      delay(sensorValue);
      digitalWrite(ledPin, LOW);
      sensorValue = analogRead(sensorPin);
      delay(sensorValue);
          if (ledPin = 9) {
            reverse = 1;  }
          else ledPin = (ledPin + 1);
  }
[u][b] else // count down[/b][/u]
      digitalWrite(ledPin, HIGH);
      sensorValue = analogRead(sensorPin);
      delay(sensorValue);
      digitalWrite(ledPin, LOW);
      sensorValue = analogRead(sensorPin);
      delay(sensorValue);
          if (ledPin = 2) {
            reverse = 0;  }
          else ledPin = (ledPin - 1);
 


However, I am getting
Quote
sketch_jun30b.cpp: In function 'void loop()':
sketch_jun30b:29: error: 'else' without a previous 'if'
with the bolded underlined portion of code being pointed out.  Why isn't that "else" associated with the very 1st "if"?

liudr

Code: [Select]
else // count down[/b][/u]
      digitalWrite(ledPin, HIGH);
      sensorValue = analogRead(sensorPin);
      delay(sensorValue);
      digitalWrite(ledPin, LOW);
      sensorValue = analogRead(sensorPin);
      delay(sensorValue);
          if (ledPin = 2) {
            reverse = 0;  }
          else ledPin = (ledPin - 1);


The else is only coupled with digitalWrite(ledPin,HIGH);

Thought you may want to know that. Your first if structure indicates everything after the else should be inside {} but you didn't put them  in.

Plus, all if statements are wrong, at least not giving you what you want, use "==" instead of "=" if you compare values like if (ledPin==2) not if (ledPin=2).

Fix the above possible mistake and see if you get what you want.

montecito

Remove the semicolon after the "if" condition.

Code: [Select]

  if   (reverse = 0); { // count up


mabvs

Cool.. I'll give that a go and check it out. Thank you.. I'll update success of failure  :)

mabvs

Well, removing the ";" fixed the missing if error, and I changed to the double "=", but it wont move past pin 2. My "count up" is failing.. back to the drawing board.

montecito

#5
Jul 01, 2011, 05:00 am Last Edit: Jul 01, 2011, 05:01 am by montecito Reason: 1
Can you post your code properly formatted ??  Get rid of the annotations in brackets and write the "if/else" conditions in a neater style such as this:

Code: [Select]

if (cond) {
  doSomething();
} else {
 doSomethingElse();
}


I have a hard time parsing your code and understanding what it is trying to do.

mabvs

Okay.

Code: [Select]
int sensorPin = A0;
int ledPin = 2;
int sensorValue = 0;
int reverse = 0;

void setup() {
  pinMode(2, OUTPUT);
  pinMode(3, OUTPUT);
  pinMode(4, OUTPUT);
  pinMode(5, OUTPUT);
  pinMode(6, OUTPUT);
  pinMode(7, OUTPUT);
  pinMode(8, OUTPUT);
  pinMode(9, OUTPUT);   

}

void loop() {
  if   (reverse = 0) { // count up
      digitalWrite(ledPin, HIGH);
      sensorValue = analogRead(sensorPin);
      delay(sensorValue);
      digitalWrite(ledPin, LOW);
      sensorValue = analogRead(sensorPin);
      delay(sensorValue);
          if (ledPin = 9) {
            reverse = 1; 
          } else {
            ledPin = (ledPin+1);
          }
 
  } else { // count down
      digitalWrite(ledPin, HIGH);
      sensorValue = analogRead(sensorPin);
      delay(sensorValue);
      digitalWrite(ledPin, LOW);
      sensorValue = analogRead(sensorPin);
      delay(sensorValue);
          if (ledPin = 2) {
            reverse = 0; 
          } else {
            ledPin = (ledPin-1);
          }
 
  }
}

montecito

This is a problem:

Code: [Select]

          if (ledPin = 2) {


Replace with

Code: [Select]

          if (ledPin == 2) {



I don't know if that will fix everything but it's an obvious problem.

mabvs

Yeah, I reverted back trying to debug. As soon as I changed it back to "==", all is working as desired. Thank you all for your help  :)

montecito

This too:

Code: [Select]

          if (ledPin = 9) {


Comparisons are == not = as the other poster pointed out.  Check all our "if" statements for this error.

liudr


Yeah, I reverted back trying to debug. As soon as I changed it back to "==", all is working as desired. Thank you all for your help  :)


There is no reason to revert. The single equal is value assignment, double is comparison. I suggest you learn the operators && and ++. Your code may work but is very bloated without proper use of them.

FalconFour

A little explanation of "=" vs "==" is in order...

"=" means "assign", as in "a=2" -> "set a to 2". This compiles properly, because "a=2" also passes-through the value "2", so things like "a = b = c = d = 2" are possible, setting a, b, c, and d to "2" by referencing each other.

"==" means "check if equal". It returns "1" if they are the same, and "0" if they aren't the same. Basically.

So if you want to check if two things are the same, use "==". And if you want to "assign and check" - like, "if (x = buttonCheck())", which would mean that "x" is set to the value returned by buttonCheck() and the loop would run if the value is non-zero (e.g. the function returns 0 if nothing is pressed, otherwise it returns the button that was pressed), but not have to run buttonCheck() a second time.

Go Up