hidapi
hidapi copied to clipboard
The hid_write return value is too large on windows
The Windows HID api always expects to receive a fixed size buffer (corresponding to the largest report supported by the device). Therefore the hidapi library internally pads the buffer with zeros to the expected size, but apparently it also returns the size of the padded buffer! Thus, for small writes it returns a value that is larger than the buffer that was passed. And that leads to very unexpected results when you actually use the return value.
For example, if I write 2 bytes to a device with an output report size of 32 bytes, then hid_write will return 32 instead of 2!
For example, if I write 2 bytes to a device with an output report size of 32 bytes, then hid_write will return 32 instead of 2!
I don't think that is correct. I'm pretty sure I get back the right size. What type of report are you using? Maybe there is a bug. I mainly use feature reports and they work correctly. I can double check.
Unfortunately you are wrong. The bug is easy to spot when you look at the code:
if (length >= dev->output_report_length) {
...
} else {
...
length = dev->output_report_length;
}
res = WriteFile(dev->device_handle, buf, length, NULL, &ol);
...
res = GetOverlappedResult(dev->device_handle, &ol, &bytes_written, TRUE/*wait*/);
...
return bytes_written;
If the length (2 bytes) is smaller than the output report size (32 bytes), the length is set to the expected size of the output report. Because that's the amount that gets passed to the WriteFile call, that's also the amount that GetOverlappedResult will report as bytes_written. And thus the return value of the hid_write function (32 bytes) is larger then the amount of bytes that was passed (only 2 bytes)!
You're right I had this back-words. I was thinking of hid_read. So yes if writing to device, you always get the size that is on the descriptor. I was under the impression that is how USB worked. This was always a pain to me. So I was forced to make a few input reports as I didn't want to send with a global size for all reports.
It's how Windows works, unfortunately. You have to give it the length of the longest report, and it gives you the length of the longest report. It's only an issue for devices with multiple (numbered) reports, so it doesn't come up very often.
I knew you had to match the report size but I didn't know it had to be the size of thee greatest reports. That is odd.
The problem of this Windows "feature" is that it can have some nasty consequences if you're not aware of it and you try to use the return value. For example it's common practice to compare the return value with the passed length to check whether the entire write operation was successful (and not just a partial success):
nbytes = hid_write(device, data, size);
if (nbytes < 0 || nbytes != size) {
/* Error or partial write */
}
Suddenly an error is reported on Windows (but not on Linux or Mac), even if the operation was actually successful!
You can also easily end up with a buffer overflow in the application, if you try to use the return value as a byte count later on:
memcpy(buffer, data, nbytes); /* Oops, copying 32 bytes to/from a 2 byte buffer! */
Since you know how many bytes the caller passed, you could easily return that size as the return value instead of the bytes_written value returned from the WriteFile call. A user of a cross-platform library shouldn't need to know about this implementation quirk.
Now I use this workaround on the caller side:
#ifdef _WIN32
if (nbytes > size) {
nbytes = size;
}
#endif
and everything works fine again. But I reported this issue because I discovered it the hard way, and other people might run into it as well.
I have that same issue, and had to pass in my intended size. reportId, size, payload... I check on the other end and ignore the padding.
I actually I took it a bit further and do this now.
reportId, size, command, payload...
on my device end I check the Rid, size and use the command for my intention. This way I have 1 report for my 0-8 range, 0-16 range, and so one. This way I do not have to use a lot of report ID's. Not needed if you only have a few.
if (reportId == 20 ) /my 8 size reports) switch (command) ...
if (reportId == 21) /my 16 size reports) switch (command) ...
if (reportId == 22 ) /my 32 size reports) switch (command) ...
You get the idea.