hidapi documentation about number of bytes written and read #589
Comments
|
Relevant discussion before, which fixed the reported Input Report number of reading bytes
PR #232 is for sure correct, since the original code wrongly assumes the buffer does not contain the report ID. Probably similar patch is requrired for Output report. The Windows behavior is not wrong per se, Windows HID driver indeed adds the report ID 0 (no report ID) as the first byte in the buffer. However the report ID 0 is not really send on the USB bus. Since HIDAPI is a cross-platform library, I think we should make Windows to behave the same as Linux and macOS. Therefore we should not report 65 bytes in the above example for Windows, but rather 64 bytes which are the number of bytes the device really received. When there is repot ID, Windows will still require the first byte to be the report ID, which is transmitted on the USB Bus. Idealy we should always report the number of bytes transmitted on the USB bus, no matter there is a report ID or not. In that case, we will get the same number of bytes, for HID Input report, Output report and Feature report. |
mcuee
changed the title
mcuee
changed the title
|
Test using Jan Axelson's generic HID example here. Windows reported 3 for Feature report writing and reading, one byter longer than the real device (2 bytes Input report, Output Report and Feature report, no report IDs). And same for Output report, Windows reports 3 bytes written. Only for Input report, Windows is correct to report 2 bytes have been read, thanks to PR #232. WIndows also always requires to send report ID 0 for the Output report for devices without report ID (and this causes the extra one byte length reported for the write), whereas Linux does not. But in this case, hidapitester under Linux has the same behavior in terms of feature report as Windows. And we have already a commit similar to PR #232 for Feature report. So maybe the device really has a three bytes Feature report. |
|
So I am pretty sure we need to fix number of bytes written Output Report under Windows, with all my test results in #478. I am not so sure about Feature report yet since Linux behaves the same as Windows. Current implementation under Windows. It mentions that 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 which is shorter than that. I think we need to report the length minus one, at least for the case when there is no report ID. |
|
I think you have very good understanding of the issue and the necessary test devices. Please take a look at this issue as well when you got the chance. |
|
I think for Feature report, Linux hidraw implemenation is kind of the same as Windows. So that probably explains why I see the same behavior under Windows and Linux, since we just report the buffer length (without minus one for the case when there is no repor ID) and not the data on the USB bus. Commits 6fcb0bb is for sure correct, since the original code wrongly assues the buffer does not contain the Feature Report ID. Now the question is whether we need to minus 1 for the case of no Report ID, to reflect the data on the USB Bus, and not the OS driver. https://www.kernel.org/doc/Documentation/hid/hidraw.txt
|
|
As of now, hidapitester using hidapi-libusb (custom build) behaves pretty same as hidapi-hidraw. So that is a good thing. |
mcuee
changed the title
mcuee
changed the title
As per HIDAPI documentation, we should include the |
|
Therefore only values from 1 to 255 can be Report IDs. The value 0 can be used in the ReportDescriptor, to define that the reports do not contain a ReportID. Windows implements it as specified in the class specification and adds the Report ID prefix byte only for devices that use Report IDs. I know that we need to keep backward compatibility, but from a clean API I would expect a clear seperation between address and data in two arguments. |
Just wondering what you mean by the above. Are you proposing some new APIs? I think that will be okay if there are real benefits. We can always keep the older APIs (and in the future to deprecate them in HIDAPI 1.x version if necessary). |
|
Just tested under macOS (13.4, Mac Mini M1) and the behavior is the same as Linux. So only Windows is out for the Output Report. |
I believe that is a though process "out loud", i.e. "it is probably would be clean to have an API where we send the report ID as a separate argument and only the report body in the buffer". I think that will not change anything in terms of this issue. We still have to fight with different drivers/OS implementation to match the behavior the way we want. And because as per current documentation - we should always count the report ID as part of the data returned - having it as a separate argument would essentially mean |
|
But pre-pending the |
|
That's exactly my point. |
|
If you want to unify the behaviour between platforms you need either to prepend the |
There are two aspects of the issue. 1. Buffer handling -- platform specific. I think the current codes already do that. For example, Windows codes have already done that, at least for Output Report and Feature Report (first byte of the report is the report ID, 0 when there is no report ID). And I believe all platforms are doing that for Feature report, based on my testing results. I tend to believe the current codes are already fine in this aspect, i.e, we do not have issues in terms of getting thngs done (the device will receive the right data, host will also receive the right data). 2. Read/Write lentgh reporting I think this is the issue now. It is not consistent. Two ways of reporting -- now we are mixing the two. b) length of the data bytes appearing on the USB Bus -- Input Report (not including the 0 byte for device without Report ID). For Windows, this could be due to the way we handle Input Report differently from Output Report and Feature report. Currently I believe there is no behaviour differences for Input Report across platforms. c) mix of the two ways -- Output Report (Windows always including the 0 byte for device without Report ID, other platforms may or may not including). So Windows may report one byte more than the other platforms or may report the same length as the other platforms. I think we should at least fix this one. 3. Potential Solution for Issue 2. b) If we just want to unify the behavior across platforms, then we only need to fix Output Report write length reporting to the user. I tend to think we only need to make Windows consistent with others. So only one platform (Windows) and one report type (Output Report) needs to be fixed. |
+1 |
mcuee
changed the title
Okay, I agree this is the way to go to keep backwards compatibillity and with minimum changes. We may also need to improve the documentation as well. |
|
I will try to compare the Linux hidraw implementation versus Windows implementation. Let's talk about INPUT Report first. One thing hidapitester does not test is to use 1) Linux hidraw and Input Report Linux hidraw docuemtation shows there will be no discrepancies using
2) Windows HID API and Input Report Windows So we will have a discrepancy here within the Windows implementations betwen using By the way, Windows Reference for Windows IOCTL_HID_GET_INPUT_REPORT IOCTL (hidclass.h) Input buffer If the collection includes report IDs, the requester must set the first byte of the output buffer to a nonzero report ID. Otherwise, the requester must set the first byte of the output buffer to zero. Input buffer length Reference for Linux: https://docs.kernel.org/hid/hidraw.html read() On a device which uses numbered reports, the first byte of the returned data will be the report number; the report data follows, beginning in the second byte. For devices which do not use numbered reports, the report data will begin at the first byte. write() The first byte of the buffer passed to write() should be set to the report number. If the device does not use numbered reports, the first byte should be set to 0. The report data itself should begin at the second byte. HIDIOCSFEATURE(len): This ioctl will send a feature report to the device. Per the HID specification, feature reports are always sent using the control endpoint. Set the first byte of the supplied buffer to the report number. For devices which do not use numbered reports, set the first byte to 0. The report data begins in the second byte. Make sure to set len accordingly, to one more than the length of the report (to account for the report number). HIDIOCGFEATURE(len): This ioctl will request a feature report from the device using the control endpoint. The first byte of the supplied buffer should be set to the report number of the requested report. For devices which do not use numbered reports, set the first byte to 0. The returned report buffer will contain the report number in the first byte, followed by the report data read from the device. For devices which do not use numbered reports, the report data will begin at the first byte of the returned buffer. HIDIOCSINPUT(len): This ioctl will send an input report to the device, using the control endpoint. In most cases, setting an input HID report on a device is meaningless and has no effect, but some devices may choose to use this to set or reset an initial state of a report. The format of the buffer issued with this report is identical to that of HIDIOCSFEATURE. HIDIOCGINPUT(len): This ioctl will request an input report from the device using the control endpoint. This is slower on most devices where a dedicated In endpoint exists for regular input reports, but allows the host to request the value of a specific report number. Typically, this is used to request the initial states of an input report of a device, before an application listens for normal reports via the regular device read() interface. The format of the buffer issued with this report is identical to that of HIDIOCGFEATURE. HIDIOCSOUTPUT(len): This ioctl will send an output report to the device, using the control endpoint. This is slower on most devices where a dedicated Out endpoint exists for regular output reports, but is added for completeness. Typically, this is used to set the initial states of an output report of a device, before an application sends updates via the regular device write() interface. The format of the buffer issued with this report is identical to that of HIDIOCSFEATURE. HIDIOCGOUTPUT(len): This ioctl will request an output report from the device using the control endpoint. Typically, this is used to retrieve the initial state of an output report of a device, before an application updates it as necessary either via a HIDIOCSOUTPUT request, or the regular device write() interface. The format of the buffer issued with this report is identical to that of HIDIOCGFEATURE. |
|
For Feature Report: 1) Linux hidraw for Feature Report Linux hidraw uses the following IOCTLs for Sending and Receiving Feature Report. The Linux hidraw documentation seems to indicate that Linux HIDAPI https://docs.kernel.org/hid/hidraw.html This ioctl will send a feature report to the device. Per the HID specification, feature reports are always sent using the control endpoint. Set the first byte of the supplied buffer to the report number. For devices which do not use numbered reports, set the first byte to 0. The report data begins in the second byte. Make sure to set len accordingly, to one more than the length of the report (to account for the report number). HIDIOCGFEATURE(len): This ioctl will request a feature report from the device using the control endpoint. The first byte of the supplied buffer should be set to the report number of the requested report. For devices which do not use numbered reports, set the first byte to 0. The returned report buffer will contain the report number in the first byte, followed by the report data read from the device. For devices which do not use numbered reports, the report data will begin at the first byte of the returned buffer. 2) Windows: From the documentations and testing results, Windows implementation matches the HIDAPI documentation.
3) Refererence for Windows 1) HidD_SetFeature function (hidsdi.h) [in] ReportBuffer Pointer to a caller-allocated feature report buffer that the caller uses to specify a HID report ID. For more information about this parameter, see the Remarks section. [in] ReportBufferLength The size of the report buffer in bytes. The report buffer must be large enough to hold the feature report plus one additional byte that specifies a nonzero report ID. If report ID is not used, the ID value is zero. 2) IOCTL_HID_GET_FEATURE IOCTL (hidclass.h) https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/hidclass/ni-hidclass-ioctl_hid_get_feature Input buffer If the collection includes report IDs, the requester must set the first byte of the output buffer to a nonzero report ID. Otherwise, the requester must set the first byte of the output buffer to zero. Input buffer length |
|
For Output Report: 1) Linux hidraw implementations. From the documentation it seems to be in line with the above documentation. However, in testing, it seems to support both uses cases -- to ignore the report ID 0 or not to ignore the report ID 0 for devices without Report ID. https://docs.kernel.org/hid/hidraw.html The first byte of the buffer passed to write() should be set to the report number. If the device does not use numbered reports, the first byte should be set to 0. The report data itself should begin at the second byte. 2) Windows implementation Even though I mentioned that Windows may report one more byte than the other platforms, I tend to think it is actually according to the HIDAPI documentation. From the comments: using Windows Take note we do not use either |
|
Summary: It seems to me HIDAPI Windows implementation is actually consistent with the HIDAPI documentation, even though the behavior seems to be a bit different from the other platforms, at least for the Output Reports. To do:
I will try to use a modified version of hidapitester.
|
My conclusion: I am guessing that Linux hidraw documentations on
Confusions about
From Linux kernel source code, I think indeed this is the case. https://github.com/torvalds/linux/blob/master/drivers/hid/hidraw.c
|
Windows testing results with device with Report ID. Windows testing results with device without numbered Report ID. My modification: |
|
Linux hidraw test results 1) For devices with numbered report ID Results are consistent with Windows. 2) For devices without numbered report ID. Result are not always consistent with Windows. But if we follow HIDAPI documentatation exactly, it seems to be consistent. |
Linux libusb test results 1) For devices with numbered report ID Results are consistent with Windows 2) For devices without numbered report ID. |
|
Somehow based on the test results, if I follow exactly the existing HIDAPI documentation, there are no issues, Windows implementations are consistent with Linux hidraw and Linux libusb implementations. It is just that Linux hidraw and libusb implementations are more flexible in terms of output reports.
And I think it is not crticial that hidapi is a bit lax in terms of reporting to the user number of bytes written, as long as the device gets the right data.
Edit: there is one thing to fix about documentation for The last part is not correct. It should be changed to the following. |
|
If possible, please test on your end to see if you come to the same conclusion as I or not. Thanks. My conclusion is based on my test devices and it could be totally wrong if the firmware has issues. |
You mentioned the following.
If my conclusion is correct that Linux hidraw documentation about HIDIOCGFEATURE is incorrect, then your confusion will be cleared. Now we need to confirm with Linux HID developer side. Since you are also very familiar with hidapi and hidraw, please check on your side as well under Linux. Thanks. |
I think my modification is working even though it is a quick and dirty fix. It turns out I did not modify Jan Axeson's original HID FW correctly for devices without numbered report ID. I have fixed that for report length <=63 Bytes. I still have one issue with the FW for Report length = 64 Bytes but I will try to fix that later. Here are the results under Windows and I can see that Windows implementation is as per the documentation, using the FW without numbered report ID and report length = 63 Bytes. We can see that |
|
Linux hidraw testing results are the same as Windows if I use the same commands (as per HIDAPI documentation). |
|
Linux libusb testing results are the same as Windows if I use the same commands (as per HIDAPI documentation). |
|
In the end, if my testing results are correct, then there is only one thing to fix about documentation for It should be changed to the following. One thing I would like to do is to check with linux-input mailing list to confirm that Linux hidraw documentation is not correct for the two IOCTLs: https://docs.kernel.org/hid/hidraw.html HIDIOCGFEATURE(len): I beleive this shoudl be: HIDIOCGINPUT(len): |
That I agree, it will be way more consistent overall. I'd make one general statement: from a good API/architecture point of view: if we're passing a buffer (regardless of its nature) and expect it to be read or written to - it is best if API function (the return value) gives that buffer size (from the start) in case of success. |
Great. I have actually similar thoughts when I wrote the following in the above comment. If we look at the testing results, only |
Question asked in linux-input mailing list. |
I don't think we should change the behavior of that one either. And, of course, breaking backward compativility is something we should to avoid as much as we can. |
|
Modified testing code from the following Linux kernel example using hidraw for the above test device |
mcuee
changed the title
|
Moreover the kernel codes here seem to prove that Linux documentation on hidraw https://github.com/torvalds/linux/blob/master/include/uapi/linux/hidraw.h |
…out report IDs" This reverts commit 69e4ee6. In the commit (from #1217) it was wrongly assumed what the transfer length actually includes, and the change caused wrong values for Output Report and Feature Report. See also #1285 where the regression was first analyzed. More discussion about lengths reported from HIDAPI point of view: libusb/hidapi#589 References #1217 References #1285
https://lore.kernel.org/linux-input/[email protected]/T/#m458a231a4f3c9eb91553656b9d7d50d86cacd4be Answer from the linux-input core maintainer. ++++++++++++++++++++++ This should be read: FWIW, the difficulty to find out what the code does is because this part Looking at drivers/hid/usbhid/hid-core.c, lines 869+, the function |
|
@Youw @tormodvolden @jonasmalacofilho @JoergAtGithub @todbot Hopefully the above answer clear the doubt on this issue. |
|
that is consistent with the "general statement" above |
|
BTW, I believe the following kernel code explains the behavior of Output Report handling under Linux. For HID device without numbered report, it is okay to skip the report ID 0 for Output Report (not following HIDAPI documentation) under Linux hidraw backend. But it is better to follow HIDAPI documentation to include Report ID 0 in the buffer in order to be cross-platform code. https://github.com/torvalds/linux/blob/master/drivers/hid/usbhid/hid-core.c Same for I2C HID. |
|
But I can not find similar things for bluetooth, maybe the underline Bluetooth subsystem is handling that. |
Discusison here:
As @todbot mentioned, this has been an issue bugging him. Same for me.
Testing device: Circuit Python rawhid example, no report IDs.
boot.py
code.py
hidapitester output under Windows, it says 65 bytes written
hidapitester output under Linux (Ubuntu 20.04 x64): it says 64 bytes written.
hidapitester output under macOS 13.4 (Mac Mini M1): it says 64 bytes written.
Device Found
type: 2e8a 102e
path: /dev/hidraw3
serial_number: DE6185100F4D6522
Manufacturer: VCC-GND Studio
Product: YD-RP2040
Release: 100
Interface: 3
Usage (page): 0x1 (0xff00)
Bus type: 1 (USB)
Report Descriptor: (34 bytes)
0x06, 0x00, 0xff, 0x09, 0x01, 0xa1, 0x01, 0x09, 0x02, 0x15,
0x00, 0x26, 0xff, 0x00, 0x75, 0x08, 0x95, 0x40, 0x81, 0x02,
0x09, 0x03, 0x15, 0x00, 0x26, 0xff, 0x00, 0x75, 0x08, 0x95,
0x40, 0x91, 0x02, 0xc0,
The text was updated successfully, but these errors were encountered: