Added hid_send_output_report() function #677

Merged
mbcinergy merged 7 commits from fix/SendOutputReport into master 2024-06-12 11:40:09 -05:00
mbcinergy commented 2024-04-15 10:04:16 -05:00 (Migrated from github.com)

Where hid_get_input_report() was already available for some time, hid_send_output_report() was still missing.

We have used our own fork with this function added for a couple of years now, but believe it's better to add this to the main repository as well.

The changes are tested for Windows and on Linux (hidraw and libusb).

The functions for mac and netbsd are also added but not tested, since we currently have no hardware available for that. It would be great if somebody can test this function for those platforms as well, but the functions are very straightforward so I don't expect any issues with them.

Where hid_get_input_report() was already available for some time, hid_send_output_report() was still missing. We have used our own fork with this function added for a couple of years now, but believe it's better to add this to the main repository as well. The changes are tested for Windows and on Linux (hidraw and libusb). The functions for mac and netbsd are also added but not tested, since we currently have no hardware available for that. It would be great if somebody can test this function for those platforms as well, but the functions are very straightforward so I don't expect any issues with them.
Youw commented 2024-04-15 10:58:34 -05:00 (Migrated from github.com)

So far I haven't encountered cases when you have to use hid_send_output_report specifically, instead of using hid_write.

Most implementations of hid_write would try to use interrupt pipe to send the data if possible first, and would fallback to "hid_send_output_report" if there is no interrupt (output) pipe for a specific device.

On some platforms, the implementation of hid_write is identical to suggested hid_send_output_report, and on others - should work identically (but "better").
Have you tried it?

So far I haven't encountered cases when you have to use `hid_send_output_report` specifically, instead of using `hid_write`. Most implementations of `hid_write` would try to use interrupt pipe to send the data if possible first, and would fallback to `"hid_send_output_report"` if there is no interrupt (output) pipe for a specific device. On some platforms, the implementation of `hid_write` is identical to suggested `hid_send_output_report`, and on others - should work identically (but "better"). Have you tried it?
mbcinergy commented 2024-04-16 05:42:27 -05:00 (Migrated from github.com)

So far I haven't encountered cases when you have to use hid_send_output_report specifically, instead of using hid_write.

Most implementations of hid_write would try to use interrupt pipe to send the data if possible first, and would fallback to "hid_send_output_report" if there is no interrupt (output) pipe for a specific device.

On some platforms, the implementation of hid_write is identical to suggested hid_send_output_report, and on others - should work identically (but "better"). Have you tried it?

I've tried hid_write, but it does not work in this case. The issue is that we have a couple of devices which do have an interrupt output pipe but use the control pipe for some vendor specific reports. That's why the hid_write does not work out of the box for those devices. So we need a function which is able to specify the pipe we need to write to.

> So far I haven't encountered cases when you have to use `hid_send_output_report` specifically, instead of using `hid_write`. > > Most implementations of `hid_write` would try to use interrupt pipe to send the data if possible first, and would fallback to `"hid_send_output_report"` if there is no interrupt (output) pipe for a specific device. > > On some platforms, the implementation of `hid_write` is identical to suggested `hid_send_output_report`, and on others - should work identically (but "better"). Have you tried it? I've tried `hid_write`, but it does not work in this case. The issue is that we have a couple of devices which do have an interrupt output pipe but use the control pipe for some vendor specific reports. That's why the `hid_write` does not work out of the box for those devices. So we need a function which is able to specify the pipe we need to write to.
Youw (Migrated from github.com) reviewed 2024-04-16 06:45:53 -05:00
Youw (Migrated from github.com) commented 2024-04-16 06:45:53 -05:00

Lets add to the documentation something like "see also @ref hid_write".
And lets update the documentation for hid_write, specifically:

hid_write() will send the data on the first OUT endpoint, if one exists. If it does not, it will send the data through the Control Endpoint (Endpoint 0).

Would be great to update and specify that the behavior is as (newly introduced) hid_send_output_report in case if interrupt pipe does not exist.

Lets add to the documentation something like `"see also @ref hid_write"`. And lets update the documentation for `hid_write`, specifically: > hid_write() will send the data on the first OUT endpoint, if one exists. If it does not, it will send the data through the Control Endpoint (Endpoint 0). Would be great to update and specify that the behavior is as (newly introduced) `hid_send_output_report` in case if interrupt pipe does not exist.
Youw (Migrated from github.com) reviewed 2024-04-16 06:48:06 -05:00
Youw (Migrated from github.com) commented 2024-04-16 06:48:06 -05:00

would be great to avoid code duplication among this function and hid_write

