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

HID_REPORT_REQUEST_Feature definitions missing #80

Open
NicoHood opened this Issue Apr 9, 2016 · 4 comments

Comments

Projects
None yet
2 participants
@NicoHood
Copy link
Contributor

NicoHood commented Apr 9, 2016

Currently there is only HID_REPORT_ITEM_Feature which is used to generate the RAWHID report descriptor. However with HID you can also differentiate between In, Out, Feature inside the Get/Set_Report Request. Those values differ from the definition above (+1). Something like HID_REPORT_TYPE_Feature is wished.

Currently available:
http://www.fourwalledcubicle.com/files/LUFA/Doc/151115/html/group___group___u_s_b_class_h_i_d_common.html#gga3c0de6e2f6380c88937a5f09bcbf022ea468b3fdb884153aaefb634c9823f64bb

Requested:
https://github.com/arduino/Arduino/blob/master/hardware/arduino/avr/libraries/HID/src/HID.h#L57

Source in the USB Spec:
HID Request Type HID1.11 Page 51 7.2.1 Get_Report Request

Please add those 3 values, as it is really helpful.

@NicoHood NicoHood changed the title HID_REPORT_TYPE_Feature definitions missing HID_REPORT_REQUEST_Feature definitions missing Apr 9, 2016

@NicoHood NicoHood referenced a pull request that will close this issue Apr 9, 2016

Open

Added HID_ReportRequestTypes_t #81

@abcminiuser

This comment has been minimized.

Copy link
Owner

abcminiuser commented Apr 10, 2016

Hrm. A long time ago when I implemented this, I just use code in the HID device class driver to adapt the off-by-one values to the existing common HID_REPORT_ITEM_* values, since it seemed simpler to have the single set of values (at the time). Having the two sets isn't a bad idea, but changing the adapter code could lead to subtle code bugs in previously working user code.

I can fix it and add it to the migration notes, but I'm not completely convinced people actually read those (most people would expect to be able to just dump in a different version of LUFA and recompile blindly, which is frankly a reasonable expectation).

@NicoHood

This comment has been minimized.

Copy link
Contributor

NicoHood commented Apr 10, 2016

Huh?
I dont get the problem. All I am doing is to add another 3 definitions. Everything stays compatible. Use the new definitions or not. I do not change old definitions. See my PR (1st commit)

@abcminiuser

This comment has been minimized.

Copy link
Owner

abcminiuser commented Apr 10, 2016

Yes, I understand - but the CALLBACK_HID_Device_CreateHIDReport() callback has a ReportType parameter already, which uses the existing HID_REPORT_ITEM_* values. Having the new definitions around would confuse people unless I change the class driver to use them, which presents the compatibility problem above.

@NicoHood

This comment has been minimized.

Copy link
Contributor

NicoHood commented Apr 10, 2016

Oh now I see. Well I do not use the class driver at all.
So I'd just keep it as it is for the class driver and for low level one can use the other definitions that i've added.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment