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.

Sign up for GitHub

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

Jump to bottom

Missing one byte feature report read under Windows for devices without report id #328

Closed
mcuee opened this issue Aug 30, 2021 · 10 comments · Fixed by #334
Closed

Missing one byte feature report read under Windows for devices without report id #328

mcuee opened this issue Aug 30, 2021 · 10 comments · Fixed by #334
Labels
bug Something isn't working Windows Related to Windows backend

Comments

@mcuee
Copy link
Member

mcuee commented Aug 30, 2021

Reference here:
#285

Now I start to question the commit 6fcb0bb which I proposed in #232 with regard to feature report. It seems to be correct when the report id is not zero. However, it seems to report one less byte when there is no report id. It seems to me for Windows, we may need to report one more byte for reading of feature report when the report id is zero.
@mcuee mcuee added the Windows Related to Windows backend label Aug 30, 2021
@mcuee
Copy link
Member Author

mcuee commented Aug 30, 2021

#285 (comment)

Just tried hidapitester with hidapi-0.10.1 release and hidapi git head, with regard to input report, the result is the same since it is using hid_read_timeout(). So #232 does not matter in that case.
https://github.com/todbot/hidapitester/blob/master/hidapitester.c#L379

With regard to feature report, then the result will be different as it uses hid_get_feature_report(). So commit 6fcb0bb does matter for getting the feature report. In this particular case without report id (report id zero), it seems to me the commit is not correct.
https://github.com/todbot/hidapitester/blob/master/hidapitester.c#L403

$ ./hidapitester_release0101.exe --vidpid=0925:7001 --open --send-feature 0,0x40,0x41 --read-feature 0
Opening device, vid/pid: 0x0925/0x7001
Writing 64-byte feature report...wrote 64 bytes:
 00 40 41 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Reading 64-byte feature report, report_id 0...read 3 bytes:
 00 40 41 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Closing device

$ ./hidapitester_git4d63a50.exe --vidpid=0925:7001 --open --send-feature 0,0x40,0x41 --read-feature 0
Opening device, vid/pid: 0x0925/0x7001
Writing 64-byte feature report...wrote 64 bytes:
 00 40 41 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Reading 64-byte feature report, report_id 0...read 2 bytes:
 00 40 41 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Closing device

@mcuee mcuee mentioned this issue Aug 30, 2021
Closed
@Youw
Copy link
Member

Youw commented Sep 4, 2021

Do you think we need to do the same as we do for macOS?

@mcuee
Copy link
Member Author

mcuee commented Sep 5, 2021

Do you think we need to do the same as we do for macOS?

Yes I think that will fix this issue.

@mcuee mcuee added the bug Something isn't working label Sep 13, 2021
@Youw
Copy link
Member

Youw commented Sep 22, 2021

I think this one is worth fixing before releasing v0.11.0

@Youw
Copy link
Member

Youw commented Sep 22, 2021

After reading the documentation for IOCTL_HID_GET_INPUT_REPORT and IOCTL_HID_GET_FEATURE one more time, combined with your test results, I believe input_report is as broken as feature_report

