Better bitwise conversion

[Robototes2412]

Hello,

I am working on a program to control an arduino from a ds, and I have this code that converts all of the button input bools to a byte.

				//pack buttons into a byte
				if(keysHeld() & KEY_A) {
				    amount += 1;
				} else if (keysHeld() & KEY_B) {
				    amount += 2;
				} else if (keysHeld() & KEY_Y) {
				    amount += 4;
				} else if (keysHeld() & KEY_X) {
				    amount += 8;
				} else if (keysHeld() & KEY_L) {
				    amount += 16;
				} else if (keysHeld() & KEY_R) {
				    amount += 32;
				} else if (keysHeld() & KEY_START) {
				    amount += 64;
				} else if (keysHeld() & KEY_SELECT) {
				    amount += 128;
				} else {
				    amount = 0;
				}

amount is a byte.

How can i do this more elegantly?

[mikets]

Is there a reason why you are translating one bit position to another? It looks like keysHeld() is already returning a bit per key. Can't you just do the following?

amount = keysHeld();

The only difference is keysHeld() can return more than one bit set and "amount" in your code can only have one bit set.
If you want to isolate one bit from all the set bits returned by keysHeld(), you can do this:

int rawBits = keysHeld();
int amount = rawBits & ~(rawBits ^ -rawBits);

Or is keysHeld() returning more than one byte (e.g. 16-bit) and you are trying to compress it into one byte? If so you may do the following:

int rawBits = keysHeld();
byte amount = 0;
 
if (rawBits & KEY_A) amount |= 0x01;
if (rawBits & KEY_B) amount |= 0x02;
if (rawBits & KEY_Y) amount |= 0x04;
if (rawBits & KEY_X) amount |= 0x08;
if (rawBits & KEY_L) amount |= 0x10;
if (rawBits & KEY_R) amount |= 0x20;
if (rawBits & KEY_START) amount |= 0x40;
if (rawBits & KEY_SELECT) amount |= 0x80;

Just curious, these looks like xbox controller buttons, what are you trying to do?

[Robototes2412]

its for a ds interface to an arduino robot my team is working on

[dbeckwith]

Well one way you could do it is with a for loop, but that would only really be efficient if the bit masks are consecutive. I only know Java, so I might get the C++ syntax wrong, but here is what it might look like:

int keys = keysHeld();
byte packedKeys = 0;

for (int i = 0; i < 8; i++) {
  if (keys & (1 << i)) packedKeys |= 1 << i;
}

That would pack the values of the keys, but only if the bit masks were all in a line. You could also start the i variable at the first mask if it isn't zero. I have the same question as mikets though. Why do you even need to do this if keysHeld() already returns the key values? It seems redundant to me.

[mikets]

Originally Posted by dbeckwith

Well one way you could do it is with a for loop, but that would only really be efficient if the bit masks are consecutive. I only know Java, so I might get the C++ syntax wrong, but here is what it might look like:

int keys = keysHeld();
byte packedKeys = 0;
 
for (int i = 0; i < 8; i++) {
  if (keys & (1 << i)) packedKeys |= 1 << i;
}

That would pack the values of the keys, but only if the bit masks were all in a line. You could also start the i variable at the first mask if it isn't zero. I have the same question as mikets though. Why do you even need to do this if keysHeld() already returns the key values? It seems redundant to me.

But this for-loop is copying the exact same bit pattern from keys to packedKeys. So what's the difference between that and this?

packedKeys = keys;

[demosthenes2k8]

keys stores them differently, according to this:

typedef enum KEYPAD_BITS {
  KEY_A      = BIT(0),  //!< Keypad A button.
  KEY_B      = BIT(1),  //!< Keypad B button.
  KEY_SELECT = BIT(2),  //!< Keypad SELECT button.
  KEY_START  = BIT(3),  //!< Keypad START button.
  KEY_RIGHT  = BIT(4),  //!< Keypad RIGHT button.
  KEY_LEFT   = BIT(5),  //!< Keypad LEFT button.
  KEY_UP     = BIT(6),  //!< Keypad UP button.
  KEY_DOWN   = BIT(7),  //!< Keypad DOWN button.
  KEY_R      = BIT(8),  //!< Right shoulder button.
  KEY_L      = BIT(9),  //!< Left shoulder button.
  KEY_X      = BIT(10), //!< Keypad X button.
  KEY_Y      = BIT(11), //!< Keypad Y button.
  KEY_TOUCH  = BIT(12), //!< Touchscreen pendown.
  KEY_LID    = BIT(13)  //!< Lid state.
} KEYPAD_BITS;