2015-04-14 22:06:55

by Marie Janssen

[permalink] [raw]
Subject: [PATCH BlueZ v2 0/6] Improvements to the LE Advertising API

Adds a method to add the TX Power attribute to the advertisements that are sent
through the D-Bus interface, and supports more than one advertisement based on
the maximum number reported by the kernel.

v1 -> v2:
* Rebase
* Bugfixes
* Improve error messages from MGMT api
* Fix possible issue with dbus_bool_t type
* Patch to fix object matching
* Fix instance ids

Michael Janssen (6):
core/advertising: Fix dbus object matching
doc: Add IncludeTXPower to LE Advertising API
core/advertising: Parse IncludeTXPower
core/advertising: Support more than one advertisement.
test: add IncludeTXPower to example-advertisement
core/advertising: improve errors for add advertising

doc/advertising-api.txt | 5 ++
src/advertising.c | 150 ++++++++++++++++++++++++++++++++++++++-------
test/example-advertisement | 4 ++
3 files changed, 137 insertions(+), 22 deletions(-)

--
2.2.0.rc0.207.ga3a616c



2015-04-16 15:05:43

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH BlueZ v2 0/6] Improvements to the LE Advertising API

Hi Michael,

On Wed, Apr 15, 2015 at 1:06 AM, Michael Janssen <[email protected]> wrote:
> Adds a method to add the TX Power attribute to the advertisements that are sent
> through the D-Bus interface, and supports more than one advertisement based on
> the maximum number reported by the kernel.
>
> v1 -> v2:
> * Rebase
> * Bugfixes
> * Improve error messages from MGMT api
> * Fix possible issue with dbus_bool_t type
> * Patch to fix object matching
> * Fix instance ids
>
> Michael Janssen (6):
> core/advertising: Fix dbus object matching
> doc: Add IncludeTXPower to LE Advertising API
> core/advertising: Parse IncludeTXPower
> core/advertising: Support more than one advertisement.
> test: add IncludeTXPower to example-advertisement
> core/advertising: improve errors for add advertising
>
> doc/advertising-api.txt | 5 ++
> src/advertising.c | 150 ++++++++++++++++++++++++++++++++++++++-------
> test/example-advertisement | 4 ++
> 3 files changed, 137 insertions(+), 22 deletions(-)
>
> --
> 2.2.0.rc0.207.ga3a616c

Applied, thanks.


--
Luiz Augusto von Dentz

2015-04-14 22:06:56

by Marie Janssen

[permalink] [raw]
Subject: [PATCH BlueZ v2 1/6] core/advertising: Fix dbus object matching

Objects should be matched on both their path and the sending
application.
---
src/advertising.c | 37 ++++++++++++++++++++++++++-----------
1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/src/advertising.c b/src/advertising.c
index 2f8e539..78178f2 100644
--- a/src/advertising.c
+++ b/src/advertising.c
@@ -63,12 +63,23 @@ struct advertisement {
uint8_t instance;
};

-static bool match_advertisement_path(const void *a, const void *b)
+struct dbus_obj_match {
+ const char *owner;
+ const char *path;
+};
+
+static bool match_advertisement(const void *a, const void *b)
{
const struct advertisement *ad = a;
- const char *path = b;
+ const struct dbus_obj_match *match = b;
+
+ if (match->owner && !g_strcmp0(ad->owner, match->owner))
+ return false;
+
+ if (match->path && !g_strcmp0(ad->path, match->path))
+ return false;

- return g_strcmp0(ad->path, path);
+ return true;
}

