2021-03-05 23:56:39

by Daniel Winkler

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

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


2021-03-05 23:59:03

by Daniel Winkler

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

Add supported features to advertising dbus api.

Reviewed-by: Miao-chen Chou <[email protected]>
---

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

2021-03-05 23:59:03

by Daniel Winkler

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

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 <[email protected]>
---

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

2021-03-06 01:17:17

by bluez.test.bot

[permalink] [raw]
Subject: RE: Expose extended adv feature support via bluez API

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

2021-03-08 18:39:04

by Luiz Augusto von Dentz

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

Hi Daniel,

On Fri, Mar 5, 2021 at 3:59 PM Daniel Winkler <[email protected]> wrote:
>
> Add supported features to advertising dbus api.
>
> Reviewed-by: Miao-chen Chou <[email protected]>
> ---
>
> 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

2021-03-08 18:41:12

by Luiz Augusto von Dentz

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

Hi Daniel,

On Fri, Mar 5, 2021 at 3:59 PM Daniel Winkler <[email protected]> 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 <[email protected]>
> ---
>
> 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