All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH BlueZ 0/4] Improvements to the LE Advertising API
@ 2015-04-10 19:06 Michael Janssen
  2015-04-10 19:06 ` [PATCH BlueZ 1/4] doc: Add IncludeTXPower to " Michael Janssen
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Michael Janssen @ 2015-04-10 19:06 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Michael Janssen

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.

Michael Janssen (4):
  doc: Add IncludeTXPower to LE Advertising API
  core/advertising: Parse IncludeTXPower
  core/advertising: Support more than one advertisement.
  test: add IncludeTXPower to example-advertisement

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

-- 
2.2.0.rc0.207.ga3a616c


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH BlueZ 1/4] doc: Add IncludeTXPower to LE Advertising API
  2015-04-10 19:06 [PATCH BlueZ 0/4] Improvements to the LE Advertising API Michael Janssen
@ 2015-04-10 19:06 ` Michael Janssen
  2015-04-10 19:06 ` [PATCH BlueZ 2/4] core/advertising: Parse IncludeTXPower Michael Janssen
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Michael Janssen @ 2015-04-10 19:06 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Michael Janssen

---
 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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH BlueZ 2/4] core/advertising: Parse IncludeTXPower
  2015-04-10 19:06 [PATCH BlueZ 0/4] Improvements to the LE Advertising API Michael Janssen
  2015-04-10 19:06 ` [PATCH BlueZ 1/4] doc: Add IncludeTXPower to " Michael Janssen
@ 2015-04-10 19:06 ` Michael Janssen
  2015-04-13 12:13   ` Szymon Janc
  2015-04-10 19:06 ` [PATCH BlueZ 3/4] core/advertising: Support more than one advertisement Michael Janssen
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Michael Janssen @ 2015-04-10 19:06 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Michael Janssen

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 | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 74 insertions(+), 5 deletions(-)

diff --git a/src/advertising.c b/src/advertising.c
index 2f8e539..8acd5b4 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;
 };
@@ -361,6 +363,22 @@ fail:
 	return false;
 }
 
+static bool parse_advertising_include_tx_power(GDBusProxy *proxy,
+							bool *included)
+{
+	DBusMessageIter iter;
+
+	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, included);
+
+	return true;
+}
+
 static void add_advertising_callback(uint8_t status, uint16_t length,
 					  const void *param, void *user_data)
 {
@@ -375,19 +393,44 @@ 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;
+
+	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",
@@ -406,9 +449,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);
@@ -457,6 +498,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:
@@ -636,6 +683,20 @@ 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");
+		return;
+	}
+
+	manager->max_adv_len = feat->max_adv_data_len;
+}
+
 static struct btd_advertising *
 advertising_manager_create(struct btd_adapter *adapter)
 {
@@ -657,6 +718,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("Cannot read advertising features, MGMT version too low");
+		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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH BlueZ 3/4] core/advertising: Support more than one advertisement.
  2015-04-10 19:06 [PATCH BlueZ 0/4] Improvements to the LE Advertising API Michael Janssen
  2015-04-10 19:06 ` [PATCH BlueZ 1/4] doc: Add IncludeTXPower to " Michael Janssen
  2015-04-10 19:06 ` [PATCH BlueZ 2/4] core/advertising: Parse IncludeTXPower Michael Janssen
@ 2015-04-10 19:06 ` Michael Janssen
  2015-04-10 19:06 ` [PATCH BlueZ 4/4] test: add IncludeTXPower to example-advertisement Michael Janssen
  2015-04-13 13:38 ` [PATCH BlueZ 0/4] Improvements to the LE Advertising API Luiz Augusto von Dentz
  4 siblings, 0 replies; 11+ messages in thread
From: Michael Janssen @ 2015-04-10 19:06 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Michael Janssen

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 | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/advertising.c b/src/advertising.c
index 8acd5b4..c3e9a72 100644
--- a/src/advertising.c
+++ b/src/advertising.c
@@ -47,6 +47,7 @@ struct btd_advertising {
 	struct mgmt *mgmt;
 	uint16_t mgmt_index;
 	uint8_t max_adv_len;
+	uint8_t max_ads;
 };
 
 #define AD_TYPE_BROADCAST 0
@@ -610,9 +611,8 @@ static DBusMessage *register_advertisement(DBusConnection *conn,
 	if (queue_find(manager->ads, match_advertisement_path, path))
 		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);
 
