device not removed from SDL_HIDAPI_devices on surprise removal

[shuffle2]

I have this observed buggy behavior on SDL 1.0.21 on windows 11:

1. connect ps4 controller via usb
2. device gets recognized and HIDAPI SDL driver claims it (good!)
3. my code calls SDL_JoystickOpen, SDL_GameControllerOpen
4. disconnect controller from usb
5. my code calls SDL_GameControllerClose, SDL_JoystickClose. I can see that SDL frees the SDL_Joystick* (no more refs)
6. removal triggers HIDAPI_JoystickDisconnected to be called from HIDAPI_DriverPS4_UpdateDevice
7. reconnect usb
8. SDL attaches the WINDOWS driver to it instead of HIDAPI (bad!)

This appears be because EnumJoystickDetectCallback -> HIDAPI_IsDevicePresent -> HIDAPI_UpdateDeviceList still sees the old device in the SDL_HIDAPI_devices list, and therefor skips calling HIDAPI_AddDevice (instead, it notices the list entry is dangling and removes it). I tested that settings SDL_HIDAPI_devices to NULL between steps 6 and 7 makes the code behave as expected (the reconnect attaches HIDAPI driver again).

I'm curious if I'm using the API incorrectly, if this is a real bug, etc. :) If it's a bug, possible fix could just be duplicating the HIDAPI_DelDevice loop in HIDAPI_UpdateDeviceList to the first walk of the list, as well?

It looks like HIDAPI_JoystickDetect is supposed to detect the change in device count and trigger the HIDAPI_UpdateDeviceList which would get HIDAPI_DelDevice called, but it doesn't see the device count change (stays at 1).

In HIDAPI_InitializeDiscovery, RegisterDeviceNotification does return success, but ControllerWndProc is never called.

I ripped out the WM_DEVICECHANGE-related code into a standalone test app, and it seems the issue is that the thread which created the m_hwndMsg must also be processing window messages. Seems conflicting as the SDL thread of the app falls into a SDL_WaitEvent loop (why doesn't something within SDL_WaitEvent handle window messages?)

OK, so I've found similar code in the Joystick code. It has a minor issue - the class name it uses is too generic ("Message") and so RegisterClassEx fails because something already registered that name. However, the non-threaded method in this file (using cfgmgr32) was already working, it's just that the variable they control (s_bWindowsDeviceChanged) is not used for the HIDAPI layer.

So...I propose:

1. Replacing window-message based notification in SDL_hidapi.c with the cfgmgr32 method.
2. changing the wndclass registered by SDL_CreateDeviceNotification to something more unique.
3. extending WINDOWS_JoystickInit to (optionally, based on a hint?) disable window message-based detection of device change all together, since this is already accomplished by SDL_CreateDeviceNotificationFunc.

I've tested, and replacing RegisterDeviceNotification with CM_Register_Notification in SDL_hidapi.c is working, with either GUID_DEVINTERFACE_USB_DEVICE or GUID_DEVINTERFACE_HID.

[slouken]

Great investigation. Can you create a PR that implements your changes for review?

[slouken]

@shuffle2, do you have a tested patch that implements your change?

[shuffle2]

Sorry, I haven't gotten around to it. I came across the issue because I was fixing someone else's PR dealing with SDL, I'm not really routinely using SDL myself.