RE: Re: HoG: Support multi output report

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi David and Luiz,
	I am not familiar to the implementation of the kernel driver for Blue-z so my comments are more general from the BLE HID service perspective.

> -----Original Message-----
> From: Luiz Augusto von Dentz [mailto:luiz.dentz@xxxxxxxxx]
> Sent: 05 August 2014 13:22
> To: David Herrmann
> Cc: Murat Kilivan; Aravind Parvathala; linux-bluetooth@xxxxxxxxxxxxxxx
> Subject: Re: Re: HoG: Support multi output report
> 
> Hi David,
> 
> On Tue, Aug 5, 2014 at 2:55 PM, David Herrmann <dh.herrmann@xxxxxxxxx>
> wrote:
> > Hi
> >
> > On Mon, Aug 4, 2014 at 10:21 AM, Luiz Augusto von Dentz
> > <luiz.dentz@xxxxxxxxx> wrote:
> >>> What did you mean by another instance? Is there a way to configure the
> blue-z stack to create separate instances (are they hidraw device instances or
> something else) for each of the HID output report?
> >>
> >> Take a look at android/hog.c it supports multiple instances already
> >> you basically can have more than one HID service in your primaries
> >> which I believe is what PTS does.
> >>
> >>> Any suggestions on how to achieve that? Can the HID report map be
> configured such that there are two different collections, each with one
> output report so that the kernel enumerates them as two different device
> instances?
> >>
> >> They are two completely separate instances with their own report map
> >> and in that case we would create each individual instance as a
> >> separate uHID device.
> >>
> >> @David: If we got multiple output reports how do the driver discover
> them?
> >
> > Ok, I now took the time to read through the HoG specs. It says:
> >
> >     "For data transferred to HID Device from Report Host,
> >      Report ID is removed from data received from a
> >      USB HID Class driver before being transmitted to the
> >      HID Device (usually a write command to a Report
> >      characteristic value or as a write request to a
> >      Report characteristic value for HID Service data)."
> >
> > Each UHID_OUTPUT message received by BlueZ contains the report-ID as
> > 1-byte prefix of the message. Therefore, you're supposed to strip it
> > from the message and forward it to the right target.
> >
> > I have no idea how GATT works, but I assume you write to an attribute
> > that is named after the report-ID of the output report. So you parse
> > this 1-byte prefix, select the correct attribute based on this
> > information and then send the message (without the 1-byte prefix) to
> > the selected attribute.

For each report (note that it is not for each report ID, because you could have an input, an output as well as a feature report all of which are using the same report ID), there is a corresponding characteristic which is discoverable. The report map indicates which are all the report IDs and the corresponding type (purpose). As shown by Murat, the debug logs show that all those report IDs were discovered from the report map. 

Each dedicated characteristic for each of the reports, carry a "Report Reference characteristic descriptor" which specifies the report ID and report type associated with that characteristic. Hence I would imagine the blue-z driver would need to keep the mapping of report ID/Type Vs the Characteristic handle.

For an output report, the driver would direct the contents of the report over the corresponding handle as "Write request". This data will not have (and does not need to have) the report ID as the BLE device already knows which report ID is associated with the characteristic handle.

For an input report, it’s the same except that the BLE device will issue a write request to the Blue-z host on the corresponding handle. Again the payload will not have any report ID, because the Blue-z host is expected to be able to map the payload of the originating characteristic handle to the corresponding input report ID.

Same is applicable to the feature report as well but in this case , I imagine it is a read/write request-response type of exchange as you would reading/writing static configuration data rather.


So it is not really a Characteristic  attribute named after the report-ID, but mapped to a report ID. As stated earlier, the associated " Report Reference characteristic descriptor " holds the information which is key to translate from the BLE data to the HID data and vice-versa.

> >
> > Looking at hog.c in the input-profile, I can see this:
> >
> >
> >         static int report_type_cmp(gconstpointer a,
> >                                                 gconstpointer b)
> >         {
> >                const struct report *report = a;
> >                uint8_t type = GPOINTER_TO_UINT(b);
> >
> >                return report->type - type;
> >         }
> >
> >         ...forward_report()...
> >         l = g_slist_find_custom(hogdev->reports,
> >                                                 GUINT_TO_POINTER(type),
> >                                                 report_type_cmp);
> >         if (!l)
> >                return;
> >
> > Now I assume you rather want this:
> >
> >         static int report_type_cmp(gconstpointer a,
> >                                                 gconstpointer b)
> >         {
> >                 const struct report *report = a;
> >                 const struct report *cmp = b;
> >
> >                 if (report->type != cmp->type)
> >                        return report->type - cmp->type;
> >
> >                 return report->id - cmp->id;
> >         }
> 
> But that is the very question, how can the driver know what ids to use to
> makes us able to match by id like you did above, when we create the device
> we don't seem to inform the report ids, perhaps we should try to match by id
> but if that fails use the current logic to match by type otherwise it will
> probably be very hard to come up with a generic driver that doesn't know
> what report id to use or perhaps the driver should never send any output
> without a proper report id?
> 
> >         ...forward_report()...
> >         struct report rep = { .type = HOGP_REPORT_TYPE_OUTPUT,
> >                                 .id = hogdev->has_report_id ?
> > ev->u.output.data[0] : 0 };
> >         l = g_slist_find_custom(hogdev->reports,
> >                                                 &rep,
> >                                                 report_type_cmp);
> >         if (!l)
> >                return;
> >
> > (sorry for the formatting.. I hope you get my point)
> >
> > Note that your hack with "hogdev->has_report_id" is no longer needed
> > in my new UHID extensions. The kernel then informs you whether the
> > device uses report-IDs or not. That is, you know for sure whether the
> > first byte of a message is a report-ID or whether it must be skipped.
> 
> I just recently start hacking on it so I don't know exactly why we did have
> has_report_id in the first place, anyway good that we are evolving this code.
> 
> --
> Luiz Augusto von Dentz
> 
> 
>  To report this email as spam click
> https://www.mailcontrol.com/sr/MZbqvYs5QwJvpeaetUwhCQ==
> P7H!770p6BqWEek3u1!FYHtNluRcx4Zmx9o2WGkmkeK0Rphlb7DTA== .


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Keep up to date with CSR on our technical blog, www.csr.com/blog, CSR people blog, www.csr.com/people, YouTube, www.youtube.com/user/CSRplc, Facebook, www.facebook.com/pages/CSR/191038434253534, or follow us on Twitter at www.twitter.com/CSR_plc.
New for 2014, you can now access the wide range of products powered by aptX at www.aptx.com.
��.n��������+%������w��{.n�����{����^n�r������&��z�ޗ�zf���h���~����������_��+v���)ߣ�


[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux