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)
>>
>
>

Reply via email to