[Bluez PATCH v3 0/3] Expose extended adv feature support via bluez API

[Bluez PATCH v3 0/3] Expose extended adv feature support via bluez API

[Daniel Winkler]

Hi Maintainers,

I believe this series fell through the cracks. It is listed on
patchworks as "Accepted", but I have not been able to find it in the
tree. I am re-posting it here, please advise if I am mistaken.

This change adds a SupportedFeatures member to the LEAdvertisingManager
interface, which allows us to expose support for hardware offloading and
setting TX power on advertisements.

Best,
Daniel

Changes in v3:
- Rebased onto master

Changes in v2:
- Expose empty SupportedFeatures if no support available
- Doc: Expect empty SupportedFeatures if no support available

Daniel Winkler (3):
  advertising: Add SupportedFeatures to LEAdvertisingManager1
  client: Add adv SupportedFeatures to bluetoothctl
  doc/advertising-api: Add adv SupportedFeatures to doc

 client/main.c           |  1 +
 doc/advertising-api.txt | 20 +++++++++++++++++++
 lib/mgmt.h              |  2 ++
 src/advertising.c       | 43 +++++++++++++++++++++++++++++++++++++++--
 4 files changed, 64 insertions(+), 2 deletions(-)

-- 
2.30.1.766.gb4fecdf3b7-goog

[Bluez PATCH v3 1/3] advertising: Add SupportedFeatures to LEAdvertisingManager1

[Daniel Winkler]

The new SupportedFeatures member tells advertising clients whether the
platform has hardware support for advertising or capability to set tx
power of advertisements.

Additionally, fix small typo in "secondary_exists" function name.

Change is tested on hatch and kukui chromebooks by using dbus-send to
verify that SupportedFeatures always exists, and is only populated when
extended advertising is available.

Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
---

Changes in v3: None
Changes in v2:
- Expose empty SupportedFeatures if no support available

 lib/mgmt.h        |  2 ++
 src/advertising.c | 43 +++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/lib/mgmt.h b/lib/mgmt.h
index 76a03c9c2..c0021abd8 100644
--- a/lib/mgmt.h
+++ b/lib/mgmt.h
@@ -503,6 +503,8 @@ struct mgmt_rp_add_advertising {
 #define MGMT_ADV_FLAG_SEC_1M		(1 << 7)
 #define MGMT_ADV_FLAG_SEC_2M		(1 << 8)
 #define MGMT_ADV_FLAG_SEC_CODED		(1 << 9)
+#define MGMT_ADV_FLAG_CAN_SET_TX_POWER	(1 << 10)
+#define MGMT_ADV_FLAG_HW_OFFLOAD	(1 << 11)
 #define MGMT_ADV_PARAM_DURATION		(1 << 12)
 #define MGMT_ADV_PARAM_TIMEOUT		(1 << 13)
 #define MGMT_ADV_PARAM_INTERVALS	(1 << 14)
diff --git a/src/advertising.c b/src/advertising.c
index 15a343e52..dd6008cb9 100644
--- a/src/advertising.c
+++ b/src/advertising.c
@@ -1616,7 +1616,8 @@ static void append_secondary(struct btd_adv_manager *manager,
 	}
 }
 
