* [PATCH BlueZ 2/4] doc/gatt-api: Merge RegisterProfile with RegisterApplication
2016-05-06 19:10 [PATCH BlueZ 1/4] doc/gatt-api: Add options dictionary to ReadValue/WriteValue Luiz Augusto von Dentz
@ 2016-05-06 19:11 ` Luiz Augusto von Dentz
2016-05-07 20:15 ` Vinicius Costa Gomes
2016-05-06 19:11 ` [PATCH BlueZ 3/4] doc/gatt-api: Add secure flags Luiz Augusto von Dentz
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Luiz Augusto von Dentz @ 2016-05-06 19:11 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Since RegisterApplication makes use of ObjectManager it is also possible
to verify the existance of GattProfile objects unifying the API for both
services (GATT server) and profiles (GATT client).
---
doc/gatt-api.txt | 37 ++---
src/gatt-database.c | 414 +++++++++++++++++++++-------------------------------
2 files changed, 172 insertions(+), 279 deletions(-)
diff --git a/doc/gatt-api.txt b/doc/gatt-api.txt
index 683b1b7..9404986 100644
--- a/doc/gatt-api.txt
+++ b/doc/gatt-api.txt
@@ -218,8 +218,8 @@ Properties string UUID [read-only]
"encrypt-authenticated-read"
"encrypt-authenticated-write"
-Profile hierarcy
-================
+GATT Profile hierarcy
+=====================
Local profile (GATT client) instance. By registering this type of object
an application effectively indicates support for a specific GATT profile
@@ -238,6 +238,10 @@ Methods void Release()
profile, because when this method gets called it has
already been unregistered.
+Properties array{string} UUIDs [read-only]
+
+ 128-bit GATT service UUIDs.
+
GATT Manager hierarchy
======================
@@ -306,11 +310,12 @@ Object path [variable prefix]/{hci0,hci1,...}
Methods void RegisterApplication(object application, dict options)
Registers a local GATT services hierarchy as described
- above.
+ above (GATT Server) and/or GATT profiles (GATT Client).
The application object path together with the D-Bus
system bus connection ID define the identification of
- the application registering a GATT based service.
+ the application registering a GATT based
+ service or profile.
Possible errors: org.bluez.Error.InvalidArguments
org.bluez.Error.AlreadyExists
@@ -324,27 +329,3 @@ Methods void RegisterApplication(object application, dict options)
Possible errors: org.bluez.Error.InvalidArguments
org.bluez.Error.DoesNotExist
-
- void RegisterProfile(object profile, array{string} UUIDs,
- dict options)
-
- Registers a GATT (client role) profile exported
- under interface GattProfile1. The array of UUIDs
- specifies the mandatory set of remote service
- UUIDs that should all be available for the
- remote device to match this profile. Matching
- devices will be added to the auto-connection
- list and connected whenever available.
-
- Possible errors: org.bluez.Error.InvalidArguments
- org.bluez.Error.AlreadyExists
-
- void UnregisterProfile(object profile)
-
- This unregisters the profile that has been
- previously registered. The object path parameter
- must match the same value that has been used
- on registration.
-
- Possible errors: org.bluez.Error.InvalidArguments
- org.bluez.Error.DoesNotExist
diff --git a/src/gatt-database.c b/src/gatt-database.c
index 7f36ffd..594972a 100644
--- a/src/gatt-database.c
+++ b/src/gatt-database.c
@@ -54,6 +54,7 @@
#endif
#define GATT_MANAGER_IFACE "org.bluez.GattManager1"
+#define GATT_PROFILE_IFACE "org.bluez.GattProfile1"
#define GATT_SERVICE_IFACE "org.bluez.GattService1"
#define GATT_CHRC_IFACE "org.bluez.GattCharacteristic1"
#define GATT_DESC_IFACE "org.bluez.GattDescriptor1"
@@ -88,6 +89,7 @@ struct gatt_app {
DBusMessage *reg;
GDBusClient *client;
bool failed;
+ struct queue *profiles;
struct queue *services;
struct queue *proxies;
};
@@ -103,10 +105,8 @@ struct external_service {
};
struct external_profile {
- struct btd_gatt_database *database;
- char *owner;
- char *path; /* Path to GattProfile1 */
- unsigned int id;
+ struct gatt_app *app;
+ GDBusProxy *proxy;
struct queue *profiles; /* btd_profile list */
};
@@ -362,30 +362,6 @@ static void service_free(void *data)
free(service);
}
-static void app_free(void *data)
-{
- struct gatt_app *app = data;
-
- queue_destroy(app->services, service_free);
- queue_destroy(app->proxies, NULL);
-
- if (app->client) {
- g_dbus_client_set_disconnect_watch(app->client, NULL, NULL);
- g_dbus_client_set_proxy_handlers(app->client, NULL, NULL,
- NULL, NULL);
- g_dbus_client_set_ready_watch(app->client, NULL, NULL);
- g_dbus_client_unref(app->client);
- }
-
- if (app->reg)
- dbus_message_unref(app->reg);
-
- g_free(app->owner);
- g_free(app->path);
-
- free(app);
-}
-
static void profile_remove(void *data)
{
struct btd_profile *p = data;
@@ -403,20 +379,10 @@ static void profile_remove(void *data)
static void profile_release(struct external_profile *profile)
{
- DBusMessage *msg;
-
- if (!profile->id)
- return;
-
- DBG("Releasing \"%s\"", profile->owner);
-
- g_dbus_remove_watch(btd_get_dbus_connection(), profile->id);
+ DBG("Releasing \"%s\"", profile->app->owner);
- msg = dbus_message_new_method_call(profile->owner, profile->path,
- "org.bluez.GattProfile1",
- "Release");
- if (msg)
- g_dbus_send_message(btd_get_dbus_connection(), msg);
+ g_dbus_proxy_method_call(profile->proxy, "Release", NULL, NULL, NULL,
+ NULL);
}
static void profile_free(void *data)
@@ -427,12 +393,36 @@ static void profile_free(void *data)
profile_release(profile);
- g_free(profile->owner);
- g_free(profile->path);
+ g_dbus_proxy_unref(profile->proxy);
free(profile);
}
+static void app_free(void *data)
+{
+ struct gatt_app *app = data;
+
+ queue_destroy(app->profiles, profile_free);
+ queue_destroy(app->services, service_free);
+ queue_destroy(app->proxies, NULL);
+
+ if (app->client) {
+ g_dbus_client_set_disconnect_watch(app->client, NULL, NULL);
+ g_dbus_client_set_proxy_handlers(app->client, NULL, NULL,
+ NULL, NULL);
+ g_dbus_client_set_ready_watch(app->client, NULL, NULL);
+ g_dbus_client_unref(app->client);
+ }
+
+ if (app->reg)
+ dbus_message_unref(app->reg);
+
+ g_free(app->owner);
+ g_free(app->path);
+
+ free(app);
+}
+
static void gatt_database_free(void *data)
{
struct btd_gatt_database *database = data;
@@ -2209,9 +2199,6 @@ static bool database_add_app(struct gatt_app *app)
{
const struct queue_entry *entry;
- if (queue_isempty(app->services))
- return false;
-
entry = queue_get_entries(app->services);
while (entry) {
if (!database_add_service(entry->data)) {
@@ -2225,6 +2212,132 @@ static bool database_add_app(struct gatt_app *app)
return true;
}
+static int profile_device_probe(struct btd_service *service)
+{
+ struct btd_profile *p = btd_service_get_profile(service);
+
+ DBG("%s probed", p->name);
+
+ return 0;
+}
+
+static void profile_device_remove(struct btd_service *service)
+{
+ struct btd_profile *p = btd_service_get_profile(service);
+
+ DBG("%s removed", p->name);
+}
+
+static int profile_add(struct external_profile *profile, const char *uuid)
+{
+ struct btd_profile *p;
+
+ p = new0(struct btd_profile, 1);
+
+ /* Assign directly to avoid having extra fields */
+ p->name = (const void *) g_strdup_printf("%s%s/%s", profile->app->owner,
+ g_dbus_proxy_get_path(profile->proxy), uuid);
+ if (!p->name) {
+ free(p);
+ return -ENOMEM;
+ }
+
+ p->remote_uuid = (const void *) g_strdup(uuid);
+ if (!p->remote_uuid) {
+ g_free((void *) p->name);
+ free(p);
+ return -ENOMEM;
+ }
+
+ p->device_probe = profile_device_probe;
+ p->device_remove = profile_device_remove;
+ p->auto_connect = true;
+ p->external = true;
+
+ queue_push_tail(profile->profiles, p);
+
+ DBG("Added \"%s\"", p->name);
+
+ return 0;
+}
+
+static void add_profile(void *data, void *user_data)
+{
+ struct btd_adapter *adapter = user_data;
+
+ btd_profile_register(data);
+ adapter_add_profile(adapter, data);
+}
+
+static struct external_profile *create_profile(struct gatt_app *app,
+ GDBusProxy *proxy,
+ const char *path)
+
+{
+ struct external_profile *profile;
+ DBusMessageIter iter, array;
+
+ if (!path || !g_str_has_prefix(path, "/"))
+ return NULL;
+
+ profile = new0(struct external_profile, 1);
+
+ profile->app = app;
+ profile->proxy = g_dbus_proxy_ref(proxy);
+ profile->profiles = queue_new();
+
+ if (!g_dbus_proxy_get_property(proxy, "UUIDs", &iter)) {
+ DBG("UUIDs property not found");
+ goto fail;
+ }
+
+ dbus_message_iter_recurse(&iter, &array);
+
+ while (dbus_message_iter_get_arg_type(&array) == DBUS_TYPE_STRING) {
+ const char *uuid;
+
+ dbus_message_iter_get_basic(&array, &uuid);
+
+ if (profile_add(profile, uuid) < 0)
+ goto fail;
+
+ dbus_message_iter_next(&array);
+ }
+
+ if (queue_isempty(profile->profiles))
+ goto fail;
+
+ queue_foreach(profile->profiles, add_profile, app->database->adapter);
+ queue_push_tail(app->profiles, profile);
+
+ return profile;
+
+fail:
+ profile_free(profile);
+ return NULL;
+}
+
+static void register_profile(void *data, void *user_data)
+{
+ struct gatt_app *app = user_data;
+ GDBusProxy *proxy = data;
+ const char *iface = g_dbus_proxy_get_interface(proxy);
+ const char *path = g_dbus_proxy_get_path(proxy);
+
+ if (app->failed)
+ return;
+
+ if (g_strcmp0(iface, GATT_PROFILE_IFACE) == 0) {
+ struct external_profile *profile;
+
+ profile = create_profile(app, proxy, path);
+ if (!profile) {
+ app->failed = true;
+ return;
+ }
+ }
+}
+
static void register_service(void *data, void *user_data)
{
struct gatt_app *app = user_data;
@@ -2307,11 +2420,13 @@ static void client_ready_cb(GDBusClient *client, void *user_data)
goto reply;
}
+ queue_foreach(app->proxies, register_profile, app);
queue_foreach(app->proxies, register_service, app);
queue_foreach(app->proxies, register_characteristic, app);
queue_foreach(app->proxies, register_descriptor, app);
- if (!app->services || app->failed) {
+ if ((queue_isempty(app->services) && queue_isempty(app->profiles)) ||
+ app->failed) {
error("No valid external GATT objects found");
fail = true;
reply = btd_error_failed(app->reg,
@@ -2364,6 +2479,7 @@ static struct gatt_app *create_app(DBusConnection *conn, DBusMessage *msg,
goto fail;
app->services = queue_new();
+ app->profiles = queue_new();
app->proxies = queue_new();
app->reg = dbus_message_ref(msg);
@@ -2450,203 +2566,6 @@ static DBusMessage *manager_unregister_app(DBusConnection *conn,
return dbus_message_new_method_return(msg);
}
-static void profile_exited(DBusConnection *conn, void *user_data)
-{
- struct external_profile *profile = user_data;
-
- DBG("\"%s\" exited", profile->owner);
-
- profile->id = 0;
-
- queue_remove(profile->database->profiles, profile);
-
- profile_free(profile);
-}
-
-static int profile_device_probe(struct btd_service *service)
-{
- struct btd_profile *p = btd_service_get_profile(service);
-
- DBG("%s probed", p->name);
-
- return 0;
-}
-
-static void profile_device_remove(struct btd_service *service)
-{
- struct btd_profile *p = btd_service_get_profile(service);
-
- DBG("%s removed", p->name);
-}
-
-static int profile_add(struct external_profile *profile, const char *uuid)
-{
- struct btd_profile *p;
-
- p = new0(struct btd_profile, 1);
-
- /* Assign directly to avoid having extra fields */
- p->name = (const void *) g_strdup_printf("%s%s/%s", profile->owner,
- profile->path, uuid);
- if (!p->name) {
- free(p);
- return -ENOMEM;
- }
-
- p->remote_uuid = (const void *) g_strdup(uuid);
- if (!p->remote_uuid) {
- g_free((void *) p->name);
- free(p);
- return -ENOMEM;
- }
-
- p->device_probe = profile_device_probe;
- p->device_remove = profile_device_remove;
- p->auto_connect = true;
- p->external = true;
-
- queue_push_tail(profile->profiles, p);
-
- DBG("Added \"%s\"", p->name);
-
- return 0;
-}
-
-static void add_profile(void *data, void *user_data)
-{
- struct btd_adapter *adapter = user_data;
-
- btd_profile_register(data);
- adapter_add_profile(adapter, data);
-}
-
-static int profile_create(DBusConnection *conn,
- struct btd_gatt_database *database,
- const char *sender, const char *path,
- DBusMessageIter *iter)
-{
- struct external_profile *profile;
- DBusMessageIter uuids;
-
- if (!path || !g_str_has_prefix(path, "/"))
- return -EINVAL;
-
- profile = new0(struct external_profile, 1);
-
- profile->owner = g_strdup(sender);
- if (!profile->owner)
- goto fail;
-
- profile->path = g_strdup(path);
- if (!profile->path)
- goto fail;
-
- profile->profiles = queue_new();
- profile->database = database;
- profile->id = g_dbus_add_disconnect_watch(conn, sender, profile_exited,
- profile, NULL);
-
- dbus_message_iter_recurse(iter, &uuids);
-
- while (dbus_message_iter_get_arg_type(&uuids) == DBUS_TYPE_STRING) {
- const char *uuid;
-
- dbus_message_iter_get_basic(&uuids, &uuid);
-
- if (profile_add(profile, uuid) < 0)
- goto fail;
-
- dbus_message_iter_next(&uuids);
- }
-
- if (queue_isempty(profile->profiles))
- goto fail;
-
- queue_foreach(profile->profiles, add_profile, database->adapter);
- queue_push_tail(database->profiles, profile);
-
- return 0;
-
-fail:
- profile_free(profile);
- return -EINVAL;
-}
-
-static bool match_profile(const void *a, const void *b)
-{
- const struct external_profile *profile = a;
- const struct svc_match_data *data = b;
-
- return g_strcmp0(profile->path, data->path) == 0 &&
- g_strcmp0(profile->owner, data->sender) == 0;
-}
-
-static DBusMessage *manager_register_profile(DBusConnection *conn,
- DBusMessage *msg, void *user_data)
-{
- struct btd_gatt_database *database = user_data;
- const char *sender = dbus_message_get_sender(msg);
- DBusMessageIter args;
- const char *path;
- struct svc_match_data match_data;
-
- DBG("sender %s", sender);
-
- if (!dbus_message_iter_init(msg, &args))
- return btd_error_invalid_args(msg);
-
- if (dbus_message_iter_get_arg_type(&args) != DBUS_TYPE_OBJECT_PATH)
- return btd_error_invalid_args(msg);
-
- dbus_message_iter_get_basic(&args, &path);
-
- match_data.path = path;
- match_data.sender = sender;
-
- if (queue_find(database->profiles, match_profile, &match_data))
- return btd_error_already_exists(msg);
-
- dbus_message_iter_next(&args);
- if (dbus_message_iter_get_arg_type(&args) != DBUS_TYPE_ARRAY)
- return btd_error_invalid_args(msg);
-
- if (profile_create(conn, database, sender, path, &args) < 0)
- return btd_error_failed(msg, "Failed to register profile");
-
- return dbus_message_new_method_return(msg);
-}
-
-static DBusMessage *manager_unregister_profile(DBusConnection *conn,
- DBusMessage *msg, void *user_data)
-{
- struct btd_gatt_database *database = user_data;
- const char *sender = dbus_message_get_sender(msg);
- const char *path;
- DBusMessageIter args;
- struct external_profile *profile;
- struct svc_match_data match_data;
-
- if (!dbus_message_iter_init(msg, &args))
- return btd_error_invalid_args(msg);
-
- if (dbus_message_iter_get_arg_type(&args) != DBUS_TYPE_OBJECT_PATH)
- return btd_error_invalid_args(msg);
-
- dbus_message_iter_get_basic(&args, &path);
-
- match_data.path = path;
- match_data.sender = sender;
-
- profile = queue_remove_if(database->profiles, match_profile,
- &match_data);
- if (!profile)
- return btd_error_does_not_exist(msg);
-
- profile_free(profile);
-
- return dbus_message_new_method_return(msg);
-}
-
static const GDBusMethodTable manager_methods[] = {
{ GDBUS_EXPERIMENTAL_ASYNC_METHOD("RegisterApplication",
GDBUS_ARGS({ "application", "o" },
@@ -2655,13 +2574,6 @@ static const GDBusMethodTable manager_methods[] = {
{ GDBUS_EXPERIMENTAL_ASYNC_METHOD("UnregisterApplication",
GDBUS_ARGS({ "application", "o" }),
NULL, manager_unregister_app) },
- { GDBUS_EXPERIMENTAL_ASYNC_METHOD("RegisterProfile",
- GDBUS_ARGS({ "profile", "o" }, { "UUIDs", "as" },
- { "options", "a{sv}" }), NULL,
- manager_register_profile) },
- { GDBUS_EXPERIMENTAL_ASYNC_METHOD("UnregisterProfile",
- GDBUS_ARGS({ "profile", "o" }),
- NULL, manager_unregister_profile) },
{ }
};
--
2.5.5
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH BlueZ 2/4] doc/gatt-api: Merge RegisterProfile with RegisterApplication
2016-05-06 19:11 ` [PATCH BlueZ 2/4] doc/gatt-api: Merge RegisterProfile with RegisterApplication Luiz Augusto von Dentz
@ 2016-05-07 20:15 ` Vinicius Costa Gomes
0 siblings, 0 replies; 11+ messages in thread
From: Vinicius Costa Gomes @ 2016-05-07 20:15 UTC (permalink / raw)
To: Luiz Augusto von Dentz, linux-bluetooth
Hi Luiz,
Luiz Augusto von Dentz <luiz.dentz@gmail.com> writes:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> Since RegisterApplication makes use of ObjectManager it is also possible
> to verify the existance of GattProfile objects unifying the API for both
> services (GATT server) and profiles (GATT client).
> ---
Just cosmetic stuff.
> doc/gatt-api.txt | 37 ++---
> src/gatt-database.c | 414 +++++++++++++++++++++-------------------------------
> 2 files changed, 172 insertions(+), 279 deletions(-)
>
> diff --git a/doc/gatt-api.txt b/doc/gatt-api.txt
> index 683b1b7..9404986 100644
> --- a/doc/gatt-api.txt
> +++ b/doc/gatt-api.txt
> @@ -218,8 +218,8 @@ Properties string UUID [read-only]
> "encrypt-authenticated-read"
> "encrypt-authenticated-write"
>
[...]
> +static void add_profile(void *data, void *user_data)
> +{
> + struct btd_adapter *adapter = user_data;
> +
> + btd_profile_register(data);
> + adapter_add_profile(adapter, data);
> +}
> +
> +static struct external_profile *create_profile(struct gatt_app *app,
> + GDBusProxy *proxy,
> + const char *path)
> +
Extra empty line.
> +{
> + struct external_profile *profile;
> + DBusMessageIter iter, array;
> +
> + if (!path || !g_str_has_prefix(path, "/"))
> + return NULL;
> +
> + profile = new0(struct external_profile, 1);
> +
> + profile->app = app;
> + profile->proxy = g_dbus_proxy_ref(proxy);
> + profile->profiles = queue_new();
> +
> + if (!g_dbus_proxy_get_property(proxy, "UUIDs", &iter)) {
> + DBG("UUIDs property not found");
> + goto fail;
> + }
> +
> + dbus_message_iter_recurse(&iter, &array);
> +
> + while (dbus_message_iter_get_arg_type(&array) == DBUS_TYPE_STRING) {
> + const char *uuid;
> +
> + dbus_message_iter_get_basic(&array, &uuid);
> +
> + if (profile_add(profile, uuid) < 0)
> + goto fail;
> +
> + dbus_message_iter_next(&array);
> + }
> +
> + if (queue_isempty(profile->profiles))
> + goto fail;
> +
> + queue_foreach(profile->profiles, add_profile, app->database->adapter);
> + queue_push_tail(app->profiles, profile);
> +
> + return profile;
> +
> +fail:
> + profile_free(profile);
> + return NULL;
> +}
> +
Cheers,
--
Vinicius
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH BlueZ 3/4] doc/gatt-api: Add secure flags
2016-05-06 19:10 [PATCH BlueZ 1/4] doc/gatt-api: Add options dictionary to ReadValue/WriteValue Luiz Augusto von Dentz
2016-05-06 19:11 ` [PATCH BlueZ 2/4] doc/gatt-api: Merge RegisterProfile with RegisterApplication Luiz Augusto von Dentz
@ 2016-05-06 19:11 ` Luiz Augusto von Dentz
2016-05-06 19:11 ` [PATCH BlueZ 4/4] test: Update GATT examples with the new API Luiz Augusto von Dentz
` (2 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Luiz Augusto von Dentz @ 2016-05-06 19:11 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This add secure-{read,write} which shall be used by servers that want
to restrict attribute access to secure connection only (BT_SECURITY_FIPS)
---
doc/gatt-api.txt | 4 +++
lib/bluetooth.h | 1 +
src/gatt-database.c | 90 ++++++++++++++++++++----------------------------
src/shared/att-types.h | 5 +++
src/shared/gatt-server.c | 3 ++
5 files changed, 51 insertions(+), 52 deletions(-)
diff --git a/doc/gatt-api.txt b/doc/gatt-api.txt
index 9404986..cb4e196 100644
--- a/doc/gatt-api.txt
+++ b/doc/gatt-api.txt
@@ -150,6 +150,8 @@ Properties string UUID [read-only]
"encrypt-write"
"encrypt-authenticated-read"
"encrypt-authenticated-write"
+ "secure-read" (Server only)
+ "secure-write" (Server only)
Characteristic Descriptors hierarchy
====================================
@@ -217,6 +219,8 @@ Properties string UUID [read-only]
"encrypt-write"
"encrypt-authenticated-read"
"encrypt-authenticated-write"
+ "secure-read" (Server Only)
+ "secure-write" (Server Only)
GATT Profile hierarcy
=====================
diff --git a/lib/bluetooth.h b/lib/bluetooth.h
index 852a6b2..eb27926 100644
--- a/lib/bluetooth.h
+++ b/lib/bluetooth.h
@@ -69,6 +69,7 @@ struct bt_security {
#define BT_SECURITY_LOW 1
#define BT_SECURITY_MEDIUM 2
#define BT_SECURITY_HIGH 3
+#define BT_SECURITY_FIPS 4
#define BT_DEFER_SETUP 7
diff --git a/src/gatt-database.c b/src/gatt-database.c
index 594972a..627a416 100644
--- a/src/gatt-database.c
+++ b/src/gatt-database.c
@@ -116,6 +116,7 @@ struct external_chrc {
GDBusProxy *proxy;
uint8_t props;
uint8_t ext_props;
+ uint32_t perm;
struct gatt_db_attribute *attrib;
struct gatt_db_attribute *ccc;
struct queue *pending_reads;
@@ -1113,7 +1114,7 @@ static bool incr_attr_count(struct external_service *service, uint16_t incr)
}
static bool parse_chrc_flags(DBusMessageIter *array, uint8_t *props,
- uint8_t *ext_props)
+ uint8_t *ext_props, uint32_t *perm)
{
const char *flag;
@@ -1127,34 +1128,51 @@ static bool parse_chrc_flags(DBusMessageIter *array, uint8_t *props,
if (!strcmp("broadcast", flag))
*props |= BT_GATT_CHRC_PROP_BROADCAST;
- else if (!strcmp("read", flag))
+ else if (!strcmp("read", flag)) {
*props |= BT_GATT_CHRC_PROP_READ;
- else if (!strcmp("write-without-response", flag))
+ *perm |= BT_ATT_PERM_READ;
+ } else if (!strcmp("write-without-response", flag)) {
*props |= BT_GATT_CHRC_PROP_WRITE_WITHOUT_RESP;
- else if (!strcmp("write", flag))
+ *perm |= BT_ATT_PERM_WRITE;
+ } else if (!strcmp("write", flag)) {
*props |= BT_GATT_CHRC_PROP_WRITE;
- else if (!strcmp("notify", flag))
+ *perm |= BT_ATT_PERM_WRITE;
+ } else if (!strcmp("notify", flag)) {
*props |= BT_GATT_CHRC_PROP_NOTIFY;
- else if (!strcmp("indicate", flag))
+ } else if (!strcmp("indicate", flag)) {
*props |= BT_GATT_CHRC_PROP_INDICATE;
- else if (!strcmp("authenticated-signed-writes", flag))
+ } else if (!strcmp("authenticated-signed-writes", flag)) {
*props |= BT_GATT_CHRC_PROP_AUTH;
- else if (!strcmp("reliable-write", flag))
+ *perm |= BT_ATT_PERM_WRITE;
+ } else if (!strcmp("reliable-write", flag)) {
*ext_props |= BT_GATT_CHRC_EXT_PROP_RELIABLE_WRITE;
- else if (!strcmp("writable-auxiliaries", flag))
+ *perm |= BT_ATT_PERM_WRITE;
+ } else if (!strcmp("writable-auxiliaries", flag)) {
*ext_props |= BT_GATT_CHRC_EXT_PROP_WRITABLE_AUX;
- else if (!strcmp("encrypt-read", flag)) {
+ } else if (!strcmp("encrypt-read", flag)) {
*props |= BT_GATT_CHRC_PROP_READ;
*ext_props |= BT_GATT_CHRC_EXT_PROP_ENC_READ;
+ *perm |= BT_ATT_PERM_READ | BT_ATT_PERM_READ_ENCRYPT;
} else if (!strcmp("encrypt-write", flag)) {
*props |= BT_GATT_CHRC_PROP_WRITE;
*ext_props |= BT_GATT_CHRC_EXT_PROP_ENC_WRITE;
+ *perm |= BT_ATT_PERM_WRITE | BT_ATT_PERM_WRITE_ENCRYPT;
} else if (!strcmp("encrypt-authenticated-read", flag)) {
*props |= BT_GATT_CHRC_PROP_READ;
*ext_props |= BT_GATT_CHRC_EXT_PROP_AUTH_READ;
+ *perm |= BT_ATT_PERM_READ | BT_ATT_PERM_READ_AUTHEN;
} else if (!strcmp("encrypt-authenticated-write", flag)) {
*props |= BT_GATT_CHRC_PROP_WRITE;
*ext_props |= BT_GATT_CHRC_EXT_PROP_AUTH_WRITE;
+ *perm |= BT_ATT_PERM_WRITE | BT_ATT_PERM_WRITE_AUTHEN;
+ } else if (!strcmp("secure-read", flag)) {
+ *props |= BT_GATT_CHRC_PROP_READ;
+ *ext_props |= BT_GATT_CHRC_EXT_PROP_AUTH_READ;
+ *perm |= BT_ATT_PERM_WRITE | BT_ATT_PERM_READ_SECURE;
+ } else if (!strcmp("secure-write", flag)) {
+ *props |= BT_GATT_CHRC_PROP_WRITE;
+ *ext_props |= BT_GATT_CHRC_EXT_PROP_AUTH_WRITE;
+ *perm |= BT_ATT_PERM_WRITE | BT_ATT_PERM_WRITE_SECURE;
} else {
error("Invalid characteristic flag: %s", flag);
return false;
@@ -1191,6 +1209,10 @@ static bool parse_desc_flags(DBusMessageIter *array, uint32_t *perm)
*perm |= BT_ATT_PERM_READ | BT_ATT_PERM_READ_AUTHEN;
else if (!strcmp("encrypt-authenticated-write", flag))
*perm |= BT_ATT_PERM_WRITE | BT_ATT_PERM_WRITE_AUTHEN;
+ else if (!strcmp("secure-read", flag))
+ *perm |= BT_ATT_PERM_READ | BT_ATT_PERM_READ_AUTHEN;
+ else if (!strcmp("secure-write", flag))
+ *perm |= BT_ATT_PERM_WRITE | BT_ATT_PERM_WRITE_AUTHEN;
else {
error("Invalid descriptor flag: %s", flag);
return false;
@@ -1204,6 +1226,7 @@ static bool parse_flags(GDBusProxy *proxy, uint8_t *props, uint8_t *ext_props,
uint32_t *perm)
{
DBusMessageIter iter, array;
+ const char *iface;
if (!g_dbus_proxy_get_property(proxy, "Flags", &iter))
return false;
@@ -1213,10 +1236,11 @@ static bool parse_flags(GDBusProxy *proxy, uint8_t *props, uint8_t *ext_props,
dbus_message_iter_recurse(&iter, &array);
- if (perm)
+ iface = g_dbus_proxy_get_interface(proxy);
+ if (!strcmp(iface, GATT_DESC_IFACE))
return parse_desc_flags(&array, perm);
- return parse_chrc_flags(&array, props, ext_props);
+ return parse_chrc_flags(&array, props, ext_props, perm);
}
static struct external_chrc *chrc_create(struct gatt_app *app,
@@ -1264,7 +1288,7 @@ static struct external_chrc *chrc_create(struct gatt_app *app,
* are used to determine if any special descriptors should be
* created.
*/
- if (!parse_flags(proxy, &chrc->props, &chrc->ext_props, NULL)) {
+ if (!parse_flags(proxy, &chrc->props, &chrc->ext_props, &chrc->perm)) {
error("Failed to parse characteristic properties");
goto fail;
}
@@ -1754,37 +1778,6 @@ static void send_write(struct btd_device *device,
gatt_db_attribute_write_result(attrib, id, ecode);
}
-static uint32_t permissions_from_props(uint8_t props, uint8_t ext_props)
-{
- uint32_t perm = 0;
-
- if (props & BT_GATT_CHRC_PROP_WRITE ||
- props & BT_GATT_CHRC_PROP_WRITE_WITHOUT_RESP ||
- ext_props & BT_GATT_CHRC_EXT_PROP_RELIABLE_WRITE ||
- ext_props & BT_GATT_CHRC_EXT_PROP_ENC_WRITE ||
- ext_props & BT_GATT_CHRC_EXT_PROP_AUTH_WRITE)
- perm |= BT_ATT_PERM_WRITE;
-
- if (props & BT_GATT_CHRC_PROP_READ ||
- ext_props & BT_GATT_CHRC_EXT_PROP_ENC_READ ||
- ext_props & BT_GATT_CHRC_EXT_PROP_AUTH_READ)
- perm |= BT_ATT_PERM_READ;
-
- if (ext_props & BT_GATT_CHRC_EXT_PROP_ENC_READ)
- perm |= BT_ATT_PERM_READ_ENCRYPT;
-
- if (ext_props & BT_GATT_CHRC_EXT_PROP_ENC_WRITE)
- perm |= BT_ATT_PERM_WRITE_ENCRYPT;
-
- if (ext_props & BT_GATT_CHRC_EXT_PROP_AUTH_READ)
- perm |= BT_ATT_PERM_READ_AUTHEN;
-
- if (ext_props & BT_GATT_CHRC_EXT_PROP_AUTH_WRITE)
- perm |= BT_ATT_PERM_WRITE_AUTHEN;
-
- return perm;
-}
-
static uint8_t ccc_write_cb(uint16_t value, void *user_data)
{
struct external_chrc *chrc = user_data;
@@ -2086,7 +2079,6 @@ static bool database_add_chrc(struct external_service *service,
struct external_chrc *chrc)
{
bt_uuid_t uuid;
- uint32_t perm;
const struct queue_entry *entry;
if (!parse_uuid(chrc->proxy, &uuid)) {
@@ -2099,14 +2091,8 @@ static bool database_add_chrc(struct external_service *service,
return false;
}
- /*
- * TODO: Once shared/gatt-server properly supports permission checks,
- * set the permissions based on a D-Bus property of the external
- * characteristic.
- */
- perm = permissions_from_props(chrc->props, chrc->ext_props);
chrc->attrib = gatt_db_service_add_characteristic(service->attrib,
- &uuid, perm,
+ &uuid, chrc->perm,
chrc->props, chrc_read_cb,
chrc_write_cb, chrc);
if (!chrc->attrib) {
diff --git a/src/shared/att-types.h b/src/shared/att-types.h
index c3062c0..4a9b67f 100644
--- a/src/shared/att-types.h
+++ b/src/shared/att-types.h
@@ -31,6 +31,7 @@
#define BT_ATT_SECURITY_LOW 1
#define BT_ATT_SECURITY_MEDIUM 2
#define BT_ATT_SECURITY_HIGH 3
+#define BT_ATT_SECURITY_FIPS 4
#define BT_ATT_DEFAULT_LE_MTU 23
#define BT_ATT_MAX_LE_MTU 517
@@ -123,6 +124,10 @@ struct bt_att_pdu_error_rsp {
BT_ATT_PERM_WRITE_AUTHEN)
#define BT_ATT_PERM_AUTHOR 0x40
#define BT_ATT_PERM_NONE 0x80
+#define BT_ATT_PERM_READ_SECURE 0x0100
+#define BT_ATT_PERM_WRITE_SECURE 0x0200
+#define BT_ATT_PERM_SECURE (BT_ATT_PERM_READ_SECURE | \
+ BT_ATT_PERM_WRITE_SECURE)
/* GATT Characteristic Properties Bitfield values */
#define BT_GATT_CHRC_PROP_BROADCAST 0x01
diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
index 123d9c1..79e01c8 100644
--- a/src/shared/gatt-server.c
+++ b/src/shared/gatt-server.c
@@ -398,6 +398,9 @@ static uint8_t check_permissions(struct bt_gatt_server *server,
return 0;
security = bt_att_get_security(server->att);
+ if (perm & BT_ATT_PERM_SECURE && security < BT_ATT_SECURITY_FIPS)
+ return BT_ATT_ERROR_AUTHENTICATION;
+
if (perm & BT_ATT_PERM_AUTHEN && security < BT_ATT_SECURITY_HIGH)
return BT_ATT_ERROR_AUTHENTICATION;
--
2.5.5
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH BlueZ 4/4] test: Update GATT examples with the new API
2016-05-06 19:10 [PATCH BlueZ 1/4] doc/gatt-api: Add options dictionary to ReadValue/WriteValue Luiz Augusto von Dentz
2016-05-06 19:11 ` [PATCH BlueZ 2/4] doc/gatt-api: Merge RegisterProfile with RegisterApplication Luiz Augusto von Dentz
2016-05-06 19:11 ` [PATCH BlueZ 3/4] doc/gatt-api: Add secure flags Luiz Augusto von Dentz
@ 2016-05-06 19:11 ` Luiz Augusto von Dentz
2016-05-06 22:18 ` [PATCH BlueZ 1/4] doc/gatt-api: Add options dictionary to ReadValue/WriteValue Vinicius Costa Gomes
2016-05-07 20:08 ` Vinicius Costa Gomes
4 siblings, 0 replies; 11+ messages in thread
From: Luiz Augusto von Dentz @ 2016-05-06 19:11 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
test/example-gatt-client | 2 +-
test/example-gatt-server | 90 +++++++++++++++++++++++++-------
test/test-gatt-profile | 130 ++++++++++++++++++++++++++++++++++++-----------
3 files changed, 172 insertions(+), 50 deletions(-)
diff --git a/test/example-gatt-client b/test/example-gatt-client
index b77a627..4d1df2f 100755
--- a/test/example-gatt-client
+++ b/test/example-gatt-client
@@ -113,7 +113,7 @@ def hr_msrmt_changed_cb(iface, changed_props, invalidated_props):
def start_client():
# Read the Body Sensor Location value and print it asynchronously.
- body_snsr_loc_chrc[0].ReadValue(reply_handler=body_sensor_val_cb,
+ body_snsr_loc_chrc[0].ReadValue({}, reply_handler=body_sensor_val_cb,
error_handler=generic_error_cb,
dbus_interface=GATT_CHRC_IFACE)
diff --git a/test/example-gatt-server b/test/example-gatt-server
index 93cf068..71aeb1b 100755
--- a/test/example-gatt-server
+++ b/test/example-gatt-server
@@ -166,13 +166,15 @@ class Characteristic(dbus.service.Object):
return self.get_properties[GATT_CHRC_IFACE]
- @dbus.service.method(GATT_CHRC_IFACE, out_signature='ay')
- def ReadValue(self):
+ @dbus.service.method(GATT_CHRC_IFACE,
+ in_signature='a{sv}',
+ out_signature='ay')
+ def ReadValue(self, options):
print('Default ReadValue called, returning error')
raise NotSupportedException()
- @dbus.service.method(GATT_CHRC_IFACE, in_signature='ay')
- def WriteValue(self, value):
+ @dbus.service.method(GATT_CHRC_IFACE, in_signature='aya{sv}')
+ def WriteValue(self, value, options):
print('Default WriteValue called, returning error')
raise NotSupportedException()
@@ -222,13 +224,15 @@ class Descriptor(dbus.service.Object):
return self.get_properties[GATT_CHRC_IFACE]
- @dbus.service.method(GATT_DESC_IFACE, out_signature='ay')
- def ReadValue(self):
+ @dbus.service.method(GATT_DESC_IFACE,
+ in_signature='a{sv}',
+ out_signature='ay')
+ def ReadValue(self, options):
print ('Default ReadValue called, returning error')
raise NotSupportedException()
- @dbus.service.method(GATT_DESC_IFACE, in_signature='ay')
- def WriteValue(self, value):
+ @dbus.service.method(GATT_DESC_IFACE, in_signature='aya{sv}')
+ def WriteValue(self, value, options):
print('Default WriteValue called, returning error')
raise NotSupportedException()
@@ -317,7 +321,7 @@ class BodySensorLocationChrc(Characteristic):
['read'],
service)
- def ReadValue(self):
+ def ReadValue(self, options):
# Return 'Chest' as the sensor location.
return [ 0x01 ]
@@ -331,7 +335,7 @@ class HeartRateControlPointChrc(Characteristic):
['write'],
service)
- def WriteValue(self, value):
+ def WriteValue(self, value, options):
print('Heart Rate Control Point WriteValue called')
if len(value) != 1:
@@ -393,7 +397,7 @@ class BatteryLevelCharacteristic(Characteristic):
self.notify_battery_level()
return True
- def ReadValue(self):
+ def ReadValue(self, options):
print('Battery Level read: ' + repr(self.battery_lvl))
return [dbus.Byte(self.battery_lvl)]
@@ -425,6 +429,7 @@ class TestService(Service):
Service.__init__(self, bus, index, self.TEST_SVC_UUID, False)
self.add_characteristic(TestCharacteristic(bus, 0, self))
self.add_characteristic(TestEncryptCharacteristic(bus, 1, self))
+ self.add_characteristic(TestSecureCharacteristic(bus, 2, self))
class TestCharacteristic(Characteristic):
"""
@@ -445,11 +450,11 @@ class TestCharacteristic(Characteristic):
self.add_descriptor(
CharacteristicUserDescriptionDescriptor(bus, 1, self))
- def ReadValue(self):
+ def ReadValue(self, options):
print('TestCharacteristic Read: ' + repr(self.value))
return self.value
- def WriteValue(self, value):
+ def WriteValue(self, value, options):
print('TestCharacteristic Write: ' + repr(value))
self.value = value
@@ -468,7 +473,7 @@ class TestDescriptor(Descriptor):
['read', 'write'],
characteristic)
- def ReadValue(self):
+ def ReadValue(self, options):
return [
dbus.Byte('T'), dbus.Byte('e'), dbus.Byte('s'), dbus.Byte('t')
]
@@ -491,10 +496,10 @@ class CharacteristicUserDescriptionDescriptor(Descriptor):
['read', 'write'],
characteristic)
- def ReadValue(self):
+ def ReadValue(self, options):
return self.value
- def WriteValue(self, value):
+ def WriteValue(self, value, options):
if not self.writable:
raise NotPermittedException()
self.value = value
@@ -517,11 +522,11 @@ class TestEncryptCharacteristic(Characteristic):
self.add_descriptor(
CharacteristicUserDescriptionDescriptor(bus, 3, self))
- def ReadValue(self):
+ def ReadValue(self, options):
print('TestCharacteristic Read: ' + repr(self.value))
return self.value
- def WriteValue(self, value):
+ def WriteValue(self, value, options):
print('TestCharacteristic Write: ' + repr(value))
self.value = value
@@ -539,7 +544,54 @@ class TestEncryptDescriptor(Descriptor):
['encrypt-read', 'encrypt-write'],
characteristic)
- def ReadValue(self):
+ def ReadValue(self, options):
+ return [
+ dbus.Byte('T'), dbus.Byte('e'), dbus.Byte('s'), dbus.Byte('t')
+ ]
+
+
+class TestSecureCharacteristic(Characteristic):
+ """
+ Dummy test characteristic requiring secure connection.
+
+ """
+ TEST_CHRC_UUID = '12345678-1234-5678-1234-56789abcdef5'
+
+ def __init__(self, bus, index, service):
+ Characteristic.__init__(
+ self, bus, index,
+ self.TEST_CHRC_UUID,
+ ['secure-read', 'secure-write'],
+ service)
+ self.value = []
+ self.add_descriptor(TestEncryptDescriptor(bus, 2, self))
+ self.add_descriptor(
+ CharacteristicUserDescriptionDescriptor(bus, 3, self))
+
+ def ReadValue(self, options):
+ print('TestCharacteristic Read: ' + repr(self.value))
+ return self.value
+
+ def WriteValue(self, value, options):
+ print('TestCharacteristic Write: ' + repr(value))
+ self.value = value
+
+
+class TestSecureDescriptor(Descriptor):
+ """
+ Dummy test descriptor requiring secure connection. Returns a static value.
+
+ """
+ TEST_DESC_UUID = '12345678-1234-5678-1234-56789abcdef6'
+
+ def __init__(self, bus, index, characteristic):
+ Descriptor.__init__(
+ self, bus, index,
+ self.TEST_DESC_UUID,
+ ['secure-read', 'secure-write'],
+ characteristic)
+
+ def ReadValue(self, options):
return [
dbus.Byte('T'), dbus.Byte('e'), dbus.Byte('s'), dbus.Byte('t')
]
diff --git a/test/test-gatt-profile b/test/test-gatt-profile
index ad320b1..995a659 100755
--- a/test/test-gatt-profile
+++ b/test/test-gatt-profile
@@ -15,46 +15,116 @@ except ImportError:
import gobject as GObject
import bluezutils
-class GattProfile(dbus.service.Object):
- @dbus.service.method("org.bluez.GattProfile1",
- in_signature="", out_signature="")
- def Release(self):
- print("Release")
- mainloop.quit()
+BLUEZ_SERVICE_NAME = 'org.bluez'
+GATT_MANAGER_IFACE = 'org.bluez.GattManager1'
+DBUS_OM_IFACE = 'org.freedesktop.DBus.ObjectManager'
+DBUS_PROP_IFACE = 'org.freedesktop.DBus.Properties'
-if __name__ == '__main__':
- dbus.mainloop.glib.DBusGMainLoop(set_as_default=True)
+GATT_PROFILE_IFACE = 'org.bluez.GattProfile1'
+
+
+class InvalidArgsException(dbus.exceptions.DBusException):
+ _dbus_error_name = 'org.freedesktop.DBus.Error.InvalidArgs'
+
+
+class Application(dbus.service.Object):
+ def __init__(self, bus):
+ self.path = '/'
+ self.profiles = []
+ dbus.service.Object.__init__(self, bus, self.path)
+
+ def get_path(self):
+ return dbus.ObjectPath(self.path)
+
+ def add_profile(self, profile):
+ self.profiles.append(profile)
+
+ @dbus.service.method(DBUS_OM_IFACE, out_signature='a{oa{sa{sv}}}')
+ def GetManagedObjects(self):
+ response = {}
+ print('GetManagedObjects')
+
+ for profile in self.profiles:
+ response[profile.get_path()] = profile.get_properties()
+
+ return response
+
+
+class Profile(dbus.service.Object):
+ PATH_BASE = '/org/bluez/example/profile'
- bus = dbus.SystemBus()
+ def __init__(self, bus, uuids):
+ self.path = self.PATH_BASE
+ self.bus = bus
+ self.uuids = uuids
+ dbus.service.Object.__init__(self, bus, self.path)
+
+ def get_properties(self):
+ return {
+ GATT_PROFILE_IFACE: {
+ 'UUIDs': self.uuids,
+ }
+ }
+
+ def get_path(self):
+ return dbus.ObjectPath(self.path)
+
+ @dbus.service.method(GATT_PROFILE_IFACE,
+ in_signature="",
+ out_signature="")
+ def Release(self):
+ print("Release")
+ mainloop.quit()
+
+ @dbus.service.method(DBUS_PROP_IFACE,
+ in_signature='s',
+ out_signature='a{sv}')
+ def GetAll(self, interface):
+ if interface != GATT_PROFILE_IFACE:
+ raise InvalidArgsException()
+
+ return self.get_properties[GATT_PROFILE_IFACE]
+
+
+def register_app_cb():
+ print('GATT application registered')
+
+
+def register_app_error_cb(error):
+ print('Failed to register application: ' + str(error))
+ mainloop.quit()
+
+if __name__ == '__main__':
+ dbus.mainloop.glib.DBusGMainLoop(set_as_default=True)
- path = bluezutils.find_adapter().object_path
+ bus = dbus.SystemBus()
- manager = dbus.Interface(bus.get_object("org.bluez", path),
- "org.bluez.GattManager1")
+ path = bluezutils.find_adapter().object_path
- option_list = [
- make_option("-u", "--uuid", action="store",
- type="string", dest="uuid",
- default=None),
- make_option("-p", "--path", action="store",
- type="string", dest="path",
- default="/foo/bar/profile"),
- ]
+ manager = dbus.Interface(bus.get_object("org.bluez", path),
+ GATT_MANAGER_IFACE)
- opts = dbus.Dictionary({ }, signature='sv')
+ option_list = [make_option("-u", "--uuid", action="store",
+ type="string", dest="uuid",
+ default=None),
+ ]
- parser = OptionParser(option_list=option_list)
+ opts = dbus.Dictionary({}, signature='sv')
- (options, args) = parser.parse_args()
+ parser = OptionParser(option_list=option_list)
- profile = GattProfile(bus, options.path)
+ (options, args) = parser.parse_args()
- mainloop = GObject.MainLoop()
+ mainloop = GObject.MainLoop()
- if not options.uuid:
- options.uuid = str(uuid.uuid4())
+ if not options.uuid:
+ options.uuid = str(uuid.uuid4())
- uuids = { options.uuid }
- manager.RegisterProfile(options.path, uuids, opts)
+ app = Application(bus)
+ profile = Profile(bus, [options.uuid])
+ app.add_profile(profile)
+ manager.RegisterApplication(app.get_path(), {},
+ reply_handler=register_app_cb,
+ error_handler=register_app_error_cb)
- mainloop.run()
+ mainloop.run()
--
2.5.5
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH BlueZ 1/4] doc/gatt-api: Add options dictionary to ReadValue/WriteValue
2016-05-06 19:10 [PATCH BlueZ 1/4] doc/gatt-api: Add options dictionary to ReadValue/WriteValue Luiz Augusto von Dentz
` (2 preceding siblings ...)
2016-05-06 19:11 ` [PATCH BlueZ 4/4] test: Update GATT examples with the new API Luiz Augusto von Dentz
@ 2016-05-06 22:18 ` Vinicius Costa Gomes
2016-05-07 7:21 ` Luiz Augusto von Dentz
2016-05-07 20:08 ` Vinicius Costa Gomes
4 siblings, 1 reply; 11+ messages in thread
From: Vinicius Costa Gomes @ 2016-05-06 22:18 UTC (permalink / raw)
To: Luiz Augusto von Dentz, linux-bluetooth
Luiz Augusto von Dentz <luiz.dentz@gmail.com> writes:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> This adds the possibility to pass an offset to these operations, and
> also in the server case to give the device object.
> ---
> doc/gatt-api.txt | 20 ++++--
> src/gatt-client.c | 135 +++++++++++++++++++++++++++++----------
> src/gatt-database.c | 177 +++++++++++++++++++++++++++++++++++++---------------
> 3 files changed, 245 insertions(+), 87 deletions(-)
>
> diff --git a/doc/gatt-api.txt b/doc/gatt-api.txt
> index ad2ab16..683b1b7 100644
> --- a/doc/gatt-api.txt
> +++ b/doc/gatt-api.txt
> @@ -61,23 +61,29 @@ Service org.bluez
> Interface org.bluez.GattCharacteristic1 [Experimental]
> Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/serviceXX/charYYYY
>
> -Methods array{byte} ReadValue()
> +Methods array{byte} ReadValue(dict options)
>
> Issues a request to read the value of the
> characteristic and returns the value if the
> operation was successful.
>
> + Possible options: "offset": uint16 offset
> + "device": Object Device (Server only)
> +
[...]
>
> +static int parse_options(DBusMessage *msg, uint16_t *offset)
> +{
> + DBusMessageIter args, flags;
> +
> + if (!dbus_message_iter_init(msg, &args))
> + return -EINVAL;
> +
> + if (dbus_message_iter_get_arg_type(&args) != DBUS_TYPE_ARRAY)
> + return -EINVAL;
> +
> + dbus_message_iter_recurse(&args, &flags);
> + if (dbus_message_iter_get_arg_type(&flags) != DBUS_TYPE_DICT_ENTRY)
> + return -EINVAL;
> +
I would think that from the documentation, "Possible options", that providing
the offset would be optional. I don't think much is gained making this mandatory.
> + while (dbus_message_iter_get_arg_type(&flags) == DBUS_TYPE_DICT_ENTRY) {
> + const char *key;
> + DBusMessageIter value, entry;
> + int var;
> +
> + dbus_message_iter_recurse(&flags, &entry);
> + dbus_message_iter_get_basic(&entry, &key);
> +
> + dbus_message_iter_next(&entry);
> + dbus_message_iter_recurse(&entry, &value);
> +
> + var = dbus_message_iter_get_arg_type(&value);
> + if (strcasecmp(key, "offset") == 0) {
> + if (var != DBUS_TYPE_UINT16)
> + return -EINVAL;
> + dbus_message_iter_get_basic(&value, offset);
> + }
> + }
> +
> + return 0;
> +}
> +
Cheers,
--
Vinicius
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH BlueZ 1/4] doc/gatt-api: Add options dictionary to ReadValue/WriteValue
2016-05-06 22:18 ` [PATCH BlueZ 1/4] doc/gatt-api: Add options dictionary to ReadValue/WriteValue Vinicius Costa Gomes
@ 2016-05-07 7:21 ` Luiz Augusto von Dentz
2016-05-07 19:57 ` Vinicius Costa Gomes
0 siblings, 1 reply; 11+ messages in thread
From: Luiz Augusto von Dentz @ 2016-05-07 7:21 UTC (permalink / raw)
To: Vinicius Costa Gomes; +Cc: linux-bluetooth@vger.kernel.org
Hi Vinicius,
On Sat, May 7, 2016 at 1:18 AM, Vinicius Costa Gomes
<vinicius.gomes@intel.com> wrote:
> Luiz Augusto von Dentz <luiz.dentz@gmail.com> writes:
>
>> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>>
>> This adds the possibility to pass an offset to these operations, and
>> also in the server case to give the device object.
>> ---
>> doc/gatt-api.txt | 20 ++++--
>> src/gatt-client.c | 135 +++++++++++++++++++++++++++++----------
>> src/gatt-database.c | 177 +++++++++++++++++++++++++++++++++++++---------------
>> 3 files changed, 245 insertions(+), 87 deletions(-)
>>
>> diff --git a/doc/gatt-api.txt b/doc/gatt-api.txt
>> index ad2ab16..683b1b7 100644
>> --- a/doc/gatt-api.txt
>> +++ b/doc/gatt-api.txt
>> @@ -61,23 +61,29 @@ Service org.bluez
>> Interface org.bluez.GattCharacteristic1 [Experimental]
>> Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/serviceXX/charYYYY
>>
>> -Methods array{byte} ReadValue()
>> +Methods array{byte} ReadValue(dict options)
>>
>> Issues a request to read the value of the
>> characteristic and returns the value if the
>> operation was successful.
>>
>> + Possible options: "offset": uint16 offset
>> + "device": Object Device (Server only)
>> +
>
> [...]
>
>>
>> +static int parse_options(DBusMessage *msg, uint16_t *offset)
>> +{
>> + DBusMessageIter args, flags;
>> +
>> + if (!dbus_message_iter_init(msg, &args))
>> + return -EINVAL;
>> +
>> + if (dbus_message_iter_get_arg_type(&args) != DBUS_TYPE_ARRAY)
>> + return -EINVAL;
>> +
>> + dbus_message_iter_recurse(&args, &flags);
>> + if (dbus_message_iter_get_arg_type(&flags) != DBUS_TYPE_DICT_ENTRY)
>> + return -EINVAL;
>> +
>
> I would think that from the documentation, "Possible options", that providing
> the offset would be optional. I don't think much is gained making this mandatory.
I guess you are referring to empty array, so yes we should probably
drop the check for dict entry so it can be empty since it is optional.
>> + while (dbus_message_iter_get_arg_type(&flags) == DBUS_TYPE_DICT_ENTRY) {
>> + const char *key;
>> + DBusMessageIter value, entry;
>> + int var;
>> +
>> + dbus_message_iter_recurse(&flags, &entry);
>> + dbus_message_iter_get_basic(&entry, &key);
>> +
>> + dbus_message_iter_next(&entry);
>> + dbus_message_iter_recurse(&entry, &value);
>> +
>> + var = dbus_message_iter_get_arg_type(&value);
>> + if (strcasecmp(key, "offset") == 0) {
>> + if (var != DBUS_TYPE_UINT16)
>> + return -EINVAL;
>> + dbus_message_iter_get_basic(&value, offset);
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>
>
> Cheers,
> --
> Vinicius
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH BlueZ 1/4] doc/gatt-api: Add options dictionary to ReadValue/WriteValue
2016-05-07 7:21 ` Luiz Augusto von Dentz
@ 2016-05-07 19:57 ` Vinicius Costa Gomes
2016-05-08 13:54 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 11+ messages in thread
From: Vinicius Costa Gomes @ 2016-05-07 19:57 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth@vger.kernel.org
Hi Luiz,
Luiz Augusto von Dentz <luiz.dentz@gmail.com> writes:
>>> +static int parse_options(DBusMessage *msg, uint16_t *offset)
>>> +{
>>> + DBusMessageIter args, flags;
>>> +
>>> + if (!dbus_message_iter_init(msg, &args))
>>> + return -EINVAL;
>>> +
>>> + if (dbus_message_iter_get_arg_type(&args) != DBUS_TYPE_ARRAY)
>>> + return -EINVAL;
>>> +
>>> + dbus_message_iter_recurse(&args, &flags);
>>> + if (dbus_message_iter_get_arg_type(&flags) != DBUS_TYPE_DICT_ENTRY)
>>> + return -EINVAL;
>>> +
>>
>> I would think that from the documentation, "Possible options", that providing
>> the offset would be optional. I don't think much is gained making this mandatory.
>
> I guess you are referring to empty array, so yes we should probably
> drop the check for dict entry so it can be empty since it is optional.
>
I was thinking that a ReadValue() message without any arguments would
make sense to be valid. And one less thing to change in applications if
they don't need the offset.
>>> + while (dbus_message_iter_get_arg_type(&flags) == DBUS_TYPE_DICT_ENTRY) {
>>> + const char *key;
>>> + DBusMessageIter value, entry;
>>> + int var;
>>> +
>>> + dbus_message_iter_recurse(&flags, &entry);
>>> + dbus_message_iter_get_basic(&entry, &key);
>>> +
>>> + dbus_message_iter_next(&entry);
>>> + dbus_message_iter_recurse(&entry, &value);
>>> +
>>> + var = dbus_message_iter_get_arg_type(&value);
>>> + if (strcasecmp(key, "offset") == 0) {
>>> + if (var != DBUS_TYPE_UINT16)
>>> + return -EINVAL;
>>> + dbus_message_iter_get_basic(&value, offset);
>>> + }
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>
>>
>> Cheers,
>> --
>> Vinicius
>
>
>
> --
> Luiz Augusto von Dentz
Cheers,
--
Vinicius
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH BlueZ 1/4] doc/gatt-api: Add options dictionary to ReadValue/WriteValue
2016-05-07 19:57 ` Vinicius Costa Gomes
@ 2016-05-08 13:54 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 11+ messages in thread
From: Luiz Augusto von Dentz @ 2016-05-08 13:54 UTC (permalink / raw)
To: Vinicius Costa Gomes; +Cc: linux-bluetooth@vger.kernel.org
Hi Vinicius,
On Sat, May 7, 2016 at 10:57 PM, Vinicius Costa Gomes
<vinicius.gomes@intel.com> wrote:
> Hi Luiz,
>
> Luiz Augusto von Dentz <luiz.dentz@gmail.com> writes:
>
>>>> +static int parse_options(DBusMessage *msg, uint16_t *offset)
>>>> +{
>>>> + DBusMessageIter args, flags;
>>>> +
>>>> + if (!dbus_message_iter_init(msg, &args))
>>>> + return -EINVAL;
>>>> +
>>>> + if (dbus_message_iter_get_arg_type(&args) != DBUS_TYPE_ARRAY)
>>>> + return -EINVAL;
>>>> +
>>>> + dbus_message_iter_recurse(&args, &flags);
>>>> + if (dbus_message_iter_get_arg_type(&flags) != DBUS_TYPE_DICT_ENTRY)
>>>> + return -EINVAL;
>>>> +
>>>
>>> I would think that from the documentation, "Possible options", that providing
>>> the offset would be optional. I don't think much is gained making this mandatory.
>>
>> I guess you are referring to empty array, so yes we should probably
>> drop the check for dict entry so it can be empty since it is optional.
>>
>
> I was thinking that a ReadValue() message without any arguments would
> make sense to be valid. And one less thing to change in applications if
> they don't need the offset.
We can't do that because D-Bus doesn't support method overloading thus
the signature must be a{sv}, but it can be an empty container so we
would default to offset 0 as it is currently.
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH BlueZ 1/4] doc/gatt-api: Add options dictionary to ReadValue/WriteValue
2016-05-06 19:10 [PATCH BlueZ 1/4] doc/gatt-api: Add options dictionary to ReadValue/WriteValue Luiz Augusto von Dentz
` (3 preceding siblings ...)
2016-05-06 22:18 ` [PATCH BlueZ 1/4] doc/gatt-api: Add options dictionary to ReadValue/WriteValue Vinicius Costa Gomes
@ 2016-05-07 20:08 ` Vinicius Costa Gomes
2016-05-08 13:52 ` Luiz Augusto von Dentz
4 siblings, 1 reply; 11+ messages in thread
From: Vinicius Costa Gomes @ 2016-05-07 20:08 UTC (permalink / raw)
To: Luiz Augusto von Dentz, linux-bluetooth
Hi Luiz,
Luiz Augusto von Dentz <luiz.dentz@gmail.com> writes:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> This adds the possibility to pass an offset to these operations, and
> also in the server case to give the device object.
> ---
> doc/gatt-api.txt | 20 ++++--
> src/gatt-client.c | 135 +++++++++++++++++++++++++++++----------
> src/gatt-database.c | 177 +++++++++++++++++++++++++++++++++++++---------------
> 3 files changed, 245 insertions(+), 87 deletions(-)
>
> diff --git a/doc/gatt-api.txt b/doc/gatt-api.txt
> index ad2ab16..683b1b7 100644
> --- a/doc/gatt-api.txt
> +++ b/doc/gatt-api.txt
> @@ -61,23 +61,29 @@ Service org.bluez
> Interface org.bluez.GattCharacteristic1 [Experimental]
> Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/serviceXX/charYYYY
>
> -Methods array{byte} ReadValue()
> +Methods array{byte} ReadValue(dict options)
>
> Issues a request to read the value of the
> characteristic and returns the value if the
> operation was successful.
>
> + Possible options: "offset": uint16 offset
> + "device": Object Device (Server only)
> +
> Possible Errors: org.bluez.Error.Failed
> org.bluez.Error.InProgress
> org.bluez.Error.NotPermitted
> org.bluez.Error.NotAuthorized
> org.bluez.Error.NotSupported
>
> - void WriteValue(array{byte} value)
> + void WriteValue(array{byte} value, dict options)
>
> Issues a request to write the value of the
> characteristic.
>
> + Possible options: "offset": Start offset
> + "device": Device path (Server only)
> +
> Possible Errors: org.bluez.Error.Failed
> org.bluez.Error.InProgress
> org.bluez.Error.NotPermitted
> @@ -154,23 +160,29 @@ Service org.bluez
> Interface org.bluez.GattDescriptor1 [Experimental]
> Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/serviceXX/charYYYY/descriptorZZZ
>
> -Methods array{byte} ReadValue()
> +Methods array{byte} ReadValue(dict flags)
>
> Issues a request to read the value of the
> characteristic and returns the value if the
> operation was successful.
>
> + Possible options: "offset": Start offset
> + "device": Device path (Server only)
> +
> Possible Errors: org.bluez.Error.Failed
> org.bluez.Error.InProgress
> org.bluez.Error.NotPermitted
> org.bluez.Error.NotAuthorized
> org.bluez.Error.NotSupported
>
> - void WriteValue(array{byte} value)
> + void WriteValue(array{byte} value, dict flags)
>
> Issues a request to write the value of the
> characteristic.
>
> + Possible options: "offset": Start offset
> + "device": Device path (Server only)
> +
> Possible Errors: org.bluez.Error.Failed
> org.bluez.Error.InProgress
> org.bluez.Error.NotPermitted
> diff --git a/src/gatt-client.c b/src/gatt-client.c
> index 16a1f6c..c51cdc8 100644
> --- a/src/gatt-client.c
> +++ b/src/gatt-client.c
> @@ -23,6 +23,7 @@
>
> #include <stdbool.h>
> #include <stdint.h>
> +#include <errno.h>
>
> #include <dbus/dbus.h>
>
> @@ -390,12 +391,67 @@ fail:
> return;
> }
>
> +static int parse_options(DBusMessage *msg, uint16_t *offset)
> +{
> + DBusMessageIter args, flags;
> +
> + if (!dbus_message_iter_init(msg, &args))
> + return -EINVAL;
> +
> + if (dbus_message_iter_get_arg_type(&args) != DBUS_TYPE_ARRAY)
> + return -EINVAL;
> +
> + dbus_message_iter_recurse(&args, &flags);
> + if (dbus_message_iter_get_arg_type(&flags) != DBUS_TYPE_DICT_ENTRY)
> + return -EINVAL;
> +
> + while (dbus_message_iter_get_arg_type(&flags) == DBUS_TYPE_DICT_ENTRY) {
> + const char *key;
> + DBusMessageIter value, entry;
> + int var;
> +
> + dbus_message_iter_recurse(&flags, &entry);
> + dbus_message_iter_get_basic(&entry, &key);
> +
> + dbus_message_iter_next(&entry);
> + dbus_message_iter_recurse(&entry, &value);
> +
> + var = dbus_message_iter_get_arg_type(&value);
> + if (strcasecmp(key, "offset") == 0) {
> + if (var != DBUS_TYPE_UINT16)
> + return -EINVAL;
> + dbus_message_iter_get_basic(&value, offset);
> + }
> + }
> +
> + return 0;
> +}
> +
> +static unsigned int read_value(struct bt_gatt_client *gatt, uint16_t handle,
> + bt_gatt_client_read_callback_t callback,
> + struct async_dbus_op *op)
> +{
> + if (op->offset)
> + return bt_gatt_client_read_long_value(gatt, handle, op->offset,
> + callback,
> + async_dbus_op_ref(op),
> + async_dbus_op_unref);
> + else
> + return bt_gatt_client_read_value(gatt, handle, callback,
> + async_dbus_op_ref(op),
> + async_dbus_op_unref);
> +}
> +
> static DBusMessage *descriptor_read_value(DBusConnection *conn,
> DBusMessage *msg, void *user_data)
> {
> struct descriptor *desc = user_data;
> struct bt_gatt_client *gatt = desc->chrc->service->client->gatt;
> struct async_dbus_op *op;
> + uint16_t offset = 0;
> +
> + if (parse_options(msg, &offset))
> + return btd_error_invalid_args(msg);
>
> if (!gatt)
> return btd_error_failed(msg, "Not connected");
> @@ -406,17 +462,16 @@ static DBusMessage *descriptor_read_value(DBusConnection *conn,
> op = new0(struct async_dbus_op, 1);
> op->msg = dbus_message_ref(msg);
> op->data = desc;
> + op->offset = offset;
>
> - desc->read_id = bt_gatt_client_read_value(gatt, desc->handle,
> - desc_read_cb,
> - async_dbus_op_ref(op),
> - async_dbus_op_unref);
> + desc->read_id = read_value(gatt, desc->handle, desc_read_cb, op);
> if (desc->read_id)
> return NULL;
>
> async_dbus_op_free(op);
>
> return btd_error_failed(msg, "Failed to send read request");
> +
Extra empty line.
> }
>
> static void write_result_cb(bool success, bool reliable_error,
> @@ -459,7 +514,8 @@ static void write_cb(bool success, uint8_t att_ecode, void *user_data)
> static unsigned int start_long_write(DBusMessage *msg, uint16_t handle,
> struct bt_gatt_client *gatt,
> bool reliable, const uint8_t *value,
> - size_t value_len, void *data,
> + size_t value_len, uint16_t offset,
> + void *data,
> async_dbus_op_complete_t complete)
> {
> struct async_dbus_op *op;
> @@ -469,9 +525,10 @@ static unsigned int start_long_write(DBusMessage *msg, uint16_t handle,
> op->msg = dbus_message_ref(msg);
> op->data = data;
> op->complete = complete;
> + op->offset = offset;
>
> - id = bt_gatt_client_write_long_value(gatt, reliable, handle,
> - 0, value, value_len,
> + id = bt_gatt_client_write_long_value(gatt, reliable, handle, offset,
> + value, value_len,
> write_result_cb, op,
> async_dbus_op_free);
>
> @@ -524,6 +581,10 @@ static DBusMessage *descriptor_write_value(DBusConnection *conn,
> struct bt_gatt_client *gatt = desc->chrc->service->client->gatt;
> uint8_t *value = NULL;
> size_t value_len = 0;
> + uint16_t offset = 0;
> +
> + if (parse_options(msg, &offset))
> + return btd_error_invalid_args(msg);
>
> if (!gatt)
> return btd_error_failed(msg, "Not connected");
> @@ -546,15 +607,15 @@ static DBusMessage *descriptor_write_value(DBusConnection *conn,
> * Based on the value length and the MTU, either use a write or a long
> * write.
> */
> - if (value_len <= (unsigned) bt_gatt_client_get_mtu(gatt) - 3)
> + if (value_len <= (unsigned) bt_gatt_client_get_mtu(gatt) - 3 && !offset)
> desc->write_id = start_write_request(msg, desc->handle,
> gatt, value,
> value_len, desc,
> desc_write_complete);
> else
> - desc->write_id = start_long_write(msg, desc->handle,
> - gatt, false, value,
> - value_len, desc,
> + desc->write_id = start_long_write(msg, desc->handle, gatt,
> + false, value,
> + value_len, offset, desc,
> desc_write_complete);
>
> if (!desc->write_id)
> @@ -574,13 +635,15 @@ static const GDBusPropertyTable descriptor_properties[] = {
> };
>
> static const GDBusMethodTable descriptor_methods[] = {
> - { GDBUS_EXPERIMENTAL_ASYNC_METHOD("ReadValue", NULL,
> - GDBUS_ARGS({ "value", "ay" }),
> - descriptor_read_value) },
> + { GDBUS_EXPERIMENTAL_ASYNC_METHOD("ReadValue",
> + GDBUS_ARGS({ "options", "a{sv}" }),
> + GDBUS_ARGS({ "value", "ay" }),
> + descriptor_read_value) },
> { GDBUS_EXPERIMENTAL_ASYNC_METHOD("WriteValue",
> - GDBUS_ARGS({ "value", "ay" }),
> - NULL,
> - descriptor_write_value) },
> + GDBUS_ARGS({ "value", "ay" },
> + { "options", "a{sv}" }),
> + NULL,
> + descriptor_write_value) },
> { }
> };
>
> @@ -838,6 +901,10 @@ static DBusMessage *characteristic_read_value(DBusConnection *conn,
> struct characteristic *chrc = user_data;
> struct bt_gatt_client *gatt = chrc->service->client->gatt;
> struct async_dbus_op *op;
> + uint16_t offset = 0;
> +
> + if (parse_options(msg, &offset))
> + return btd_error_invalid_args(msg);
>
> if (!gatt)
> return btd_error_failed(msg, "Not connected");
> @@ -848,11 +915,9 @@ static DBusMessage *characteristic_read_value(DBusConnection *conn,
> op = new0(struct async_dbus_op, 1);
> op->msg = dbus_message_ref(msg);
> op->data = chrc;
> + op->offset = offset;
>
> - chrc->read_id = bt_gatt_client_read_value(gatt, chrc->value_handle,
> - chrc_read_cb,
> - async_dbus_op_ref(op),
> - async_dbus_op_unref);
> + chrc->read_id = read_value(gatt, chrc->value_handle, chrc_read_cb, op);
> if (chrc->read_id)
> return NULL;
>
> @@ -882,6 +947,10 @@ static DBusMessage *characteristic_write_value(DBusConnection *conn,
> uint8_t *value = NULL;
> size_t value_len = 0;
> bool supported = false;
> + uint16_t offset = 0;
> +
> + if (parse_options(msg, &offset))
> + return btd_error_invalid_args(msg);
>
> if (!gatt)
> return btd_error_failed(msg, "Not connected");
> @@ -906,7 +975,7 @@ static DBusMessage *characteristic_write_value(DBusConnection *conn,
> if ((chrc->ext_props & BT_GATT_CHRC_EXT_PROP_RELIABLE_WRITE)) {
> supported = true;
> chrc->write_id = start_long_write(msg, chrc->value_handle, gatt,
> - true, value, value_len,
> + true, value, value_len, offset,
> chrc, chrc_write_complete);
> if (chrc->write_id)
> return NULL;
> @@ -920,7 +989,7 @@ static DBusMessage *characteristic_write_value(DBusConnection *conn,
> if (!mtu)
> return btd_error_failed(msg, "No ATT transport");
>
> - if (value_len <= (unsigned) mtu - 3)
> + if (value_len <= (unsigned) mtu - 3 && !offset)
> chrc->write_id = start_write_request(msg,
> chrc->value_handle,
> gatt, value, value_len,
> @@ -928,7 +997,7 @@ static DBusMessage *characteristic_write_value(DBusConnection *conn,
> else
> chrc->write_id = start_long_write(msg,
> chrc->value_handle, gatt,
> - false, value, value_len,
> + false, value, value_len, offset,
> chrc, chrc_write_complete);
>
> if (chrc->write_id)
> @@ -1242,17 +1311,19 @@ static const GDBusPropertyTable characteristic_properties[] = {
> };
>
> static const GDBusMethodTable characteristic_methods[] = {
> - { GDBUS_EXPERIMENTAL_ASYNC_METHOD("ReadValue", NULL,
> - GDBUS_ARGS({ "value", "ay" }),
> - characteristic_read_value) },
> + { GDBUS_EXPERIMENTAL_ASYNC_METHOD("ReadValue",
> + GDBUS_ARGS({ "options", "a{sv}" }),
> + GDBUS_ARGS({ "value", "ay" }),
> + characteristic_read_value) },
> { GDBUS_EXPERIMENTAL_ASYNC_METHOD("WriteValue",
> - GDBUS_ARGS({ "value", "ay" }),
> - NULL,
> - characteristic_write_value) },
> + GDBUS_ARGS({ "value", "ay" },
> + { "options", "a{sv}" }),
> + NULL,
> + characteristic_write_value) },
> { GDBUS_EXPERIMENTAL_ASYNC_METHOD("StartNotify", NULL, NULL,
> - characteristic_start_notify) },
> + characteristic_start_notify) },
> { GDBUS_EXPERIMENTAL_METHOD("StopNotify", NULL, NULL,
> - characteristic_stop_notify) },
> + characteristic_stop_notify) },
> { }
> };
>
> diff --git a/src/gatt-database.c b/src/gatt-database.c
> index b8da955..7f36ffd 100644
> --- a/src/gatt-database.c
> +++ b/src/gatt-database.c
> @@ -135,6 +135,7 @@ struct external_desc {
> };
>
> struct pending_op {
> + struct btd_device *device;
> unsigned int id;
> struct gatt_db_attribute *attrib;
> struct queue *owner_queue;
> @@ -1592,7 +1593,8 @@ static void pending_op_free(void *data)
> free(op);
> }
>
> -static struct pending_op *pending_read_new(struct queue *owner_queue,
> +static struct pending_op *pending_read_new(struct btd_device *device,
> + struct queue *owner_queue,
> struct gatt_db_attribute *attrib,
> unsigned int id)
> {
> @@ -1601,6 +1603,7 @@ static struct pending_op *pending_read_new(struct queue *owner_queue,
> op = new0(struct pending_op, 1);
>
> op->owner_queue = owner_queue;
> + op->device = device;
Increase the device's reference counting?
> op->attrib = attrib;
> op->id = id;
> queue_push_tail(owner_queue, op);
> @@ -1608,17 +1611,44 @@ static struct pending_op *pending_read_new(struct queue *owner_queue,
> return op;
> }
>
> -static void send_read(struct gatt_db_attribute *attrib, GDBusProxy *proxy,
> - struct queue *owner_queue,
> - unsigned int id)
> +static void append_options(DBusMessageIter *iter, void *user_data)
> +{
> + struct pending_op *op = user_data;
> + const char *path = device_get_path(op->device);
> +
> + dict_append_entry(iter, "device", DBUS_TYPE_OBJECT_PATH, &path);
> +}
> +
> +static void read_setup_cb(DBusMessageIter *iter, void *user_data)
> +{
> + struct pending_op *op = user_data;
> + DBusMessageIter dict;
> +
> + 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);
> +
> + append_options(&dict, op);
> +
> + dbus_message_iter_close_container(iter, &dict);
> +}
> +
> +static void send_read(struct btd_device *device,
> + struct gatt_db_attribute *attrib,
> + GDBusProxy *proxy,
> + struct queue *owner_queue,
> + unsigned int id)
> {
> struct pending_op *op;
> uint8_t ecode = BT_ATT_ERROR_UNLIKELY;
>
> - op = pending_read_new(owner_queue, attrib, id);
> + op = pending_read_new(device, owner_queue, attrib, id);
>
> - if (g_dbus_proxy_method_call(proxy, "ReadValue", NULL, read_reply_cb,
> - op, pending_op_free) == TRUE)
> + if (g_dbus_proxy_method_call(proxy, "ReadValue", read_setup_cb,
> + read_reply_cb, op, pending_op_free) == TRUE)
> return;
>
> pending_op_free(op);
> @@ -1629,12 +1659,28 @@ static void send_read(struct gatt_db_attribute *attrib, GDBusProxy *proxy,
> static void write_setup_cb(DBusMessageIter *iter, void *user_data)
> {
> struct pending_op *op = user_data;
> - DBusMessageIter array;
> + DBusMessageIter array, dict;
>
> dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, "y", &array);
> dbus_message_iter_append_fixed_array(&array, DBUS_TYPE_BYTE,
> &op->data.iov_base, op->data.iov_len);
> dbus_message_iter_close_container(iter, &array);
> +
> + 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);
> +
> + append_options(&dict, op);
> +
> + dbus_message_iter_close_container(iter, &dict);
> +
> + if (!op->owner_queue) {
> + gatt_db_attribute_write_result(op->attrib, op->id, 0);
> + pending_op_free(op);
> + }
> }
>
> static void write_reply_cb(DBusMessage *message, void *user_data)
> @@ -1673,7 +1719,8 @@ done:
> gatt_db_attribute_write_result(op->attrib, op->id, ecode);
> }
>
> -static struct pending_op *pending_write_new(struct queue *owner_queue,
> +static struct pending_op *pending_write_new(struct btd_device *device,
> + struct queue *owner_queue,
> struct gatt_db_attribute *attrib,
> unsigned int id,
> const uint8_t *value,
> @@ -1686,6 +1733,7 @@ static struct pending_op *pending_write_new(struct queue *owner_queue,
> op->data.iov_base = (uint8_t *) value;
> op->data.iov_len = len;
>
> + op->device = device;
> op->owner_queue = owner_queue;
> op->attrib = attrib;
> op->id = id;
> @@ -1694,7 +1742,9 @@ static struct pending_op *pending_write_new(struct queue *owner_queue,
> return op;
> }
>
> -static void send_write(struct gatt_db_attribute *attrib, GDBusProxy *proxy,
> +static void send_write(struct btd_device *device,
> + struct gatt_db_attribute *attrib,
> + GDBusProxy *proxy,
> struct queue *owner_queue,
> unsigned int id,
> const uint8_t *value, size_t len)
> @@ -1702,11 +1752,11 @@ static void send_write(struct gatt_db_attribute *attrib, GDBusProxy *proxy,
> struct pending_op *op;
> uint8_t ecode = BT_ATT_ERROR_UNLIKELY;
>
> - op = pending_write_new(owner_queue, attrib, id, value, len);
> + op = pending_write_new(device, owner_queue, attrib, id, value, len);
>
> if (g_dbus_proxy_method_call(proxy, "WriteValue", write_setup_cb,
> - write_reply_cb, op,
> - pending_op_free) == TRUE)
> + owner_queue ? write_reply_cb : NULL,
> + op, pending_op_free) == TRUE)
> return;
>
> pending_op_free(op);
> @@ -1895,19 +1945,58 @@ static bool database_add_cep(struct external_service *service,
> return true;
> }
>
> +static struct btd_device *att_get_device(struct bt_att *att)
> +{
> + GIOChannel *io = NULL;
> + GError *gerr = NULL;
> + bdaddr_t src, dst;
> + uint8_t dst_type;
> + struct btd_adapter *adapter;
> +
> + io = g_io_channel_unix_new(bt_att_get_fd(att));
> + if (!io)
> + return false;
return NULL?
> +
> + bt_io_get(io, &gerr, BT_IO_OPT_SOURCE_BDADDR, &src,
> + BT_IO_OPT_DEST_BDADDR, &dst,
> + BT_IO_OPT_DEST_TYPE, &dst_type,
> + BT_IO_OPT_INVALID);
> + if (gerr) {
> + error("bt_io_get: %s", gerr->message);
> + g_error_free(gerr);
> + g_io_channel_unref(io);
> + return false;
also here.
> + }
> +
> + g_io_channel_unref(io);
> +
> + adapter = adapter_find(&src);
> + if (!adapter) {
> + error("Unable to find adapter object");
> + return false;
and here.
> + }
> +
> + return btd_adapter_find_device(adapter, &dst, dst_type);
> +}
> +
> static void desc_read_cb(struct gatt_db_attribute *attrib,
> unsigned int id, uint16_t offset,
> uint8_t opcode, struct bt_att *att,
> void *user_data)
> {
> struct external_desc *desc = user_data;
> + struct btd_device *device;
>
> if (desc->attrib != attrib) {
> error("Read callback called with incorrect attribute");
> return;
> }
>
> - send_read(attrib, desc->proxy, desc->pending_reads, id);
> + device = att_get_device(att);
> + if (!device)
> + error("Unable to find device object");
> +
> + send_read(device, attrib, desc->proxy, desc->pending_reads, id);
> }
>
> static void desc_write_cb(struct gatt_db_attribute *attrib,
> @@ -1917,13 +2006,19 @@ static void desc_write_cb(struct gatt_db_attribute *attrib,
> void *user_data)
> {
> struct external_desc *desc = user_data;
> + struct btd_device *device;
>
> if (desc->attrib != attrib) {
> error("Read callback called with incorrect attribute");
> return;
> }
>
> - send_write(attrib, desc->proxy, desc->pending_writes, id, value, len);
> + device = att_get_device(att);
> + if (!device)
> + error("Unable to find device object");
I guess you should return here. There are couple of places like this. If
this is not a mistake, I guess you could make append_options() not
append the NULL path, if there's no device associated.
> +
> + send_write(device, attrib, desc->proxy, desc->pending_writes, id,
> + value, len);
> }
>
> static bool database_add_desc(struct external_service *service,
> @@ -1956,43 +2051,18 @@ static void chrc_read_cb(struct gatt_db_attribute *attrib,
> void *user_data)
> {
> struct external_chrc *chrc = user_data;
> + struct btd_device *device;
>
> if (chrc->attrib != attrib) {
> error("Read callback called with incorrect attribute");
> return;
> }
>
> - send_read(attrib, chrc->proxy, chrc->pending_reads, id);
> -}
> -
> -static void write_without_response_setup_cb(DBusMessageIter *iter,
> - void *user_data)
> -{
> - struct iovec *iov = user_data;
> - DBusMessageIter array;
> -
> - dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, "y", &array);
> - dbus_message_iter_append_fixed_array(&array, DBUS_TYPE_BYTE,
> - &iov->iov_base, iov->iov_len);
> - dbus_message_iter_close_container(iter, &array);
> -}
> -
> -static void send_write_without_response(struct gatt_db_attribute *attrib,
> - GDBusProxy *proxy, unsigned int id,
> - const uint8_t *value, size_t len)
> -{
> - struct iovec iov;
> - uint8_t ecode = 0;
> -
> - iov.iov_base = (uint8_t *) value;
> - iov.iov_len = len;
> -
> - if (!g_dbus_proxy_method_call(proxy, "WriteValue",
> - write_without_response_setup_cb,
> - NULL, &iov, NULL))
> - ecode = BT_ATT_ERROR_UNLIKELY;
> + device = att_get_device(att);
> + if (!device)
> + error("Unable to find device object");
>
> - gatt_db_attribute_write_result(attrib, id, ecode);
> + send_read(device, attrib, chrc->proxy, chrc->pending_reads, id);
> }
>
> static void chrc_write_cb(struct gatt_db_attribute *attrib,
> @@ -2002,19 +2072,24 @@ static void chrc_write_cb(struct gatt_db_attribute *attrib,
> void *user_data)
> {
> struct external_chrc *chrc = user_data;
> + struct btd_device *device;
> + struct queue *queue;
>
> if (chrc->attrib != attrib) {
> error("Write callback called with incorrect attribute");
> return;
> }
>
> - if (chrc->props & BT_GATT_CHRC_PROP_WRITE_WITHOUT_RESP) {
> - send_write_without_response(attrib, chrc->proxy, id, value,
> - len);
> - return;
> - }
> + if (!(chrc->props & BT_GATT_CHRC_PROP_WRITE_WITHOUT_RESP))
> + queue = chrc->pending_writes;
> + else
> + queue = NULL;
> +
> + device = att_get_device(att);
> + if (!device)
> + error("Unable to find device object");
>
> - send_write(attrib, chrc->proxy, chrc->pending_writes, id, value, len);
> + send_write(device, attrib, chrc->proxy, queue, id, value, len);
> }
>
> static bool database_add_chrc(struct external_service *service,
> --
> 2.5.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Cheers,
--
Vinicius
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH BlueZ 1/4] doc/gatt-api: Add options dictionary to ReadValue/WriteValue
2016-05-07 20:08 ` Vinicius Costa Gomes
@ 2016-05-08 13:52 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 11+ messages in thread
From: Luiz Augusto von Dentz @ 2016-05-08 13:52 UTC (permalink / raw)
To: Vinicius Costa Gomes; +Cc: linux-bluetooth@vger.kernel.org
Hi Vinicius,
On Sat, May 7, 2016 at 11:08 PM, Vinicius Costa Gomes
<vinicius.gomes@intel.com> wrote:
> Hi Luiz,
>
> Luiz Augusto von Dentz <luiz.dentz@gmail.com> writes:
>
>> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>>
>> This adds the possibility to pass an offset to these operations, and
>> also in the server case to give the device object.
>> ---
>> doc/gatt-api.txt | 20 ++++--
>> src/gatt-client.c | 135 +++++++++++++++++++++++++++++----------
>> src/gatt-database.c | 177 +++++++++++++++++++++++++++++++++++++---------------
>> 3 files changed, 245 insertions(+), 87 deletions(-)
>>
>> diff --git a/doc/gatt-api.txt b/doc/gatt-api.txt
>> index ad2ab16..683b1b7 100644
>> --- a/doc/gatt-api.txt
>> +++ b/doc/gatt-api.txt
>> @@ -61,23 +61,29 @@ Service org.bluez
>> Interface org.bluez.GattCharacteristic1 [Experimental]
>> Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/serviceXX/charYYYY
>>
>> -Methods array{byte} ReadValue()
>> +Methods array{byte} ReadValue(dict options)
>>
>> Issues a request to read the value of the
>> characteristic and returns the value if the
>> operation was successful.
>>
>> + Possible options: "offset": uint16 offset
>> + "device": Object Device (Server only)
>> +
>> Possible Errors: org.bluez.Error.Failed
>> org.bluez.Error.InProgress
>> org.bluez.Error.NotPermitted
>> org.bluez.Error.NotAuthorized
>> org.bluez.Error.NotSupported
>>
>> - void WriteValue(array{byte} value)
>> + void WriteValue(array{byte} value, dict options)
>>
>> Issues a request to write the value of the
>> characteristic.
>>
>> + Possible options: "offset": Start offset
>> + "device": Device path (Server only)
>> +
>> Possible Errors: org.bluez.Error.Failed
>> org.bluez.Error.InProgress
>> org.bluez.Error.NotPermitted
>> @@ -154,23 +160,29 @@ Service org.bluez
>> Interface org.bluez.GattDescriptor1 [Experimental]
>> Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/serviceXX/charYYYY/descriptorZZZ
>>
>> -Methods array{byte} ReadValue()
>> +Methods array{byte} ReadValue(dict flags)
>>
>> Issues a request to read the value of the
>> characteristic and returns the value if the
>> operation was successful.
>>
>> + Possible options: "offset": Start offset
>> + "device": Device path (Server only)
>> +
>> Possible Errors: org.bluez.Error.Failed
>> org.bluez.Error.InProgress
>> org.bluez.Error.NotPermitted
>> org.bluez.Error.NotAuthorized
>> org.bluez.Error.NotSupported
>>
>> - void WriteValue(array{byte} value)
>> + void WriteValue(array{byte} value, dict flags)
>>
>> Issues a request to write the value of the
>> characteristic.
>>
>> + Possible options: "offset": Start offset
>> + "device": Device path (Server only)
>> +
>> Possible Errors: org.bluez.Error.Failed
>> org.bluez.Error.InProgress
>> org.bluez.Error.NotPermitted
>> diff --git a/src/gatt-client.c b/src/gatt-client.c
>> index 16a1f6c..c51cdc8 100644
>> --- a/src/gatt-client.c
>> +++ b/src/gatt-client.c
>> @@ -23,6 +23,7 @@
>>
>> #include <stdbool.h>
>> #include <stdint.h>
>> +#include <errno.h>
>>
>> #include <dbus/dbus.h>
>>
>> @@ -390,12 +391,67 @@ fail:
>> return;
>> }
>>
>> +static int parse_options(DBusMessage *msg, uint16_t *offset)
>> +{
>> + DBusMessageIter args, flags;
>> +
>> + if (!dbus_message_iter_init(msg, &args))
>> + return -EINVAL;
>> +
>> + if (dbus_message_iter_get_arg_type(&args) != DBUS_TYPE_ARRAY)
>> + return -EINVAL;
>> +
>> + dbus_message_iter_recurse(&args, &flags);
>> + if (dbus_message_iter_get_arg_type(&flags) != DBUS_TYPE_DICT_ENTRY)
>> + return -EINVAL;
>> +
>> + while (dbus_message_iter_get_arg_type(&flags) == DBUS_TYPE_DICT_ENTRY) {
>> + const char *key;
>> + DBusMessageIter value, entry;
>> + int var;
>> +
>> + dbus_message_iter_recurse(&flags, &entry);
>> + dbus_message_iter_get_basic(&entry, &key);
>> +
>> + dbus_message_iter_next(&entry);
>> + dbus_message_iter_recurse(&entry, &value);
>> +
>> + var = dbus_message_iter_get_arg_type(&value);
>> + if (strcasecmp(key, "offset") == 0) {
>> + if (var != DBUS_TYPE_UINT16)
>> + return -EINVAL;
>> + dbus_message_iter_get_basic(&value, offset);
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static unsigned int read_value(struct bt_gatt_client *gatt, uint16_t handle,
>> + bt_gatt_client_read_callback_t callback,
>> + struct async_dbus_op *op)
>> +{
>> + if (op->offset)
>> + return bt_gatt_client_read_long_value(gatt, handle, op->offset,
>> + callback,
>> + async_dbus_op_ref(op),
>> + async_dbus_op_unref);
>> + else
>> + return bt_gatt_client_read_value(gatt, handle, callback,
>> + async_dbus_op_ref(op),
>> + async_dbus_op_unref);
>> +}
>> +
>> static DBusMessage *descriptor_read_value(DBusConnection *conn,
>> DBusMessage *msg, void *user_data)
>> {
>> struct descriptor *desc = user_data;
>> struct bt_gatt_client *gatt = desc->chrc->service->client->gatt;
>> struct async_dbus_op *op;
>> + uint16_t offset = 0;
>> +
>> + if (parse_options(msg, &offset))
>> + return btd_error_invalid_args(msg);
>>
>> if (!gatt)
>> return btd_error_failed(msg, "Not connected");
>> @@ -406,17 +462,16 @@ static DBusMessage *descriptor_read_value(DBusConnection *conn,
>> op = new0(struct async_dbus_op, 1);
>> op->msg = dbus_message_ref(msg);
>> op->data = desc;
>> + op->offset = offset;
>>
>> - desc->read_id = bt_gatt_client_read_value(gatt, desc->handle,
>> - desc_read_cb,
>> - async_dbus_op_ref(op),
>> - async_dbus_op_unref);
>> + desc->read_id = read_value(gatt, desc->handle, desc_read_cb, op);
>> if (desc->read_id)
>> return NULL;
>>
>> async_dbus_op_free(op);
>>
>> return btd_error_failed(msg, "Failed to send read request");
>> +
>
> Extra empty line.
>
>> }
>>
>> static void write_result_cb(bool success, bool reliable_error,
>> @@ -459,7 +514,8 @@ static void write_cb(bool success, uint8_t att_ecode, void *user_data)
>> static unsigned int start_long_write(DBusMessage *msg, uint16_t handle,
>> struct bt_gatt_client *gatt,
>> bool reliable, const uint8_t *value,
>> - size_t value_len, void *data,
>> + size_t value_len, uint16_t offset,
>> + void *data,
>> async_dbus_op_complete_t complete)
>> {
>> struct async_dbus_op *op;
>> @@ -469,9 +525,10 @@ static unsigned int start_long_write(DBusMessage *msg, uint16_t handle,
>> op->msg = dbus_message_ref(msg);
>> op->data = data;
>> op->complete = complete;
>> + op->offset = offset;
>>
>> - id = bt_gatt_client_write_long_value(gatt, reliable, handle,
>> - 0, value, value_len,
>> + id = bt_gatt_client_write_long_value(gatt, reliable, handle, offset,
>> + value, value_len,
>> write_result_cb, op,
>> async_dbus_op_free);
>>
>> @@ -524,6 +581,10 @@ static DBusMessage *descriptor_write_value(DBusConnection *conn,
>> struct bt_gatt_client *gatt = desc->chrc->service->client->gatt;
>> uint8_t *value = NULL;
>> size_t value_len = 0;
>> + uint16_t offset = 0;
>> +
>> + if (parse_options(msg, &offset))
>> + return btd_error_invalid_args(msg);
>>
>> if (!gatt)
>> return btd_error_failed(msg, "Not connected");
>> @@ -546,15 +607,15 @@ static DBusMessage *descriptor_write_value(DBusConnection *conn,
>> * Based on the value length and the MTU, either use a write or a long
>> * write.
>> */
>> - if (value_len <= (unsigned) bt_gatt_client_get_mtu(gatt) - 3)
>> + if (value_len <= (unsigned) bt_gatt_client_get_mtu(gatt) - 3 && !offset)
>> desc->write_id = start_write_request(msg, desc->handle,
>> gatt, value,
>> value_len, desc,
>> desc_write_complete);
>> else
>> - desc->write_id = start_long_write(msg, desc->handle,
>> - gatt, false, value,
>> - value_len, desc,
>> + desc->write_id = start_long_write(msg, desc->handle, gatt,
>> + false, value,
>> + value_len, offset, desc,
>> desc_write_complete);
>>
>> if (!desc->write_id)
>> @@ -574,13 +635,15 @@ static const GDBusPropertyTable descriptor_properties[] = {
>> };
>>
>> static const GDBusMethodTable descriptor_methods[] = {
>> - { GDBUS_EXPERIMENTAL_ASYNC_METHOD("ReadValue", NULL,
>> - GDBUS_ARGS({ "value", "ay" }),
>> - descriptor_read_value) },
>> + { GDBUS_EXPERIMENTAL_ASYNC_METHOD("ReadValue",
>> + GDBUS_ARGS({ "options", "a{sv}" }),
>> + GDBUS_ARGS({ "value", "ay" }),
>> + descriptor_read_value) },
>> { GDBUS_EXPERIMENTAL_ASYNC_METHOD("WriteValue",
>> - GDBUS_ARGS({ "value", "ay" }),
>> - NULL,
>> - descriptor_write_value) },
>> + GDBUS_ARGS({ "value", "ay" },
>> + { "options", "a{sv}" }),
>> + NULL,
>> + descriptor_write_value) },
>> { }
>> };
>>
>> @@ -838,6 +901,10 @@ static DBusMessage *characteristic_read_value(DBusConnection *conn,
>> struct characteristic *chrc = user_data;
>> struct bt_gatt_client *gatt = chrc->service->client->gatt;
>> struct async_dbus_op *op;
>> + uint16_t offset = 0;
>> +
>> + if (parse_options(msg, &offset))
>> + return btd_error_invalid_args(msg);
>>
>> if (!gatt)
>> return btd_error_failed(msg, "Not connected");
>> @@ -848,11 +915,9 @@ static DBusMessage *characteristic_read_value(DBusConnection *conn,
>> op = new0(struct async_dbus_op, 1);
>> op->msg = dbus_message_ref(msg);
>> op->data = chrc;
>> + op->offset = offset;
>>
>> - chrc->read_id = bt_gatt_client_read_value(gatt, chrc->value_handle,
>> - chrc_read_cb,
>> - async_dbus_op_ref(op),
>> - async_dbus_op_unref);
>> + chrc->read_id = read_value(gatt, chrc->value_handle, chrc_read_cb, op);
>> if (chrc->read_id)
>> return NULL;
>>
>> @@ -882,6 +947,10 @@ static DBusMessage *characteristic_write_value(DBusConnection *conn,
>> uint8_t *value = NULL;
>> size_t value_len = 0;
>> bool supported = false;
>> + uint16_t offset = 0;
>> +
>> + if (parse_options(msg, &offset))
>> + return btd_error_invalid_args(msg);
>>
>> if (!gatt)
>> return btd_error_failed(msg, "Not connected");
>> @@ -906,7 +975,7 @@ static DBusMessage *characteristic_write_value(DBusConnection *conn,
>> if ((chrc->ext_props & BT_GATT_CHRC_EXT_PROP_RELIABLE_WRITE)) {
>> supported = true;
>> chrc->write_id = start_long_write(msg, chrc->value_handle, gatt,
>> - true, value, value_len,
>> + true, value, value_len, offset,
>> chrc, chrc_write_complete);
>> if (chrc->write_id)
>> return NULL;
>> @@ -920,7 +989,7 @@ static DBusMessage *characteristic_write_value(DBusConnection *conn,
>> if (!mtu)
>> return btd_error_failed(msg, "No ATT transport");
>>
>> - if (value_len <= (unsigned) mtu - 3)
>> + if (value_len <= (unsigned) mtu - 3 && !offset)
>> chrc->write_id = start_write_request(msg,
>> chrc->value_handle,
>> gatt, value, value_len,
>> @@ -928,7 +997,7 @@ static DBusMessage *characteristic_write_value(DBusConnection *conn,
>> else
>> chrc->write_id = start_long_write(msg,
>> chrc->value_handle, gatt,
>> - false, value, value_len,
>> + false, value, value_len, offset,
>> chrc, chrc_write_complete);
>>
>> if (chrc->write_id)
>> @@ -1242,17 +1311,19 @@ static const GDBusPropertyTable characteristic_properties[] = {
>> };
>>
>> static const GDBusMethodTable characteristic_methods[] = {
>> - { GDBUS_EXPERIMENTAL_ASYNC_METHOD("ReadValue", NULL,
>> - GDBUS_ARGS({ "value", "ay" }),
>> - characteristic_read_value) },
>> + { GDBUS_EXPERIMENTAL_ASYNC_METHOD("ReadValue",
>> + GDBUS_ARGS({ "options", "a{sv}" }),
>> + GDBUS_ARGS({ "value", "ay" }),
>> + characteristic_read_value) },
>> { GDBUS_EXPERIMENTAL_ASYNC_METHOD("WriteValue",
>> - GDBUS_ARGS({ "value", "ay" }),
>> - NULL,
>> - characteristic_write_value) },
>> + GDBUS_ARGS({ "value", "ay" },
>> + { "options", "a{sv}" }),
>> + NULL,
>> + characteristic_write_value) },
>> { GDBUS_EXPERIMENTAL_ASYNC_METHOD("StartNotify", NULL, NULL,
>> - characteristic_start_notify) },
>> + characteristic_start_notify) },
>> { GDBUS_EXPERIMENTAL_METHOD("StopNotify", NULL, NULL,
>> - characteristic_stop_notify) },
>> + characteristic_stop_notify) },
>> { }
>> };
>>
>> diff --git a/src/gatt-database.c b/src/gatt-database.c
>> index b8da955..7f36ffd 100644
>> --- a/src/gatt-database.c
>> +++ b/src/gatt-database.c
>> @@ -135,6 +135,7 @@ struct external_desc {
>> };
>>
>> struct pending_op {
>> + struct btd_device *device;
>> unsigned int id;
>> struct gatt_db_attribute *attrib;
>> struct queue *owner_queue;
>> @@ -1592,7 +1593,8 @@ static void pending_op_free(void *data)
>> free(op);
>> }
>>
>> -static struct pending_op *pending_read_new(struct queue *owner_queue,
>> +static struct pending_op *pending_read_new(struct btd_device *device,
>> + struct queue *owner_queue,
>> struct gatt_db_attribute *attrib,
>> unsigned int id)
>> {
>> @@ -1601,6 +1603,7 @@ static struct pending_op *pending_read_new(struct queue *owner_queue,
>> op = new0(struct pending_op, 1);
>>
>> op->owner_queue = owner_queue;
>> + op->device = device;
>
> Increase the device's reference counting?
Actually this can be a weak reference since we only use it on
append_options which is actually called synchronously so there is no
chance the device would be freed in the meantime.
>> op->attrib = attrib;
>> op->id = id;
>> queue_push_tail(owner_queue, op);
>> @@ -1608,17 +1611,44 @@ static struct pending_op *pending_read_new(struct queue *owner_queue,
>> return op;
>> }
>>
>> -static void send_read(struct gatt_db_attribute *attrib, GDBusProxy *proxy,
>> - struct queue *owner_queue,
>> - unsigned int id)
>> +static void append_options(DBusMessageIter *iter, void *user_data)
>> +{
>> + struct pending_op *op = user_data;
>> + const char *path = device_get_path(op->device);
>> +
>> + dict_append_entry(iter, "device", DBUS_TYPE_OBJECT_PATH, &path);
>> +}
>> +
>> +static void read_setup_cb(DBusMessageIter *iter, void *user_data)
>> +{
>> + struct pending_op *op = user_data;
>> + DBusMessageIter dict;
>> +
>> + 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);
>> +
>> + append_options(&dict, op);
>> +
>> + dbus_message_iter_close_container(iter, &dict);
>> +}
>> +
>> +static void send_read(struct btd_device *device,
>> + struct gatt_db_attribute *attrib,
>> + GDBusProxy *proxy,
>> + struct queue *owner_queue,
>> + unsigned int id)
>> {
>> struct pending_op *op;
>> uint8_t ecode = BT_ATT_ERROR_UNLIKELY;
>>
>> - op = pending_read_new(owner_queue, attrib, id);
>> + op = pending_read_new(device, owner_queue, attrib, id);
>>
>> - if (g_dbus_proxy_method_call(proxy, "ReadValue", NULL, read_reply_cb,
>> - op, pending_op_free) == TRUE)
>> + if (g_dbus_proxy_method_call(proxy, "ReadValue", read_setup_cb,
>> + read_reply_cb, op, pending_op_free) == TRUE)
>> return;
>>
>> pending_op_free(op);
>> @@ -1629,12 +1659,28 @@ static void send_read(struct gatt_db_attribute *attrib, GDBusProxy *proxy,
>> static void write_setup_cb(DBusMessageIter *iter, void *user_data)
>> {
>> struct pending_op *op = user_data;
>> - DBusMessageIter array;
>> + DBusMessageIter array, dict;
>>
>> dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, "y", &array);
>> dbus_message_iter_append_fixed_array(&array, DBUS_TYPE_BYTE,
>> &op->data.iov_base, op->data.iov_len);
>> dbus_message_iter_close_container(iter, &array);
>> +
>> + 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);
>> +
>> + append_options(&dict, op);
>> +
>> + dbus_message_iter_close_container(iter, &dict);
>> +
>> + if (!op->owner_queue) {
>> + gatt_db_attribute_write_result(op->attrib, op->id, 0);
>> + pending_op_free(op);
>> + }
>> }
>>
>> static void write_reply_cb(DBusMessage *message, void *user_data)
>> @@ -1673,7 +1719,8 @@ done:
>> gatt_db_attribute_write_result(op->attrib, op->id, ecode);
>> }
>>
>> -static struct pending_op *pending_write_new(struct queue *owner_queue,
>> +static struct pending_op *pending_write_new(struct btd_device *device,
>> + struct queue *owner_queue,
>> struct gatt_db_attribute *attrib,
>> unsigned int id,
>> const uint8_t *value,
>> @@ -1686,6 +1733,7 @@ static struct pending_op *pending_write_new(struct queue *owner_queue,
>> op->data.iov_base = (uint8_t *) value;
>> op->data.iov_len = len;
>>
>> + op->device = device;
>> op->owner_queue = owner_queue;
>> op->attrib = attrib;
>> op->id = id;
>> @@ -1694,7 +1742,9 @@ static struct pending_op *pending_write_new(struct queue *owner_queue,
>> return op;
>> }
>>
>> -static void send_write(struct gatt_db_attribute *attrib, GDBusProxy *proxy,
>> +static void send_write(struct btd_device *device,
>> + struct gatt_db_attribute *attrib,
>> + GDBusProxy *proxy,
>> struct queue *owner_queue,
>> unsigned int id,
>> const uint8_t *value, size_t len)
>> @@ -1702,11 +1752,11 @@ static void send_write(struct gatt_db_attribute *attrib, GDBusProxy *proxy,
>> struct pending_op *op;
>> uint8_t ecode = BT_ATT_ERROR_UNLIKELY;
>>
>> - op = pending_write_new(owner_queue, attrib, id, value, len);
>> + op = pending_write_new(device, owner_queue, attrib, id, value, len);
>>
>> if (g_dbus_proxy_method_call(proxy, "WriteValue", write_setup_cb,
>> - write_reply_cb, op,
>> - pending_op_free) == TRUE)
>> + owner_queue ? write_reply_cb : NULL,
>> + op, pending_op_free) == TRUE)
>> return;
>>
>> pending_op_free(op);
>> @@ -1895,19 +1945,58 @@ static bool database_add_cep(struct external_service *service,
>> return true;
>> }
>>
>> +static struct btd_device *att_get_device(struct bt_att *att)
>> +{
>> + GIOChannel *io = NULL;
>> + GError *gerr = NULL;
>> + bdaddr_t src, dst;
>> + uint8_t dst_type;
>> + struct btd_adapter *adapter;
>> +
>> + io = g_io_channel_unix_new(bt_att_get_fd(att));
>> + if (!io)
>> + return false;
>
> return NULL?
Yep, gonna fix these errors, thanks for the review.
>> +
>> + bt_io_get(io, &gerr, BT_IO_OPT_SOURCE_BDADDR, &src,
>> + BT_IO_OPT_DEST_BDADDR, &dst,
>> + BT_IO_OPT_DEST_TYPE, &dst_type,
>> + BT_IO_OPT_INVALID);
>> + if (gerr) {
>> + error("bt_io_get: %s", gerr->message);
>> + g_error_free(gerr);
>> + g_io_channel_unref(io);
>> + return false;
>
> also here.
>
>> + }
>> +
>> + g_io_channel_unref(io);
>> +
>> + adapter = adapter_find(&src);
>> + if (!adapter) {
>> + error("Unable to find adapter object");
>> + return false;
>
> and here.
>
>> + }
>> +
>> + return btd_adapter_find_device(adapter, &dst, dst_type);
>> +}
>> +
>> static void desc_read_cb(struct gatt_db_attribute *attrib,
>> unsigned int id, uint16_t offset,
>> uint8_t opcode, struct bt_att *att,
>> void *user_data)
>> {
>> struct external_desc *desc = user_data;
>> + struct btd_device *device;
>>
>> if (desc->attrib != attrib) {
>> error("Read callback called with incorrect attribute");
>> return;
>> }
>>
>> - send_read(attrib, desc->proxy, desc->pending_reads, id);
>> + device = att_get_device(att);
>> + if (!device)
>> + error("Unable to find device object");
>> +
>> + send_read(device, attrib, desc->proxy, desc->pending_reads, id);
>> }
>>
>> static void desc_write_cb(struct gatt_db_attribute *attrib,
>> @@ -1917,13 +2006,19 @@ static void desc_write_cb(struct gatt_db_attribute *attrib,
>> void *user_data)
>> {
>> struct external_desc *desc = user_data;
>> + struct btd_device *device;
>>
>> if (desc->attrib != attrib) {
>> error("Read callback called with incorrect attribute");
>> return;
>> }
>>
>> - send_write(attrib, desc->proxy, desc->pending_writes, id, value, len);
>> + device = att_get_device(att);
>> + if (!device)
>> + error("Unable to find device object");
>
> I guess you should return here. There are couple of places like this. If
> this is not a mistake, I guess you could make append_options() not
> append the NULL path, if there's no device associated.
The idea is to skip the device but this in fact shall never fail since
we do create the device once connected so perhaps returning here is
fine.
>
>> +
>> + send_write(device, attrib, desc->proxy, desc->pending_writes, id,
>> + value, len);
>> }
>>
>> static bool database_add_desc(struct external_service *service,
>> @@ -1956,43 +2051,18 @@ static void chrc_read_cb(struct gatt_db_attribute *attrib,
>> void *user_data)
>> {
>> struct external_chrc *chrc = user_data;
>> + struct btd_device *device;
>>
>> if (chrc->attrib != attrib) {
>> error("Read callback called with incorrect attribute");
>> return;
>> }
>>
>> - send_read(attrib, chrc->proxy, chrc->pending_reads, id);
>> -}
>> -
>> -static void write_without_response_setup_cb(DBusMessageIter *iter,
>> - void *user_data)
>> -{
>> - struct iovec *iov = user_data;
>> - DBusMessageIter array;
>> -
>> - dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, "y", &array);
>> - dbus_message_iter_append_fixed_array(&array, DBUS_TYPE_BYTE,
>> - &iov->iov_base, iov->iov_len);
>> - dbus_message_iter_close_container(iter, &array);
>> -}
>> -
>> -static void send_write_without_response(struct gatt_db_attribute *attrib,
>> - GDBusProxy *proxy, unsigned int id,
>> - const uint8_t *value, size_t len)
>> -{
>> - struct iovec iov;
>> - uint8_t ecode = 0;
>> -
>> - iov.iov_base = (uint8_t *) value;
>> - iov.iov_len = len;
>> -
>> - if (!g_dbus_proxy_method_call(proxy, "WriteValue",
>> - write_without_response_setup_cb,
>> - NULL, &iov, NULL))
>> - ecode = BT_ATT_ERROR_UNLIKELY;
>> + device = att_get_device(att);
>> + if (!device)
>> + error("Unable to find device object");
>>
>> - gatt_db_attribute_write_result(attrib, id, ecode);
>> + send_read(device, attrib, chrc->proxy, chrc->pending_reads, id);
>> }
>>
>> static void chrc_write_cb(struct gatt_db_attribute *attrib,
>> @@ -2002,19 +2072,24 @@ static void chrc_write_cb(struct gatt_db_attribute *attrib,
>> void *user_data)
>> {
>> struct external_chrc *chrc = user_data;
>> + struct btd_device *device;
>> + struct queue *queue;
>>
>> if (chrc->attrib != attrib) {
>> error("Write callback called with incorrect attribute");
>> return;
>> }
>>
>> - if (chrc->props & BT_GATT_CHRC_PROP_WRITE_WITHOUT_RESP) {
>> - send_write_without_response(attrib, chrc->proxy, id, value,
>> - len);
>> - return;
>> - }
>> + if (!(chrc->props & BT_GATT_CHRC_PROP_WRITE_WITHOUT_RESP))
>> + queue = chrc->pending_writes;
>> + else
>> + queue = NULL;
>> +
>> + device = att_get_device(att);
>> + if (!device)
>> + error("Unable to find device object");
>>
>> - send_write(attrib, chrc->proxy, chrc->pending_writes, id, value, len);
>> + send_write(device, attrib, chrc->proxy, queue, id, value, len);
>> }
>>
>> static bool database_add_chrc(struct external_service *service,
>> --
>> 2.5.5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
> Cheers,
> --
> Vinicius
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 11+ messages in thread