Go Down

Topic: Multiple sets of "if" statements? (Read 5 times) previous topic - next topic

ikestarm

I am trying to make a code where there are if statements and when you press a button, if some things are true, then a green light goes on. I also want it to turn a red light on when the same button is pressed but other things are true. So far, when I tried it it only followed the first set and totally disregarded the second one. Please help! :)

AWOL

#1
Oct 18, 2009, 06:06 pm Last Edit: Oct 18, 2009, 06:07 pm by AWOL Reason: 1
Show us the code.
We're not psychic!
"Pete, it's a fool looks for logic in the chambers of the human heart." Ulysses Everett McGill.
Do not send technical questions via personal messaging - they will be ignored.

jezuz

You probably want to do something like this:

if (something == true && something-else == true && ... )

The && stand for and, so you can create an if statement which will only execute something if all of a certain group of parameters are met.

ikestarm

Code: [Select]
  if (val == val2) {
    if (val != buttonstate) {
      if (val == LOW) {
        if (redvalue + 30 >= redVal) {
          if (redvalue - 30 <= redVal) {
            if (bluevalue + 30 >= blueVal) {
              if (bluevalue - 30 <= blueVal) {
                if (greenvalue + 30 >= greenVal) {
                  if (greenvalue - 30 <= greenVal) {
 
        digitalWrite(led, HIGH);
               
              if (val == val2) {
    if (val != buttonstate) {
      if (val == LOW) {
        if (redvalue + 30 <= redVal || redvalue - 30 >= redVal || greenvalue + 30 <= greenVal || greenvalue - 30 >= greenVal || bluevalue + 30 <= blueVal || bluevalue - 30 >= blueVal) {
        digitalWrite(ledtwo, HIGH);

This is the chunk of code I am trying to get to work, it only responds to the first part but does nothing with the second part.

jezuz


Goofballtech

I havn't been coding for very long (like 4 days ) but that loks really confusing to me, maybe it's because i don't see the rest or how the other variables are being derived i can't see the flow. any whoooo


IMHO it looks like you only want to turn on 'led' if all of the first statements are true.. in which case jezuz has it pegged, work it just like you set up the || in the bottom statement for 'ledtwo' but use && instead.

Just curious...

how can 'redvalue + 30' be >= to 'redVal' just befor you are testing to see if 'redvalue - 30' is <= to 'redVal' with nothing in between to change either of those values? same goes for the other values for each color and both LED's.... like i said maybe it's because i can't see the rest of the code but i can't seem to understand your flow.

Fjornir

#6
Oct 19, 2009, 04:53 am Last Edit: Oct 19, 2009, 04:55 am by Fjornir Reason: 1
Really quite hard to say given the fragment you posted: you omitted every closing brace... That said you can probably simplify a fair bit of the logic

Code: [Select]
  if (val == val2) {
    if (val != buttonstate) {


...is equivalent to
Code: [Select]

  if ( (val == val2) && (val != buttonstate)  ) {


...unless there is an else on that first if.

When you start nesting if()s that deeply then it is a good indication that you haven't thought your program's logic flow and structure through far enough.

For logic flow: go ahead and draw a flowchart of the desired behavior. If your logic really is that hairy then you'll be glad of the time spent doing so when you get to implementing. If it isn't then it will become apparent that you went overboard on the if() statements and you'll be glad of the time not spent troubleshooting that mess.

For program structure:

  • Pick USEFUL variable names.  
    Code: [Select]

    if (redvalue + 30 >= redVal) {



    This is quite meaningless -- why are you comparing redvalue to redVal? They're both red values! What the heck does this mean? How is anyone supposed to know how and why redvalue and redVal differ? It makes no sense.

  • Factor out repetitive portions and put them into functions

    Code: [Select]

            if (redvalue + 30 >= redVal) {
              if (redvalue - 30 <= redVal) {
                if (bluevalue + 30 >= blueVal) {
                  if (bluevalue - 30 <= blueVal) {
                    if (greenvalue + 30 >= greenVal) {
                      if (greenvalue - 30 <= greenVal) {


    I have no idea why you're doing these +/- 30 tests. The code doesn't express the meaning behind this. But something like...

    Code: [Select]

    int isValueInBounds( value, midPoint )
    {
      if( (value+30>=midPoint) && (value-30<=midPoint)
      {
          return true;
      }
      return false;
    }


    ...could be reused in the function you pasted like:

    Code: [Select]

    if( isValueInBounds( redvalue, redval )
    {
      // whatever you're supposed to do if the condition is met
    }



    Good luck!

ikestarm

I don't care what the values are named and about everything else. I just want to know how to make both the parts work because no matter what, led2 will never go on but if the criteria is met, the first led will.

mem

#8
Oct 21, 2009, 05:41 am Last Edit: Oct 21, 2009, 05:41 am by mem Reason: 1
Although it may be clear to you, its difficult to tell from the information you posted what you want your code to do.

Can you say in words what your application is and how you want your logic to work. Don't worry about the syntax, just describe the specific conditions that will cause the lights to go on and off.

Fjornir

#9
Oct 21, 2009, 07:43 am Last Edit: Oct 21, 2009, 07:43 am by Fjornir Reason: 1
Quote
I don't care what the values are named and about everything else. I just want to know how to make both the parts work because no matter what, led2 will never go on but if the criteria is met, the first led will.


Without the full code you're trying to get working and a thorough description of what you're trying to accomplish no one here is going to be able to help you. Based on the fragment you provided I offered some suggestions on how you might get to the solution you're looking for. If that's not enough to get you going then you have two real options. 1) Post full code and a description of what you want to do -or- 2) break the problem down into the smallest thing you don't know how to do, post full code and a description for that and ask a specific question about that.

But right now no one can help you because no one seems to understand what your problem is because you've asked broad and vague questions with fragmentary code to examine.

ikestarm

Okay, sorry guys. Here is the full code:

Code: [Select]
int ledtwo = 8;
int led = 7;

int redvalue;
int greenvalue;
int bluevalue;

int buttonstate;
int val;
int val2;
int switchpin = 2;

int redPin = 3;
int bluePin = 5;
int greenPin = 6;

int redIn = 2;
int greenIn = 3;
int blueIn = 4;

int redVal;
int greenVal;
int blueVal;

#ifndef RGBCommonLED_H
#define RGBCommonLED_H

#include "WProgram.h"


class RGBCommonLED {
public:
 RGBCommonLED(int rpin, int gpin, int bpin);
 ~RGBCommonLED();

 void setRGB(byte r, byte g, byte b);
 void setHSV(byte h, byte s, byte v);
 void setColor(unsigned long v);
 void setHue(byte level);
 void setSat(byte level);
 void setVal(byte level);

 void adjHue(int level);
 void adjSat(int level);
 void adjVal(int level);

 void setRandomColor();
 void setRandomHue();
 void setRandomSat();
 void setRandomVal();

 void setHSVLevel(int section, byte level);
 void adjHSVLevel(int section, int level);
 void setRandomHSVLevel(int section);

 void updateLED();

 void HSVToRGB( unsigned int inHue, unsigned int inSaturation, unsigned int inValue,
   unsigned int *oR, unsigned int *oG, unsigned int *oB );

 void RGBToHSV( unsigned int inRed, unsigned int inGreen, unsigned int inBlue,
   unsigned int *oH, unsigned int *oS, unsigned int *oV );


 void updateHSVFromRGB();


 int pins[3];

 int rgb[3];

 int hsv[3];

private:

 boolean hsvSet;
 void setRGBFromHSV();
 void setHSVFromRGB();
};

#endif



RGBCommonLED::RGBCommonLED(int rpin, int gpin, int bpin){
 pins[0] = rpin;
 pins[1] = gpin;
 pins[2] = bpin;
 hsvSet = false;
}


RGBCommonLED::~RGBCommonLED(){
}

void RGBCommonLED::setColor(unsigned long v)
{
 this->setRGB((v & 255),((v >> 8) & 255),(v >> 16) & 255);
}


void RGBCommonLED::setRGB(byte r, byte g, byte b){
 rgb[0] = r;
 rgb[1] = g;
 rgb[2] = b;
 hsvSet = false;
}

void RGBCommonLED::setHSV(byte h, byte s, byte v){
 hsv[0] = h;
 hsv[1] = s;
 hsv[2] = v;
 this->setRGBFromHSV();
 hsvSet = true;
}


void RGBCommonLED::setHSVLevel(int section, byte level){
 updateHSVFromRGB();
 hsv[section] = level;
 this->setRGBFromHSV();
}

void RGBCommonLED::setHue(byte level){
 this->setHSVLevel(0,level);
}

void RGBCommonLED::setSat(byte level){
 this->setHSVLevel(1,level);
}

void RGBCommonLED::setVal(byte level){
 this->setHSVLevel(2,level);
}

void RGBCommonLED::adjHSVLevel(int section, int level){
 updateHSVFromRGB();
 int th = hsv[section] + level;

 if( th < 0 ){
   th = 255 + th;
 }
 else if( th > 255 ) {
   th = 255 - th;
 }

 th = constrain(th,0,255);
 hsv[section] = th;
 this->setRGBFromHSV();
}

void RGBCommonLED::adjHue(int level){
 this->adjHSVLevel(0,level);
}

void RGBCommonLED::adjSat(int level){
 this->adjHSVLevel(1,level);
}

void RGBCommonLED::adjVal(int level){
 this->adjHSVLevel(2,level);
}



void RGBCommonLED::RGBToHSV( unsigned int inRed, unsigned int inGreen, unsigned int inBlue,
unsigned int *oH, unsigned int *oS, unsigned int *oV )
{
 double vals[3];
 unsigned char maxc=0, minc=0;
 double hue, sat, val;

 vals[0]=inRed;
 vals[1]=inGreen;
 vals[2]=inBlue;
 //red is set as maximum and minimum
 if(vals[1]>vals[maxc]) maxc=1;
 if(vals[2]>vals[maxc]) maxc=2;
 if(vals[1]<vals[minc]) minc=1;
 if(vals[2]<vals[minc]) minc=2;
 val = vals[maxc];
 if(vals[maxc]==0)
   sat = hue = 0;
 else
 {
   sat=255*(1-(vals[minc]/vals[maxc]));
   hue = 60 * ((maxc*2) + (vals[(maxc+1)%3] - vals[(maxc+2)%3])/(vals[maxc] - vals[minc]));
 }
 if(hue < 0) hue += 360; //corrector for hues in -60 to 0 range
 *oH = hue; //map(hue,0,360,0,255);
 *oS = sat;
 *oV = val;
}

void RGBCommonLED::HSVToRGB( unsigned int inHue, unsigned int inSaturation, unsigned int inValue,
unsigned int *oR, unsigned int *oG, unsigned int *oB )
{
 if( inSaturation == 0 )
 {
   // achromatic (grey)
   *oR = *oG = *oB = inValue;
 }
 else
 {
   unsigned int scaledHue = (inHue * 6);
   unsigned int sector = scaledHue >> 8; // sector 0 to 5 around the color wheel
   unsigned int offsetInSector = scaledHue - (sector << 8);      // position within the sector
   unsigned int p = (inValue * ( 255 - inSaturation )) >> 8;
   unsigned int q = (inValue * ( 255 - ((inSaturation * offsetInSector) >> 8) )) >> 8;
   unsigned int t = (inValue * ( 255 - ((inSaturation * ( 255 - offsetInSector )) >> 8) )) >> 8;

   switch( sector ) {
   case 0:
     *oR = inValue;
     *oG = t;
     *oB = p;
     break;
   case 1:
     *oR = q;
     *oG = inValue;
     *oB = p;
     break;
   case 2:
     *oR = p;
     *oG = inValue;
     *oB = t;
     break;
   case 3:
     *oR = p;
     *oG = q;
     *oB = inValue;
     break;
   case 4:
     *oR = t;
     *oG = p;
     *oB = inValue;
     break;
   default:            // case 5:
     *oR = inValue;
     *oG = p;
     *oB = q;
     break;
   }
 }
}

void RGBCommonLED::setRandomColor(){
 this->setColor((unsigned long)random(0x01000000));
}

void RGBCommonLED::setRandomHue(){
 this->setRandomHSVLevel(0);
}
void RGBCommonLED::setRandomSat(){
 this->setRandomHSVLevel(1);
}
void RGBCommonLED::setRandomVal(){
 this->setRandomHSVLevel(2);
}

void RGBCommonLED::setRandomHSVLevel(int section){
 this->setHSVLevel(section, (unsigned int)random(0x0100));
}


void RGBCommonLED::updateHSVFromRGB(){
 this->setHSVFromRGB();
 hsvSet = true;
}
void RGBCommonLED::updateLED(){
 analogWrite(this->pins[0], rgb[0]);
 analogWrite(this->pins[1], rgb[1]);
 analogWrite(this->pins[2], rgb[2]);
 
   Serial.print(" R - ");
Serial.print(rgb[0] , DEC);
Serial.print(" G - ");
Serial.print(rgb[1] , DEC);
Serial.print(" B - ");
Serial.println(rgb[2] , DEC);

 
 redvalue = rgb[0];
 greenvalue = rgb[1];
 bluevalue = rgb[2];


}

void RGBCommonLED::setRGBFromHSV(){
 unsigned int h = hsv[0];
 unsigned int s = hsv[1];
 unsigned int v = hsv[2];
 unsigned int r = 0;
 unsigned int g = 0;
 unsigned int b = 0;
 HSVToRGB(h,s,v,&r,&g,&b);
 this->setRGB(r,g,b);
}

void RGBCommonLED::setHSVFromRGB(){
 unsigned int r = rgb[0];
 unsigned int g = rgb[1];
 unsigned int b = rgb[2];
 unsigned int h;
 unsigned int s;
 unsigned int v;

 this->RGBToHSV(r,g,b,&h,&s,&v);
 hsv[0] = map(h,0,360,0,255);
 hsv[1] = s;
 hsv[2] = v;
 hsvSet = true;
 
}

RGBCommonLED led1(10,11,9);



void setup()
{
 pinMode(ledtwo, OUTPUT);
 pinMode(led, OUTPUT);
 pinMode(switchpin, INPUT);
 Serial.begin(9600);
 randomSeed(analogRead(0));

 led1.setRandomHue();
 led1.setSat(255);
 led1.setVal(255);
 led1.setRandomColor();
 led1.updateLED();
 
 buttonstate = digitalRead(switchpin);



}

void loop(){

  redVal = analogRead(redIn);
 greenVal = analogRead(greenIn);
 blueVal = analogRead(blueIn);

 
 redVal = map(redVal, 0, 1023, 0, 255);
 greenVal = map(greenVal, 0, 1023, 0, 255);
 blueVal = map(blueVal, 0, 1023, 0, 255);

 analogWrite(redPin, redVal);
 analogWrite(greenPin, greenVal);
 analogWrite(bluePin, blueVal);
 
redVal = 255 - redVal;
blueVal = 255 - blueVal;
greenVal = 255 - greenVal;

Serial.print("Red is -");
Serial.println(redVal, DEC);
Serial.print("Green is -");
Serial.println(greenVal, DEC);
Serial.print("Blue is -");
Serial.println(blueVal, DEC);

val = digitalRead(switchpin);
 delay(10);
 val2 = digitalRead(switchpin);
 
  if (val == val2) {
    if (val != buttonstate) {
      if (val == LOW) {
        if (redvalue + 30 >= redVal) {
          if (redvalue - 30 <= redVal) {
            if (bluevalue + 30 >= blueVal) {
              if (bluevalue - 30 <= blueVal) {
                if (greenvalue + 30 >= greenVal) {
                  if (greenvalue - 30 <= greenVal) {
 
        digitalWrite(led, HIGH);
               
              if (val == val2) {
    if (val != buttonstate) {
      if (val == LOW) {
        if (redvalue + 30 <= redVal || redvalue - 30 >= redVal || greenvalue + 30 <= greenVal || greenvalue - 30 >= greenVal || bluevalue + 30 <= blueVal || bluevalue - 30 >= blueVal) {
        digitalWrite(ledtwo, HIGH);
       
              }
            }
          }
        }
      }
    }
  }
            }
          }
        }
      }
    }
  }
}



It is long and sorry there are no comments but this is what it is for: I am trying to make a "game" of sorts where there are two rgb leds. One turns on to a random color. The other one, you have to try and make as close to the other one's color by using the 3 sliding potentiometers. There is a button to press that when pressed, will turn a red led(ledtwo) on if the values of the led are not within 30 "units". If the colors are close enough, the green led will turn on(led). The green led will turn on if the conditions are met, however, the red led wil not turn on no matter what. I know it's confusing but thank you for your help so far and I hope you can help me fix this. :)
P.S. the serial printing is just there for testing purposes, it has no role in the real sketch.

Fjornir

I think I understand what you're trying to do. I think that something like the following should get you on the right track.

Code: [Select]
void loop()
{
   int guessRed = analogRead(redIn);
   int guessBlue = analogRead(blueIn);
   int guessGreen = analogRead(greenIn);

   guessRed = map(guessRed, 0, 1023, 0, 255);
   guessGreen = map(guessGreen, 0, 1023, 0, 255);
   guessBlue = map(guessBlue, 0, 1023, 0, 255);

   analogWrite(redPin, guessGreen);
   analogWrite(greenPin, guessGreen);
   analogWrite(bluePin, guessGreen);


   int deltaRed = abs( guessRed-actualRed );
   int deltaGreen = abs( guessGreen-actualGreen );
   int deltaBlue = abs( guessBlue-actualBlue );

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

   if( (val==val2) && (val!=LOW) )
   {
       if( (deltaRed<=30) && (deltaGreen<=30) && (deltaBlue<=30) )
       {
           // Do what you want to do if the guess was right and in bounds
       }
       {
           // Do what you want to do if the guess was wrong and out of bounds
       }
   }
}

mattd2

#12
Oct 27, 2009, 10:24 pm Last Edit: Oct 27, 2009, 10:30 pm by mattd Reason: 1
Hi,
I don't have much experience in coding and I don't even have an Arduino yet but one question I have on this code is;
Could you use an else statement since as I understand it if all the conditions are correct you want to light the "correct" led ELSE you want to light the "incorrect" led indicating a wrong guess. Would this work and if not can someone correct me.
As I said I have limited experience in coding (mechanical engineering mantra if you can't physically see something working it must be some kind of witchcraft/magic  ;) )

Edit: Just read that last code, and it looks like my comments might not have been needed. But to clear something up, is the "else" implied as part of the "if" statement?
if(something == something})
{do this}
{else do this}

Coding Badly

Quote
is the "else" implied as part of the "if" statement?


Not in the language of the Arduino (C++).  This is how your pseudo-code works...

if(something1 == something2)
{then do this}
{always do this}

Your "else do this" is only an else-clause if the word else is included...

if(something1 == something2)
{then do this}
else
{otherwise do this}

Which leads nicely to a rule you should follow: Always include braces with 'if' statements.  In other words, write your if statements like this...

if(something1 == something2)
{
 then_do_this();
}
else
{
 otherwise_do_this();
}

...not like this...

if(something1 == something2)
 then_do_this();
else
 otherwise_do_this();

AlphaBeta

#14
Oct 27, 2009, 11:28 pm Last Edit: Oct 27, 2009, 11:30 pm by AlphaBeta Reason: 1
Quote
Which leads nicely to a rule you should follow: Always include braces with 'if' statements.


Coding Badly gives advice on how to code nicely  ;D
I just had to comment on the irony of that. ;)

[edit]I completly agree with Coding Badly about the rule by the way.[/edit]

Go Up