Optimizing parsing the individual bits in a variable

I’m working on a project to resurrect some old Gravis Gamepads by converting them to USB.
Long story short, I’ve extracted a packet from the gamepads containing bits representing each individual button, and I’m trying to parse it out efficiently and quickly, to minimize input delay.

Originally, I captured the packets as uint32_t, but then I squeezed out all of the unnecessary sync and framing stuff, and now I have two individual bytes containing the bits I need to parse. (my thinking is that it’s going to be faster to parse 8-bit values, rather than 32-bit values). I’m also only ever invoking this decision tree if a packet has changed since the last time through the loop() (i.e. if no user inputs are detected, we can skip parsing).

Here’s what I have so far. It works, but I keep thinking ‘there’s gotta be a better/faster/more elegant way’. There’s a lot of copy-pasta here.

Please be kind. I haven’t written any code since school, 20 years ago.

First some #defines that I’ll be using:

/* Keymapping table: */
//JS1 defs
#define JS1_left   KEY_LEFT_ARROW
#define JS1_right  KEY_RIGHT_ARROW
#define JS1_up     KEY_UP_ARROW
#define JS1_down   KEY_DOWN_ARROW
#define JS1_start  KEY_RETURN
#define JS1_select KEY_ESC
#define JS1_red    KEY_RIGHT_SHIFT
#define JS1_yellow ' '
#define JS1_blue   KEY_RIGHT_ALT
#define JS1_green  KEY_RIGHT_CTRL
#define JS1_l1     'l'
#define JS1_r1     ':'
#define JS1_l2     '.'
#define JS1_r2     '/'

//JS2 defs
#define JS2_left   'a'
#define JS2_right  'd'
#define JS2_up     'w'
#define JS2_down   's'
#define JS2_start  'z'
#define JS2_select 'c'
#define JS2_red    'r'
#define JS2_yellow 'f'
#define JS2_blue   't'
#define JS2_green  'g'
#define JS2_l1     'q'
#define JS2_r1     'e'
#define JS2_l2     '1'
#define JS2_r2     '3'