static void advertisement_free(void *data)
@@ -547,8 +558,8 @@ static DBusMessage *register_advertisement(DBusConnection *conn,
{
struct btd_advertising *manager = user_data;
DBusMessageIter args;
- const char *path;
struct advertisement *ad;
+ struct dbus_obj_match match;

DBG("RegisterAdvertisement");

@@ -558,9 +569,11 @@ static DBusMessage *register_advertisement(DBusConnection *conn,
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);
+ dbus_message_iter_get_basic(&args, &match.path);

- if (queue_find(manager->ads, match_advertisement_path, path))
+ match.owner = dbus_message_get_sender(msg);
+
+ if (queue_find(manager->ads, match_advertisement, &match))
return btd_error_already_exists(msg);

/* TODO: support more than one advertisement */
@@ -572,12 +585,12 @@ static DBusMessage *register_advertisement(DBusConnection *conn,
if (dbus_message_iter_get_arg_type(&args) != DBUS_TYPE_ARRAY)
return btd_error_invalid_args(msg);

- ad = advertisement_create(conn, msg, path);
+ ad = advertisement_create(conn, msg, match.path);
if (!ad)
return btd_error_failed(msg,
"Failed to register advertisement");

- DBG("Registered advertisement at path %s", path);
+ DBG("Registered advertisement at path %s", match.path);

ad->manager = manager;
queue_push_tail(manager->ads, ad);
@@ -591,8 +604,8 @@ static DBusMessage *unregister_advertisement(DBusConnection *conn,
{
struct btd_advertising *manager = user_data;
DBusMessageIter args;
- const char *path;
struct advertisement *ad;
+ struct dbus_obj_match match;

DBG("UnregisterAdvertisement");

@@ -602,9 +615,11 @@ static DBusMessage *unregister_advertisement(DBusConnection *conn,
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);
+ dbus_message_iter_get_basic(&args, &match.path);
+
+ match.owner = dbus_message_get_sender(msg);

- ad = queue_find(manager->ads, match_advertisement_path, path);
+ ad = queue_find(manager->ads, match_advertisement, &match);
if (!ad)
return btd_error_does_not_exist(msg);

--
2.2.0.rc0.207.ga3a616c


2015-04-14 22:07:00

by Marie Janssen

[permalink] [raw]
Subject: [PATCH BlueZ v2 5/6] test: add IncludeTXPower to example-advertisement

---
test/example-advertisement | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/test/example-advertisement b/test/example-advertisement
index 6e47391..1198f2e 100755
--- a/test/example-advertisement
+++ b/test/example-advertisement
@@ -51,6 +51,7 @@ class Advertisement(dbus.service.Object):
self.manufacturer_data = None
self.solicit_uuids = None
self.service_data = None
+ self.include_tx_power = None
dbus.service.Object.__init__(self, bus, self.path)

def get_properties(self):
@@ -68,6 +69,8 @@ class Advertisement(dbus.service.Object):
if self.service_data is not None:
properties['ServiceData'] = dbus.Dictionary(self.service_data,
signature='say')
+ if self.include_tx_power is not None:
+ properties['IncludeTXPower'] = dbus.Boolean(self.include_tx_power)
return {LE_ADVERTISEMENT_IFACE: properties}

def get_path(self):
@@ -117,6 +120,7 @@ class TestAdvertisement(Advertisement):
self.add_service_uuid('180F')
self.add_manufacturer_data(0xffff, [0x00, 0x01, 0x02, 0x03, 0x04])
self.add_service_data('9999', [0x00, 0x01, 0x02, 0x03, 0x04])
+ self.include_tx_power = True


def register_ad_cb():
--
2.2.0.rc0.207.ga3a616c


2015-04-14 22:06:58

by Marie Janssen

[permalink] [raw]
Subject: [PATCH BlueZ v2 3/6] core/advertising: Parse IncludeTXPower

Parse the IncludeTXPower property of the advertisement object, and
pass the appropriate flag to MGMT if it is set.

Uses MGMT Read Advertising Features Command to determine the maximum
length allowed.
---
src/advertising.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 86 insertions(+), 5 deletions(-)

diff --git a/src/advertising.c b/src/advertising.c
index 78178f2..8e803cb 100644
--- a/src/advertising.c
+++ b/src/advertising.c
@@ -46,6 +46,7 @@ struct btd_advertising {
struct queue *ads;
struct mgmt *mgmt;
uint16_t mgmt_index;
+ uint8_t max_adv_len;
};

#define AD_TYPE_BROADCAST 0
@@ -59,6 +60,7 @@ struct advertisement {
GDBusProxy *proxy;
DBusMessage *reg;
uint8_t type; /* Advertising type */
+ bool include_tx_power;
struct bt_ad *data;
uint8_t instance;
};
@@ -372,6 +374,25 @@ fail:
return false;
}

+static bool parse_advertising_include_tx_power(GDBusProxy *proxy,
+ bool *included)
+{
+ DBusMessageIter iter;
+ dbus_bool_t b;
+
+ if (!g_dbus_proxy_get_property(proxy, "IncludeTXPower", &iter))
+ return true;
+
+ if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_BOOLEAN)
+ return false;
+
+ dbus_message_iter_get_basic(&iter, &b);
+
+ *included = b;
+
+ return true;
+}
+
static void add_advertising_callback(uint8_t status, uint16_t length,
const void *param, void *user_data)
{
@@ -386,19 +407,47 @@ static void add_advertising_callback(uint8_t status, uint16_t length,
ad->instance = rp->instance;
}

+static size_t calc_max_adv_len(struct advertisement *ad, uint32_t flags)
+{
+ size_t max = ad->manager->max_adv_len;
+
+ /*
+ * Flags which reduce the amount of space available for advertising.
+ * See doc/mgmt-api.txt
+ */
+ if (flags & MGMT_ADV_FLAG_TX_POWER)
+ max -= 3;
+
+ if (flags & (MGMT_ADV_FLAG_DISCOV | MGMT_ADV_FLAG_LIMITED_DISCOV |
+ MGMT_ADV_FLAG_MANAGED_FLAGS))
+ max -= 3;
+
+ if (flags & MGMT_ADV_FLAG_APPEARANCE)
+ max -= 4;
+
+ return max;
+}
+
static DBusMessage *refresh_advertisement(struct advertisement *ad)
{
struct mgmt_cp_add_advertising *cp;
uint8_t param_len;
uint8_t *adv_data;
size_t adv_data_len;
+ uint32_t flags = 0;

DBG("Refreshing advertisement: %s", ad->path);

+ if (ad->type == AD_TYPE_PERIPHERAL)
+ flags = MGMT_ADV_FLAG_CONNECTABLE | MGMT_ADV_FLAG_DISCOV;
+
+ if (ad->include_tx_power)
+ flags |= MGMT_ADV_FLAG_TX_POWER;
+
adv_data = bt_ad_generate(ad->data, &adv_data_len);

- if (!adv_data) {
- error("Advertising data couldn't be generated.");
+ if (!adv_data || (adv_data_len > calc_max_adv_len(ad, flags))) {
+ error("Advertising data too long or couldn't be generated.");

return g_dbus_create_error(ad->reg, ERROR_INTERFACE
".InvalidLength",
@@ -417,9 +466,7 @@ static DBusMessage *refresh_advertisement(struct advertisement *ad)
return btd_error_failed(ad->reg, "Failed");
}

- if (ad->type == AD_TYPE_PERIPHERAL)
- cp->flags = MGMT_ADV_FLAG_CONNECTABLE | MGMT_ADV_FLAG_DISCOV;
-
+ cp->flags = flags;
cp->instance = ad->instance;
cp->adv_data_len = adv_data_len;
memcpy(cp->data, adv_data, adv_data_len);
@@ -468,6 +515,12 @@ static DBusMessage *parse_advertisement(struct advertisement *ad)
goto fail;
}

+ if (!parse_advertising_include_tx_power(ad->proxy,
+ &ad->include_tx_power)) {
+ error("Property \"IncludeTXPower\" failed to parse");
+ goto fail;
+ }
+
return refresh_advertisement(ad);

fail:
@@ -651,6 +704,26 @@ static void advertising_manager_destroy(void *user_data)
free(manager);
}

+static void read_adv_features_callback(uint8_t status, uint16_t length,
+ const void *param, void *user_data)
+{
+ struct btd_advertising *manager = user_data;
+ const struct mgmt_rp_read_adv_features *feat = param;
+
+ if (status || !param) {
+ error("Failed to read advertising features: %s (0x%02x)",
+ mgmt_errstr(status), status);
+ return;
+ }
+
+ if (length < sizeof(*feat)) {
+ error("Wrong size of read adv features response");
+ return;
+ }
+
+ manager->max_adv_len = feat->max_adv_data_len;
+}
+
static struct btd_advertising *
advertising_manager_create(struct btd_adapter *adapter)
{
@@ -672,6 +745,14 @@ advertising_manager_create(struct btd_adapter *adapter)

manager->mgmt_index = btd_adapter_get_index(adapter);

+ if (!mgmt_send(manager->mgmt, MGMT_OP_READ_ADV_FEATURES,
+ manager->mgmt_index, 0, NULL,
+ read_adv_features_callback, manager, NULL)) {
+ error("Failed to read advertising features");
+ advertising_manager_destroy(manager);
+ return NULL;
+ }
+
if (!g_dbus_register_interface(btd_get_dbus_connection(),
adapter_get_path(adapter),
LE_ADVERTISING_MGR_IFACE,
--
2.2.0.rc0.207.ga3a616c


2015-04-14 22:06:59

by Marie Janssen

[permalink] [raw]
Subject: [PATCH BlueZ v2 4/6] core/advertising: Support more than one advertisement.

Use the Read Advertising Features response to determine the maximum
number of advertisements, and limit the number of advertisements based
on this.
---
src/advertising.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/src/advertising.c b/src/advertising.c
index 8e803cb..46de9b4 100644
--- a/src/advertising.c
+++ b/src/advertising.c
@@ -47,6 +47,8 @@ struct btd_advertising {
struct mgmt *mgmt;
uint16_t mgmt_index;
uint8_t max_adv_len;
+ uint8_t max_ads;
+ unsigned int next_instance_id;
};

#define AD_TYPE_BROADCAST 0
@@ -570,8 +572,6 @@ static struct advertisement *advertisement_create(DBusConnection *conn,
if (!ad)
return NULL;

- ad->instance = 1;
-
ad->client = g_dbus_client_new_full(conn, sender, path, path);
if (!ad->client)
goto fail;
@@ -629,9 +629,8 @@ static DBusMessage *register_advertisement(DBusConnection *conn,
if (queue_find(manager->ads, match_advertisement, &match))
return btd_error_already_exists(msg);

- /* TODO: support more than one advertisement */
- if (!queue_isempty(manager->ads))
- return btd_error_failed(msg, "Already advertising");
+ if (queue_length(manager->ads) >= manager->max_ads)
+ return btd_error_failed(msg, "Maximum advertisements reached");

dbus_message_iter_next(&args);

@@ -645,7 +644,9 @@ static DBusMessage *register_advertisement(DBusConnection *conn,

DBG("Registered advertisement at path %s", match.path);

+ ad->instance = manager->next_instance_id++;
ad->manager = manager;
+
queue_push_tail(manager->ads, ad);

return NULL;
@@ -722,6 +723,7 @@ static void read_adv_features_callback(uint8_t status, uint16_t length,
}

manager->max_adv_len = feat->max_adv_data_len;
+ manager->max_ads = feat->max_instances;
}

static struct btd_advertising *
@@ -765,6 +767,8 @@ advertising_manager_create(struct btd_adapter *adapter)

manager->ads = queue_new();

+ manager->next_instance_id = 1;
+
return manager;
}

--
2.2.0.rc0.207.ga3a616c


2015-04-14 22:07:01

by Marie Janssen

[permalink] [raw]
Subject: [PATCH BlueZ v2 6/6] core/advertising: improve errors for add advertising

Use the error string and check the size of the response when we get
response from the add advertising command.
---
src/advertising.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/advertising.c b/src/advertising.c
index 46de9b4..7d69ee1 100644
--- a/src/advertising.c
+++ b/src/advertising.c
@@ -402,7 +402,13 @@ static void add_advertising_callback(uint8_t status, uint16_t length,
const struct mgmt_rp_add_advertising *rp = param;

if (status || !param) {
- error("Failed to add advertising MGMT");
+ error("Failed to add advertisement: %s (0x%02x)",
+ mgmt_errstr(status), status);
+ return;
+ }
+
+ if (length < sizeof(*rp)) {
+ error("Wrong size of add advertising response");
return;
}

--
2.2.0.rc0.207.ga3a616c


2015-04-14 22:06:57

by Marie Janssen

[permalink] [raw]
Subject: [PATCH BlueZ v2 2/6] doc: Add IncludeTXPower to LE Advertising API

---
doc/advertising-api.txt | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/doc/advertising-api.txt b/doc/advertising-api.txt
index 7fb34ee..54ab106 100644
--- a/doc/advertising-api.txt
+++ b/doc/advertising-api.txt
@@ -61,6 +61,11 @@ Properties string Type
Service Data elements to include. The keys are the
UUID to associate with the data.

+ bool IncludeTXPower
+
+ Includes the TX Power in the advertising packet.
+ If missing, the TX Power is not included.
+

LE Advertising Manager hierarchy
================================
--
2.2.0.rc0.207.ga3a616c