Youw added a commit that referenced this issue Sep 22, 2021
@Youw
Windows: hid_get_feature/input_report return size fix (#334)
19e809e 
- It appears when the numbered reports aren't used,
WinAPI returns size of the report excluding the data[0] which contains 0,
as an indication that numbered reports aren't used;
Explicity count that byte in the result.

Fixes: #328
@Youw Youw mentioned this issue Sep 22, 2021
Merged
Youw added a commit that referenced this issue Sep 22, 2021
@Youw
Windows: hid_get_feature/input_report return size fix (#334)
ad36065 
- It appears when the numbered reports aren't used,
WinAPI returns size of the report excluding the data[0] which contains 0,
as an indication that numbered reports aren't used;
Explicity count that byte in the result.

Fixes: #328
@mcuee
Copy link
Member Author

mcuee commented Sep 23, 2021

After reading the documentation for IOCTL_HID_GET_INPUT_REPORT and IOCTL_HID_GET_FEATURE one more time, combined with your test results, I believe input_report is as broken as feature_report

Let me test out #334 over the weekend.

@mcuee
Copy link
Member Author

mcuee commented Sep 27, 2021 •
edited
Loading

Firstly for feature report for device without report ID.

I think #334 is correct since it reports reading 3 bytes. Release 0.10.1 is actually correct but then 6fcb0bb introduced the regression. #334 fixes the issue in the current master.

$ ./hidapitester_release0101.exe --vidpid=0925:7001 --open --send-feature 0,0x40,0x41 --read-feature 0
Opening device, vid/pid: 0x0925/0x7001
Writing 64-byte feature report...wrote 64 bytes:
 00 40 41 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Reading 64-byte feature report, report_id 0...read 3 bytes:
 00 40 41 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Closing device

$ ./hidapitester_hidapi_master.exe --vidpid=0925:7001 --open --send-feature 0,0x40,0x41 --read-feature 0
Opening device, vid/pid: 0x0925/0x7001
Writing 64-byte feature report...wrote 64 bytes:
 00 40 41 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Reading 64-byte feature report, report_id 0...read 2 bytes:
 00 40 41 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Closing device

$ ./hidapitester_hidapi_fix_feature.exe --vidpid=0925:7001 --open --send-feature 0,0x40,0x41 --read-feature 0
Opening device, vid/pid: 0x0925/0x7001
Writing 64-byte feature report...wrote 64 bytes:
 00 40 41 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Reading 64-byte feature report, report_id 0...read 3 bytes:
 00 40 41 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Closing device

@mcuee
Copy link
Member Author

mcuee commented Sep 27, 2021

Secondly for devices with report ID. No regression from the current master. Release 0.10.1 is not correct and it was fixed by 6fcb0bb .

$ ./hidapitester_release0101.exe --vidpid=0925:7001 --open --send-feature 3,0x40,0x41 --read-feature 4
Opening device, vid/pid: 0x0925/0x7001
Writing 64-byte feature report...wrote 64 bytes:
 03 40 41 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Reading 64-byte feature report, report_id 4...read 4 bytes:
 04 40 41 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Closing device

$ ./hidapitester_hidapi_master.exe --vidpid=0925:7001 --open --send-feature 3,0x40,0x41 --read-feature 4
Opening device, vid/pid: 0x0925/0x7001
Writing 64-byte feature report...wrote 64 bytes:
 03 40 41 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Reading 64-byte feature report, report_id 4...read 3 bytes:
 04 40 41 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Closing device

$ ./hidapitester_hidapi_fix_feature.exe --vidpid=0925:7001 --open --send-feature 3,0x40,0x41 --read-feature 4
Opening device, vid/pid: 0x0925/0x7001
Writing 64-byte feature report...wrote 64 bytes:
 03 40 41 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Reading 64-byte feature report, report_id 4...read 3 bytes:
 04 40 41 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Closing device

@mcuee
Copy link
Member Author

mcuee commented Sep 27, 2021

hdiapitester can not be used to test #334 though as it uses hid_read_timeout().
#285 (comment)

@mcuee
Copy link
Member Author

mcuee commented Sep 27, 2021 •
edited
Loading

After reading the documentation for IOCTL_HID_GET_INPUT_REPORT and IOCTL_HID_GET_FEATURE one more time, combined with your test results, I believe input_report is as broken as feature_report

Even though I can not use hidapitester to test the input report, but I agree with you after reading the documentation one more time.

https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/hidclass/ni-hidclass-ioctl_hid_get_feature
https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/hidclass/ni-hidclass-ioctl_hid_get_input_report

@Youw Youw closed this as completed in #334 Sep 27, 2021
Youw added a commit that referenced this issue Sep 27, 2021
@Youw
Windows: hid_get_feature/input_report return size fix (#334)
b923aed 
- It appears when the numbered reports aren't used,
WinAPI returns size of the report excluding the data[0] which contains 0,
as an indication that numbered reports aren't used;
Explicity count that byte in the result.

Fixes: #328
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working Windows Related to Windows backend
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Windows: hid_get_feature/input_report return size fix libusb/hidapi
2 participants
@mcuee @Youw