Improve code output format

[ao2]

Hi, first of all thanks for hidrd-convert.

Would it be possible to add some more options to the code output format, so that it creates descriptors ready to be used in the linux kernel?

Some examples:

enclose the byte array into an actual variable declaration:

static __u8 rdesc[] = {
<TAB>value,
<TAB>value,
...
};

pre-pending one TAB to each line.

Indent the bytes too according to the descriptor structure, i.e. if the comment is indented then the value is indented too (maybe by only one or two spaces, just to give some visual clue about the descriptor structure):

static __u8 rdesc[] = {
<TAB>value,    /* usage page   */
<TAB>  value,  /*     Collection  */
...
};

Show values in both decimal and hex in the comments when this can be useful (for min and max).

Show bitfields more explicitly in the comments:

0x81, 0x03, /* Input 0x3 (Constant:1,Variable:1,Abs:0) */

These are things that other tools (like gHID: https://code.google.com/p/ghid/) do, but the comments produced by hidrd-convert are generally clearer than gHIDs.

Thanks, Antonio

[spbnick]

Hi ao2, thank you for your suggestions. My replies are below.

enclose the byte array into an actual variable declaration:

static __u8 rdesc[] = {

value, value, ... }; pre-pending one TAB to each line.

I would like to decline. This can be easily done with just a little bit of shell scripting, like this for example:

echo 'static __u8 rdesc[] = {';
hidrd-convert -o code; | sed -e 's/^/\t/'
echo '};'

Adding this would limit hidrd-convert use and would force people using it with other coding conventions to first remove the extra code, before adding their own.

Indent the bytes too according to the descriptor structure, i.e. if the comment is indented then the value is indented too (maybe by only one or two spaces, just to give some visual clue about the descriptor structure):

static __u8 rdesc[] = {

value, /\* usage page _/ value, /_ Collection */ ... };

I would say that this goes against some coding conventions, where indenting is used solely for structuring, so I wouldn't like to do it by default. Although, this can be implemented as an option, I don't find it very useful. Comments already show the structure and without them indenting wouldn't help much with understanding the binary. If you would still like to have such an option, could you please submit a separate issue for it?

Show values in both decimal and hex in the comments when this can be useful (for min and max).

In my experience, logical and physical extents are rarely chosen to be round hexadecimal numbers. Do you see much evidence to the contrary? Could you please describe why they are more useful to you in hex? Is it not enough to be able to extract the hexadecimal value from the binary representation, which is on the same line in code output? Unless there is a good reason, I would like to avoid making the output more noisy and verbose. Also, as double representations don't seem to be a part of the specification example format (being the "code" comments), I can only put the other representation in its comments, which are output with "--oo comments_comments=true" for "code" format. I would like to reserve the possibility of implementing reading the specification example format in the future and putting the second representation there directly would prevent that.

Show bitfields more explicitly in the comments:

0x81, 0x03, /* Input 0x3 (Constant:1,Variable:1,Abs:0) */

The example output you provided is confusing in itself, as it doesn't make it clear what zero values would mean for some fields, requiring reading the specification anyway. E.g. what's the opposite of "Variable"? I would have thought it would be "Constant", but it isn't. What's the opposite of "Buffered bytes"? And so on.

Current code only outputs a bit token, if it is on. The HID standard uses zero for most often used values, which makes the list short. Input items have eight, output and feature items have nine bit fields. Outputting values for each of them by default would make the output too long and verbose.

We can add an option for that. E.g. Input (Variable) would become Input (Data, Variable, Absolute, No Wrap, Linear, Preferred State, No Null position, Bit Field). While this is ridiculously long, perhaps it will be useful for education. Or, maybe, we can add an option which would make only some bit values appear always. However, I hesitate to make it the default, as it seems somewhat confusing.

What do you think?

[ao2]

I was not suggesting to make these options defaults of course.

TBH the suggestions were more inspired by the fact that I am used to the output of gHID, but gHID has many limitations, the code is not super clear and it's in haskell so I am not that comfortable modifying it.

Thinking about it a little more, most of your comments make a lot of sense:

The variable declaration and the extra indentation are easy to add externally (even if having an extra script laying around can also be inconvenient).

Showing values in hex is redundant, we have them in hex already in the byte array.

Showing all the fields of a bitfield can become confusing.

However I still think it can be useful to indent the byte values according to the descriptor structure, now I have to read the whole line and get to the comment to see the indentation, and then go back to the bytes to check the values and I have to remember the structure. Again, I may be biased just because I am used to the output of gHID, but I think I'll submit this as a separate issue so we can have a more focused discussion.

Thanks, Antonio

[spbnick]

Sure. Please do submit the issue for byte indenting, I agree it might be useful. Meanwhile, I'll close this one. Thank you.