@@ -695,6 +695,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 *
-- 
2.2.0.rc0.207.ga3a616c


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH BlueZ 4/4] test: add IncludeTXPower to example-advertisement
  2015-04-10 19:06 [PATCH BlueZ 0/4] Improvements to the LE Advertising API Michael Janssen
                   ` (2 preceding siblings ...)
  2015-04-10 19:06 ` [PATCH BlueZ 3/4] core/advertising: Support more than one advertisement Michael Janssen
@ 2015-04-10 19:06 ` Michael Janssen
  2015-04-13 13:38 ` [PATCH BlueZ 0/4] Improvements to the LE Advertising API Luiz Augusto von Dentz
  4 siblings, 0 replies; 11+ messages in thread
From: Michael Janssen @ 2015-04-10 19:06 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Michael Janssen

---
 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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH BlueZ 2/4] core/advertising: Parse IncludeTXPower
  2015-04-10 19:06 ` [PATCH BlueZ 2/4] core/advertising: Parse IncludeTXPower Michael Janssen
@ 2015-04-13 12:13   ` Szymon Janc
  0 siblings, 0 replies; 11+ messages in thread
From: Szymon Janc @ 2015-04-13 12:13 UTC (permalink / raw)
  To: Michael Janssen; +Cc: linux-bluetooth

Hi Michael,

On Friday 10 of April 2015 12:06:06 Michael Janssen wrote:
> 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 | 79
> +++++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 74
> insertions(+), 5 deletions(-)
> 
> diff --git a/src/advertising.c b/src/advertising.c
> index 2f8e539..8acd5b4 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;
>  };
> @@ -361,6 +363,22 @@ fail:
>  	return false;
>  }
> 
> +static bool parse_advertising_include_tx_power(GDBusProxy *proxy,
> +							bool *included)
> +{
> +	DBusMessageIter iter;
> +
> +	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, included);

Due to historical reasons dbus_bool_t is 4 bytes long (check dbus-types.h)
To be on a safe side I'd do:

dbus_bool_t b;

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)
>  {
> @@ -375,19 +393,44 @@ 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)

nitpick: this should fit in single line

> +{
> +	size_t max = ad->manager->max_adv_len;
> +
> +	if (flags & MGMT_ADV_FLAG_TX_POWER)
> +		max -= 3;

At least comment on where those 3/4 bytes came from.

> +
> +	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",
> @@ -406,9 +449,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);
> @@ -457,6 +498,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:
> @@ -636,6 +683,20 @@ 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");

I'd print status error here as well.

> +		return;
> +	}

Usually we check if daemon got enough bytes in response before accessing it:

if (length < sizeof(*rp)) {
         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)