/* Our compressed data packet consists of 2 bytes, as follows:
  B0: Yellow Red L1 R1 Up Down Right Left
  B1: 0 0 Select Start R2 Blue L2 Green
  Bitmap to express:
 //Byte 0
#define DECODE_left   0b00000001
#define DECODE_right  0b00000010
#define DECODE_down   0b00000100
#define DECODE_up     0b00001000
#define DECODE_r1     0b00010000
#define DECODE_l1     0b00100000
#define DECODE_red    0b01000000
#define DECODE_yellow 0b10000000
 //Byte 1
#define DECODE_green  0b00000001
#define DECODE_l2     0b00000010
#define DECODE_blue   0b00000100
#define DECODE_r2     0b00001000
#define DECODE_start  0b00010000
#define DECODE_select 0b00100000

and now the ugly:

  byte mask[2];
  mask[0]=cmp_packet[0]^cmp_previous[0];    //any bits that have *changed* since last time are masked

  if (JSnum == 1) {
      digitalWrite(TXLED, LOW);  // turn on the LED
      if (mask[0] & DECODE_left){     //if the 'left' d-pad state has changed
        if(mask[0] & cmp_packet[0]){  //figure our if the change was a 0->1
          Serial.println(F(" JS1: Left Pressed "));}
        else{                                      //or a 1->0
         Serial.println(F(" JS1: Left Released "));}}
      if (mask[0] & DECODE_right){
        if(mask[0] & cmp_packet[0]){
          Serial.println(F(" JS1: Right Pressed "));}
          Serial.println(F(" JS1: Right Released "));}}
      if (mask[0] & DECODE_down){  
        if(mask[0] & cmp_packet[0]){
          Serial.println(F(" JS1: Down Pressed "));}
          Serial.println(F(" JS1: Down Released "));}}
      if (mask[0] & DECODE_up){
        if(mask[0] & cmp_packet[0]){
          Serial.println(F(" JS1: Up Pressed "));}
          Serial.println(F(" JS1: Up Released "));}}
      if (mask[0] & DECODE_r1){
        if(mask[0] & cmp_packet[0]){
          Serial.println(F(" JS1: R1 Pressed "));}
          Serial.println(F(" JS1: R1 Released "));}}
      if (mask[0] & DECODE_l1){
        if(mask[0] & cmp_packet[0]){
          Serial.println(F(" JS1: L1 Pressed "));}
          Serial.println(F(" JS1: L1 Released "));}}
      if (mask[0] & DECODE_red){
        if(mask[0] & cmp_packet[0]){
          Serial.println(F(" JS1: Red Pressed "));}
          Serial.println(F(" JS1: Red Released "));}}
      if (mask[0] & DECODE_yellow){
        if(mask[0] & cmp_packet[0]){
          Serial.println(F(" JS1: Yellow Pressed "));}
          Serial.println(F(" JS1: Yellow Released "));}}
      if (mask[1] & DECODE_green){
        if(mask[1] & cmp_packet[1]){
          Serial.println(F(" JS1: Green Pressed "));}

[b]<snip> - you get the idea. Then go through all this again for the 2nd gamepad[/b]

Complete code on github: https://github.com/prosper00/GRiP-duino

Just using JS1 as an example like you did, you could put the JS1_xxx keys in a table. Next you would have a loop with 8 iterations of index that would test and shift right both of your mask[0] and cmp_packet[0] bytes. If the mask[0] LSbit is a 1 you grab the index element in your JS1 key array, otherwise you continue the loop. If the cmp_packet[0] LSbit is a 1 then you press the JS1 key or if the cmp_packet[0] LSbit is a 0 then you release the JS1 key.

I'm in a hurry but if I have time later I will try to at least partially code it.

Here is some code. I haven’t compiled it and it is just a code fragment.

If you put both packet bytes in an unsigned int then you can decode the packet in one loop.

char JS1_Keys[] = {KEY_LEFT_ARROW, KEY_RIGHT_ARROW, KEY_UP_ARROW, KEY_DOWN_ARROW, KEY_RETURN, KEY_ESC, 	                          KEY_RIGHT_SHIFT, ' ', KEY_RIGHT_ALT, KEY_RIGHT_CTRL, 'l', ':', '.', '/'}

  unsigned int cmp_packet;
  unsigned int cmp_previous;
  unsigned int mask;

  // !!!!!! Assume cmp_packet has already been populated when we get here !!!!!!

  mask = cmp_packet ^ cmp_previous;    //any bits that have *changed* since last time are masked
  cmp_previous = cmp_packet;
  for (index = 0; index < 14; index++)
    if (mask & 0x01)
      if (cmp_packet & 0x01)
    mask >>= 1;
    cmp_packet  >>= 1;

awesome, I'll play with it a bit and see what happens.


SO much cleaner now! I still want to benchmark the two approaches. I suspect that doing two loops on two ‘bytes’ is faster than 1 loop on an int - the arduino works on 8 bits at a time, so, 16-bit compares would take at least twice as long.

void SendKeys(uint32_t packet, uint32_t previous, byte JSnum) { //JSnum = 0 for the first JS, and 1 for the 2nd

  uint16_t mask=packet^previous;

  for (int i = 0; i < 14; i++) {
    if (mask & 0x01) {
      if (packet & 0x01) {
      else {
    mask >>= 1;
    packet  >>= 1;
/* Keymapping table: 
* Our compressed data packet consists of 2 bytes, as follows:
* 0 0 Select Start R2 Blue L2 Green Yellow Red L1 R1 Up Down Right Left
//JS1 keymap
const char JS_keys[2][14] = {
  KEY_LEFT_ARROW,  // JS1_left   bit 0000000000000001
  KEY_RIGHT_ARROW, // JS1_right  bit 0000000000000010
  KEY_DOWN_ARROW,  // JS1_down   bit 0000000000000100
  KEY_UP_ARROW,    // JS1_up     bit 0000000000001000
  ':',             // JS1_r1     bit 0000000000010000
  'l',             // JS1_l1     bit 0000000000100000
  KEY_RIGHT_SHIFT, // JS1_red    bit 0000000001000000
  ' ',             // JS1_yellow bit 0000000010000000
  KEY_RIGHT_CTRL,  // JS1_green  bit 0000000100000000
  '.',             // JS1_l2     bit 0000001000000000
  KEY_RIGHT_ALT,   // JS1_blue   bit 0000010000000000
  '/',             // JS1_r2     bit 0000100000000000
  KEY_RETURN,      // JS1_start  bit 0001000000000000
  KEY_ESC          // JS1_select bit 0010000000000000

//JS2 keymap
  'a', // JS2_left   bit 0000000000000001
  'd', // JS2_right  bit 0000000000000010
  's', // JS2_down   bit 0000000000000100
  'w', // JS2_up     bit 0000000000001000
  'e', // JS2_r1     bit 0000000000010000
  'q', // JS2_l1     bit 0000000000100000
  'r', // JS2_red    bit 0000000001000000
  'f', // JS2_yellow bit 0000000010000000
  'g', // JS2_green  bit 0000000100000000
  '1', // JS2_l2     bit 0000001000000000
  't', // JS2_blue   bit 0000010000000000
  '3', // JS2_r2     bit 0000100000000000
  'z', // JS2_start  bit 0001000000000000
  'c'  // JS2_select bit 0010000000000000