Re: [PATCH v2 08/10] gatt: Implement AcquireNotify for server

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

 



Hi, Luiz

May I have several questions about this feature?

On Wed, Sep 20, 2017 at 12:44 AM, Luiz Augusto von Dentz
<luiz.dentz@xxxxxxxxx> wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
>
> This enables IO via file descriptors using AcquireWrite if server
> implements it.
> ---
>  src/gatt-database.c | 116 +++++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 111 insertions(+), 5 deletions(-)
>
> diff --git a/src/gatt-database.c b/src/gatt-database.c
> index 239fe5384..4932568dd 100644
> --- a/src/gatt-database.c
> +++ b/src/gatt-database.c
> @@ -24,6 +24,7 @@
>  #include <stdint.h>
>  #include <stdlib.h>
>  #include <errno.h>
> +#include <unistd.h>
>
>  #include "lib/bluetooth.h"
>  #include "lib/sdp.h"
> @@ -118,7 +119,9 @@ struct external_chrc {
>         uint8_t props;
>         uint8_t ext_props;
>         uint32_t perm;
> +       uint16_t mtu;
>         struct io *write_io;
> +       struct io *notify_io;
>         struct gatt_db_attribute *attrib;
>         struct gatt_db_attribute *ccc;
>         struct queue *pending_reads;
> @@ -152,7 +155,8 @@ struct device_state {
>         struct queue *ccc_states;
>  };
>
> -typedef uint8_t (*btd_gatt_database_ccc_write_t) (uint16_t value,
> +typedef uint8_t (*btd_gatt_database_ccc_write_t) (struct bt_att *att,
> +                                                       uint16_t value,
>                                                         void *user_data);
>  typedef void (*btd_gatt_database_destroy_t) (void *data);
>
> @@ -328,6 +332,7 @@ static void chrc_free(void *data)
>         struct external_chrc *chrc = data;
>
>         io_destroy(chrc->write_io);
> +       io_destroy(chrc->notify_io);
>
>         queue_destroy(chrc->pending_reads, cancel_pending_read);
>         queue_destroy(chrc->pending_writes, cancel_pending_write);
> @@ -792,7 +797,8 @@ static void gatt_ccc_write_cb(struct gatt_db_attribute *attrib,
>                 goto done;
>
>         if (ccc_cb->callback)
> -               ecode = ccc_cb->callback(get_le16(value), ccc_cb->user_data);
> +               ecode = ccc_cb->callback(att, get_le16(value),
> +                                               ccc_cb->user_data);
>
>         if (!ecode) {
>                 ccc->value[0] = value[0];
> @@ -1801,12 +1807,34 @@ static bool pipe_hup(struct io *io, void *user_data)
>
>         if (io == chrc->write_io)
>                 chrc->write_io = NULL;
> +       else
> +               chrc->notify_io = NULL;
>
>         io_destroy(io);
>
>         return false;
>  }
>
> +static bool pipe_io_read(struct io *io, void *user_data)
> +{
> +       struct external_chrc *chrc = user_data;
> +       uint8_t buf[512];
> +       int fd = io_get_fd(io);
> +       ssize_t bytes_read;
> +
> +       bytes_read = read(fd, buf, sizeof(buf));
> +       if (bytes_read < 0)
> +               return false;
> +
> +       send_notification_to_devices(chrc->service->app->database,
> +                               gatt_db_attribute_get_handle(chrc->attrib),
> +                               buf, bytes_read,
> +                               gatt_db_attribute_get_handle(chrc->ccc),
> +                               false, NULL);
> +
> +       return true;
> +}
> +
>  static struct io *pipe_io_new(int fd, void *user_data)
>  {
>         struct io *io;
> @@ -1815,6 +1843,8 @@ static struct io *pipe_io_new(int fd, void *user_data)
>
>         io_set_close_on_destroy(io, true);
>
> +       io_set_read_handler(io, pipe_io_read, user_data, NULL);
> +
>         io_set_disconnect_handler(io, pipe_hup, user_data, NULL);
>
>         return io;
> @@ -1915,9 +1945,64 @@ static struct pending_op *acquire_write(struct external_chrc *chrc,
>         return NULL;
>  }
>
> -static uint8_t ccc_write_cb(uint16_t value, void *user_data)
> +static void acquire_notify_reply(DBusMessage *message, void *user_data)
>  {
>         struct external_chrc *chrc = user_data;
> +       DBusError err;
> +       int fd;
> +       uint16_t mtu;
> +
> +       dbus_error_init(&err);
> +
> +       if (dbus_set_error_from_message(&err, message) == TRUE) {
> +               error("Failed to acquire notify: %s\n", err.name);
> +               dbus_error_free(&err);
> +               goto retry;
> +       }
> +
> +       if ((dbus_message_get_args(message, NULL, DBUS_TYPE_UNIX_FD, &fd,
> +                                       DBUS_TYPE_UINT16, &mtu,
> +                                       DBUS_TYPE_INVALID) == false)) {
> +               error("Invalid AcquirNotify response\n");
> +               goto retry;
> +       }
> +
> +       DBG("AcquireNotify success: fd %d MTU %u\n", fd, mtu);
> +
> +       chrc->notify_io = pipe_io_new(fd, chrc);

Why do we need to use pipe and notify_io when implementing this
feature? I think fd and mtu has been pass using dbus message. Could
you clarify more details?

> +
> +       __sync_fetch_and_add(&chrc->ntfy_cnt, 1);
> +
> +       return;
> +
> +retry:
> +       g_dbus_proxy_method_call(chrc->proxy, "StartNotify", NULL, NULL,
> +                                                       NULL, NULL);
> +
> +       __sync_fetch_and_add(&chrc->ntfy_cnt, 1);
> +}
> +
> +static void acquire_notify_setup(DBusMessageIter *iter, void *user_data)
> +{
> +       DBusMessageIter dict;
> +       struct external_chrc *chrc = user_data;
> +
> +       dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY,
> +                                       DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
> +                                       DBUS_TYPE_STRING_AS_STRING
> +                                       DBUS_TYPE_VARIANT_AS_STRING
> +                                       DBUS_DICT_ENTRY_END_CHAR_AS_STRING,
> +                                       &dict);
> +
> +       dict_append_entry(&dict, "MTU", DBUS_TYPE_UINT16, &chrc->mtu);
> +
> +       dbus_message_iter_close_container(iter, &dict);
> +}
> +
> +static uint8_t ccc_write_cb(struct bt_att *att, uint16_t value, void *user_data)
> +{
> +       struct external_chrc *chrc = user_data;
> +       DBusMessageIter iter;
>
>         DBG("External CCC write received with value: 0x%04x", value);
>
> @@ -1929,6 +2014,12 @@ static uint8_t ccc_write_cb(uint16_t value, void *user_data)
>                 if (__sync_sub_and_fetch(&chrc->ntfy_cnt, 1))
>                         return 0;
>
> +               if (chrc->notify_io) {
> +                       io_destroy(chrc->notify_io);
> +                       chrc->notify_io = NULL;
> +                       return 0;
> +               }
> +
>                 /*
>                  * Send request to stop notifying. This is best-effort
>                  * operation, so simply ignore the return the value.
> @@ -1949,12 +2040,27 @@ static uint8_t ccc_write_cb(uint16_t value, void *user_data)
>                 (value == 2 && !(chrc->props & BT_GATT_CHRC_PROP_INDICATE)))
>                 return BT_ERROR_CCC_IMPROPERLY_CONFIGURED;
>
> +       if (chrc->notify_io) {
> +               __sync_fetch_and_add(&chrc->ntfy_cnt, 1);
> +               return 0;
> +       }
What is the purpose to call __sync_fetch_and_add when there is
charc->notify_io? What is the scenario here?

> +
> +       chrc->mtu = bt_att_get_mtu(att);
> +
> +       /* Make use of AcquireNotify if supported */
> +       if (g_dbus_proxy_get_property(chrc->proxy, "NotifyAcquired", &iter)) {
> +               if (g_dbus_proxy_method_call(chrc->proxy, "AcquireNotify",
> +                                               acquire_notify_setup,
> +                                               acquire_notify_reply,
> +                                               chrc, NULL))
> +                       return 0;
> +       }
When ble central subscribe this characteristic,  if NotifyAcquired is
set in ble peripheral application, then it return, but how can we
subscribe this characteristic? May we call "AcquireNotify" when ble
central subscribe this characteristic or enable notification?

> +
>         /*
>          * Always call StartNotify for an incoming enable and ignore the return
>          * value for now.
>          */
> -       if (g_dbus_proxy_method_call(chrc->proxy,
> -                                               "StartNotify", NULL, NULL,
> +       if (g_dbus_proxy_method_call(chrc->proxy, "StartNotify", NULL, NULL,
>                                                 NULL, NULL) == FALSE)
>                 return BT_ATT_ERROR_UNLIKELY;
>
> --
> 2.13.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[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