Sorry for the spamming.
After some research and finding that my fix for issue nr: 2 (
hid_usage_in_page() )
will break the functionality inside /usr.bin/usbhidaction/usbhidaction.c
https://goo.gl/1cWFtR (link to usbhidaction.c)
I now change my patch to only include a fix for issue nr: 1
More details is described in the previous mail
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 29 May 2018 19:45:25 -0000
@@ -265,8 +265,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 +279,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;
}
comments? ok?
2018-05-28 13:01 GMT+02:00 David Bern <[email protected]>:
> 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)
>>>>
>>>
>>>
>>
>
