-
Notifications
You must be signed in to change notification settings - Fork 397
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
Added support for Get Input Report to linux, mac, windows and libusb. #59
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Windows implementation better be same as for hid_send_feature_report
, to return actual length
(tested in my projects)
mac implementation sends report ID as part of data, as it was in hid_get_feature_report
before #3
hidraw implementation does not respect length
parameter, which may lead to memory corruption
libusb implementation seems fine to me
Thanks!
I was going to have it implemented for some while, this is a good start.
Some notes:
- This is a major change (new API), we should update version too
- Along with new version, I think it makes sense add a way to determine library version in compile/run-time.
…re_Report so the returned length matches.
Co-Authored-By: Ihor Dutchak <ihor.youw@gmail.com>
Windows implementation better be same as for hid_send_feature_report, to return actual length mac implementation sends report ID as part of data, as it was in hid_get_feature_report before #3 hidraw implementation does not respect length parameter, which may lead to memory corruption |
pass pointer to stack-allocated data that is guaranteed to be large enough for the API, and then copy up to |
regarding mac and #60 - lets wait for comments of other maintainers/users |
something doesn't adds up in hidraw implementation: if I understand how I'll spend more time investigating it a bit later |
It is possible, I wrote (and studied) the windows, mac and libusb implementations. Another developer that I worked with wrote the Linux one (I saw your issue at signal11/hidapi and am trying to up-stream now). I simply verified that it worked with both test boards that I have. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, with a few minor comments.
I am working on a new device with HID and also on the host side. This pull request is rather timely for me as the day it was created was the same day I was getting ready to implement the same functionality. |
HID Spec v1.11 in section 5.6 also specifies how to know if a device is using numbered reports:
@DanielVanNoord, @DavidCNelson can you confirm, that your devices defines corresponding Report IDs tags in the Report descriptor? |
Yes, the device I'm working with does define report IDs in the descriptor, and so we do use report IDs for all transfers. |
Same here, the device I am working on uses report IDs in the descriptors for all transfers. |
Co-Authored-By: Ihor Dutchak <ihor.youw@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DanielVanNoord please resolve merge conflict, and:
- update mac implementation as per macOS: send report id conditionally for hid_get_feature_report #70
- update windows\ddk_build\hidapi.def
@Youw Should be taken care of now |
This was tested for all operating systems and each worked identically.