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

Zero padding frames on Linux #4

Open
VictorBonjour opened this issue May 4, 2021 · 3 comments
Open

Zero padding frames on Linux #4

VictorBonjour opened this issue May 4, 2021 · 3 comments
Labels
bug Something isn't working

Comments

@VictorBonjour
Copy link

Dear Benedek, dear Luke,

I am using your marvelous library to communicate with some devices on Linux (raspbian).
While doing so I noticed that my device would only reply when the buffer was filled with Device.GetMaxOutputReportLength() bytes. In my case, this value is 32.
So if I am writting only 8 bytes to the device, I would have to pad my frame with 24 extra bytes (zeros) so the buffer would be flushed.

I took a look at your code and this is the fix I propose.
Commit is 87ad1d3 - RELEASE 2.1.0 (May 4, 2019)
File is HidSharp\Platform\SysHidStream.cs, line 112
Replace
byte[] packet = new byte[count];
with
byte[] packet = new byte[maxOutputReportLength];

Please find patch file attached.
ZeroPaddingLinux.patch.zip

This fixes the issue on my Linux system.
Bug is not present on Windows.

Thank you for your work.
I remain available in case of questions.
Victor

@benedekkupper
Copy link
Member

Hi Victor,

Thanks for bringing this issue to our attention. The library itself isn't our work, the base is the mirror of James F. Bellinger's library.

Before I jump into merging this (which would be better filed as a pull request than an issue), can you explain to me the HID device's reports layout? Does this device have multiple OUT reports? What's the size of the OUT report that you're trying to set? I hope you are aware that HID requires the full length of any report to be transferred, partial report updates aren't supported.

@VictorBonjour
Copy link
Author

VictorBonjour commented May 12, 2021

Hi Benedek,

I may be a bit confused here, please bear with me.

I'm using only one OUT report and its length is 32.
I might have mistakenly imagined that I could send only 8 bytes and that HidSharp would zero-pad it to 32 bytes for me. If I read you well, this is not the expected behavior.

I'm a bit confused because on Windows, I can send my 8 bytes to HidSharp and it will send 32 bytes to my device regardless. Have a look at WinHidStream.Write(byte[] buffer, int offset, int count), line 164:

int minOut = Device.GetMaxOutputReportLength();
if (minOut <= 0) { throw new IOException("Can't write to this device."); }
if (_writeBuffer == null || _writeBuffer.Length < Math.Max(count, minOut)) { Array.Resize(ref _writeBuffer, Math.Max(count, minOut)); }
Array.Copy(buffer, offset, _writeBuffer, 0, count);

If the actual message length ("count", in my example this would be 8) is less than the report length ("minOut", in my example 32), then the write buffer is 32.
Perhaps this is not a behavior I should rely on?

Essentially I'm confused because the behavior is different on Windows and Linux.
Cheers
Victor

PS: Sorry for not making a pull request, will do next time!

@benedekkupper
Copy link
Member

Hi Victor,

Yes, it seems that the windows behavior is the faulty one, I will verify and correct it to only accept full size report buffers. Thank you for bringing this to our attention.

@benedekkupper benedekkupper added the bug Something isn't working label May 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants