I was suggested off list to give an explanation on what the patch does.
So please, tell me if I need to clarify more, or make further changes to
the code.
The patch tries to fix two things.
1. Changes in hid_parse_usage_in_page() fixes problems in parsing usages
defined as: * Button %d
hid_parse_usage_in_page():
Previously - With input "Button:Button_1" returns -1
Now - With input "Button:Button_1" returns 589825
In the scenario of parsing Button:Button_1 we will not find a usage name
matching that string. For example Button:Button_1 is defined as
Button %d in the table.
We are still able to calculate the proper usage number in the same way we
are
able to calculate the proper usage name in hid_usage_in_page().
The first step is to identify if usage name is shortened. If it is,
usage will hold a value of -1. Then I try to locate a separator char in the
name as "_".
If a separator char is found I use it to read the value as "_%d" to get the
corresponding usage number
>+ us = pages[k].page_contents[j].usage;
>+ if (us == -1) {
>+ usage_sep = strchr(sep, '_');
>+ if (usage_sep == NULL)
>+ return -1;
>+ if (sscanf(usage_sep, "_%d", &parsed_usage))
>+ return (pages[k].usage << 16) |
>+ parsed_usage;
2. The text-string that is returned by hid_usage_in_page() misses page
information.
So the changes made in hid_usage_in_page() is to make it the inverse of
hid_parse_usage_in_page()
In details what the code previously did and now does.
hid_usage_in_page():
Previously - With input 589825 returns Button_1
Now - With input 589825 returns Button:Button_1
The change just adds a pages[k].name to the string, a format that
hid_parse_usage_in_page() expects it to have.
I make formatting in two steps when us == -1 which it is when usage is
shortened
as for example: * Button %d.
>+ snprintf(fmt, sizeof fmt,
>+ "%%s:%s", pages[k].page_contents[j].name);
>+ snprintf(b, sizeof b, fmt, pages[k].name, i);
The first step is to create a format string that will result in something
like
"%s:Button_%d".
The last step I use the fmt-string to create a complete string that will
result in
"Button:Button_1"
2018-05-24 18:44 GMT+02:00 David Bern <[email protected]>:
> While I was waiting for comments and feedback I came up with some
> improvements.
> The "logic" is still the same, but the execution is hopefully more sane.
>
> Index: usage.c
> ===================================================================
> RCS file: /cvs/src/lib/libusbhid/usage.c,v
> retrieving revision 1.16
> diff -u -p -r1.16 usage.c
> --- usage.c 8 Oct 2014 04:49:36 -0000 1.16
> +++ usage.c 24 May 2018 16:37:54 -0000
> @@ -224,6 +224,7 @@ hid_usage_in_page(unsigned int u)
> {
> int i = HID_USAGE(u), j, k, us;
> int page = HID_PAGE(u);
> + char fmt[100];
> static char b[100];
>
> for (k = 0; k < npages; k++)
> @@ -234,12 +235,16 @@ hid_usage_in_page(unsigned int u)
> for (j = 0; j < pages[k].pagesize; j++) {
> us = pages[k].page_contents[j].usage;
> if (us == -1) {
> - snprintf(b, sizeof b,
> - pages[k].page_contents[j].name, i);
> + snprintf(fmt, sizeof fmt,
> + "%%s:%s", pages[k].page_contents[j].name);
> + snprintf(b, sizeof b, fmt, pages[k].name, i);
> + return b;
> + }
> + if (us == i) {
> + snprintf(b, sizeof b, "%s:%s", pages[k].name,
> + pages[k].page_contents[j].name);
> return b;
> }
> - if (us == i)
> - return pages[k].page_contents[j].name;
> }
> bad:
> snprintf(b, sizeof b, "0x%04x", i);
> @@ -265,8 +270,9 @@ int
> hid_parse_usage_in_page(const char *name)
> {
> const char *sep;
> + const char *usage_sep;
> unsigned int l;
> - int k, j;
> + int k, j, us, parsed_usage;
>
> sep = strchr(name, ':');
> if (sep == NULL)
> @@ -278,9 +284,19 @@ hid_parse_usage_in_page(const char *name
> return -1;
> found:
> sep++;
> - for (j = 0; j < pages[k].pagesize; j++)
> + for (j = 0; j < pages[k].pagesize; j++) {
> + us = pages[k].page_contents[j].usage;
> + if (us == -1) {
> + usage_sep = strchr(sep, '_');
> + if (usage_sep == NULL)
> + return -1;
> + if (sscanf(usage_sep, "_%d", &parsed_usage))
> + return (pages[k].usage << 16) |
> + parsed_usage;
> + }
> if (strcmp(pages[k].page_contents[j].name, sep) == 0)
> return (pages[k].usage << 16) |
> pages[k].page_contents[j].usage;
> + }
> return -1;
> }
>
>
>
> 2018-05-21 23:12 GMT+02:00 David Bern <[email protected]>:
>
>> First diff "solves" point 1 & 2. Second diff is on the parts of the
>> manual that does not match the usbhid.h
>>
>> Index: usage.c
>> ===================================================================
>> RCS file: /cvs/src/lib/libusbhid/usage.c,v
>> retrieving revision 1.16
>> diff -u -p -r1.16 usage.c
>> --- usage.c 8 Oct 2014 04:49:36 -0000 1.16
>> +++ usage.c 21 May 2018 21:06:24 -0000
>> @@ -224,6 +224,7 @@ hid_usage_in_page(unsigned int u)
>> {
>> int i = HID_USAGE(u), j, k, us;
>> int page = HID_PAGE(u);
>> + char usage_name[100];
>> static char b[100];
>>
>> for (k = 0; k < npages; k++)
>> @@ -234,12 +235,18 @@ hid_usage_in_page(unsigned int u)
>> for (j = 0; j < pages[k].pagesize; j++) {
>> us = pages[k].page_contents[j].usage;
>> if (us == -1) {
>> - snprintf(b, sizeof b,
>> + snprintf(usage_name, sizeof usage_name,
>> pages[k].page_contents[j].name, i);
>> + snprintf(b, sizeof b,
>> + "%s:%s", pages[k].name, usage_name);
>> + return b;
>> + }
>> + if (us == i) {
>> + snprintf(b, sizeof b,
>> + "%s:%s", pages[k].name,
>> + pages[k].page_contents[j].name);
>> return b;
>> }
>> - if (us == i)
>> - return pages[k].page_contents[j].name;
>> }
>> bad:
>> snprintf(b, sizeof b, "0x%04x", i);
>> @@ -265,8 +272,9 @@ int
>> hid_parse_usage_in_page(const char *name)
>> {
>> const char *sep;
>> + const char *usage_sep;
>> unsigned int l;
>> - int k, j;
>> + int k, j, us, parsed_usage;
>>
>> sep = strchr(name, ':');
>> if (sep == NULL)
>> @@ -277,10 +285,20 @@ hid_parse_usage_in_page(const char *name
>> goto found;
>> return -1;
>> found:
>> + usage_sep = strchr(sep, '_');
>> sep++;
>> - for (j = 0; j < pages[k].pagesize; j++)
>> + for (j = 0; j < pages[k].pagesize; j++) {
>> + us = pages[k].page_contents[j].usage;
>> + if (us == -1) {
>> + if (usage_sep == NULL)
>> + return -1;
>> + if (sscanf(usage_sep, "_%d", &parsed_usage))
>> + return (pages[k].usage << 16 |
>> + parsed_usage);
>> + }
>> if (strcmp(pages[k].page_contents[j].name, sep) == 0)
>> return (pages[k].usage << 16) |
>> pages[k].page_contents[j].usage;
>> + }
>> return -1;
>> }
>>
>> Index: usbhid.3
>> ===================================================================
>> RCS file: /cvs/src/lib/libusbhid/usbhid.3,v
>> retrieving revision 1.17
>> diff -u -p -r1.17 usbhid.3
>> --- usbhid.3 13 May 2014 14:05:02 -0000 1.17
>> +++ usbhid.3 21 May 2018 21:02:21 -0000
>> @@ -65,22 +65,22 @@
>> .Fn hid_report_size "report_desc_t d" "hid_kind_t k" "int id"
>> .Ft int
>> .Fn hid_locate "report_desc_t d" "u_int usage" "hid_kind_t k"
>> "hid_item_t *h" "int id"
>> -.Ft char *
>> +.Ft const char *
>> .Fn hid_usage_page "int i"
>> -.Ft char *
>> +.Ft const char *
>> .Fn hid_usage_in_page "u_int u"
>> .Ft int
>> .Fn hid_parse_usage_page "const char *"
>> -.Ft char *
>> +.Ft int
>> .Fn hid_parse_usage_in_page "const char *"
>> .Ft void
>> .Fn hid_init "char *file"
>> .Ft int
>> .Fn hid_start "char *file"
>> -.Ft int
>> +.Ft int32_t
>> .Fn hid_get_data "void *data" "hid_item_t *h"
>> .Ft void
>> -.Fn hid_set_data "void *data" "hid_item_t *h" "u_int data"
>> +.Fn hid_set_data "void *data" "hid_item_t *h" "int32_t data"
>> .Sh DESCRIPTION
>> The
>> .Nm
>>
>>
>>
>> 2018-05-21 12:07 GMT+02:00 Martin Pieuchot <[email protected]>:
>>
>>> On 18/05/18(Fri) 10:01, David Bern wrote:
>>> > Hello!
>>> >
>>> > Have been using libusbhid and discovered a couple of discrepancies in
>>> > the man-page (libusbhid.3) and the source of usage.c
>>> >
>>> > Some are just factual misses, but I also got (what I think is) 2
>>> errors.
>>> > I will try to explain them;
>>> >
>>> > 1. (This is the I think is an error but not sure). The man-page tells
>>> me
>>> > that hid_usage_in_page and hid_parse_usage_in_page are each
>>> > others inverse.
>>> > If I haven't misunderstood the practical meaning of inverse in this
>>> > case then this should be true:
>>> > x == hid_parse_usage_in_page(hid_usage_in_page(x)).
>>> >
>>> > My observation:
>>> > The main reason to why this isnt true, is that hid_usage_in_page()
>>> > returns the data of pages[k].page_contents[j].name
>>> > while hid_parse_usage_in_page() expects the data to
>>> > contain "%s:%s", pages[k].name, pages[k].page_contents[j].name
>>> >
>>> > The reason I ask instead of just posting working code is this:
>>> > Am I misunderstanding the manual? In that case, the solution I want
>>> > to send in is a change in that sentence
>>> > Or Is the manual correct and the behavior of hid_usage_in_page() wrong,
>>> > is this something I can correct without breaking other systems?
>>> >
>>> >
>>> > 2. The second error I found is located in hid_parse_usage_in_page().
>>> > It is unable to parse values found in page Button.
>>> >
>>> > My observation:
>>> > usage.c is using a standard table named usb_hid_pages. In that table
>>> > we got a page called Buttons.
>>> > the usages in that page is shortened to "* Button %d".
>>> > I believe this is the cause of why hid_parse_usage_in_page() is getting
>>> > the pagesize "wrong" and therefor unable to parse any button in the
>>> > button page. I guess this is the case with other similar cases as well.
>>> >
>>> > my conclusion is that it would be possible to handle similar cases in a
>>> > similar way as it is handled in hid_usage_in_page().
>>> >
>>> >
>>> > As this is my first "issue" I would love to get as much feedback as
>>> > possible so I can work on delivering a desirable and usable patch in
>>> > my first try.
>>>
>>> Just send a diff, then we'll look at it 8)
>>>
>>
>>
>