would be great to avoid code duplication among this function and `hid_write`
Youw (Migrated from github.com) reviewed 2024-04-16 06:53:43 -05:00
@ -1313,6 +1315,45 @@ int HID_API_EXPORT HID_API_CALL hid_get_feature_report(hid_device *dev, unsigned
return hid_get_report(dev, IOCTL_HID_GET_FEATURE, data, length);
Youw (Migrated from github.com) commented 2024-04-16 06:53:42 -05:00

I'm afraid this is not enought

take a look at hid_send_feature_report and try to extract common aproach/implementation, so that the two function would behave alike

I'm afraid this is not enought take a look at `hid_send_feature_report` and try to extract common aproach/implementation, so that the two function would behave alike
Youw (Migrated from github.com) approved these changes 2024-04-16 10:02:27 -05:00
JoergAtGithub commented 2024-04-16 13:37:37 -05:00 (Migrated from github.com)

Wouldn't it be better to read the Endpoint Descriptor to determine if the particular interface of the USB device has a Interupt Out?

Wouldn't it be better to read the `Endpoint Descriptor` to determine if the particular interface of the USB device has a `Interupt Out`?
Youw commented 2024-04-16 15:27:25 -05:00 (Migrated from github.com)

Not sure what do you mean.
Right now libusb backend looks for a first interrupt endpoint of a particular interface. But it reads from a USB config descriptor.

I didn't check the linux/hidraw implementation, but Windows and macOS implementation is not exactly known.

Not sure what do you mean. Right now libusb backend looks for a first interrupt endpoint of a _particular interface_. But it reads from a USB config descriptor. I didn't check the linux/hidraw implementation, but Windows and macOS implementation is not exactly known.
JoergAtGithub commented 2024-04-18 13:36:08 -05:00 (Migrated from github.com)
I meant this: https://learn.microsoft.com/en-us/windows-hardware/drivers/usbcon/standard-usb-descriptors#usb-endpoint-descriptor
Youw (Migrated from github.com) reviewed 2024-06-12 06:51:20 -05:00
@ -1313,6 +1315,45 @@ int HID_API_EXPORT HID_API_CALL hid_get_feature_report(hid_device *dev, unsigned
return hid_get_report(dev, IOCTL_HID_GET_FEATURE, data, length);
Youw (Migrated from github.com) commented 2024-06-12 06:51:20 -05:00

I'm not sure why I marked this as resolved - it is clearly not.
See here:
https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/hidsdi/nf-hidsdi-hidd_setoutputreport#remarks
and see hid_send_feature_report implementation.

I'm not sure why I marked this as resolved - it is clearly not. See here: https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/hidsdi/nf-hidsdi-hidd_setoutputreport#remarks and see `hid_send_feature_report` implementation.
mbcinergy (Migrated from github.com) reviewed 2024-06-12 08:05:30 -05:00
@ -1313,6 +1315,45 @@ int HID_API_EXPORT HID_API_CALL hid_get_feature_report(hid_device *dev, unsigned
return hid_get_report(dev, IOCTL_HID_GET_FEATURE, data, length);
mbcinergy (Migrated from github.com) commented 2024-06-12 08:05:30 -05:00

This is fixed in our branch. I think something happened with the references when I rebased on the latest master branch using the update button from this pull request (probably should have done a merge instead of rebase?).

This is fixed in our branch. I think something happened with the references when I rebased on the latest master branch using the update button from this pull request (probably should have done a merge instead of rebase?).
Youw (Migrated from github.com) reviewed 2024-06-12 09:04:23 -05:00
@ -1313,6 +1315,45 @@ int HID_API_EXPORT HID_API_CALL hid_get_feature_report(hid_device *dev, unsigned
return hid_get_report(dev, IOCTL_HID_GET_FEATURE, data, length);
Youw (Migrated from github.com) commented 2024-06-12 09:04:22 -05:00

There are inifinite ways one can mess up git history. And there is (almost) always ways to fix it. Give it a try.

There are inifinite ways one can mess up git history. And there is (almost) always ways to fix it. Give it a try.
Youw (Migrated from github.com) approved these changes 2024-06-12 10:54:40 -05:00
Youw commented 2024-06-12 10:56:36 -05:00 (Migrated from github.com)

I meant this: https://learn.microsoft.com/en-us/windows-hardware/drivers/usbcon/standard-usb-descriptors#usb-endpoint-descriptor

I'm pretty sure that's what we do on libusb backend.
On other backends we don't have any control, as functions like write are the OS/Platform implementation, which appear to have in a similar way.

> I meant this: https://learn.microsoft.com/en-us/windows-hardware/drivers/usbcon/standard-usb-descriptors#usb-endpoint-descriptor I'm pretty sure that's what we do on libusb backend. On other backends we don't have any control, as functions like `write` are the OS/Platform implementation, which appear to have in a similar way.
Sign in to join this conversation.
No reviewers
No milestone
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gh-libusb/hidapi!677
No description provided.