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

ReceiveReport function? #94

Open
patratacus opened this issue Feb 22, 2022 · 16 comments
Open

ReceiveReport function? #94

patratacus opened this issue Feb 22, 2022 · 16 comments

Comments

@patratacus
Copy link

patratacus commented Feb 22, 2022

Is there a way to just receive an Input Report without having to do a SendReport first? There's a ReceiveFeatureReport function but why couldn't a ReceiveReport be done directly? I understand that Feature Report is bi-directional while Input and Output reports are one way. Maybe I don't understand this USB HID concept too clearly, but there's a receive Input Report implementation in other libraries like Csharp.HID so I'm not sure why it's not available here.

I couldn't seem to get any input report to come in even when I do a Device.Open() or send a report. The device I'm working with is an STM32L46x MCU that implements USB HID protocol. I can actually enumerate the report from the device.collections and see all the reports. I could even write to it successfully. I just can't seem to ever get any reports back using the "inputreport" event listener.

@beaufortfrancois
Copy link
Contributor

@nondebug
Copy link
Collaborator

nondebug commented Apr 8, 2022

The USB HID specification (section 6.2.2.5) describes two ways to get input reports:

  • Input items define input reports accessible via the Control pipe with a Get_Report (Input) request.
  • Input type reports are also sent at the polling rate via the Interrupt In pipe.

Input reports can be initiated either by the host (via the Control pipe) or the device (via the Interrupt In pipe). Based on your description, I would guess your device wants input reports to be host-initiated and that's why you're seeing this behavior.

Since you mentioned C# I'm assuming Windows, but this is a protocol-level issue so it's possible that this affects other platforms. I think the way to work around this on Windows is to use HidD_GetInputReport. However, MSDN documentation warns against using this API. "an application should only use the HidD_GetXxx routines to obtain the current state of a device. If an application attempts to use HidD_GetInputReport to continuously obtain input reports, the reports can be lost. In addition, some devices might not support HidD_GetInputReport, and will become unresponsive if this routine is used."

Losing data and causing devices to become unresponsive seems bad, so WebHID in Chrome doesn't attempt to use HidD_GetInputReport. Based on the documentation I suspect HidD_GetInputReport sends Get_Report on the Control pipe but I haven't verified it. Chrome uses the ReadFile API which is meant to work for either input report mode and lets the Windows HID driver handle it.

According to USB Complete by Jan Axelson:

The Windows HID driver causes the host controller to request Input reports. The driver stores received reports in a buffer. ReadFile retrieves one or more reports from the buffer. If the buffer is empty, ReadFile waits for a report to arrive. In other words, ReadFile doesn’t cause a device to send a report but just reads reports that the driver has requested.

Perhaps the Windows HID driver doesn't request any reports from the device or perhaps the driver requests the report but it doesn't get delivered to applications. Can you tell whether the Windows HID driver requested a report by sending Get_Report on the Control pipe?

other libraries like Csharp.HID

I'm not familiar with that one, link? Are you maybe talking about SharpLibHid?

It looks like SharpLibHid uses a third Windows HID API: Raw Input. Chrome uses Raw Input in the Gamepad API backend but not for WebHID. Can you tell if there's any difference from the device's perspective when the device is opened through WebHID or SharpLibHid?

@reillyeon
Copy link
Contributor

After reading through the StackOverflow post it seems like the device here is not implementing the USB HID specification correctly. Input reports should be sent to the host as responses to the IN PIDs sent by the host on the interrupt endpoint. The GET_REPORT control transfer is an alternative way of getting input reports from the device but as the Windows documentation mentions, this request may cause problems for some devices which don't expect it as it is designed for use with feature reports. It is also wasteful for script to poll the device like this instead of registering an inputreport event listener.

I am aware that some devices have this buggy behavior and so providing a getReport() method would be useful but in this case, since we're dealing with a device the developer has control over, the right answer is to fix the firmware so that it responds on the interrupt endpoint correctly.

@patratacus
Copy link
Author

The main usage for getReport() is definitely not for continuous polling. In my hardware example, it's mainly to just grab the stored settings from the hardware. I do agree that the on_received even type is better for that if you have a way to run the device with interrupt.

@reillyeon I think you are perhaps correct that the firmware should be updated to correctly match the USB HID specification. That would make the most sense. Thank you for looking into this and we'll see what we could do to make the firmware comply to the standard more either using Feature Reports or using the proper implementation. Probably it doesn't make sense to make WedHID do something to support non-standard USB HID implementation.

