Commit 4ec776c334f6ed07ad82270e146aca33741178e9

Sam Lantinga 2020-12-22T13:29:23

Don't switch the PS5 controller out of DirectInput mode by default

diff --git a/include/SDL_hints.h b/include/SDL_hints.h
index 50a5a08..a2fb2fa 100644
--- a/include/SDL_hints.h
+++ b/include/SDL_hints.h
@@ -605,6 +605,21 @@ extern "C" {
 #define SDL_HINT_JOYSTICK_HIDAPI_PS4 "SDL_JOYSTICK_HIDAPI_PS4"
 
 /**
+ *  \brief  A variable controlling whether extended input reports should be used for PS4 controllers when using the HIDAPI driver.
+ *
+ *  This variable can be set to the following values:
+ *    "0"       - extended reports are not enabled (the default)
+ *    "1"       - extended reports
+ *
+ *  Extended input reports allow rumble on Bluetooth PS4 controllers, but
+ *  break DirectInput handling for applications that don't use SDL.
+ *
+ *  Once extended reports are enabled, they can not be disabled without
+ *  power cycling the controller.
+ */
+#define SDL_HINT_JOYSTICK_HIDAPI_PS4_RUMBLE "SDL_JOYSTICK_HIDAPI_PS4_RUMBLE"
+
+/**
  *  \brief  A variable controlling whether the HIDAPI driver for PS5 controllers should be used.
  *
  *  This variable can be set to the following values:
@@ -616,19 +631,19 @@ extern "C" {
 #define SDL_HINT_JOYSTICK_HIDAPI_PS5 "SDL_JOYSTICK_HIDAPI_PS5"
 
 /**
- *  \brief  A variable controlling whether extended input reports should be used for PS4 controllers when using the HIDAPI driver.
+ *  \brief  A variable controlling whether extended input reports should be used for PS5 controllers when using the HIDAPI driver.
  *
  *  This variable can be set to the following values:
  *    "0"       - extended reports are not enabled (the default)
  *    "1"       - extended reports
  *
- *  Extended input reports allow rumble on Bluetooth PS4 controllers, but
+ *  Extended input reports allow rumble on Bluetooth PS5 controllers, but
  *  break DirectInput handling for applications that don't use SDL.
  *
  *  Once extended reports are enabled, they can not be disabled without
  *  power cycling the controller.
  */
-#define SDL_HINT_JOYSTICK_HIDAPI_PS4_RUMBLE "SDL_JOYSTICK_HIDAPI_PS4_RUMBLE"
+#define SDL_HINT_JOYSTICK_HIDAPI_PS5_RUMBLE "SDL_JOYSTICK_HIDAPI_PS5_RUMBLE"
 
 /**
  *  \brief  A variable controlling whether the HIDAPI driver for Steam Controllers should be used.
diff --git a/src/joystick/SDL_gamecontroller.c b/src/joystick/SDL_gamecontroller.c
index a667450..c7d90e0 100644
--- a/src/joystick/SDL_gamecontroller.c
+++ b/src/joystick/SDL_gamecontroller.c
@@ -604,7 +604,7 @@ static ControllerMapping_t *SDL_CreateMappingForHIDAPIController(SDL_JoystickGUI
                 break;
             case SDL_CONTROLLER_TYPE_PS5:
                 /* PS5 controllers have a microphone button and an additional touchpad button */
-                SDL_strlcat(mapping_string, "misc1:b15,touchpad:b16", sizeof(mapping_string));
+                SDL_strlcat(mapping_string, "touchpad:b15,misc1:b16", sizeof(mapping_string));
                 break;
             case SDL_CONTROLLER_TYPE_NINTENDO_SWITCH_PRO:
                 /* Nintendo Switch Pro controllers have a screenshot button */
diff --git a/src/joystick/hidapi/SDL_hidapi_ps5.c b/src/joystick/hidapi/SDL_hidapi_ps5.c
index fc05541..49fb31f 100644
--- a/src/joystick/hidapi/SDL_hidapi_ps5.c
+++ b/src/joystick/hidapi/SDL_hidapi_ps5.c
@@ -151,6 +151,7 @@ typedef struct {
 
 typedef struct {
     SDL_bool is_bluetooth;
+    SDL_bool effects_supported;
     SDL_bool report_sensors;
     SDL_bool hardware_calibration;
     IMUCalibrationData calibration[6];
@@ -360,6 +361,9 @@ HIDAPI_DriverPS5_UpdateEffects(SDL_HIDAPI_Device *device, EDS5Effect effect)
     int *pending_size;
     int maximum_size;
 
+    if (!ctx->effects_supported) {
+        return SDL_Unsupported();
+    }
 
     SDL_zero(data);
 
@@ -462,17 +466,6 @@ HIDAPI_DriverPS5_UpdateEffects(SDL_HIDAPI_Device *device, EDS5Effect effect)
 }
 
 static void
-HIDAPI_DriverPS5_SetBluetooth(SDL_HIDAPI_Device *device, SDL_bool is_bluetooth)
-{
-    SDL_DriverPS5_Context *ctx = (SDL_DriverPS5_Context *)device->context;
-
-    if (ctx->is_bluetooth != is_bluetooth) {
-        ctx->is_bluetooth = is_bluetooth;
-        HIDAPI_DriverPS5_UpdateEffects(device, k_EDS5EffectLED);
-    }
-}
-
-static void
 HIDAPI_DriverPS5_CheckPendingLEDReset(SDL_HIDAPI_Device *device)
 {
     SDL_DriverPS5_Context *ctx = (SDL_DriverPS5_Context *)device->context;
@@ -512,7 +505,8 @@ static SDL_bool
 HIDAPI_DriverPS5_OpenJoystick(SDL_HIDAPI_Device *device, SDL_Joystick *joystick)
 {
     SDL_DriverPS5_Context *ctx;
-    Uint8 data[USB_PACKET_LENGTH];
+    Uint8 data[USB_PACKET_LENGTH*2];
+    int size;
 
     ctx = (SDL_DriverPS5_Context *)SDL_calloc(1, sizeof(*ctx));
     if (!ctx) {
@@ -529,14 +523,55 @@ HIDAPI_DriverPS5_OpenJoystick(SDL_HIDAPI_Device *device, SDL_Joystick *joystick)
     }
     device->context = ctx;
 
-    /* Read the serial number (Bluetooth address in reverse byte order)
-       This will also enable enhanced reports over Bluetooth
-    */
-    if (ReadFeatureReport(device->dev, k_EPS5FeatureReportIdSerialNumber, data, sizeof(data)) >= 7) {
+    /* Read a report to see what mode we're in */
+    size = hid_read_timeout(device->dev, data, sizeof(data), 16);
+#ifdef DEBUG_PS5_PROTOCOL
+    if (size > 0) {
+        HIDAPI_DumpPacket("PS5 first packet: size = %d", data, size);
+    } else {
+        SDL_Log("PS5 first packet: size = %d\n", size);
+    }
+#endif
+    if (size == 64) {
+        /* Connected over USB */
+        ctx->is_bluetooth = SDL_FALSE;
+        ctx->effects_supported = SDL_TRUE;
+    } else if (size > 0 && data[0] == k_EPS5ReportIdBluetoothEffects) {
+        /* Connected over Bluetooth, using enhanced reports */
+        ctx->is_bluetooth = SDL_TRUE;
+        ctx->effects_supported = SDL_TRUE;
+    } else {
+        /* Connected over Bluetooth, using simple reports (DirectInput enabled) */
+        ctx->is_bluetooth = SDL_TRUE;
+        ctx->effects_supported = SDL_GetHintBoolean(SDL_HINT_JOYSTICK_HIDAPI_PS5_RUMBLE, SDL_FALSE);
+    }
+
+    if (ctx->effects_supported) {
+        /* Read the serial number (Bluetooth address in reverse byte order)
+           This will also enable enhanced reports over Bluetooth
+        */
+        if (ReadFeatureReport(device->dev, k_EPS5FeatureReportIdSerialNumber, data, sizeof(data)) >= 7) {
+            char serial[18];
+
+            SDL_snprintf(serial, sizeof(serial), "%.2x-%.2x-%.2x-%.2x-%.2x-%.2x",
+                data[6], data[5], data[4], data[3], data[2], data[1]);
+            joystick->serial = SDL_strdup(serial);
+        }
+    }
+
+    if (!joystick->serial && SDL_strlen(device->serial) == 12) {
+        int i, j;
         char serial[18];
 
-        SDL_snprintf(serial, sizeof(serial), "%.2x-%.2x-%.2x-%.2x-%.2x-%.2x",
-            data[6], data[5], data[4], data[3], data[2], data[1]);
+        j = -1;
+        for (i = 0; i < 12; i += 2) {
+            j += 1;
+            SDL_memcpy(&serial[j], &device->serial[i], 2);
+            j += 2;
+            serial[j] = '-';
+        }
+        serial[j] = '\0';
+
         joystick->serial = SDL_strdup(serial);
     }
 
@@ -602,6 +637,10 @@ HIDAPI_DriverPS5_SetJoystickSensorsEnabled(SDL_HIDAPI_Device *device, SDL_Joysti
 {
     SDL_DriverPS5_Context *ctx = (SDL_DriverPS5_Context *)device->context;
 
+    if (!ctx->effects_supported) {
+        return SDL_Unsupported();
+    }
+
     if (enabled) {
         HIDAPI_DriverPS5_LoadCalibrationData(device);
     }
@@ -783,8 +822,8 @@ HIDAPI_DriverPS5_HandleStatePacket(SDL_Joystick *joystick, hid_device *dev, SDL_
         Uint8 data = packet->rgucButtonsAndHat[2];
 
         SDL_PrivateJoystickButton(joystick, SDL_CONTROLLER_BUTTON_GUIDE, (data & 0x01) ? SDL_PRESSED : SDL_RELEASED);
-        SDL_PrivateJoystickButton(joystick, 15, (data & 0x04) ? SDL_PRESSED : SDL_RELEASED);
-        SDL_PrivateJoystickButton(joystick, 16, (data & 0x02) ? SDL_PRESSED : SDL_RELEASED);
+        SDL_PrivateJoystickButton(joystick, 15, (data & 0x02) ? SDL_PRESSED : SDL_RELEASED);
+        SDL_PrivateJoystickButton(joystick, 16, (data & 0x04) ? SDL_PRESSED : SDL_RELEASED);
     }
 
     axis = ((int)packet->ucTriggerLeft * 257) - 32768;
@@ -869,20 +908,22 @@ HIDAPI_DriverPS5_UpdateDevice(SDL_HIDAPI_Device *device)
 
         switch (data[0]) {
         case k_EPS5ReportIdState:
-            if (size == 10) {
-                HIDAPI_DriverPS5_SetBluetooth(device, SDL_TRUE);    /* Simple state packet over Bluetooth */
+            if (size == 10 || size == 78) {
                 HIDAPI_DriverPS5_HandleSimpleStatePacket(joystick, device->dev, ctx, (PS5SimpleStatePacket_t *)&data[1]);
             } else {
-                HIDAPI_DriverPS5_SetBluetooth(device, SDL_FALSE);
                 HIDAPI_DriverPS5_HandleStatePacket(joystick, device->dev, ctx, (PS5StatePacket_t *)&data[1]);
             }
             break;
         case k_EPS5ReportIdBluetoothState:
-            HIDAPI_DriverPS5_SetBluetooth(device, SDL_TRUE);
             HIDAPI_DriverPS5_HandleStatePacket(joystick, device->dev, ctx, (PS5StatePacket_t *)&data[2]);
             if (ctx->led_reset_state == k_EDS5LEDResetStatePending) {
                 HIDAPI_DriverPS5_CheckPendingLEDReset(device);
             }
+            if (!ctx->effects_supported) {
+                /* This is the extended report, we can enable effects now */
+                ctx->effects_supported = SDL_TRUE;
+                HIDAPI_DriverPS5_UpdateEffects(device, k_EDS5EffectLED);
+            }
             break;
         default:
 #ifdef DEBUG_JOYSTICK