Hi Luiz,
I have revised the patch to using the notifyid to detect double
registration. Please take a another look. Thanks!
On Mon, Jan 11, 2021 at 9:47 AM Luiz Augusto von Dentz
<luiz.dentz@xxxxxxxxx> wrote:
>
> Hi Sonny,
>
> On Sat, Jan 9, 2021 at 4:34 PM Sonny Sasaka <sonnysasaka@xxxxxxxxxxxx> wrote:
> >
> > In commit 23b69ab3e484 ("input/hog: Cache the HID report map"), we
> > optimized HOG reconnection by registering report value callbacks early,
> > but there was a bug where we also re-register the same report value
> > callbacks after at CCC write callback. We should handle this case by
> > avoiding the second callback register if we know we have done it
> > earlier.
> >
> > ---
> > profiles/input/hog-lib.c | 12 ++++++++++++
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/profiles/input/hog-lib.c b/profiles/input/hog-lib.c
> > index 1f132aa4c..089f42826 100644
> > --- a/profiles/input/hog-lib.c
> > +++ b/profiles/input/hog-lib.c
> > @@ -80,6 +80,7 @@ struct bt_hog {
> > struct bt_uhid *uhid;
> > int uhid_fd;
> > bool uhid_created;
> > + bool report_value_cb_registered;
> > gboolean has_report_id;
> > uint16_t bcdhid;
> > uint8_t bcountrycode;
> > @@ -336,6 +337,13 @@ static void report_ccc_written_cb(guint8 status, const guint8 *pdu,
> > return;
> > }
> >
> > + /* If we already had the report map cache, we must have registered UHID
> > + * and the report value callbacks. In that case, don't re-register the
> > + * report value callbacks here.
> > + */
> > + if (hog->report_value_cb_registered)
> > + return;
> > +
>
> Can't we check the report->notifyid instead of introducing yet another
> flag that seems to have the same intent of tracking if the report has
> been subscribed? In fact it seem there is something odd related to how
> we handle the CCC here, we do read it on ccc_read_cb but we don't
> check if its value is already set. Pehaps something like the following
> would be more complete solution, though we should really look into
> convert hog-lib to use bt_gatt_client instead of keep using GAttrib:
>
> diff --git a/profiles/input/hog-lib.c b/profiles/input/hog-lib.c
> index 1f132aa4c..34a4d7c3b 100644
> --- a/profiles/input/hog-lib.c
> +++ b/profiles/input/hog-lib.c
> @@ -360,15 +360,30 @@ static void ccc_read_cb(guint8 status, const
> guint8 *pdu, guint16 len,
> {
> struct gatt_request *req = user_data;
> struct report *report = req->user_data;
> + uint16_t value;
>
> destroy_gatt_req(req);
>
> - if (status != 0) {
> + if (status || !len) {
> error("Error reading CCC value: %s", att_ecode2str(status));
> return;
> }
>
> - write_ccc(report->hog, report->hog->attrib, report->ccc_handle, report);
> + if (len == 1)
> + value = *pdu;
> + else
> + value = get_le16(pdu);
> +
> + if (!value) {
> + write_ccc(report->hog, report->hog->attrib, report->ccc_handle,
> + report);
> + return;
> + }
> +
> + report->notifyid = g_attrib_register(report->hog->attrib,
> + ATT_OP_HANDLE_NOTIFY,
> + report->value_handle,
> + report_value_cb, report, NULL);
> }
