-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Update HIDAPI to match upstream hidapi 0.14.0 #7736
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
535d740
to
73fea71
Compare
The SDL coding standard is C89 plus C++ style comments and mixed code and declarations.
c7c1689
to
f4ab960
Compare
@flibitijibibo, how important is having libusb/hid.c compile on Windows? |
It would definitely be a nice-to-have but right now I'm only enabling it on Linux. |
698e430
to
d6efd85
Compare
daa2c31
to
18ae036
Compare
Some windows notes: (Disclaimer: I tested build using an old mingw toolchain.)
After manually adding diff --git a/src/hidapi/SDL_hidapi_windows.h b/src/hidapi/SDL_hidapi_windows.h
index da61fa2..fba405c 100644
--- a/src/hidapi/SDL_hidapi_windows.h
+++ b/src/hidapi/SDL_hidapi_windows.h
@@ -22,6 +22,7 @@
#include <stdio.h> /* Include early so we can redefine snprintf */
/* Define standard library functions in terms of SDL */
+#define HIDAPI_USING_SDL_RUNTIME
#define calloc SDL_calloc
#define free SDL_free
#define malloc SDL_malloc
diff --git a/src/hidapi/windows/hid.c b/src/hidapi/windows/hid.c
index 99712ad..7610376 100644
--- a/src/hidapi/windows/hid.c
+++ b/src/hidapi/windows/hid.c
@@ -38,11 +38,12 @@ extern "C" {
typedef LONG NTSTATUS;
#endif
-#ifdef __MINGW32__
-#include <ntdef.h>
-#include <winbase.h>
+#ifndef WC_ERR_INVALID_CHARS
#define WC_ERR_INVALID_CHARS 0x00000080
#endif
+#ifndef _WIN32_WINNT_WIN8
+#define _WIN32_WINNT_WIN8 0x0602
+#endif
#ifdef __CYGWIN__
#include <ntdef.h>
@@ -56,9 +57,11 @@ typedef LONG NTSTATUS;
#include "hidapi_hidclass.h"
#include "hidapi_hidsdi.h"
+#ifndef HIDAPI_USING_SDL_RUNTIME
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
+#endif
#ifdef MIN
#undef MIN
@@ -117,6 +120,11 @@ static void free_library_handles()
cfgmgr32_lib_handle = NULL;
}
+#if defined(__GNUC__)
+# pragma GCC diagnostic push
+# pragma GCC diagnostic ignored "-Wcast-function-type"
+#endif
+
static int lookup_functions()
{
hid_lib_handle = LoadLibraryW(L"hid.dll");
@@ -129,10 +137,6 @@ static int lookup_functions()
goto err;
}
-#if defined(__GNUC__)
-# pragma GCC diagnostic push
-# pragma GCC diagnostic ignored "-Wcast-function-type"
-#endif
#define RESOLVE(lib_handle, x) x = (x##_)GetProcAddress(lib_handle, #x); if (!x) goto err;
RESOLVE(hid_lib_handle, HidD_GetHidGuid);
@@ -158,9 +162,6 @@ static int lookup_functions()
RESOLVE(cfgmgr32_lib_handle, CM_Get_Device_Interface_ListW);
#undef RESOLVE
-#if defined(__GNUC__)
-# pragma GCC diagnostic pop
-#endif
return 0;
@@ -169,6 +170,10 @@ err:
return -1;
}
+#if defined(__GNUC__)
+# pragma GCC diagnostic pop
+#endif
+
#endif /* HIDAPI_USE_DDK */
struct hid_device_ {
diff --git a/src/hidapi/windows/hidapi_cfgmgr32.h b/src/hidapi/windows/hidapi_cfgmgr32.h
index 4685b76..09bf4d7 100644
--- a/src/hidapi/windows/hidapi_cfgmgr32.h
+++ b/src/hidapi/windows/hidapi_cfgmgr32.h
@@ -66,7 +66,7 @@ typedef CONFIGRET(__stdcall* CM_Get_Parent_)(PDEVINST pdnDevInst, DEVINST dnDevI
typedef CONFIGRET(__stdcall* CM_Get_DevNode_PropertyW_)(DEVINST dnDevInst, CONST DEVPROPKEY* PropertyKey, DEVPROPTYPE* PropertyType, PBYTE PropertyBuffer, PULONG PropertyBufferSize, ULONG ulFlags);
typedef CONFIGRET(__stdcall* CM_Get_Device_Interface_PropertyW_)(LPCWSTR pszDeviceInterface, CONST DEVPROPKEY* PropertyKey, DEVPROPTYPE* PropertyType, PBYTE PropertyBuffer, PULONG PropertyBufferSize, ULONG ulFlags);
typedef CONFIGRET(__stdcall* CM_Get_Device_Interface_List_SizeW_)(PULONG pulLen, LPGUID InterfaceClassGuid, DEVINSTID_W pDeviceID, ULONG ulFlags);
-typedef CONFIGRET(__stdcall* CM_Get_Device_Interface_ListW_)(LPGUID InterfaceClassGuid, DEVINSTID_W pDeviceID, PZZWSTR Buffer, ULONG BufferLen, ULONG ulFlags);
+typedef CONFIGRET(__stdcall* CM_Get_Device_Interface_ListW_)(LPGUID InterfaceClassGuid, DEVINSTID_W pDeviceID, WCHAR* /*PZZWSTR*/ Buffer, ULONG BufferLen, ULONG ulFlags);
// from devpkey.h
static DEVPROPKEY DEVPKEY_NAME = { { 0xb725f130, 0x47ef, 0x101a, {0xa5, 0xf1, 0x02, 0x60, 0x8c, 0x9e, 0xeb, 0xac} }, 10 }; // DEVPROP_TYPE_STRING
diff --git a/src/hidapi/windows/hidapi_hidsdi.h b/src/hidapi/windows/hidapi_hidsdi.h
index a4bac4b..95e2fcb 100644
--- a/src/hidapi/windows/hidapi_hidsdi.h
+++ b/src/hidapi/windows/hidapi_hidsdi.h
@@ -40,8 +40,6 @@ typedef struct _HIDD_ATTRIBUTES{
USHORT VersionNumber;
} HIDD_ATTRIBUTES, *PHIDD_ATTRIBUTES;
-typedef struct _HIDP_PREPARSED_DATA * PHIDP_PREPARSED_DATA;
-
typedef void (__stdcall *HidD_GetHidGuid_)(LPGUID hid_guid);
typedef BOOLEAN (__stdcall *HidD_GetAttributes_)(HANDLE device, PHIDD_ATTRIBUTES attrib);
typedef BOOLEAN (__stdcall *HidD_GetSerialNumberString_)(HANDLE device, PVOID buffer, ULONG buffer_len); |
Also: If you accept doing |
Also, curious: does this compile using VS2010? I.e.: does VS2010 support for loop initial declarations? |
No, this change drops support for Visual Studio 2010. Thanks for your other patches! I'll apply them and make the other change to remove stdio.h as well. |
@cgutman, I carefully reviewed all the changes that we've made to see if they're needed on the new hidapi, but would you mind taking a glance and make sure everything you wanted is in? |
Touching HID devices with keyboard usages will trigger a keyboard capture permission prompt on macOS 11+. See libsdl-org#4887 Like the IOKit joystick backend, we accept HID devices that have joystick, gamepad, or multi-axis controller usages. We also allow the Valve VID for the Steam Controller, just like the Windows HIDAPI implementation does. Signed-off-by: Cameron Gutman <aicommander@gmail.com> Signed-off-by: Sam Lantinga <slouken@libsdl.org>
Signed-off-by: Sam Lantinga <slouken@libsdl.org>
Signed-off-by: Sam Lantinga <slouken@libsdl.org>
Current windows build exports hidapi procedures. The following fixes that: diff -u a/src/hidapi/SDL_hidapi.c b/src/hidapi/SDL_hidapi.c
--- a/src/hidapi/SDL_hidapi.c
+++ b/src/hidapi/SDL_hidapi.c
@@ -32,6 +32,7 @@
#include "SDL_hidapi_c.h"
/* Initial type declarations */
+#define HID_API_NO_EXPORT_DEFINE /* do not export hidapi procedures */
#include "hidapi/hidapi.h"
#ifndef SDL_HIDAPI_DISABLED |
Signed-off-by: Sam Lantinga <slouken@libsdl.org>
…eadFile in hidapi not being canceled for all threads before device close - hidapi already called CancelIo on hid_close but that only cancels pending IO for the current thread. Controller read/writes originate from multiple threads (serialized, but on a different thread nonetheless) but device destruction was always done on the main device thread which left any pending overlapped reads still running after hidapi's internal read buffer is deallocated leading to intermittent free list corruption. Signed-off-by: Sam Lantinga <slouken@libsdl.org>
…out() This is unsafe because the event is auto-reset, therefore the call to WaitForSingleObject() resets the event which GetOverlappedResult() will try to wait on. Even though the overlapped operation is guaranteed to be completed at the point we call GetOverlappedResult(), it will still wait on the event handle for a short time to trigger the reset for auto-reset events. This amounts to roughly a 100 ms sleep each time GetOverlappedResult() is called for a completed I/O with a non-signalled event. In the context of HIDAPI, this extra sleep means that callers that loop on hid_read_timeout() with timeout=0 will loop forever, since the 100 ms sleep each iteration ensures ReadFile() will always have new data. Signed-off-by: Cameron Gutman <aicommander@gmail.com> Signed-off-by: Sam Lantinga <slouken@libsdl.org>
Signed-off-by: Sam Lantinga <slouken@libsdl.org>
Signed-off-by: Sam Lantinga <slouken@libsdl.org>
…th EPIPE (e.g. the device stalled) Signed-off-by: Sam Lantinga <slouken@libsdl.org>
Signed-off-by: Sam Lantinga <slouken@libsdl.org>
…e ignored in SDL_hid_enumerate()
I went ahead and fixed the compiling issues. They were entirely around the threading implementation, which was pretty easily abstracted. |
We have a new SDL2 incompatibility: |
The FreeBSD VM in github CI has been failing since this PR. I don't know how this PR ever effected that... |
From what I understand, the BSD's only have libusb to do the low-level hidapi things: Lines 175 to 181 in e4eab18
In that case, Lines 469 to 475 in e4eab18
Is this assumption still correct? |
I think FreeBSD also supports hidraw, and I'm guessing the newer FreeBSD versions have a usable libusb. Can you disable the override for now, and I'll investigate when I get a chance? |
FreeBSD doesn't contain hidraw yet. But there are discussions with the FreeBSD maintainers to adapt it to their kernel. Currently all BSDs require the libusb backend. |
The linux driver (hidapi/linux/hid.c) seems to require kernel >= 2.6.39 |
Yeah, that seems fine. I went ahead and added it to SDL_hidapi_linux.h |
The changes generally look good to me. My only concern is if we still need f21e172. It looks like we're back to using |
Yeah, I was wondering about that, but I was getting called back with a closed handle in our version of hidapi, and not with 0.14.0. I suspect the upstream code is properly cleaning up and our issue was self-inflicted. I figured we could leave it out and see whether we actually need it. Are you able to reproduce the original issue with the new update? |
Fixes #7727