The existing behaviour for a HOG device with report IDs was broken, and
likely wouldn't have worked at all as the first byte of the response was
being discarded.
The missing prepended report ID is theoretically something that would
cause an observable difference to userspace, but as it's only added in
the multiple report case that was broken I don't think anyone will be
affected.
Single report HOG devices make up for the majority of consumer HID
devices, which is probably why this has gone unnoticed for so long, and
the behaviour of those is unaffected by my patch.
The only use case I can see that would be broken would be a userspace
app using HIDRAW to communicate with a multiple report HOG device, which
was already tolerant of missing the first byte if the report.
Cheers,
- Dean
On 21/11/2020 6:56 am, mathieu.stephan@xxxxxxxxx wrote:
Hello All,
Is there a way to communicate to users that particular change?
I'm reacting as this is something our team heavily relies upon
(https://github.com/mooltipass/moolticute/blob/master/src/MPDevice_linux.cpp#L91
<https://github.com/mooltipass/moolticute/blob/master/src/MPDevice_linux.cpp#L91>)
and I'm guessing we're far from the only ones :)
Regards,
Mathieu
On Fri, Nov 20, 2020 at 6:52 PM Luiz Augusto von Dentz
<luiz.dentz@xxxxxxxxx <mailto:luiz.dentz@xxxxxxxxx>> wrote:
Hi Dean,
On Thu, Nov 19, 2020 at 5:47 PM Dean Camera
<dean@xxxxxxxxxxxxxxxxxxxxx <mailto:dean@xxxxxxxxxxxxxxxxxxxxx>> wrote:
>
> If the HID subsystem requests a HID report to be read from the
> device, we currently incorrectly strip off the first byte of the
> response, if the device has report IDs set in the HID report
> descriptor.
>
> This is incorrect; unlike USB HID, the report ID is *not* included
> in the HOG profile's HID reports, and instead exists out of band
> in a descriptor on the report's bluetooth characteristic in the
> device.
>
> In this patch, we remove the erroneous stripping of the first
> byte of the report, and (if report IDs are enabled) prepend the
> report ID to the front of the result. This makes the HID report
> returned indentical in format to that of a USB HID report, so
> that the upper HID drivers can consume HOG device reports in the
> same way as USB.
> ---
> profiles/input/hog-lib.c | 18 +++++++++++-------
> 1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/profiles/input/hog-lib.c b/profiles/input/hog-lib.c
> index 78018aad3..49d459e21 100644
> --- a/profiles/input/hog-lib.c
> +++ b/profiles/input/hog-lib.c
> @@ -779,7 +779,8 @@ fail:
> static void get_report_cb(guint8 status, const guint8 *pdu,
guint16 len,
> gpointer
user_data)
> {
> - struct bt_hog *hog = user_data;
> + struct report *report = user_data;
> + struct bt_hog *hog = report->hog;
> struct uhid_event rsp;
> int err;
>
> @@ -808,13 +809,16 @@ static void get_report_cb(guint8 status, const
> guint8 *pdu, guint16 len,
>
> --len;
> ++pdu;
> +
> if (hog->has_report_id && len > 0) {
> - --len;
> - ++pdu;
> + rsp.u.get_report_reply.size = len + 1;
> + rsp.u.get_report_reply.data[0] = report->id;
> + memcpy(&rsp.u.get_report_reply.data[1], pdu, len);
> + }
> + else {
> + rsp.u.get_report_reply.size = len;
> + memcpy(rsp.u.get_report_reply.data, pdu, len);
> }
> -
> - rsp.u.get_report_reply.size = len;
> - memcpy(rsp.u.get_report_reply.data, pdu, len);
>
> exit:
> rsp.u.get_report_reply.err = status;
> @@ -846,7 +850,7 @@ static void get_report(struct uhid_event *ev,
void
> *user_data)
>
> hog->getrep_att = gatt_read_char(hog->attrib,
> report->value_handle,
> - get_report_cb, hog);
> + get_report_cb,
report);
> if (!hog->getrep_att) {
> err = ENOMEM;
> goto fail;
> --
> 2.29.2.windows.2
>
Applied, thanks.
--
Luiz Augusto von Dentz