>  {
> @@ -657,6 +718,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("Cannot read advertising features, MGMT version too low");

I'm not sure if you get unsupported failure here since that needs to come from 
kernel in response. So this error message doesn't seem right.


> +		advertising_manager_destroy(manager);
> +		return NULL;
> +	}
> +
>  	if (!g_dbus_register_interface(btd_get_dbus_connection(),
>  						adapter_get_path(adapter),
>  						LE_ADVERTISING_MGR_IFACE,

-- 
BR
Szymon Janc

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH BlueZ 0/4] Improvements to the LE Advertising API
  2015-04-10 19:06 [PATCH BlueZ 0/4] Improvements to the LE Advertising API Michael Janssen
                   ` (3 preceding siblings ...)
  2015-04-10 19:06 ` [PATCH BlueZ 4/4] test: add IncludeTXPower to example-advertisement Michael Janssen
@ 2015-04-13 13:38 ` Luiz Augusto von Dentz
  2015-04-13 17:22   ` Michael Janssen
  4 siblings, 1 reply; 11+ messages in thread
From: Luiz Augusto von Dentz @ 2015-04-13 13:38 UTC (permalink / raw)
  To: Michael Janssen; +Cc: linux-bluetooth

Hi Michael,

On Fri, Apr 10, 2015 at 10:06 PM, Michael Janssen <jamuraa@chromium.org> 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.

That doesn't say why we should expose the IncludeTXPower, I thought we
would let bluetoothd figure if that should be included or not, we may
not have enough space or don't have this information from the
controller.

> Michael Janssen (4):
>   doc: Add IncludeTXPower to LE Advertising API
>   core/advertising: Parse IncludeTXPower
>   core/advertising: Support more than one advertisement.
>   test: add IncludeTXPower to example-advertisement
>
>  doc/advertising-api.txt    |  5 +++
>  src/advertising.c          | 86 +++++++++++++++++++++++++++++++++++++++++-----
>  test/example-advertisement |  4 +++
>  3 files changed, 87 insertions(+), 8 deletions(-)
>
> --
> 2.2.0.rc0.207.ga3a616c
>
> --
> 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



-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH BlueZ 0/4] Improvements to the LE Advertising API
  2015-04-13 13:38 ` [PATCH BlueZ 0/4] Improvements to the LE Advertising API Luiz Augusto von Dentz
@ 2015-04-13 17:22   ` Michael Janssen
  2015-04-13 18:16     ` Marcel Holtmann
  0 siblings, 1 reply; 11+ messages in thread
From: Michael Janssen @ 2015-04-13 17:22 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

Hi Luiz,

On Mon, Apr 13, 2015 at 6:38 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Michael,
>
> On Fri, Apr 10, 2015 at 10:06 PM, Michael Janssen <jamuraa@chromium.org> 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.
>
> That doesn't say why we should expose the IncludeTXPower, I thought we
> would let bluetoothd figure if that should be included or not, we may
> not have enough space or don't have this information from the
> controller.
>

This just exposes the flag that is available on the MGMT API. If we
don't have a reason to turn it on and off from user space, we should
remove that flag too.

I would rather get an error when I want to include TX Power and there
isn't enough space to include it instead of having to discover by
testing that it is not there when I want to use it.

>> Michael Janssen (4):
>>   doc: Add IncludeTXPower to LE Advertising API
>>   core/advertising: Parse IncludeTXPower
>>   core/advertising: Support more than one advertisement.
>>   test: add IncludeTXPower to example-advertisement
>>
>>  doc/advertising-api.txt    |  5 +++
>>  src/advertising.c          | 86 +++++++++++++++++++++++++++++++++++++++++-----
>>  test/example-advertisement |  4 +++
>>  3 files changed, 87 insertions(+), 8 deletions(-)
>>
>> --
>> 2.2.0.rc0.207.ga3a616c
>>
>> --
>> 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
>
>
>
> --
> Luiz Augusto von Dentz

-- 
M Janssen

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH BlueZ 0/4] Improvements to the LE Advertising API
  2015-04-13 17:22   ` Michael Janssen
@ 2015-04-13 18:16     ` Marcel Holtmann
  2015-04-13 18:27       ` Michael Janssen
  0 siblings, 1 reply; 11+ messages in thread
From: Marcel Holtmann @ 2015-04-13 18:16 UTC (permalink / raw)
  To: Michael Janssen; +Cc: Luiz Augusto von Dentz, linux-bluetooth

Hi Michael,

>>> 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.
>> 
>> That doesn't say why we should expose the IncludeTXPower, I thought we
>> would let bluetoothd figure if that should be included or not, we may
>> not have enough space or don't have this information from the
>> controller.
>> 
> 
> This just exposes the flag that is available on the MGMT API. If we
> don't have a reason to turn it on and off from user space, we should
> remove that flag too.

the kernel side needs this flag. You can not emulate it from userspace. The reason is that userspace has no idea what the actual TX power is. Only the kernel knows. If you build the value in userspace by yourself, then you are plain faking / hardcoding it.

Regards

