Incorrect comments in HID.cpp [imported]

[cmaglie]

This is Issue 1107 moved from a Google Code project.
Added by 2012-11-10T12:46:23.000Z by Lem...@gmail.com.
Please review that bug for more context and additional comments, but update this bug.

Original labels: Type-Defect, Priority-Medium

Original description

I'm poking around in the HID.cpp file, working on turning my arduino leonardo into a joystick (which is working), and have found a few incorrect comments.

In the RAWHID section:

if RAWHID_ENABLED

//  RAW HID
0x06, LSB(RAWHID_USAGE_PAGE), MSB(RAWHID_USAGE_PAGE),   // 30
0x0A, LSB(RAWHID_USAGE), MSB(RAWHID_USAGE),

0xA1, 0x01,             // Collection 0x01
0x85, 0x03,             // REPORT_ID (3)
0x75, 0x08,             // report size = 8 bits
0x15, 0x00,             // logical minimum = 0
0x26, 0xFF, 0x00,       // logical maximum = 255

0x95, 64,               // report count TX
0x09, 0x01,             // usage
0x81, 0x02,             // Input (array)

0x95, 64,               // report count RX
0x09, 0x02,             // usage
0x91, 0x02,             // Output (array)
0xC0                    // end collection

endif

Specifically:
0x81, 0x02, // Input (array)
0x91, 0x02, // Output (array)

In both of these cases, the comment specifies that the data type is an array.
However, if you look up the bit-mapping from the HID specs:

Bit 0 {Data (0) | Constant (1)}
Bit 1 {Array (0) | Variable (1)}
Bit 2 {Absolute (0) | Relative (1)}
...

You can see that a value of 0x02 for the data type specifies a variable, not an array.
It should probably also be specified that since bit 0 is not set, this is data, and since bit 3 is not set, the value is absolute, to make this section match the other descriptors in the file.

Anyways, I have been going through and fairly extensively clarified the whole HID descriptor section with some #defines, replacing the various hex constants with what they actually mean as a #define macro.

The attached file can replace the existing HID.cpp file with no changes.
It also has the the descriptor needed to turn a leonardo into a joystick, though it's currently turned off using a #ifdef.

To make things work completely, you also need to add a few lines to USBAPI.h, so I'm attaching that too.

You can see an example for how to use the joystick additions on my website, http://www.imaginaryindustries.com/blog/?p=80

[nospam2000]

I can't find the mentioned attachments, maybe the got lost when moving from Google Code? Can you please add them again?

In my pull request #1488 I already changed some constants of the USB code from numeric values to #defines to make the code better readable.

[shfitz]

@cmaglie Not sure this is a documentation issue.

From google code :
Nov 10, 2012 #2 Lemuix
Hangon, there is a #define collision somewhere, that the arduino compiler is not reporting.

Trying to troubleshoot now.

Nov 10, 2012 #4 Lemuix
Never mind, it wound up being a stupid assumption about how I could pack pack multiple-bytes into an single statement in a variable definition, along with the fact that the arduino environment eats all the compiler warnings.

Here is the patched HID.cpp

https://arduino.googlecode.com/issues/attachment?aid=11070004000&name=HID.cpp&token=RHW-51PTyJ2QuQSgiMotzvlI9LQ%3A1383549825558

the USBAPI.h

https://arduino.googlecode.com/issues/attachment?aid=11070004001&name=USBAPI.h&token=-NQDamHRDYyESvMVYOO04u4YrJ0%3A1383549825558

[agdl]

@shfitz can we close this?