I'll close this issue. Thank you.

@reillyeon
Copy link
Contributor

If anything, I think that GET_REPORT(Input) could be reasonable for that kind of "get the initial state of the device" use case. For settings on the other hand, that's what feature reports are for and that's why they are bidirectional.

@nondebug
Copy link
Collaborator

If this is indeed an implementation bug, I suspect the bug is fairly common and there may be many popular devices affected by it. The platform-level HID APIs tend to be fairly tolerant of implementation errors and I feel WebHID should have a similar strategy. We shouldn't bake incompatibility into the API by assuming more compliance with the spec than the platform actually enforces.

I think it's reasonable to make a small API change to support buggy devices if we can demonstrate that there's enough demand for that feature. So far I don't know that any devices have this bug except yours. Let's find a popular device with this bug before we consider changing the API.

@patratacus patratacus reopened this May 9, 2022
@patratacus
Copy link
Author

patratacus commented May 9, 2022

After spending quite a bit of time with the native USB "controltransfer" function, I think the GET_REPORT(Input) should be a part of the WebHID since there's no other way to retrieve an input report if we don't implement an interrupt request for the Input Report or a Feature Report. Our device is not a one-off device that implement this sort of input report retrieval especially for reading a setting from the device. Many examples from PIC microcontroller mentioned the use of get_input_report especially for use in Windows. I have managed to use the controltransfer function to do Set and Get on Android and Linux now. WebHID doesn't have a cotroltransfer function exposed which is probably a good thing due so many misuses or bad arguments.

I don't think this is an unreasonable request since WebHID already has the Set_Report function working. As you probably already know, it's basically the same control transfer function underneath except that you change the bmRequestType from 00100001 (Hex: 0x21) to 10100001 (0xA1), the bRequestfrom SET_REPORT (0x09) in to GET_REPORT (0x01), the Req_value is a bit more tricky since the report type has to be the high byte and ReportID as the low byte. So it should just be (0x01 << 8) | ReportID for Input Report. The Set Report didn't need to do any bit shifting if you make the output reportID as the first index in the command byte array.

Technically get/set feature report is also using the same controltransfer function with report type being 0x03. So for the completeness of the WebHID library, why not implement GET_InputReport function too? Get/Set Feature Report would have the same concern with not being good for polling either, but they are still parts of WebHID. As nondebug mentioned, the HID APIs are pretty tolerant of implementation errors already so I don't see why WebHID needs to be more strict.

As it is, WebHID isn't too useful for our project unless we update our firmware to go with the interrupt input return or use feature reports. We already have over 3,000 of our devices deployed all over the world and the number is increasing, it would be nice to be able to introduce WebHID as another way for people to connect to the devices if they have a Chrome Web Browser. It's sad that we could do all the command send to the device but we couldn't read anything from it.

I already made a demo to do the WebHID interfacing to our device here:
https://dev.chaopricha.com/

It's only useful for setting our device into boot loader mode. Apart from that, it's not too useful.

@reillyeon
Copy link
Contributor

It's important to note that the implementation of WebHID in Chromium (and this is how I expect any other implementation to work) is not directly issuing USB control transfers. It is using the HID API provided by the platform and supports devices connected using transports other than USB. The ability to offer a receiveInputReport() function depends on a similar function being available from the operating system. This functionality appears to be available on all currently supported platforms (listed below) but I want to point out that it is not as simple as adjusting the parameters on a control transfer.

@patratacus
Copy link
Author

@reillyeon If the HID API on various platforms already supports it, wouldn't it make sense for WebHID to use it too? I still couldn't find the reason why not use or make the function available other than it shouldn't be used for polling to get the input report. In our use case, it's actually a legitimate use since we also only query it once, in the beginning, to read the settings on the device and not polling constantly except for one parameter which is only polled once a second. Feature reports would have been better for sure since that was its intended usage. However, the firmware was developed this way using input/output reports rather than feature reports.

The reason I brought up control transfer was because of the lack of that get_input or set_output report via an HID API implementation in the Android USB library. I looked around quite a bit and there wasn't really one available. I came across a Python pyUSB library that was implementing a control transfer in Windows and Linux to do report get/set on a USB HID device. That's how I was able to re-construct similar functions in Android and they seem to be working well. I really would have loved to be able to do something like IOHIDDEviceGetReport like on macOS or HidD_GetInputReport on Windows. on the Android platform.

