libusbhid bug?

[Hans Petter Selasky]

Hi,

I think Markus Brueffer is working on a new HID parser.

--HPS

On Tuesday 15 May 2007 01:34, Stefan `Sec` Zehl wrote:
> Hi,
>
> I've been playing around with a Bluetooth HID device. Bluetooth uses the
> same HID format as USB, so libusbhid is used to parse these messages.
>
> While debugging, I've found what I think is a mistake in parsing
> "Array"-type input messages:
>
> In my HID descriptor, there is the following snippet:
> | 95 06   |    Report Count(6)
> | 75 08   |    Report Size(8)
> | 15 00   |    Logical Minimum(0)
> | 26 a4 00|    Logical Maximum(164)
> | 05 07   |    Usage Page(Key Codes)
> | 19 00   |    Usage Minimum(0x00)
> | 29 a4   |    Usage Maximum(0xa4)
> | 81 00   |    Input(Data,Array,Absolute)
>
> The current usbhid only parses one item of Report_Size (8bit). According
>
> to the spec it should parse Report_Count items:
> | An array provides an alternate means for
> | describing the data returned from a group of
> | buttons. Arrays are more efficient, if less flexible
> | than variable items. Rather than returning a single
> | bit for each button in the group, an array returns an
> | index in each field that corresponds to the pressed
> | button (like keyboard scan codes). An out-of range
> | value in and array field is considered no controls
> | asserted. Buttons or keys in an array that are
> | simultaneously pressed need to be reported in
> | multiple fields. Therefore, the number of fields in
> | an array input item (Report Count) dictates the
> | maximum number of simultaneous controls that
> | can be reported. A keyboard could report up to
> | three simultaneous keys using an array with three
> | 8-bit fields (Report Size = 8, Report Count = 3).
>
> I have created a patch which changes that behaviour, and attached it
> below. My sample code works fine with these changes, but as I don't have
> an USB Keyboard, it would be great if someone could check that this
> doesn't break anything.
>
> --- parse.c.org	Wed Feb 11 22:09:13 2004
> +++ parse.c	Tue May 15 01:26:08 2007
> @@ -54,6 +54,7 @@
>  	int multimax;
>  	int kindset;
>  	int reportid;
> +	int isarray;
>
>  	/*
>  	 * The start of collection item has no report ID set, so save
> @@ -100,6 +101,7 @@
>  	s->kindset = kindset;
>  	s->reportid = id;
>  	s->hassavedcoll = 0;
> +	s->isarray = 0;
>  	return (s);
>  }
>
> @@ -182,6 +184,16 @@
>  			hid_clear_local(c);
>  		}
>  	}
> +	if(s->isarray) {
> +		REPORT_SAVED_COLL;
> +		*h = *c;
> +		h->next = 0;
> +		h->pos = s->kindpos[c->kind];
> +		s->kindpos[c->kind] += c->report_size ;
> +		if(--s->isarray ==0)
> +			hid_clear_local(c);
> +		return(1);
> +	};
>  	for (;;) {
>  		p = s->p;
>  		if (p >= s->end)
> @@ -262,14 +274,9 @@
>  				} else {
>  					if (s->minset)
>  						c->usage = c->usage_minimum;
> -					*h = *c;
> -					h->next = 0;
> -					h->pos = s->kindpos[c->kind];
> -					s->kindpos[c->kind] +=
> -					    c->report_size * c->report_count;
> -					hid_clear_local(c);
>  					s->minset = 0;
> -					return (1);
> +					s->isarray=c->report_count;
> +					goto top;
>  				}
>  			case 9:		/* Output */
>  				retkind = hid_output;
>
> CU,
>     Sec