Go Down

Topic: code not working, please help! (Read 725 times) previous topic - next topic

big93

rawr, i'm trying to use if statements, and its not working, i looked at my code and it seems to make sense!

i'm using a potentiometer and dividing the total number by 3 to make 3 equal parts of 341, and making 341 mode 1, 682 mode 2 and 1023 mode 3.

but when i tell it to print in mode, it always stays at mode 3!

i used less than , and then i used less than or equal to for 1023, now please tell me why mode is stuck on 3!

p.s, i cant get the else code to work after the if statement, it tells me i need a primary expression before it, and it doesent say what primary expression to use!! w/e for now, heres the messed up code :

Code: [Select]
int value = 0;                          
int ledpin = 9;                          
int pot = 0;
int pot1= 0;
int mode = 0;
void setup()
{
 Serial.begin(9600);
 
 
}

void loop()
{
 if (pot1 < 341);
  mode = 1;


 if (pot1 < 682);
  mode = 2;


 if (pot <= 1023);
  mode = 3;


pot1 = analogRead(1);
pot = analogRead(2) / 4;  
Serial.println(pot1);
 analogWrite(ledpin, pot);

 
}



please tell me why my code is broken, it doesent make sense to me!

thanks in advance,
-big93

westfw

ALL of your if statements get executed.  So you read in 10, and the first if statement says "if (10 < 341) mode = 1", and 10 is less than that, so it sets mode to 1.  Then you get to "if (10 < 682)", and pot1 is still 10, so that's STILL true, and mode gets set to 2.  Then you get to "if (10 < 1023)", and that's true too, so mode gets set to 3.  You also have extra semicolons on the "if" lines, but I assume that's a recent bug, since as written here I would think mode would always be zero.

The easiest fix without adding complexity (like "else") is to simply change the order:

Code: [Select]
if (pot1 < 1023)
 mode = 3;
if (pot1 < 682)
 mode = 2;
if (pot1 < 341)
 mode = 1;

wurx

Westfw-

I instantly noticed that all the "if" statements were executed and were returning a virtual "yes".
Then I thought use "else".... that is the easiest fix.

WRONG!

Your solution is great! I learned something new, thanks!


big93

thank you, ill test the better code now, and what you said about the bug, is not true, i put thiose on cuz i thought they go there, but now i know!!

thanks allot!

big93

#4
Dec 16, 2007, 05:48 pm Last Edit: Dec 16, 2007, 05:56 pm by big93 Reason: 1
....... damnit, code not working again, can you take a look again, i really dont understand why it's acting funny.

ok, i made it so modes are now controlling individual leds, but when i try to dimm and brighten the led in one mode, all the leds brighten and dimm....

Code: [Select]
int value = 0;                          
int ledpin3 = 11;
int ledpin2 = 10;
int ledpin1 = 9;                          
int pot = 0;
int pot1= 0;
int mode = 0;
void setup()
{
 Serial.begin(9600);
 
 
}

void loop()
{
if (pot1 < 1023)
 mode = 3;
if (pot1 < 682)
 mode = 2;
if (pot1 < 341)
 mode = 1;
pot1 = analogRead(1);
pot = analogRead(2) / 4;  
Serial.println(mode);
 if (mode = 3)
  analogWrite(ledpin3, pot);
  else
  analogWrite(ledpin3, 0);
 if (mode = 2)
  analogWrite(ledpin2, pot);
  else
  analogWrite(ledpin2, 0);
 if (mode = 1)
  analogWrite(ledpin1, pot);
  else
  analogWrite(ledpin1, 0);
 
 

 
}


thanks again
-big93

p.s, why does it ask me for a primary expression before the freaken "else" statement sometimes?

mem

if you change:
 mode = 3
to
 mode == 3  // note double equals sign
in all lines where you want to test for equality it should work.

This is a very common error in c.  if(var = x) will  return x (it is assigning the value of x to var), you want if(var == x) which will return true if var equals x

big93

oh! thats why, i forgot you have to do two equals if ur not using a greater or less then with it... lol

thanks mem

follower

Hi...

First, a comment on coding style that may be helpful and save you unnecessary trouble in the future...

Rather than writing your code like this:
Quote
Code: [Select]

 if (mode == 3) // Corrected, as per suggestion in earlier post
  analogWrite(ledpin3, pot);
  else
  analogWrite(ledpin3, 0);

I strongly recommend writing such code using braces such as:
Code: [Select]

 if (mode == 3) { // Corrected, as per suggestion in earlier post
   analogWrite(ledpin3, pot);
 } else {
   analogWrite(ledpin3, 0);
 }


Doing this now means you'll never do the following and wonder why the second function is always called:
Code: [Select]

if (mode ==3)
 doSomething();
 doSomethingElse();


Quote
p.s, why does it ask me for a primary expression before the freaken "else" statement sometimes?

Some example code that causes this problem would be useful in diagnosing what causes it...

--Phil.

P.S. I seem to recall you mentioning in an earlier post that you are currently at school, if you're wanting to get into software development using C/C++ as a career (or just to make you hobby code better) I strongly suggest you (and anyone else) read the book Code Complete. Maybe not now, but certainly in the future as it includes good coding style suggestions and better to learn good habits early. Maybe see if your local library has a copy. ("Writing Solid Code" is also worth reading for similar reasons--in fact it might even be a better, more code-focussed starter--it's been a while since I've read them. :-) ) In my experience any book by either of the "two Steves" (no, not the Apple ones... :-) ) is worthy reading for software developers.

big93

those look like good books, and i also solved the primary expression things by adding the " }{ " things, lol, thanks!

-big93

charlieb

Also for the whole = vs == issue if you put the constant first in the expression then the compiler will catch your mistakes:

"if( 3 = val )" is a syntax error while "if( val = 3 )" is not and nor is "if( 3 == val )" and I think readability is pretty much the same.

Charlieb

Go Up