Get digital inputs for Sony DualShock 3 working

== DETAILS

- fix the bitshift math
- read the right bytes out of the ds3 data packet
- remove verbose logging in critical path
- stop caring about errors in the hid read loop -- seems to just
  be benign "device not ready" -- or at least, that's what I'm assuming
  given that the read eventually succeeds.

== TESTING
Played Mario 3 with the DS3 with no issues.

input/common/hid/device_ds3.c

@@ -78,9 +78,11 @@ static void print_error(const char *fmt, int32_t errcode)
   RARCH_ERR(fmt, err1, err2);
 }
 
-static uint32_t send_activation_packet(ds3_instance_t *instance)
+static void update_pad_state(ds3_instance_t *instance);
+
+static int32_t send_activation_packet(ds3_instance_t *instance)
 {
-   uint32_t result;
+   int32_t result;
 #if defined(WIIU)
    result = HID_SET_REPORT(instance->handle,
                   HID_REPORT_FEATURE,
@@ -91,8 +93,9 @@ static uint32_t send_activation_packet(ds3_instance_t *instance)
    HID_SEND_CONTROL(instance->handle,
                     activation_packet, sizeof(activation_packet));
 #endif
-   if(result)
+   if(result < 0)
       print_error("[ds3]: activation packet failed (%d:%d)\n", result);
+
    return result;
 }
 
@@ -100,18 +103,18 @@ static uint32_t set_protocol(ds3_instance_t *instance, int protocol)
 {
    uint32_t result = 0;
 #if defined(WIIU)
-   result = HID_SET_PROTOCOL(1);
+   result = HID_SET_PROTOCOL(instance->handle, 1);
    if(result)
       print_error("[ds3]: set protocol failed (%d:%d)\n", result);
-
 #endif
+
    return result;
 }
 
-static uint32_t send_control_packet(ds3_instance_t *instance)
+static int32_t send_control_packet(ds3_instance_t *instance)
 {
    uint8_t packet_buffer[control_packet_size];
-   uint32_t result = 0;
+   int32_t result = 0;
    memcpy(packet_buffer, control_packet, control_packet_size);
 
    packet_buffer[LED_OFFSET] = 0;
@@ -131,7 +134,7 @@ static uint32_t send_control_packet(ds3_instance_t *instance)
                   DS3_RUMBLE_REPORT_ID,
                   packet_buffer+PACKET_OFFSET,
                   control_packet_size-PACKET_OFFSET);
-   if(result)
+   if(result < 0)
       print_error("[ds3]: send control packet failed: (%d:%d)\n", result);
 #else
    HID_SEND_CONTROL(instance->handle,
@@ -152,14 +155,18 @@ static void *ds3_init(void *handle)
 
    instance->handle = handle;
 
-   RARCH_LOG("[ds3]: sending activation packet\n");
+/* maybe not necessary? */
-   if(send_activation_packet(instance))
+/*
-      errors++;
    RARCH_LOG("[ds3]: setting protocol\n");
    if(set_protocol(instance, 1))
       errors++;
+*/
    RARCH_LOG("[ds3]: sending control packet\n");
-   if(send_control_packet(instance))
+   if(send_control_packet(instance) < 0)
+      errors++;
+
+   RARCH_LOG("[ds3]: sending activation packet\n");
+   if(send_activation_packet(instance) < 0)
       errors++;
 
    if(errors)
@@ -245,11 +252,10 @@ static void ds3_get_buttons(void *data, retro_bits_t *state)
 static void ds3_packet_handler(void *data, uint8_t *packet, uint16_t size)
 {
    ds3_instance_t *instance = (ds3_instance_t *)data;
-   RARCH_LOG_BUFFER(packet, size);
 
-   if(!instance->led_set)
+   if(instance->pad && !instance->led_set)
    {
-      send_activation_packet(instance);
+      send_control_packet(instance);
       instance->led_set = true;
    }
 
@@ -261,7 +267,41 @@ static void ds3_packet_handler(void *data, uint8_t *packet, uint16_t size)
    }
 
    memcpy(instance->data, packet, size);
+   update_pad_state(instance);
+}
+
+static void update_pad_state(ds3_instance_t *instance)
+{
+   uint32_t i, pressed_keys;
+
+   static const uint32_t button_mapping[17] =
+   {
+      RETRO_DEVICE_ID_JOYPAD_SELECT,
+      RETRO_DEVICE_ID_JOYPAD_L3,
+      RETRO_DEVICE_ID_JOYPAD_R3,
+      RETRO_DEVICE_ID_JOYPAD_START,
+      RETRO_DEVICE_ID_JOYPAD_UP,
+      RETRO_DEVICE_ID_JOYPAD_RIGHT,
+      RETRO_DEVICE_ID_JOYPAD_DOWN,
+      RETRO_DEVICE_ID_JOYPAD_LEFT,
+      RETRO_DEVICE_ID_JOYPAD_L2,
+      RETRO_DEVICE_ID_JOYPAD_R2,
+      RETRO_DEVICE_ID_JOYPAD_L,
+      RETRO_DEVICE_ID_JOYPAD_R,
+      RETRO_DEVICE_ID_JOYPAD_X,
+      RETRO_DEVICE_ID_JOYPAD_A,
+      RETRO_DEVICE_ID_JOYPAD_B,
+      RETRO_DEVICE_ID_JOYPAD_Y,
+      16 /* PS button */
+   };
+
    instance->buttons = 0;
+
+   pressed_keys = instance->data[2]|(instance->data[3] << 8)|((instance->data[4] & 0x01) << 16);
+
+   for(i = 0; i < 17; i++)
+     instance->buttons |= (pressed_keys & (1 << i)) ?
+        (1 << button_mapping[i]) : 0;
 }
 
 static void ds3_set_rumble(void *data, enum retro_rumble_effect effect, uint16_t strength)
@@ -272,7 +312,14 @@ static void ds3_set_rumble(void *data, enum retro_rumble_effect effect, uint16_t
 static int16_t ds3_get_axis(void *data, unsigned axis)
 {
    ds3_instance_t *pad = (ds3_instance_t *)data;
+   int16_t val;
+
+   if(!pad || axis >= 4)
       return 0;
+
+   val = (pad->data[6+axis] << 8) - 0x8000;
+//   val = (pad->data[7+axis] << 8) - 0x8000;
+   return (val > 0x1000 || val < -0x1000) ? 0 : val;
 }
 
 static const char *ds3_get_name(void *data)
@@ -284,7 +331,10 @@ static const char *ds3_get_name(void *data)
 static bool ds3_button(void *data, uint16_t joykey)
 {
    ds3_instance_t *pad = (ds3_instance_t *)data;
+   if(!pad || joykey > 31)
       return false;
+
+   return pad->buttons & (1 << joykey);
 }
 
 pad_connection_interface_t ds3_pad_connection = {

input/include/hid_driver.h

@@ -60,8 +60,8 @@ struct hid_driver
    hid_instance.os_driver_data, pad, axis)
 #define HID_PAD_NAME(pad) \
    hid_instance.os_driver->name(hid_instance.os_driver_data, pad)
-#define HID_SET_PROTOCOL(protocol) \
+#define HID_SET_PROTOCOL(pad, protocol) \
-   hid_instance.os_driver->set_protocol(hid_instance.os_driver_data, protocol)
+   hid_instance.os_driver->set_protocol(pad, protocol)
 #define HID_SET_REPORT(pad, rpttype, rptid, data, len) \
    hid_instance.os_driver->set_report(pad, rpttype, rptid, data, len)
 #define HID_SEND_CONTROL(pad, data, len) \

input/input_autodetect_builtin.c

@@ -250,6 +250,25 @@ DECL_AXIS_EX(r_x_minus, -3, "C-stick left") \
 DECL_AXIS_EX(r_y_plus,  +2, "C-stick up") \
 DECL_AXIS_EX(r_y_minus, -2, "C-stick down")
 
+#define WIIUINPUT_DS3_DEFAULT_BINDS \
+DECL_BTN_EX(menu_toggle, 16, "Playstation") \
+DECL_BTN_EX(select, 2, "Select") \
+DECL_BTN_EX(start,  3, "Start") \
+DECL_BTN_EX(a, 8, "Circle") \
+DECL_BTN_EX(y, 1, "Triangle") \
+DECL_BTN_EX(b, 0, "Cross") \
+DECL_BTN_EX(x, 9, "Square") \
+DECL_BTN_EX(r, 11, "R1") \
+DECL_BTN_EX(l, 10, "L1") \
+DECL_BTN_EX(r2, 13, "R2") \
+DECL_BTN_EX(l2, 12, "L2") \
+DECL_BTN_EX(up, 4, "D-Pad Up") \
+DECL_BTN_EX(down, 5, "D-Pad Down") \
+DECL_BTN_EX(left, 6, "D-Pad left") \
+DECL_BTN_EX(right, 7, "D-Pad Right") \
+DECL_BTN_EX(r3, 15, "R3") \
+DECL_BTN_EX(l3, 14, "L3")
+
 #define WIIUINPUT_GAMEPAD_DEFAULT_BINDS \
 DECL_BTN_EX(menu_toggle,  1, "Home") \
 DECL_BTN_EX(select,       2, "-") \
@@ -635,7 +654,7 @@ const char* const input_builtin_autoconfs[] =
    DECL_AUTOCONF_DEVICE(PAD_NAME_CLASSIC, "wiiu", WIIUINPUT_CLASSIC_CONTROLLER_DEFAULT_BINDS),
    DECL_AUTOCONF_DEVICE(PAD_NAME_HID, "wiiu", WIIUINPUT_GAMEPAD_DEFAULT_BINDS),
    DECL_AUTOCONF_DEVICE("GameCube Controller", "wiiu", WIIUINPUT_GAMECUBE_DEFAULT_BINDS),
-   DECL_AUTOCONF_DEVICE("Sony DualShock 3", "wiiu", PS3INPUT_DEFAULT_BINDS),
+   DECL_AUTOCONF_DEVICE("Sony DualShock 3", "wiiu", WIIUINPUT_DS3_DEFAULT_BINDS),
 #endif
 #ifdef __CELLOS_LV2__
    DECL_AUTOCONF_DEVICE("SixAxis Controller", "ps3", PS3INPUT_DEFAULT_BINDS),

wiiu/input/wiiu_hid.c

@@ -181,14 +181,17 @@ static int32_t wiiu_hid_set_report(void *data, uint8_t report_type,
                uint8_t report_id, void *report_data, uint32_t report_length)
 {
    wiiu_adapter_t *adapter = (wiiu_adapter_t *)data;
-   if (!adapter)
+   if (!adapter || report_length > adapter->tx_size)
       return -1;
 
+   memset(adapter->tx_buffer, 0, adapter->tx_size);
+   memcpy(adapter->tx_buffer, report_data, report_length);
+
    return HIDSetReport(adapter->handle,
          report_type,
          report_id,
-         report_data,
+         adapter->tx_buffer,
-         report_length,
+         adapter->tx_size,
          NULL, NULL);
 }
 
@@ -333,10 +336,14 @@ static uint8_t try_init_driver(wiiu_adapter_t *adapter)
 static void synchronized_process_adapters(wiiu_hid_t *hid)
 {
    wiiu_adapter_t *adapter = NULL;
+   wiiu_adapter_t *prev = NULL, *adapter_next = NULL;
+   bool keep_prev = false;
 
    OSFastMutex_Lock(&(adapters.lock));
-   for(adapter = adapters.list; adapter != NULL; adapter = adapter->next)
+   for(adapter = adapters.list; adapter != NULL; adapter = adapter_next)
    {
+     adapter_next = adapter->next;
+
      switch(adapter->state)
      {
        case ADAPTER_STATE_NEW:
@@ -346,10 +353,24 @@ static void synchronized_process_adapters(wiiu_hid_t *hid)
        case ADAPTER_STATE_READING:
        case ADAPTER_STATE_DONE:
           break;
+       case ADAPTER_STATE_GC:
+          /* remove from the list */
+          if(prev == NULL)
+             adapters.list = adapter->next;
+          else
+             prev->next = adapter->next;
+
+          /* adapter is no longer valid after this point */
+          delete_adapter(adapter);
+          /* signal not to update prev ptr since adapter is now invalid */
+          keep_prev = true;
+          break;
        default:
           RARCH_ERR("[hid]: Invalid adapter state: %d\n", adapter->state);
           break;
      }
+     prev = keep_prev ? prev : adapter;
+     keep_prev = false;
    }
    OSFastMutex_Unlock(&(adapters.lock));
 }
@@ -373,24 +394,16 @@ static wiiu_attach_event *synchronized_get_events_list(void)
    return list;
 }
 
-static wiiu_adapter_t *synchronized_remove_from_adapters_list(uint32_t handle)
+static wiiu_adapter_t *synchronized_lookup_adapter(uint32_t handle)
 {
    OSFastMutex_Lock(&(adapters.lock));
-   wiiu_adapter_t *iterator, *prev = NULL;
+   wiiu_adapter_t *iterator;
 
    for(iterator = adapters.list; iterator != NULL; iterator = iterator->next)
    {
       if(iterator->handle == handle)
-      {
-         /* we're at the start of the list, so just re-assign head */
-         if(prev == NULL)
-           adapters.list = iterator->next;
-         else
-           prev->next = iterator->next;
          break;
    }
-      prev = iterator;
-   }
    OSFastMutex_Unlock(&(adapters.lock));
 
    return iterator;
@@ -434,12 +447,13 @@ error:
 
 static void wiiu_hid_detach(wiiu_hid_t *hid, wiiu_attach_event *event)
 {
-   wiiu_adapter_t *adapter = synchronized_remove_from_adapters_list(event->handle);
+   wiiu_adapter_t *adapter = synchronized_lookup_adapter(event->handle);
 
-   if(adapter) {
+   /* this will signal the read loop to stop for this adapter
-      RARCH_LOG("[hid]: freeing detached pad\n");
+    * the read loop method will update this to ADAPTER_STATE_GC
-      delete_adapter(adapter);
+    * so the adapter poll method can clean it up. */
-   }
+   if(adapter)
+      adapter->state = ADAPTER_STATE_DONE;
 }
 
 
@@ -495,20 +509,21 @@ static void wiiu_hid_read_loop_callback(uint32_t handle, int32_t error,
       adapter->state == ADAPTER_STATE_READING) {
 
       adapter->state = ADAPTER_STATE_READING;
-
+      /* "error" usually is something benign like "device not ready", at
-      if(error)
+       * least from my own experiments. Just ignore the error and retry
-      {
+       * the read. */
-         int16_t r1 =  (error & 0x0000FFFF);
+      if(error == 0) {
-         int16_t r2 = ((error & 0xFFFF0000) >> 16);
+         adapter->driver->handle_packet(adapter->driver_handle,
-         RARCH_ERR("[hid]: read failed: %08x (%d:%d)\n", error, r2, r1);
+            buffer, buffer_size);
-      } else {
-         adapter->driver->handle_packet(adapter->driver_handle, buffer, buffer_size);
       }
    }
 
    /* this can also get set if something goes wrong in initialization */
    if(adapter->state == ADAPTER_STATE_DONE)
+   {
+      adapter->state = ADAPTER_STATE_GC;
       return;
+   }
 
    HIDRead(adapter->handle, adapter->rx_buffer, adapter->rx_size,
       wiiu_hid_read_loop_callback, adapter);
@@ -597,7 +612,6 @@ static void wiiu_handle_ready_adapters(wiiu_hid_t *hid)
 static int wiiu_hid_polling_thread(int argc, const char **argv)
 {
    wiiu_hid_t *hid = (wiiu_hid_t *)argv;
-   int i           = 0;
 
    RARCH_LOG("[hid]: polling thread is starting\n");
 
@@ -606,9 +620,6 @@ static int wiiu_hid_polling_thread(int argc, const char **argv)
       wiiu_handle_attach_events(hid, synchronized_get_events_list());
       wiiu_handle_ready_adapters(hid);
       usleep(10000);
-      i += 10000;
-      if(i >= (1000 * 1000 * 3))
-         i = 0;
    }
 
    RARCH_LOG("[hid]: polling thread is stopping\n");
@@ -713,7 +724,6 @@ static wiiu_attach_event *new_attach_event(HIDDevice *device)
         device->vid, device->pid);
       return NULL;
    }
-   RARCH_LOG("[hid]: Found HID device driver: %s\n", driver->name);
    wiiu_attach_event *event = alloc_zeroed(4, sizeof(wiiu_attach_event));
    if(!event)
       return NULL;

wiiu/input/wiiu_hid.h

@@ -24,10 +24,16 @@
 #define DEVICE_UNUSED 0
 #define DEVICE_USED   1
 
+/* Adapter has been detected and needs to be initialized */
 #define ADAPTER_STATE_NEW     0
+/* Adapter has been initialized successfully */
 #define ADAPTER_STATE_READY   1
+/* The read loop has been started */
 #define ADAPTER_STATE_READING 2
+/* The read loop is shutting down */
 #define ADAPTER_STATE_DONE    3
+/* The read loop has fully stopped and the adapter can be freed */
+#define ADAPTER_STATE_GC      4
 
 struct wiiu_hid {
    /* used to register for HID notifications */