Hi Yunhan,
On Fri, Sep 29, 2017 at 7:47 AM, Yunhan Wang <yunhanw@xxxxxxxxxxxx> wrote:
> 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?
notify_io is the receiving end of the notification, the application
will write its notifications using the fd which is captured by
checking if the fd is readable.
>
>> +
>> + __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？
This is using the number of subscribed client as a refcount so when
there is no client left we can drop the notify_io as well.
>
>> +
>> + 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?
If NotifyAcquired is supported it will call AcquireNotify, or are you
saying you tried that? The bluetoothctl has code to utilize this
feature, you just have to register a characteristic with notify flag.
>> +
>> /*
>> * 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
--
Luiz Augusto von Dentz
--
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