Correct me if I'm wrong, but so far I don't think WebHID works on either Android or iOS devices even with the use of the Chrome app. For iOS, I think it just blocks any direct HID device if it's not MFI certified. I think only a generic USB keyboard, mass storage, and mouse HID devices work on the iOS. From what I read they might even kill that and only allow Blue Tooth connection only. For Android, I think there are a bunch more permissions required beyond just the connection promissory to use the hardware.

In any case, thanks for considering it. I think WebHID is a great project to replace the dead chrome.HID. There are already more features on WebHID. Thanks for doing great work!

@reillyeon
Copy link
Contributor

After taking a look at the cross-platform interfaces I've come around to being supportive of adding this to WebHID. I think we just need to be very clear in the specification and documentation about the difference between a receiveInputReport() method and the inputreport event and which developers should prefer.

Browsers on Android and iOS will have a difficult time implementing WebHID because those OSes don't provide a HID API like we have on desktop platforms. It is technically possible on Android but will require significant work that is discussed in more detail in Chromium issue 964441.

@nondebug
Copy link
Collaborator

Sounds good, let's add a way to request input reports via control transfer when a suitable platform API is available.

To make the API least surprising, I feel the default behavior of receiveInputReport should use the same platform API as the inputreport event listener. The (maybe dangerous) control transfer behavior should be enabled with an option to make it harder to unexpectedly invoke.

// Use the default platform HID API
device.receiveInputReport();  

// Use HIDIOCGINPUT / IOHIDDeviceGetReport / HidD_GetInputReport
device.receiveInputReport({useControlTransfer:true});

Like receiveFeatureReport, the method would return a promise that resolves to a DataView containing the received input report data, or rejects. The DataView should contain whatever the platform API returns, including the report ID as the first byte if the device uses report IDs. If the device doesn't use report IDs, return whatever the platform returns which may include a zero byte for the report ID.

receiveInputReport should reject:

  • If the useControlTransfer option is set to true but the option isn't implemented on the current platform.
  • If the platform API call encountered an error.
  • At the discretion of the user agent. For instance, if the UA recognizes that the device does not support a particular platform API call then the UA can choose to reject instead of invoking behavior that could corrupt the device state

@reillyeon
Copy link
Contributor

The default behavior of receiveInputReport() would be the equivalent of,

new Promise((resolve) => {
  device.addEventListener('inputreport', (e) => {
    resolve({ reportId: e.reportId, data: e.data });
  }, { once: true });
})

It might be a bit more complicated if we want it to queue up requests so that each call resolves with the next input report. That adds some additional complexity. I don't think it's a good idea to provide a function like this because, in addition to that complexity, it gives developers the false sense that they can use this method to receive input reports normally. They should be using an event listener instead because that prevents them from potentially losing input reports that arrive when they haven't called receiveInputReport().

I think it would've been clearer if we'd named receiveFeatureReport() as requestFeatureReport() because that would convey that calling it explicitly requests the device respond with a feature report. To that end, I'd propose that this method be called requestInputReport() to differentiate it from the event-driven model.

@codembed
Copy link

I have noticed that the Android USB host HID driver automatically requests all available input reports described by the report descriptor, when a HID device is first connected. This allows for the initial state of the input reports to be established (perhaps without having to expose a GetInputReport function to the application API).

Would it be possible for WebHID to do the same, when a connection is initiated?

For well-designed USB HID devices, this is really the only time Get_Report(Input) is required, as any subsequent changes are provided via the normal HID interrupt endpoint.

Could a collection of these initial input reports be included in the Promise returned by HIDDevice.open()?

@patratacus
Copy link
Author

How's it going? Just checking back to see whether the requestInputReport() is still possible. Thanks.

@gerritsd
Copy link

Are there any updates regarding this discussion on the requestInputReport()?

Our device also needs this request. This week we have implemented a small PoC bases on WebHID and ran into this issue. Over 60k units of this product are shipped worldwide, so changing the device side is not that easy ;).

This first experience with WebHID was really good, so it would be great if our product could be supported.

Looking forward to your feedback.

@patratacus
Copy link
Author

patratacus commented Dec 14, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants