From: Curtis Maves <curtis@maves.io>
To: "Luiz Augusto von Dentz" <luiz.dentz@gmail.com>
Cc: "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>
Subject: Re: [PATCH BlueZ] gatt-database: Fix notifying on indicatable attr
Date: Fri, 19 Feb 2021 20:18:33 -0500 [thread overview]
Message-ID: <177bd04559f.d1a7c05c116102.319856870975137121@maves.io> (raw)
In-Reply-To: <CABBYNZKakfpzTOdZaoa0tZ5Umqu9MKMmkMfNN51XY5owC_KWwQ@mail.gmail.com>
Hi Luiz,
---- On Fri, 19 Feb 2021 19:55:06 -0500 Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote ----
> Hi Curtis,
>
> On Fri, Feb 19, 2021 at 10:11 AM Curtis <curtis@maves.io> wrote:
> >
> > When a local GATT characteristic has both the indicate and notify
> > properties, notifications will not be send to clients requesting them.
> > This change fixes this, allowing for notifications to be sent.
> >
> > Also simplifies logic about when notifications/indications should
> > be sent.
> > ---
> > src/gatt-database.c | 15 ++++++---------
> > 1 file changed, 6 insertions(+), 9 deletions(-)
> >
> > diff --git a/src/gatt-database.c b/src/gatt-database.c
> > index d635c3214..bd5864bcd 100644
> > --- a/src/gatt-database.c
> > +++ b/src/gatt-database.c
> > @@ -1344,10 +1344,7 @@ static void send_notification_to_device(void *data, void *user_data)
> > }
> >
> > ccc = find_ccc_state(device_state, notify->ccc_handle);
> > - if (!ccc)
> > - return;
> > -
> > - if (!ccc->value || (notify->conf && !(ccc->value & 0x0002)))
> > + if (!ccc || !(ccc->value & 0x0003))
> > return;
> >
> > device = btd_adapter_find_device(notify->database->adapter,
> > @@ -1374,7 +1371,7 @@ static void send_notification_to_device(void *data, void *user_data)
> > * TODO: If the device is not connected but bonded, send the
> > * notification/indication when it becomes connected.
> > */
> > - if (!notify->conf) {
> > + if (!(ccc->value & 0x0002)) {
> > DBG("GATT server sending notification");
> > bt_gatt_server_send_notification(server,
> > notify->handle, notify->value,
> > @@ -2415,8 +2412,8 @@ static bool sock_io_read(struct io *io, void *user_data)
> > gatt_db_attribute_get_handle(chrc->attrib),
> > buf, bytes_read,
> > gatt_db_attribute_get_handle(chrc->ccc),
> > - chrc->props & BT_GATT_CHRC_PROP_INDICATE ?
> > - conf_cb : NULL, chrc->proxy);
> > + conf_cb,
> > + chrc->proxy);
>
> Not why are you changing this code to always set the conf_cb? This
> would then always send indication rather then notifications:
>
> https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/src/gatt-database.c#n1387
>
> We might need to check what value it stored in the ccc state if both
> indication and notification is supported.
>
> >
> > return true;
> > }
> > @@ -2725,8 +2722,8 @@ static void property_changed_cb(GDBusProxy *proxy, const char *name,
> > gatt_db_attribute_get_handle(chrc->attrib),
> > value, len,
> > gatt_db_attribute_get_handle(chrc->ccc),
> > - chrc->props & BT_GATT_CHRC_PROP_INDICATE ?
> > - conf_cb : NULL, proxy);
> > + conf_cb,
> > + proxy);
> > }
> >
> > static bool database_add_ccc(struct external_service *service,
> > --
> > 2.30.1
> >
> >
>
>
> --
> Luiz Augusto von Dentz
>
This patch changes the if-statement around sending notifications to check that the
ccc->value is not indicating rather than checking if conf_cb (notify->conf) is null.
This change makes unnecessary to conditionally pass the conf_cb. It's now simpler to always pass it.
--
Curtis Maves
next prev parent reply other threads:[~2021-02-20 1:19 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-19 17:49 [PATCH BlueZ] gatt-database: Fix notifying on indicatable attr Curtis
2021-02-19 18:37 ` [BlueZ] " bluez.test.bot
2021-02-20 0:55 ` [PATCH BlueZ] " Luiz Augusto von Dentz
2021-02-20 1:18 ` Curtis Maves [this message]
2021-02-20 6:07 ` Luiz Augusto von Dentz
2021-02-20 18:29 ` Curtis Maves
2021-02-22 18:24 ` Luiz Augusto von Dentz
2021-02-22 18:26 ` Luiz Augusto von Dentz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=177bd04559f.d1a7c05c116102.319856870975137121@maves.io \
--to=curtis@maves.io \
--cc=linux-bluetooth@vger.kernel.org \
--cc=luiz.dentz@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).