Hi, Luiz
On Tue, Oct 3, 2017 at 12:36 AM, Luiz Augusto von Dentz
<luiz.dentz@xxxxxxxxx> wrote:
> Hi Yunhan,
>
> On Tue, Oct 3, 2017 at 9:13 AM, Yunhan Wang <yunhanw@xxxxxxxxxxxx> wrote:
>> Hi, Luiz
>>
>> On Fri, Sep 29, 2017 at 12:03 AM, Luiz Augusto von Dentz
>> <luiz.dentz@xxxxxxxxx> wrote:
>>> 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.
>>>
>> In previous email, I have confirmed AcquireWrite is working, now I
>> wanna send indication via fd using AcquireNotify in application, I
>> have removed "check" for "notify" in
>> https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/client/gatt.c#n1387
>
> Does this means you don't have the notify flag at all, or you are
> using indications? I guess we should enable this for indications as
> well, though it won't have any means to confirm.
>
I am using indicate flag that expects the confirmation. In regular
indication operation, the indication confirmation would be received in
server side per the code,
https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/src/gatt-database.c#n935.
I just send a follow-up patch to add proxy in
send_notification_to_devices call that fix indication confirmation
using pipe_io_read. It is working now. Thank you
>> Then I try to use pipe_io_send to send indication in application, but
>> it seems data cannot be sent out from peripheral to central, any idea?
>> it seems there is no reference code to send data in bluetoothctl.
>>
>> static int pipe_io_send(struct io *io, const void *data, size_t len)
>> {
>> struct iovec iov;
>>
>> iov.iov_base = (void *) data;
>> iov.iov_len = len;
>>
>> return io_send(io, &iov, 1);
>> }
>
> Isn't this because of the following check:
> https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/src/gatt-database.c#n906
>
> Im afraid you have quite a different use case than we were targeting,
> it seems you do you GATT for acking delivery instead of doing it
> directly over the protocol which is what most solutions I know would
> do when emulating serial over GATT.
>
>>>>> +
>>>>> /*
>>>>> * 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
>>
>> thansk
>> best wishes
>> yunhan
>
>
>
> --
> Luiz Augusto von Dentz
Thanks
Best wishes
Yunhan
--
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