Marcel


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH BlueZ 0/4] Improvements to the LE Advertising API
  2015-04-13 18:16     ` Marcel Holtmann
@ 2015-04-13 18:27       ` Michael Janssen
  2015-04-14 18:18         ` Arman Uguray
  0 siblings, 1 reply; 11+ messages in thread
From: Michael Janssen @ 2015-04-13 18:27 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: Luiz Augusto von Dentz, linux-bluetooth

Hi Marcel:

On Mon, Apr 13, 2015 at 11:16 AM, Marcel Holtmann <marcel@holtmann.org> wro=
te:
> Hi Michael,
>
>>>> 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.
>>>
>>> That doesn't say why we should expose the IncludeTXPower, I thought we
>>> would let bluetoothd figure if that should be included or not, we may
>>> not have enough space or don't have this information from the
>>> controller.
>>>
>>
>> This just exposes the flag that is available on the MGMT API. If we
>> don't have a reason to turn it on and off from user space, we should
>> remove that flag too.
>
> the kernel side needs this flag. You can not emulate it from userspace. T=
he reason is that userspace has no idea what the actual TX power is. Only t=
he kernel knows. If you build the value in userspace by yourself, then you =
are plain faking / hardcoding it.

We are not emulating it from userspace.  This property signals to the
kernel to include the power.  It mirrors the MGMT API flag on the Add
Advertising Command:

4 Add TX Power field to Adv_Data

We do not build the value in userspace, but just set this flag when
the property is set.

>
> Regards
>
> Marcel
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH BlueZ 0/4] Improvements to the LE Advertising API
  2015-04-13 18:27       ` Michael Janssen
@ 2015-04-14 18:18         ` Arman Uguray
  0 siblings, 0 replies; 11+ messages in thread
From: Arman Uguray @ 2015-04-14 18:18 UTC (permalink / raw)
  To: Michael Janssen; +Cc: Marcel Holtmann, Luiz Augusto von Dentz, linux-bluetooth

Hi all,

> On Mon, Apr 13, 2015 at 11:27 AM, Michael Janssen <jamuraa@chromium.org> =
wrote:
> Hi Marcel:
>
> On Mon, Apr 13, 2015 at 11:16 AM, Marcel Holtmann <marcel@holtmann.org> w=
rote:
>> Hi Michael,
>>
>>>>> Adds a method to add the TX Power attribute to the advertisements tha=
t are sent
>>>>> through the D-Bus interface, and supports more than one advertisement=
 based on
>>>>> the maximum number reported by the kernel.
>>>>
>>>> That doesn't say why we should expose the IncludeTXPower, I thought we
>>>> would let bluetoothd figure if that should be included or not, we may
>>>> not have enough space or don't have this information from the
>>>> controller.
>>>>
>>>
>>> This just exposes the flag that is available on the MGMT API. If we
>>> don't have a reason to turn it on and off from user space, we should
>>> remove that flag too.
>>
>> the kernel side needs this flag. You can not emulate it from userspace. =
The reason is that userspace has no idea what the actual TX power is. Only =
the kernel knows. If you build the value in userspace by yourself, then you=
 are plain faking / hardcoding it.
>
> We are not emulating it from userspace.  This property signals to the
> kernel to include the power.  It mirrors the MGMT API flag on the Add
> Advertising Command:
>
> 4 Add TX Power field to Adv_Data
>
> We do not build the value in userspace, but just set this flag when
> the property is set.
>

Michael is simply using this property to determine whether or not the
MGMT_ADV_FLAG_TX_POWER should passed to the mgmt command. I think such
an API makes sense, but did you have something different in mind like
always setting this flag from D-Bus?

>>
>> Regards
>>
>> Marcel
>>
> --
> 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

Somewhat unrelated to the discussion but I think there is a bug in how
register_advertisement determines to return "AlreadyExists" as an
error. It looks like you're simply comparing object paths but you
should really be comparing the object path AND the sender. Basically
object paths are unique to each external application so if two of
these want to use an identical object path that should be OK.

Thanks,
Arman

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2015-04-14 18:18 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-10 19:06 [PATCH BlueZ 0/4] Improvements to the LE Advertising API Michael Janssen
2015-04-10 19:06 ` [PATCH BlueZ 1/4] doc: Add IncludeTXPower to " Michael Janssen
2015-04-10 19:06 ` [PATCH BlueZ 2/4] core/advertising: Parse IncludeTXPower Michael Janssen
2015-04-13 12:13   ` Szymon Janc
2015-04-10 19:06 ` [PATCH BlueZ 3/4] core/advertising: Support more than one advertisement Michael Janssen
2015-04-10 19:06 ` [PATCH BlueZ 4/4] test: add IncludeTXPower to example-advertisement Michael Janssen
2015-04-13 13:38 ` [PATCH BlueZ 0/4] Improvements to the LE Advertising API Luiz Augusto von Dentz
2015-04-13 17:22   ` Michael Janssen
2015-04-13 18:16     ` Marcel Holtmann
2015-04-13 18:27       ` Michael Janssen
2015-04-14 18:18         ` Arman Uguray

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.