-static gboolean secondary_exits(const GDBusPropertyTable *property, void *data)
+static gboolean secondary_exists(const GDBusPropertyTable *property,
+						void *data)
 {
 	struct btd_adv_manager *manager = data;
 
@@ -1640,6 +1641,43 @@ static gboolean get_supported_secondary(const GDBusPropertyTable *property,
 	return TRUE;
 }
 
+static struct adv_feature {
+	int flag;
+	const char *name;
+} features[] = {
+	{ MGMT_ADV_FLAG_CAN_SET_TX_POWER, "CanSetTxPower" },
+	{ MGMT_ADV_FLAG_HW_OFFLOAD, "HardwareOffload" },
+	{ },
+};
+
+static void append_features(struct btd_adv_manager *manager,
+						DBusMessageIter *iter)
+{
+	struct adv_feature *feat;
+
+	for (feat = features; feat->name; feat++) {
+		if (manager->supported_flags & feat->flag)
+			dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING,
+								&feat->name);
+	}
+}
+
+static gboolean get_supported_features(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	struct btd_adv_manager *manager = data;
+	DBusMessageIter entry;
+
+	dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY,
+					DBUS_TYPE_STRING_AS_STRING, &entry);
+
+	append_features(manager, &entry);
+
+	dbus_message_iter_close_container(iter, &entry);
+
+	return TRUE;
+}
+
 static gboolean get_supported_cap(const GDBusPropertyTable *property,
 					DBusMessageIter *iter, void *data)
 {
@@ -1678,7 +1716,8 @@ static const GDBusPropertyTable properties[] = {
 	{ "SupportedInstances", "y", get_instances, NULL, NULL },
 	{ "SupportedIncludes", "as", get_supported_includes, NULL, NULL },
 	{ "SupportedSecondaryChannels", "as", get_supported_secondary, NULL,
-							secondary_exits },
+							secondary_exists },
+	{ "SupportedFeatures", "as", get_supported_features, NULL, NULL },
 	{ "SupportedCapabilities", "a{sv}", get_supported_cap, NULL, NULL,
 					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL},
 	{ }
-- 
2.30.1.766.gb4fecdf3b7-goog

[Bluez PATCH v3 2/3] client: Add adv SupportedFeatures to bluetoothctl

[Daniel Winkler]

This change adds SupportedFeatures to be shown in "show" option of
bluetoothctl. It was tested with and without kernel support for features
to verify that they are shown or not shown correctly.

Change was tested by verifying SupportedFeatures were populated
correctly in bluetoothctl on hatch and kukui chromebooks

Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
---

Changes in v3: None
Changes in v2: None

 client/main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/client/main.c b/client/main.c
index 79658a463..1669d2c89 100644
--- a/client/main.c
+++ b/client/main.c
@@ -942,6 +942,7 @@ static void cmd_show(int argc, char *argv[])
 		print_property(adapter->ad_proxy, "SupportedIncludes");
 		print_property(adapter->ad_proxy, "SupportedSecondaryChannels");
 		print_property(adapter->ad_proxy, "SupportedCapabilities");
+		print_property(adapter->ad_proxy, "SupportedFeatures");
 	}
 
 	if (adapter->adv_monitor_proxy) {
-- 
2.30.1.766.gb4fecdf3b7-goog

[Bluez PATCH v3 3/3] doc/advertising-api: Add adv SupportedFeatures to doc

[Daniel Winkler]

Add supported features to advertising dbus api.

Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
---

Changes in v3:
- Rebased onto master

Changes in v2:
- Doc: Expect empty SupportedFeatures if no support available

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

diff --git a/doc/advertising-api.txt b/doc/advertising-api.txt
index 541c57004..ba9eacaae 100644
--- a/doc/advertising-api.txt
+++ b/doc/advertising-api.txt
@@ -235,6 +235,7 @@ Properties	byte ActiveInstances
 					 "2M"
 					 "Coded"
 
+
 		dict SupportedCapabilities [Experimental]
 
 			Enumerates Advertising-related controller capabilities
@@ -257,3 +258,22 @@ Properties	byte ActiveInstances
 				int16 MaxTxPower
 
 					Max advertising tx power (dBm)
+
+
+		array{string} SupportedFeatures [readonly, optional]
+
+			List of supported platform features. If no features
+			are available on the platform, the SupportedFeatures
+			array will be empty.
+
+			Possible values: "CanSetTxPower"
+
+						Indicates whether platform can
+						specify tx power on each
+						advertising instance.
+
+					 "HardwareOffload"
+
+						Indicates whether multiple
+						advertising will be offloaded
+						to the controller.
-- 
2.30.1.766.gb4fecdf3b7-goog

RE: Expose extended adv feature support via bluez API

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 557 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=443035

---Test result---

##############################
Test: CheckPatch - PASS

##############################
Test: CheckGitLint - PASS

##############################
Test: CheckBuild - PASS

##############################
Test: MakeCheck - PASS



---
Regards,
Linux Bluetooth

Re: [Bluez PATCH v3 3/3] doc/advertising-api: Add adv SupportedFeatures to doc

[Luiz Augusto von Dentz]

Hi Daniel,

On Fri, Mar 5, 2021 at 3:59 PM Daniel Winkler <danielwinkler@google.com> wrote:
>
> Add supported features to advertising dbus api.
>
> Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
> ---
>
> Changes in v3:
> - Rebased onto master
>
> Changes in v2:
> - Doc: Expect empty SupportedFeatures if no support available
>
>  doc/advertising-api.txt | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/doc/advertising-api.txt b/doc/advertising-api.txt
> index 541c57004..ba9eacaae 100644
> --- a/doc/advertising-api.txt
> +++ b/doc/advertising-api.txt
> @@ -235,6 +235,7 @@ Properties  byte ActiveInstances
>                                          "2M"
>                                          "Coded"
>
> +

Extra empty line.

>                 dict SupportedCapabilities [Experimental]
>
>                         Enumerates Advertising-related controller capabilities
> @@ -257,3 +258,22 @@ Properties byte ActiveInstances
>                                 int16 MaxTxPower
>
>                                         Max advertising tx power (dBm)
> +
> +
> +               array{string} SupportedFeatures [readonly, optional]

This shall probably be marked as experimental for now.

> +
> +                       List of supported platform features. If no features
> +                       are available on the platform, the SupportedFeatures
> +                       array will be empty.
> +
> +                       Possible values: "CanSetTxPower"
> +
> +                                               Indicates whether platform can
> +                                               specify tx power on each
> +                                               advertising instance.
> +
> +                                        "HardwareOffload"
> +
> +                                               Indicates whether multiple
> +                                               advertising will be offloaded
> +                                               to the controller.
> --
> 2.30.1.766.gb4fecdf3b7-goog
>


-- 
Luiz Augusto von Dentz

Re: [Bluez PATCH v3 1/3] advertising: Add SupportedFeatures to LEAdvertisingManager1

[Luiz Augusto von Dentz]

Hi Daniel,

On Fri, Mar 5, 2021 at 3:59 PM Daniel Winkler <danielwinkler@google.com> wrote:
>
> The new SupportedFeatures member tells advertising clients whether the
> platform has hardware support for advertising or capability to set tx
> power of advertisements.
>
> Additionally, fix small typo in "secondary_exists" function name.
>
> Change is tested on hatch and kukui chromebooks by using dbus-send to
> verify that SupportedFeatures always exists, and is only populated when
> extended advertising is available.
>
> Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
> ---
>
> Changes in v3: None
> Changes in v2:
> - Expose empty SupportedFeatures if no support available
>
>  lib/mgmt.h        |  2 ++
>  src/advertising.c | 43 +++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 43 insertions(+), 2 deletions(-)
>
> diff --git a/lib/mgmt.h b/lib/mgmt.h
> index 76a03c9c2..c0021abd8 100644
> --- a/lib/mgmt.h
> +++ b/lib/mgmt.h
> @@ -503,6 +503,8 @@ struct mgmt_rp_add_advertising {
>  #define MGMT_ADV_FLAG_SEC_1M           (1 << 7)
>  #define MGMT_ADV_FLAG_SEC_2M           (1 << 8)
>  #define MGMT_ADV_FLAG_SEC_CODED                (1 << 9)
> +#define MGMT_ADV_FLAG_CAN_SET_TX_POWER (1 << 10)
> +#define MGMT_ADV_FLAG_HW_OFFLOAD       (1 << 11)
>  #define MGMT_ADV_PARAM_DURATION                (1 << 12)
>  #define MGMT_ADV_PARAM_TIMEOUT         (1 << 13)
>  #define MGMT_ADV_PARAM_INTERVALS       (1 << 14)
> diff --git a/src/advertising.c b/src/advertising.c
> index 15a343e52..dd6008cb9 100644
> --- a/src/advertising.c
> +++ b/src/advertising.c
> @@ -1616,7 +1616,8 @@ static void append_secondary(struct btd_adv_manager *manager,
>         }
>  }
>
> -static gboolean secondary_exits(const GDBusPropertyTable *property, void *data)
> +static gboolean secondary_exists(const GDBusPropertyTable *property,
> +                                               void *data)
>  {
>         struct btd_adv_manager *manager = data;
>
> @@ -1640,6 +1641,43 @@ static gboolean get_supported_secondary(const GDBusPropertyTable *property,
>         return TRUE;
>  }
>
> +static struct adv_feature {
> +       int flag;
> +       const char *name;
> +} features[] = {
> +       { MGMT_ADV_FLAG_CAN_SET_TX_POWER, "CanSetTxPower" },
> +       { MGMT_ADV_FLAG_HW_OFFLOAD, "HardwareOffload" },
> +       { },
> +};
> +
> +static void append_features(struct btd_adv_manager *manager,
> +                                               DBusMessageIter *iter)
> +{
> +       struct adv_feature *feat;
> +
> +       for (feat = features; feat->name; feat++) {
> +               if (manager->supported_flags & feat->flag)
> +                       dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING,
> +                                                               &feat->name);
> +       }
> +}
> +
> +static gboolean get_supported_features(const GDBusPropertyTable *property,
> +                                       DBusMessageIter *iter, void *data)
> +{
> +       struct btd_adv_manager *manager = data;
> +       DBusMessageIter entry;
> +
> +       dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY,
> +                                       DBUS_TYPE_STRING_AS_STRING, &entry);
> +
> +       append_features(manager, &entry);
> +
> +       dbus_message_iter_close_container(iter, &entry);
> +
> +       return TRUE;
> +}
> +
>  static gboolean get_supported_cap(const GDBusPropertyTable *property,
>                                         DBusMessageIter *iter, void *data)
>  {
> @@ -1678,7 +1716,8 @@ static const GDBusPropertyTable properties[] = {
>         { "SupportedInstances", "y", get_instances, NULL, NULL },
>         { "SupportedIncludes", "as", get_supported_includes, NULL, NULL },
>         { "SupportedSecondaryChannels", "as", get_supported_secondary, NULL,
> -                                                       secondary_exits },
> +                                                       secondary_exists },
> +       { "SupportedFeatures", "as", get_supported_features, NULL, NULL },

Missing G_DBUS_PROPERTY_FLAG_EXPERIMENTAL.

>         { "SupportedCapabilities", "a{sv}", get_supported_cap, NULL, NULL,
>                                         G_DBUS_PROPERTY_FLAG_EXPERIMENTAL},
>         { }
> --
> 2.30.1.766.gb4fecdf3b7-goog
>


-- 
Luiz Augusto von Dentz