combine multiple IF statements

Hello,
is there a better way to do some IF statements than this?

void get_btn_state()	//check inputs
{	
	if (digitalRead(2)==LOW)	//pin 2, button 0
	{
		bitSet(button,(0));
	}
	else
	{
		bitClear(button,(0));
	}
	
	if (digitalRead(13)==LOW)	//pin 13, button 1
	{
		bitSet(button,(1));
	}
	else
	{
		bitClear(button,(1));
	}
	
	if (digitalRead(14)==LOW)	//pin 14, button 2
	{
		bitSet(button,(2));
	}
	else
	{
		bitClear(button,(2));
	}
	
	if (digitalRead(15)==LOW)	//pin 15, button 3
	{
		bitSet(button,(3));
	}
	else
	{
		bitClear(button,(3));
	}
	
	if (digitalRead(16)==LOW)	//pin 16, button 4
	{
		bitSet(button,(4));
	}
	else
	{
		bitClear(button,(4));
	}
	
	
}

i tried a for loop before

for (int i=13;i<18;i++)
{
	if (digitalRead(i)==LOW)	
	{
		bitSet(button,(i));
	}
	else
	{
		bitClear(button,(i));
	}
}

but with the 2. pin it messes all up…
so i did it one by one.

In the second snippet you are using bitSet() and bitClear() with bits 13 to 18 of a variable. How many bits does the button variable have ?

You could subtract 13 from the number used in the bit functions if the pins being read were contiguous. As they are not, and even if they were, put the pin numbers in an array and iterate through them with a for loop then you can read any pin in any order and the for loop variable can be used to manipulate the appropriate bit of the button variable.

I'm confused. Your first example is testing is testing the return values from a digitalRead() on pins 2, 13, 14, 15, 16 and changing bits 0 - 4. However, the second loop seems to be using digitalRead() on pins 13 - 18 and bits 13-18. Bits 13 through 18?? How does that work?

If would seem to me that it would be easier to use something like:

void get_btn_state()    //check inputs
{   
     CheckAndSet( 2, 0);
     CheckAndSet(13, 1);
     CheckAndSet(14, 2);
     CheckAndSet(15, 3);
     CheckAndSet(16, 4);


}
void CheckAndSet(int pinToRead, int bitToSet)
{
    if (digitalRead(pinToRead)==LOW)
    {
        bitSet(button, bitToSet);
    }
    else
    {
        bitClear(button, bitToSet);
    }

}

UKHeliBob:
In the second snippet you are using bitSet() and bitClear() with bits 13 to 18 of a variable. How many bits does the button variable have ?
You could subtract 13 from the number used in the bit functions if the pins being read were contiguous

oh dang! yes i did that in my code… i just forgot to write it down here :frowning:
the button variable has 8 bits (uint8_t)

my problem was when i used the for loop, i had to go from 2 to 18, but leave out the part from 3 to 12. because they are already used by a motor shield.

so how would an array for this purpose look like?

like this?

button[0]=2;
button[1]=13;
button[2]=14;
etc..

econjack: I'm confused. Your first example is testing is testing the return values from a digitalRead() on pins 2, 13, 14, 15, 16 and changing bits 0 - 4. However, the second loop seems to be using digitalRead() on pins 13 - 18 and bits 13-18. Bits 13 through 18?? How does that work?

If would seem to me that it would be easier to use something like:

void get_btn_state() //check inputs
{ 
     CheckAndSet( 2, 0);
     CheckAndSet(13, 1);
     CheckAndSet(14, 2);
     CheckAndSet(15, 3);
     CheckAndSet(16, 4);

} void CheckAndSet(int pinToRead, int bitToSet) { if (digitalRead(pinToRead)==LOW) { bitSet(button, bitToSet); } else { bitClear(button, bitToSet); }

}

well that's exactly what i was looking for!

thank you very much

Well, actually it's not exactly what you want, since I forgot to pass button to the function. While your code uses the same button and bit, the function would be more reusable if you pass in button.

void get_btn_state() //check inputs
{ 
     CheckAndSet( 2, 0, 0);
     CheckAndSet(13, 1, 1);
     CheckAndSet(14, 2, 2);
     CheckAndSet(15, 3, 3);
     CheckAndSet(16, 4, 4);


}
void CheckAndSet(int pinToRead, int bitToSet, int whichButton)
{
    if (digitalRead(pinToRead)==LOW)
    {
       bitSet(whichButton, bitToSet);
    } else {
       bitClear(whichButton, bitToSet);
    }

}

so how would an array for this purpose look like?

const byte buttonPins[] = {2, 13, 15, 14, 16};
const byte NUMBER_OF_BUTTONS = sizeof(buttonPins) / sizeof(buttonPins[0]);
int button = 0;

void setup()
{
  for (int b = 0; b < NUMBER_OF_BUTTONS; b++)
  {
    pinMode(b, INPUT_PULLUP);
  }
}

void loop()
{
  for (int b = 0; b < NUMBER_OF_BUTTONS; b++)
  {
    bitSet(button, digitalRead(buttonPins[b]));
  }
  //do something with the value of the button variable
}

In your code:

  for (int b = 0; b < NUMBER_OF_BUTTONS; b++)
  {
    pinMode(b, INPUT_PULLUP);
  }

does he really want to set pins 0 through 4??

i think the code has to look like this since the functionis: bitSet(var,bit)

     void get_btn_state() //check inputs
{
     CheckAndSet( 2, 0, button);
     CheckAndSet(13, 1, button);
     CheckAndSet(14, 2, button);
     CheckAndSet(15, 3, button);
     CheckAndSet(16, 4, button);

 
}
void CheckAndSet(int pinToRead, int bitToSet, int whichVariable)
{
    if (digitalRead(pinToRead)==LOW)
    {
       bitSet(whichVariable, bitToSet);
    } else {
       bitClear(whichVariable, bitToSet);
    }

}

does he really want to set pins 0 through 4??

Good spot. Neither of us are on form today, it seems.

  for (int b = 0; b < NUMBER_OF_BUTTONS; b++)
  {
    pinMode(b, INPUT_PULLUP);
  }

Should, of course, be

  for (int b = 0; b < NUMBER_OF_BUTTONS; b++)
  {
    pinMode(buttonPins[b], INPUT_PULLUP);
  }

thanks you two, you really helped me :)

the code is now as follows and its just working fine!

void get_btn_state() //check inputs
{
     CheckAndSet( 2, 0, button);
     CheckAndSet(13, 1, button);
     CheckAndSet(14, 2, button);
     CheckAndSet(15, 3, button);
     CheckAndSet(16, 4, button);
     CheckAndSet(17, 5, button);
}

void CheckAndSet(int pinToRead, int bitToSet, int &varToSave)
{
 if (digitalRead(pinToRead)==LOW)
 {
 bitSet(varToSave, bitToSet);
 }
 else
 {
 bitClear(varToSave, bitToSet);
 }
}

it also introduced me to references --> &