Skip to content
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

hidapi/windows: fixed PS4 controllers over Bluetooth on Windows 7 #578

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions windows/hid.c
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ static HidD_GetPreparsedData_ HidD_GetPreparsedData;
static HidD_FreePreparsedData_ HidD_FreePreparsedData;
static HidP_GetCaps_ HidP_GetCaps;
static HidD_SetNumInputBuffers_ HidD_SetNumInputBuffers;
static HidD_SetOutputReport_ HidD_SetOutputReport;

static CM_Locate_DevNodeW_ CM_Locate_DevNodeW = NULL;
static CM_Get_Parent_ CM_Get_Parent = NULL;
Expand Down Expand Up @@ -156,6 +157,7 @@ static int lookup_functions()
RESOLVE(hid_lib_handle, HidD_FreePreparsedData);
RESOLVE(hid_lib_handle, HidP_GetCaps);
RESOLVE(hid_lib_handle, HidD_SetNumInputBuffers);
RESOLVE(hid_lib_handle, HidD_SetOutputReport);

RESOLVE(cfgmgr32_lib_handle, CM_Locate_DevNodeW);
RESOLVE(cfgmgr32_lib_handle, CM_Get_Parent);
Expand Down Expand Up @@ -192,8 +194,28 @@ struct hid_device_ {
OVERLAPPED ol;
OVERLAPPED write_ol;
struct hid_device_info* device_info;
BOOL use_hid_write_output_report;
};

static BOOL hid_internal_is_windows_version_or_greater(WORD wMajorVersion, WORD wMinorVersion, WORD wServicePackMajor)
{
OSVERSIONINFOEXW osvi;
DWORDLONG const dwlConditionMask = VerSetConditionMask(
VerSetConditionMask(
VerSetConditionMask(
0, VER_MAJORVERSION, VER_GREATER_EQUAL ),
VER_MINORVERSION, VER_GREATER_EQUAL ),
VER_SERVICEPACKMAJOR, VER_GREATER_EQUAL );

memset(&osvi, 0, sizeof(osvi));
osvi.dwOSVersionInfoSize = sizeof( osvi );
osvi.dwMajorVersion = wMajorVersion;
osvi.dwMinorVersion = wMinorVersion;
osvi.wServicePackMajor = wServicePackMajor;

return VerifyVersionInfoW(&osvi, VER_MAJORVERSION | VER_MINORVERSION | VER_SERVICEPACKMAJOR, dwlConditionMask) != FALSE;
}

static hid_device *new_hid_device()
{
hid_device *dev = (hid_device*) calloc(1, sizeof(hid_device));
Expand Down Expand Up @@ -1039,6 +1061,11 @@ HID_API_EXPORT hid_device * HID_API_CALL hid_open_path(const char *path)
dev->read_buf = (char*) malloc(dev->input_report_length);
dev->device_info = hid_internal_get_device_info(interface_path, dev->device_handle);

/* On Windows 7, we need to use hid_write_output_report() over Bluetooth */
if (dev->output_report_length > 512) {
dev->use_hid_write_output_report = !hid_internal_is_windows_version_or_greater( HIBYTE( _WIN32_WINNT_WIN8 ), LOBYTE( _WIN32_WINNT_WIN8 ), 0 );
}

end_of_function:
free(interface_path);
CloseHandle(device_handle);
Expand All @@ -1050,6 +1077,17 @@ HID_API_EXPORT hid_device * HID_API_CALL hid_open_path(const char *path)
return dev;
}

static int hid_internal_write_output_report(hid_device *dev, const unsigned char *data, size_t length)
{
BOOL res;
res = HidD_SetOutputReport(dev->device_handle, (void *)data, (ULONG)length);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please see #224
I'm confident we need to do the same thing for HidD_SetOutputReport as well.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@slouken any thoughs/comments?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@slouken It seems, you missed to respond to this question

Copy link
Contributor Author

@slouken slouken Mar 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like #224 was just pre-allocating the buffer. Were you referring to another change that made sure the buffer size matched feature_report_length? I think, but I'm not sure, that the feature_report_length is the wrong size in this case.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean that the buffer size has to be at least output_report_length bytes in size.
If user passes less data - logically it is not a proble, but WinAPI implementation is known to crash if bufer is too small.
The workaround (as in #224) is to pre-allocate buffer that is large-enough if needed.

if (!res) {
register_winapi_error(dev, L"HidD_SetOutputReport");
return -1;
slouken marked this conversation as resolved.
Show resolved Hide resolved
}
return (int)length;
}

int HID_API_EXPORT HID_API_CALL hid_write(hid_device *dev, const unsigned char *data, size_t length)
{
DWORD bytes_written = 0;
Expand All @@ -1066,6 +1104,10 @@ int HID_API_EXPORT HID_API_CALL hid_write(hid_device *dev, const unsigned char *

register_string_error(dev, NULL);

if (dev->use_hid_write_output_report) {
return hid_internal_write_output_report(dev, data, length);
}

/* Make sure the right number of bytes are passed to WriteFile. Windows
expects the number of bytes which are in the _longest_ report (plus
one for the report number) bytes even if the data is a report
Expand Down
1 change: 1 addition & 0 deletions windows/hidapi_hidsdi.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ typedef BOOLEAN (__stdcall *HidD_GetIndexedString_)(HANDLE handle, ULONG string_
typedef BOOLEAN (__stdcall *HidD_GetPreparsedData_)(HANDLE handle, PHIDP_PREPARSED_DATA *preparsed_data);
typedef BOOLEAN (__stdcall *HidD_FreePreparsedData_)(PHIDP_PREPARSED_DATA preparsed_data);
typedef BOOLEAN (__stdcall *HidD_SetNumInputBuffers_)(HANDLE handle, ULONG number_buffers);
typedef BOOLEAN (__stdcall *HidD_SetOutputReport_)(HANDLE HidDeviceObject, PVOID ReportBuffer, ULONG ReportBufferLength);

#endif

Expand Down
Loading