Write not working with Windows 7

[diogofu]

Hi,

this might be related to issue #13. We were having problems using the write function in HidDevice - we had it working with Windows 8.1 and 10, but it didn't work in Windows 7.Digging a little bit we found about the Report ID problem with Windows platforms, that it should be set to 0x00 (as seen here: https://github.com/signal11/hidapi/issues/253). We are working with version 0.4.0.

The problem might be within the write called from HidApi, where the if clause for reportId == 0 ends up sending the data without an ID.

if (Platform.isWindows() && reportId == 0) {
      // Compensate on Windows for 0x00 report ID misalignment
      // This avoids "The parameter is incorrect" on Windows
      report = new WideStringBuffer(len);
      if (len > 1) {
        System.arraycopy(data, 0, report.buffer, 0, len);
      }
    } else {
      // Put report ID into position 0 and fill out buffer
      report = new WideStringBuffer(len + 1);
      report.buffer[0] = reportId;
      if (len > 1) {
        System.arraycopy(data, 0, report.buffer, 1, len);
      }
    }
    return hidApiLibrary.hid_write(device.ptr(), report, report.buffer.length);

We could make things work with Windows 7 by sending a data array with an extra 0x00 byte in position 0 and the data starting in position 1 (which happens to be what the else clause does for reportId != 0), and using a reportId = 0, so we kind of bypassed the write from HidApi.

Should this if clause work like this? Am I missing something here?

[gabrielpaim]

Interesting:

I tested @diogofu 's suggestion on a Win7 32 bit ultimate edition, Centrino duo @1.73GHz 1.73GHz, 2GB ram with a PIC18f47j53 and it worked for me!

Somehow, the 'if clause' on HidApi.write(HidDeviceStructure device, byte[] data, int len, byte reportId) is not working on 0.4.0 (testing with this environment). However, it is indeed working on other Windows and OSX platforms. Any considerations?

Btw, thanks @gary-rowe for this lib, great job! :)

[gary-rowe]

Thanks for raising this (and for the kind words).

Just to clarify, the code is intended to behave as follows:

• On Windows (all variants) with a report ID of 0x00, the report ID is dropped and only the data is sent.
• On Windows (all variants) with a non-zero report ID or non-Windows with any report ID, the report ID is placed in position 0 and the data buffer appended to position 1 up to the end of data buffer.

As @diogofu correctly identified, this was due to Windows HID handling causing a misalignment that lead to a "The parameter is incorrect" message popping up. Thinking back to the time, I believe I referred to signal11/hidapi #13 and uncovered some other research that indicated that a Windows quirk was occurring which required that the report ID be dropped if it was 0x00.

Looking back over this code now it seems a bit suspicious to drop the report ID entirely so I'd like to see it tested more thoroughly and a proper fix put in place - possibly ditching the "misalignment" code altogether.

@gabrielpaim Would you mind performing some additional tests on your Windows 7 machine using the current code on the develop branch? This removes the "misalignment" code and treats all platforms the same. I have a range of machines here, but Windows 7 isn't currently available for testing.

[gabrielpaim]

According to your suggestion, I performed another test with the same conditions (Windows 7 32bits and PIC18F 8bits, throughput ~ 13KB/s) using your last develop commit. Working like a charm.

Maybe @diogofu can help us performing his test on a Windows 7 64b arch. Is that possible? Thank you again, we're looking forward to test a stable release of the project. :)

[diogofu]

Tested it in 2 PCs using Windows 7, a centrino duo @1.66GHz 1.67GHz 64-bit 2GB RAM and a Core i5 @2.67GHz 2.66GHz 64-bit 4GB RAM, both work as well. Nicely done guys!

[gary-rowe]

Thanks, guys. Glad to see that Windows 7 is working.

Running without the "misalignment" code on Windows 10 yields "The parameter is incorrect". So it looks like I have to filter out the Windows version to get a reliable solution.

Does anyone have a Windows 8 machine lying around that they could quickly spin up and verify the develop code against? I used to have one and the "misalignment" code worked back then but I'd like to be sure that it generates the same error as Windows 10.

[diogofu]

I just tried it with my pc, it's Windows 8.1 64-bit, seems to be working just fine, no errors.

[gabrielpaim]

Hi there,

@gary-rowe , we've tested again the develop-snapshot on a Win 10 machine (Pro edition, i53230M 8GB ram, X64), with the same device. It worked well, without the "incorrect parameter" fault. I also tested on a OSX 10.11.3 MacbookPro, and it worked too.

Well done!

[gary-rowe]

Apologies for the slow responses from me on this issue and many thanks for the updated info from everyone.

I'm still seeing weirdness with my USB test devices (a Trezor and a KeepKey). When I use the (newly renamed) UsbHidDeviceExample test message, I still get a The parameter is incorrect response on Windows 10 (and probably 8 given its history).

This seems to imply that there is special Windows behaviour occurring in signal11/hidapi, and examining the code in the Linux version of hid_write() (see https://github.com/signal11/hidapi/blob/master/libusb/hid.c#L1009) shows that the report Id is stripped out if it is zero before being passed to libusb.

In the Windows version of hid_write() (see https://github.com/signal11/hidapi/blob/master/windows/hid.c#L604) this does not occur and the buffer is passed through. This could lead to a misinterpretation of the first byte as a zero length buffer (instead of 0x3f for 63 bytes of payload in a 64 byte buffer) which in turn triggers "the parameter is incorrect".

What's strange is that the Windows 7 behaviour is different to Windows 8 and 10, so it seems that sometimes you need report ID 0x00 at the start and sometimes you don't.

In order to make progress I've made the behaviour backward compatible with a flag to switch it on or off as required by the API consumer. The latest push contains code that does the following:

• detects Windows 8/10 only and sets the dropReportIdZero flag to true if either is present
• if the user can better determine the value of dropReportIdZero they can override for their use case/device
• if the flag is set and the report ID is zero then the old "misalignment" code is used
• in all other cases the standard (correct) handling is used

Thoughts?

Please could you test your devices with the new code just pushed to develop. It should work correctly out of the box. If not, it would be interesting to uncover a variance in the OS, JVM version or USB report count (e.g. more than one report ID on the device).

I'll mark this as Ready for Review to indicate that I think we're almost done.

[andyrozman]

Reviewed and it seems ok.

[gary-rowe]

Merged and closed.