[Bluez PATCH v2 0/2] Adding bonded flag to D-Bus property

[Bluez PATCH v2 0/2] Adding bonded flag to D-Bus property

[Zhengping Jiang]

Currently BlueZ client can't know easily whether a device is bonded or
not. This is causing issues for a number of applications. For example,
in the Nearby Share case, the peer device is paired, but not bonded.
This series will add the "Bonded" property in org.bluez.Device1 D-Bus
interface. Changes are also made in bluetoothctl to show the status of
the bonded flag as well as a list of bonded devices.

Changes in v2:
- Move one variable declaration to the top following C90 standard

Changes in v1:
- Add "Bonded" to D-Bus interface
- Send property changed signal if the bonded flag is changed
- Show the status of the "Bonded" flag in bluetoothctl
- Add option to show list of bonded devices

Zhengping Jiang (2):
  device: Add "Bonded" flag to dbus property
  client: Add bonded-devices and show Bonded flag in info

 client/main.c      | 29 +++++++++++++++++++++++++++++
 doc/device-api.txt |  4 ++++
 src/device.c       | 40 +++++++++++++++++++++++++++++++++++-----
 3 files changed, 68 insertions(+), 5 deletions(-)

-- 
2.36.0.rc0.470.gd361397f0d-goog

[Bluez PATCH v2 1/2] device: Add "Bonded" flag to dbus property

[Zhengping Jiang]

Add "Bonded" to dbus device property table. When setting the "Bonded
flag, check the status of the Bonded property first. If the Bonded
property is changed, send property changed signal.

Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org>
Reviewed-by: Yun-Hao Chung <howardchung@chromium.org>

Signed-off-by: Zhengping Jiang <jiangzp@google.com>
---

Changes in v2:
- Move one variable declaration to the top following C90 standard

Changes in v1:
- Add "Bonded" to D-Bus interface
- Send property changed signal if the bonded flag is changed

 doc/device-api.txt |  4 ++++
 src/device.c       | 40 +++++++++++++++++++++++++++++++++++-----
 2 files changed, 39 insertions(+), 5 deletions(-)

diff --git a/doc/device-api.txt b/doc/device-api.txt
index 4e824d2dec17..6162755f954c 100644
--- a/doc/device-api.txt
+++ b/doc/device-api.txt
@@ -171,6 +171,10 @@ Properties	string Address [readonly]
 
 			Indicates if the remote device is paired.
 
+		boolean Bonded [readonly]
+
+			Indicates if the remote device is bonded.
+
 		boolean Connected [readonly]
 
 			Indicates if the remote device is currently connected.
diff --git a/src/device.c b/src/device.c
index 8dc12d026827..868c41f025d9 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1042,6 +1042,22 @@ static gboolean dev_property_get_paired(const GDBusPropertyTable *property,
 	return TRUE;
 }
 
+static gboolean dev_property_get_bonded(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	struct btd_device *dev = data;
+	dbus_bool_t val;
+
+	if (dev->bredr_state.bonded || dev->le_state.bonded)
+		val = TRUE;
+	else
+		val = FALSE;
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_BOOLEAN, &val);
+
+	return TRUE;
+}
+
 static gboolean dev_property_get_legacy(const GDBusPropertyTable *property,
 					DBusMessageIter *iter, void *data)
 {
@@ -3120,6 +3136,7 @@ static const GDBusPropertyTable device_properties[] = {
 	{ "Icon", "s", dev_property_get_icon, NULL,
 					dev_property_exists_icon },
 	{ "Paired", "b", dev_property_get_paired },
+	{ "Bonded", "b", dev_property_get_bonded },
 	{ "Trusted", "b", dev_property_get_trusted, dev_property_set_trusted },
 	{ "Blocked", "b", dev_property_get_blocked, dev_property_set_blocked },
 	{ "LegacyPairing", "b", dev_property_get_legacy },
@@ -6114,17 +6131,30 @@ void btd_device_set_trusted(struct btd_device *device, gboolean trusted)
 
 void device_set_bonded(struct btd_device *device, uint8_t bdaddr_type)
 {
+	struct bearer_state *state;
+
 	if (!device)
 		return;
 
-	DBG("");
+	state = get_state(device, bdaddr_type);
 
-	if (bdaddr_type == BDADDR_BREDR)
-		device->bredr_state.bonded = true;
-	else
-		device->le_state.bonded = true;
+	if (state->bonded)
+		return;
+
+	DBG("setting bonded for device to true");
+
+	state->bonded = true;
 
 	btd_device_set_temporary(device, false);
+
+	/* If the other bearer state was already true we don't need to
+	 * send any property signals.
+	 */
+	if (device->bredr_state.bonded == device->le_state.bonded)
+		return;
+
+	g_dbus_emit_property_changed(dbus_conn, device->path,
+						DEVICE_INTERFACE, "Bonded");
 }
 
 void device_set_legacy(struct btd_device *device, bool legacy)
-- 
2.36.0.rc0.470.gd361397f0d-goog

RE: Adding bonded flag to D-Bus property

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 999 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=633121

---Test result---

Test Summary:
CheckPatch                    PASS      1.43 seconds
GitLint                       PASS      0.89 seconds
Prep - Setup ELL              PASS      49.01 seconds
Build - Prep                  PASS      0.61 seconds
Build - Configure             PASS      9.71 seconds
Build - Make                  PASS      1700.73 seconds
Make Check                    PASS      12.49 seconds
Make Check w/Valgrind         PASS      515.95 seconds
Make Distcheck                PASS      271.29 seconds
Build w/ext ELL - Configure   PASS      10.12 seconds
Build w/ext ELL - Make        PASS      1667.33 seconds
Incremental Build with patchesPASS      3404.97 seconds



---
Regards,
Linux Bluetooth

Re: [Bluez PATCH v2 1/2] device: Add "Bonded" flag to dbus property

[Luiz Augusto von Dentz]

Hi Zhengping,

On Mon, Apr 18, 2022 at 10:49 AM Zhengping Jiang <jiangzp@google.com> wrote:
>
> Add "Bonded" to dbus device property table. When setting the "Bonded
> flag, check the status of the Bonded property first. If the Bonded
> property is changed, send property changed signal.
>
> Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org>
> Reviewed-by: Yun-Hao Chung <howardchung@chromium.org>
>
> Signed-off-by: Zhengping Jiang <jiangzp@google.com>
> ---
>
> Changes in v2:
> - Move one variable declaration to the top following C90 standard
>
> Changes in v1:
> - Add "Bonded" to D-Bus interface
> - Send property changed signal if the bonded flag is changed
>
>  doc/device-api.txt |  4 ++++
>  src/device.c       | 40 +++++++++++++++++++++++++++++++++++-----
>  2 files changed, 39 insertions(+), 5 deletions(-)
>
> diff --git a/doc/device-api.txt b/doc/device-api.txt
> index 4e824d2dec17..6162755f954c 100644
> --- a/doc/device-api.txt
> +++ b/doc/device-api.txt
> @@ -171,6 +171,10 @@ Properties string Address [readonly]
>
>                         Indicates if the remote device is paired.
>
> +               boolean Bonded [readonly]
> +
> +                       Indicates if the remote device is bonded.

It is probably a good idea to add a description about Bonded vs
Paired. Btw, API documentation should be in a separate patch.

> +
>                 boolean Connected [readonly]
>
>                         Indicates if the remote device is currently connected.
> diff --git a/src/device.c b/src/device.c
> index 8dc12d026827..868c41f025d9 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -1042,6 +1042,22 @@ static gboolean dev_property_get_paired(const GDBusPropertyTable *property,
>         return TRUE;
>  }
>
> +static gboolean dev_property_get_bonded(const GDBusPropertyTable *property,
> +                                       DBusMessageIter *iter, void *data)
> +{
> +       struct btd_device *dev = data;
> +       dbus_bool_t val;
> +
> +       if (dev->bredr_state.bonded || dev->le_state.bonded)
> +               val = TRUE;
> +       else
> +               val = FALSE;
> +
> +       dbus_message_iter_append_basic(iter, DBUS_TYPE_BOOLEAN, &val);
> +
> +       return TRUE;
> +}
> +
>  static gboolean dev_property_get_legacy(const GDBusPropertyTable *property,
>                                         DBusMessageIter *iter, void *data)
>  {
> @@ -3120,6 +3136,7 @@ static const GDBusPropertyTable device_properties[] = {
>         { "Icon", "s", dev_property_get_icon, NULL,
>                                         dev_property_exists_icon },
>         { "Paired", "b", dev_property_get_paired },
> +       { "Bonded", "b", dev_property_get_bonded },
>         { "Trusted", "b", dev_property_get_trusted, dev_property_set_trusted },
>         { "Blocked", "b", dev_property_get_blocked, dev_property_set_blocked },
>         { "LegacyPairing", "b", dev_property_get_legacy },
> @@ -6114,17 +6131,30 @@ void btd_device_set_trusted(struct btd_device *device, gboolean trusted)
>
>  void device_set_bonded(struct btd_device *device, uint8_t bdaddr_type)
>  {
> +       struct bearer_state *state;
> +
>         if (!device)
>                 return;
>
> -       DBG("");
> +       state = get_state(device, bdaddr_type);
>
> -       if (bdaddr_type == BDADDR_BREDR)
> -               device->bredr_state.bonded = true;
> -       else
> -               device->le_state.bonded = true;
> +       if (state->bonded)
> +               return;
> +
> +       DBG("setting bonded for device to true");
> +
> +       state->bonded = true;
>
>         btd_device_set_temporary(device, false);
> +
> +       /* If the other bearer state was already true we don't need to
> +        * send any property signals.
> +        */
> +       if (device->bredr_state.bonded == device->le_state.bonded)
> +               return;
> +
> +       g_dbus_emit_property_changed(dbus_conn, device->path,
> +                                               DEVICE_INTERFACE, "Bonded");
>  }
>
>  void device_set_legacy(struct btd_device *device, bool legacy)
> --
> 2.36.0.rc0.470.gd361397f0d-goog
>


-- 
Luiz Augusto von Dentz

Re: [Bluez PATCH v2 1/2] device: Add "Bonded" flag to dbus property

[Luiz Augusto von Dentz]

Hi Zhengping,

On Mon, Apr 18, 2022 at 3:41 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Zhengping,
>
> On Mon, Apr 18, 2022 at 10:49 AM Zhengping Jiang <jiangzp@google.com> wrote:
> >
> > Add "Bonded" to dbus device property table. When setting the "Bonded
> > flag, check the status of the Bonded property first. If the Bonded
> > property is changed, send property changed signal.
> >
> > Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org>
> > Reviewed-by: Yun-Hao Chung <howardchung@chromium.org>
> >
> > Signed-off-by: Zhengping Jiang <jiangzp@google.com>
> > ---
> >
> > Changes in v2:
> > - Move one variable declaration to the top following C90 standard
> >
> > Changes in v1:
> > - Add "Bonded" to D-Bus interface
> > - Send property changed signal if the bonded flag is changed
> >
> >  doc/device-api.txt |  4 ++++
> >  src/device.c       | 40 +++++++++++++++++++++++++++++++++++-----
> >  2 files changed, 39 insertions(+), 5 deletions(-)
> >
> > diff --git a/doc/device-api.txt b/doc/device-api.txt
> > index 4e824d2dec17..6162755f954c 100644
> > --- a/doc/device-api.txt
> > +++ b/doc/device-api.txt
> > @@ -171,6 +171,10 @@ Properties string Address [readonly]
> >
> >                         Indicates if the remote device is paired.
> >
> > +               boolean Bonded [readonly]
> > +
> > +                       Indicates if the remote device is bonded.
>
> It is probably a good idea to add a description about Bonded vs
> Paired. Btw, API documentation should be in a separate patch.

Will you be updating following this comments or you are waiting more
feedback from upstream?

> > +
> >                 boolean Connected [readonly]
> >
> >                         Indicates if the remote device is currently connected.
> > diff --git a/src/device.c b/src/device.c
> > index 8dc12d026827..868c41f025d9 100644
> > --- a/src/device.c
> > +++ b/src/device.c
> > @@ -1042,6 +1042,22 @@ static gboolean dev_property_get_paired(const GDBusPropertyTable *property,
> >         return TRUE;
> >  }
> >
> > +static gboolean dev_property_get_bonded(const GDBusPropertyTable *property,
> > +                                       DBusMessageIter *iter, void *data)
> > +{
> > +       struct btd_device *dev = data;
> > +       dbus_bool_t val;
> > +
> > +       if (dev->bredr_state.bonded || dev->le_state.bonded)
> > +               val = TRUE;
> > +       else
> > +               val = FALSE;
> > +
> > +       dbus_message_iter_append_basic(iter, DBUS_TYPE_BOOLEAN, &val);
> > +
> > +       return TRUE;
> > +}
> > +
> >  static gboolean dev_property_get_legacy(const GDBusPropertyTable *property,
> >                                         DBusMessageIter *iter, void *data)
> >  {
> > @@ -3120,6 +3136,7 @@ static const GDBusPropertyTable device_properties[] = {
> >         { "Icon", "s", dev_property_get_icon, NULL,
> >                                         dev_property_exists_icon },
> >         { "Paired", "b", dev_property_get_paired },
> > +       { "Bonded", "b", dev_property_get_bonded },
> >         { "Trusted", "b", dev_property_get_trusted, dev_property_set_trusted },
> >         { "Blocked", "b", dev_property_get_blocked, dev_property_set_blocked },
> >         { "LegacyPairing", "b", dev_property_get_legacy },
> > @@ -6114,17 +6131,30 @@ void btd_device_set_trusted(struct btd_device *device, gboolean trusted)
> >
> >  void device_set_bonded(struct btd_device *device, uint8_t bdaddr_type)
> >  {
> > +       struct bearer_state *state;
> > +
> >         if (!device)
> >                 return;
> >
> > -       DBG("");
> > +       state = get_state(device, bdaddr_type);
> >
> > -       if (bdaddr_type == BDADDR_BREDR)
> > -               device->bredr_state.bonded = true;
> > -       else
> > -               device->le_state.bonded = true;
> > +       if (state->bonded)
> > +               return;
> > +
> > +       DBG("setting bonded for device to true");
> > +
> > +       state->bonded = true;
> >
> >         btd_device_set_temporary(device, false);
> > +
> > +       /* If the other bearer state was already true we don't need to
> > +        * send any property signals.
> > +        */
> > +       if (device->bredr_state.bonded == device->le_state.bonded)
> > +               return;
> > +
> > +       g_dbus_emit_property_changed(dbus_conn, device->path,
> > +                                               DEVICE_INTERFACE, "Bonded");
> >  }
> >
> >  void device_set_legacy(struct btd_device *device, bool legacy)
> > --
> > 2.36.0.rc0.470.gd361397f0d-goog
> >
>
>
> --
> Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz

Re: [Bluez PATCH v2 1/2] device: Add "Bonded" flag to dbus property

[Luiz Augusto von Dentz]

Hi Zhengping,

On Mon, May 2, 2022 at 2:20 PM Zhengping Jiang <jiangzp@google.com> wrote:
>
> Hi Luiz,
>
> Sorry for the delay. I just submitted a new patch for internal review. Will be sent upstream very soon.
> Just to confirm, I will remove the option "paired-devices" and add optional arguments to "devices" command, so it can generate a list of devices based on filters.

Great, thanks for letting me know. And yes, I think listing devices
based on filters is a better option than yet another command to list a
subset of devices.

> Thanks,
> Zhengping
>
> On Mon, May 2, 2022 at 2:12 PM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
>>
>> Hi Zhengping,
>>
>> On Mon, Apr 18, 2022 at 3:41 PM Luiz Augusto von Dentz
>> <luiz.dentz@gmail.com> wrote:
>> >
>> > Hi Zhengping,
>> >
>> > On Mon, Apr 18, 2022 at 10:49 AM Zhengping Jiang <jiangzp@google.com> wrote:
>> > >
>> > > Add "Bonded" to dbus device property table. When setting the "Bonded
>> > > flag, check the status of the Bonded property first. If the Bonded
>> > > property is changed, send property changed signal.
>> > >
>> > > Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org>
>> > > Reviewed-by: Yun-Hao Chung <howardchung@chromium.org>
>> > >
>> > > Signed-off-by: Zhengping Jiang <jiangzp@google.com>
>> > > ---
>> > >
>> > > Changes in v2:
>> > > - Move one variable declaration to the top following C90 standard
>> > >
>> > > Changes in v1:
>> > > - Add "Bonded" to D-Bus interface
>> > > - Send property changed signal if the bonded flag is changed
>> > >
>> > >  doc/device-api.txt |  4 ++++
>> > >  src/device.c       | 40 +++++++++++++++++++++++++++++++++++-----
>> > >  2 files changed, 39 insertions(+), 5 deletions(-)
>> > >
>> > > diff --git a/doc/device-api.txt b/doc/device-api.txt
>> > > index 4e824d2dec17..6162755f954c 100644
>> > > --- a/doc/device-api.txt
>> > > +++ b/doc/device-api.txt
>> > > @@ -171,6 +171,10 @@ Properties string Address [readonly]
>> > >
>> > >                         Indicates if the remote device is paired.
>> > >
>> > > +               boolean Bonded [readonly]
>> > > +
>> > > +                       Indicates if the remote device is bonded.
>> >
>> > It is probably a good idea to add a description about Bonded vs
>> > Paired. Btw, API documentation should be in a separate patch.
>>
>> Will you be updating following this comments or you are waiting more
>> feedback from upstream?
>>
>> > > +
>> > >                 boolean Connected [readonly]
>> > >
>> > >                         Indicates if the remote device is currently connected.
>> > > diff --git a/src/device.c b/src/device.c
>> > > index 8dc12d026827..868c41f025d9 100644
>> > > --- a/src/device.c
>> > > +++ b/src/device.c
>> > > @@ -1042,6 +1042,22 @@ static gboolean dev_property_get_paired(const GDBusPropertyTable *property,
>> > >         return TRUE;
>> > >  }
>> > >
>> > > +static gboolean dev_property_get_bonded(const GDBusPropertyTable *property,
>> > > +                                       DBusMessageIter *iter, void *data)
>> > > +{
>> > > +       struct btd_device *dev = data;
>> > > +       dbus_bool_t val;
>> > > +
>> > > +       if (dev->bredr_state.bonded || dev->le_state.bonded)
>> > > +               val = TRUE;
>> > > +       else
>> > > +               val = FALSE;
>> > > +
>> > > +       dbus_message_iter_append_basic(iter, DBUS_TYPE_BOOLEAN, &val);
>> > > +
>> > > +       return TRUE;
>> > > +}
>> > > +
>> > >  static gboolean dev_property_get_legacy(const GDBusPropertyTable *property,
>> > >                                         DBusMessageIter *iter, void *data)
>> > >  {
>> > > @@ -3120,6 +3136,7 @@ static const GDBusPropertyTable device_properties[] = {
>> > >         { "Icon", "s", dev_property_get_icon, NULL,
>> > >                                         dev_property_exists_icon },
>> > >         { "Paired", "b", dev_property_get_paired },
>> > > +       { "Bonded", "b", dev_property_get_bonded },
>> > >         { "Trusted", "b", dev_property_get_trusted, dev_property_set_trusted },
>> > >         { "Blocked", "b", dev_property_get_blocked, dev_property_set_blocked },
>> > >         { "LegacyPairing", "b", dev_property_get_legacy },
>> > > @@ -6114,17 +6131,30 @@ void btd_device_set_trusted(struct btd_device *device, gboolean trusted)
>> > >
>> > >  void device_set_bonded(struct btd_device *device, uint8_t bdaddr_type)
>> > >  {
>> > > +       struct bearer_state *state;
>> > > +
>> > >         if (!device)
>> > >                 return;
>> > >
>> > > -       DBG("");
>> > > +       state = get_state(device, bdaddr_type);
>> > >
>> > > -       if (bdaddr_type == BDADDR_BREDR)
>> > > -               device->bredr_state.bonded = true;
>> > > -       else
>> > > -               device->le_state.bonded = true;
>> > > +       if (state->bonded)
>> > > +               return;
>> > > +
>> > > +       DBG("setting bonded for device to true");
>> > > +
>> > > +       state->bonded = true;
>> > >
>> > >         btd_device_set_temporary(device, false);
>> > > +
>> > > +       /* If the other bearer state was already true we don't need to
>> > > +        * send any property signals.
>> > > +        */
>> > > +       if (device->bredr_state.bonded == device->le_state.bonded)
>> > > +               return;
>> > > +
>> > > +       g_dbus_emit_property_changed(dbus_conn, device->path,
>> > > +                                               DEVICE_INTERFACE, "Bonded");
>> > >  }
>> > >
>> > >  void device_set_legacy(struct btd_device *device, bool legacy)
>> > > --
>> > > 2.36.0.rc0.470.gd361397f0d-goog
>> > >
>> >
>> >
>> > --
>> > Luiz Augusto von Dentz
>>
>>
>>
>> --
>> Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz

[Bluez PATCH v2 2/2] client: Add bonded-devices and show Bonded flag in info

[Zhengping Jiang]

Add "bonded-devices" to the menu and show the "Bonded" property for
command "info".

Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org>
Reviewed-by: Yun-Hao Chung <howardchung@chromium.org>

Signed-off-by: Zhengping Jiang <jiangzp@google.com>
---

(no changes since v1)

Changes in v1:
- Show the status of the "Bonded" flag in bluetoothctl
- Add option to show list of bonded devices

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

diff --git a/client/main.c b/client/main.c
index 589268c3a68c..45c89a1de37b 100644
--- a/client/main.c
+++ b/client/main.c
@@ -1090,6 +1090,32 @@ static void cmd_paired_devices(int argc, char *argv[])
 	return bt_shell_noninteractive_quit(EXIT_SUCCESS);
 }
 
+static void cmd_bonded_devices(int argc, char *argv[])
+{
+	GList *ll;
+
+	if (check_default_ctrl() == FALSE)
+		return bt_shell_noninteractive_quit(EXIT_SUCCESS);
+
+	for (ll = g_list_first(default_ctrl->devices);
+			ll; ll = g_list_next(ll)) {
+		GDBusProxy *proxy = ll->data;
+		DBusMessageIter iter;
+		dbus_bool_t bonded;
+
+		if (g_dbus_proxy_get_property(proxy, "Bonded", &iter) == FALSE)
+			continue;
+
+		dbus_message_iter_get_basic(&iter, &bonded);
+		if (!bonded)
+			continue;
+
+		print_device(proxy, NULL);
+	}
+
+	return bt_shell_noninteractive_quit(EXIT_SUCCESS);
+}
+
 static void generic_callback(const DBusError *error, void *user_data)
 {
 	char *str = user_data;
@@ -1781,6 +1807,7 @@ static void cmd_info(int argc, char *argv[])
 	print_property(proxy, "Appearance");
 	print_property(proxy, "Icon");
 	print_property(proxy, "Paired");
+	print_property(proxy, "Bonded");
 	print_property(proxy, "Trusted");
 	print_property(proxy, "Blocked");
 	print_property(proxy, "Connected");
@@ -3116,6 +3143,8 @@ static const struct bt_shell_menu main_menu = {
 	{ "devices",      NULL,       cmd_devices, "List available devices" },
 	{ "paired-devices", NULL,     cmd_paired_devices,
 					"List paired devices"},
+	{ "bonded-devices", NULL,     cmd_bonded_devices,
+					"List bonded devices"},
 	{ "system-alias", "<name>",   cmd_system_alias,
 					"Set controller alias" },
 	{ "reset-alias",  NULL,       cmd_reset_alias,
-- 
2.36.0.rc0.470.gd361397f0d-goog

Re: [Bluez PATCH v2 2/2] client: Add bonded-devices and show Bonded flag in info

[Luiz Augusto von Dentz]

Hi Zhengping,

On Mon, Apr 18, 2022 at 10:49 AM Zhengping Jiang <jiangzp@google.com> wrote:
>
> Add "bonded-devices" to the menu and show the "Bonded" property for
> command "info".
>
> Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org>
> Reviewed-by: Yun-Hao Chung <howardchung@chromium.org>
>
> Signed-off-by: Zhengping Jiang <jiangzp@google.com>
> ---
>
> (no changes since v1)
>
> Changes in v1:
> - Show the status of the "Bonded" flag in bluetoothctl
> - Add option to show list of bonded devices
>
>  client/main.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>
> diff --git a/client/main.c b/client/main.c
> index 589268c3a68c..45c89a1de37b 100644
> --- a/client/main.c
> +++ b/client/main.c
> @@ -1090,6 +1090,32 @@ static void cmd_paired_devices(int argc, char *argv[])
>         return bt_shell_noninteractive_quit(EXIT_SUCCESS);
>  }
>
> +static void cmd_bonded_devices(int argc, char *argv[])
> +{
> +       GList *ll;
> +
> +       if (check_default_ctrl() == FALSE)
> +               return bt_shell_noninteractive_quit(EXIT_SUCCESS);
> +
> +       for (ll = g_list_first(default_ctrl->devices);
> +                       ll; ll = g_list_next(ll)) {
> +               GDBusProxy *proxy = ll->data;
> +               DBusMessageIter iter;
> +               dbus_bool_t bonded;
> +
> +               if (g_dbus_proxy_get_property(proxy, "Bonded", &iter) == FALSE)
> +                       continue;
> +
> +               dbus_message_iter_get_basic(&iter, &bonded);
> +               if (!bonded)
> +                       continue;
> +
> +               print_device(proxy, NULL);
> +       }
> +
> +       return bt_shell_noninteractive_quit(EXIT_SUCCESS);
> +}
> +
>  static void generic_callback(const DBusError *error, void *user_data)
>  {
>         char *str = user_data;
> @@ -1781,6 +1807,7 @@ static void cmd_info(int argc, char *argv[])
>         print_property(proxy, "Appearance");
>         print_property(proxy, "Icon");
>         print_property(proxy, "Paired");
> +       print_property(proxy, "Bonded");
>         print_property(proxy, "Trusted");
>         print_property(proxy, "Blocked");
>         print_property(proxy, "Connected");
> @@ -3116,6 +3143,8 @@ static const struct bt_shell_menu main_menu = {
>         { "devices",      NULL,       cmd_devices, "List available devices" },
>         { "paired-devices", NULL,     cmd_paired_devices,
>                                         "List paired devices"},
> +       { "bonded-devices", NULL,     cmd_bonded_devices,
> +                                       "List bonded devices"},

I would have done it a little be different, make devices command
create different lists:

bluetoothctl> devices [Trusted/Paired/Bonded/Connected]
Device XX:... Name (Trusted, Paired, Bonded...)
...

That way we don't have to create a command for each possible device filter.


>         { "system-alias", "<name>",   cmd_system_alias,
>                                         "Set controller alias" },
>         { "reset-alias",  NULL,       cmd_reset_alias,
> --
> 2.36.0.rc0.470.gd361397f0d-goog
>


-- 
Luiz Augusto von Dentz

[Bluez PATCH v3 1/3] device: Add "Bonded" flag to dbus property

[Zhengping Jiang]

Add "Bonded" to dbus device property table. When setting the "Bonded
flag, check the status of the Bonded property first. If the Bonded
property is changed, send property changed signal.

Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org>
Reviewed-by: Yun-Hao Chung <howardchung@chromium.org>

Signed-off-by: Zhengping Jiang <jiangzp@google.com>
---

(no changes since v2)

Changes in v2:
- Move one variable declaration to the top following C90 standard

Changes in v1:
- Add "Bonded" to D-Bus interface
- Send property changed signal if the bonded flag is changed

 src/device.c | 38 +++++++++++++++++++++++++++++++++-----
 1 file changed, 33 insertions(+), 5 deletions(-)

diff --git a/src/device.c b/src/device.c
index a62564b14f49..72804713b25b 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1143,6 +1143,22 @@ static gboolean dev_property_get_paired(const GDBusPropertyTable *property,
 	return TRUE;
 }
 
+static gboolean dev_property_get_bonded(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	struct btd_device *dev = data;
+	dbus_bool_t val;
+
+	if (dev->bredr_state.bonded || dev->le_state.bonded)
+		val = TRUE;
+	else
+		val = FALSE;
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_BOOLEAN, &val);
+
+	return TRUE;
+}
+
 static gboolean dev_property_get_legacy(const GDBusPropertyTable *property,
 					DBusMessageIter *iter, void *data)
 {
@@ -4033,6 +4049,7 @@ static const GDBusPropertyTable device_properties[] = {
 	{ "Icon", "s", dev_property_get_icon, NULL,
 					dev_property_exists_icon },
 	{ "Paired", "b", dev_property_get_paired },
+	{ "Bonded", "b", dev_property_get_bonded },
 	{ "Trusted", "b", dev_property_get_trusted, dev_property_set_trusted },
 	{ "Blocked", "b", dev_property_get_blocked, dev_property_set_blocked },
 	{ "LegacyPairing", "b", dev_property_get_legacy },
@@ -7065,14 +7082,25 @@ void device_set_bonded(struct btd_device *device, uint8_t bdaddr_type)
 	if (!device)
 		return;
 
-	DBG("");
+	struct bearer_state *state = get_state(device, bdaddr_type);
 
-	if (bdaddr_type == BDADDR_BREDR)
-		device->bredr_state.bonded = true;
-	else
-		device->le_state.bonded = true;
+	if (state->bonded)
+		return;
+
+	DBG("setting bonded for device to true");
+
+	state->bonded = true;
 
 	btd_device_set_temporary(device, false);
+
+	/* If the other bearer state was already true we don't need to
+	 * send any property signals.
+	 */
+	if (device->bredr_state.bonded == device->le_state.bonded)
+		return;
+
+	g_dbus_emit_property_changed(dbus_conn, device->path,
+						DEVICE_INTERFACE, "Bonded");
 }
 
 void device_set_legacy(struct btd_device *device, bool legacy)
-- 
2.36.0.464.gb9c8b46e94-goog

RE: Adding bonded flag to D-Bus property

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 5827 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=638455

---Test result---

Test Summary:
CheckPatch                    PASS      4.37 seconds
GitLint                       PASS      2.85 seconds
Prep - Setup ELL              PASS      43.69 seconds
Build - Prep                  PASS      0.69 seconds
Build - Configure             PASS      8.79 seconds
Build - Make                  FAIL      1282.94 seconds
Make Check                    FAIL      5.14 seconds
Make Check w/Valgrind         FAIL      308.39 seconds
Make Distcheck                PASS      235.79 seconds
Build w/ext ELL - Configure   PASS      8.96 seconds
Build w/ext ELL - Make        FAIL      1253.26 seconds
Incremental Build with patchesFAIL      1279.49 seconds

Details
##############################
Test: Build - Make - FAIL
Desc: Build the BlueZ source tree
Output:
tools/mgmt-tester.c: In function ‘main’:
tools/mgmt-tester.c:12364:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
12364 | int main(int argc, char *argv[])
      |     ^~~~
unit/test-avdtp.c: In function ‘main’:
unit/test-avdtp.c:766:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
  766 | int main(int argc, char *argv[])
      |     ^~~~
unit/test-avrcp.c: In function ‘main’:
unit/test-avrcp.c:989:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
  989 | int main(int argc, char *argv[])
      |     ^~~~
src/device.c: In function ‘device_set_bonded’:
src/device.c:6141:2: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
 6141 |  struct bearer_state *state = get_state(device, bdaddr_type);
      |  ^~~~~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:10248: src/bluetoothd-device.o] Error 1
make: *** [Makefile:4310: all] Error 2


##############################
Test: Make Check - FAIL
Desc: Run 'make check'
Output:
src/device.c: In function ‘device_set_bonded’:
src/device.c:6141:2: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
 6141 |  struct bearer_state *state = get_state(device, bdaddr_type);
      |  ^~~~~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:10248: src/bluetoothd-device.o] Error 1
make: *** [Makefile:11283: check] Error 2


##############################
Test: Make Check w/Valgrind - FAIL
Desc: Run 'make check' with Valgrind
Output:
tools/mgmt-tester.c: In function ‘main’:
tools/mgmt-tester.c:12364:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
12364 | int main(int argc, char *argv[])
      |     ^~~~
src/device.c: In function ‘device_set_bonded’:
src/device.c:6141:2: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
 6141 |  struct bearer_state *state = get_state(device, bdaddr_type);
      |  ^~~~~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:10248: src/bluetoothd-device.o] Error 1
make: *** [Makefile:4310: all] Error 2


##############################
Test: Build w/ext ELL - Make - FAIL
Desc: Build BlueZ source with '--enable-external-ell' configuration
Output:
tools/mgmt-tester.c: In function ‘main’:
tools/mgmt-tester.c:12364:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
12364 | int main(int argc, char *argv[])
      |     ^~~~
unit/test-avdtp.c: In function ‘main’:
unit/test-avdtp.c:766:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
  766 | int main(int argc, char *argv[])
      |     ^~~~
unit/test-avrcp.c: In function ‘main’:
unit/test-avrcp.c:989:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
  989 | int main(int argc, char *argv[])
      |     ^~~~
src/device.c: In function ‘device_set_bonded’:
src/device.c:6141:2: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
 6141 |  struct bearer_state *state = get_state(device, bdaddr_type);
      |  ^~~~~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:10248: src/bluetoothd-device.o] Error 1
make: *** [Makefile:4310: all] Error 2


##############################
Test: Incremental Build with patches - FAIL
Desc: Incremental build per patch in the series
Output:
tools/mgmt-tester.c: In function ‘main’:
tools/mgmt-tester.c:12364:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
12364 | int main(int argc, char *argv[])
      |     ^~~~
unit/test-avdtp.c: In function ‘main’:
unit/test-avdtp.c:766:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
  766 | int main(int argc, char *argv[])
      |     ^~~~
unit/test-avrcp.c: In function ‘main’:
unit/test-avrcp.c:989:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
  989 | int main(int argc, char *argv[])
      |     ^~~~
src/device.c: In function ‘device_set_bonded’:
src/device.c:6141:2: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
 6141 |  struct bearer_state *state = get_state(device, bdaddr_type);
      |  ^~~~~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:10248: src/bluetoothd-device.o] Error 1
make: *** [Makefile:4310: all] Error 2




---
Regards,
Linux Bluetooth

[Bluez PATCH v1 1/2] device: Add "Bonded" flag to dbus property

[Zhengping Jiang]

Add "Bonded" to dbus device property table. When setting the "Bonded
flag, check the status of the Bonded property first. If the Bonded
property is changed, send property changed signal.

Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org>
Reviewed-by: Yun-Hao Chung <howardchung@chromium.org>

Signed-off-by: Zhengping Jiang <jiangzp@google.com>
---

Changes in v1:
- Add "Bonded" to D-Bus interface
- Send property changed signal if the bonded flag is changed

 doc/device-api.txt |  4 ++++
 src/device.c       | 38 +++++++++++++++++++++++++++++++++-----
 2 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/doc/device-api.txt b/doc/device-api.txt
index 4e824d2dec17..6162755f954c 100644
--- a/doc/device-api.txt
+++ b/doc/device-api.txt
@@ -171,6 +171,10 @@ Properties	string Address [readonly]
 
 			Indicates if the remote device is paired.
 
+		boolean Bonded [readonly]
+
+			Indicates if the remote device is bonded.
+
 		boolean Connected [readonly]
 
 			Indicates if the remote device is currently connected.
diff --git a/src/device.c b/src/device.c
index 8dc12d026827..1a7afa8fc5c0 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1042,6 +1042,22 @@ static gboolean dev_property_get_paired(const GDBusPropertyTable *property,
 	return TRUE;
 }
 
+static gboolean dev_property_get_bonded(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	struct btd_device *dev = data;
+	dbus_bool_t val;
+
+	if (dev->bredr_state.bonded || dev->le_state.bonded)
+		val = TRUE;
+	else
+		val = FALSE;
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_BOOLEAN, &val);
+
+	return TRUE;
+}
+
 static gboolean dev_property_get_legacy(const GDBusPropertyTable *property,
 					DBusMessageIter *iter, void *data)
 {
@@ -3120,6 +3136,7 @@ static const GDBusPropertyTable device_properties[] = {
 	{ "Icon", "s", dev_property_get_icon, NULL,
 					dev_property_exists_icon },
 	{ "Paired", "b", dev_property_get_paired },
+	{ "Bonded", "b", dev_property_get_bonded },
 	{ "Trusted", "b", dev_property_get_trusted, dev_property_set_trusted },
 	{ "Blocked", "b", dev_property_get_blocked, dev_property_set_blocked },
 	{ "LegacyPairing", "b", dev_property_get_legacy },
@@ -6117,14 +6134,25 @@ void device_set_bonded(struct btd_device *device, uint8_t bdaddr_type)
 	if (!device)
 		return;
 
-	DBG("");
+	struct bearer_state *state = get_state(device, bdaddr_type);
 
-	if (bdaddr_type == BDADDR_BREDR)
-		device->bredr_state.bonded = true;
-	else
-		device->le_state.bonded = true;
+	if (state->bonded)
+		return;
+
+	DBG("setting bonded for device to true");
+
+	state->bonded = true;
 
 	btd_device_set_temporary(device, false);
+
+	/* If the other bearer state was already true we don't need to
+	 * send any property signals.
+	 */
+	if (device->bredr_state.bonded == device->le_state.bonded)
+		return;
+
+	g_dbus_emit_property_changed(dbus_conn, device->path,
+						DEVICE_INTERFACE, "Bonded");
 }
 
 void device_set_legacy(struct btd_device *device, bool legacy)
-- 
2.36.0.rc0.470.gd361397f0d-goog

RE: Adding bonded flag to D-Bus property

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 5829 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=632916

---Test result---

Test Summary:
CheckPatch                    PASS      3.15 seconds
GitLint                       PASS      2.12 seconds
Prep - Setup ELL              PASS      50.93 seconds
Build - Prep                  PASS      0.78 seconds
Build - Configure             PASS      10.43 seconds
Build - Make                  FAIL      1394.11 seconds
Make Check                    FAIL      5.82 seconds
Make Check w/Valgrind         FAIL      350.53 seconds
Make Distcheck                PASS      271.29 seconds
Build w/ext ELL - Configure   PASS      10.32 seconds
Build w/ext ELL - Make        FAIL      1367.40 seconds
Incremental Build with patchesFAIL      1420.73 seconds

Details
##############################
Test: Build - Make - FAIL
Desc: Build the BlueZ source tree
Output:
tools/mgmt-tester.c: In function ‘main’:
tools/mgmt-tester.c:12364:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
12364 | int main(int argc, char *argv[])
      |     ^~~~
unit/test-avdtp.c: In function ‘main’:
unit/test-avdtp.c:766:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
  766 | int main(int argc, char *argv[])
      |     ^~~~
unit/test-avrcp.c: In function ‘main’:
unit/test-avrcp.c:989:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
  989 | int main(int argc, char *argv[])
      |     ^~~~
src/device.c: In function ‘device_set_bonded’:
src/device.c:6137:2: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
 6137 |  struct bearer_state *state = get_state(device, bdaddr_type);
      |  ^~~~~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:10248: src/bluetoothd-device.o] Error 1
make: *** [Makefile:4310: all] Error 2


##############################
Test: Make Check - FAIL
Desc: Run 'make check'
Output:
src/device.c: In function ‘device_set_bonded’:
src/device.c:6137:2: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
 6137 |  struct bearer_state *state = get_state(device, bdaddr_type);
      |  ^~~~~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:10248: src/bluetoothd-device.o] Error 1
make: *** [Makefile:11283: check] Error 2


##############################
Test: Make Check w/Valgrind - FAIL
Desc: Run 'make check' with Valgrind
Output:
tools/mgmt-tester.c: In function ‘main’:
tools/mgmt-tester.c:12364:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
12364 | int main(int argc, char *argv[])
      |     ^~~~
src/device.c: In function ‘device_set_bonded’:
src/device.c:6137:2: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
 6137 |  struct bearer_state *state = get_state(device, bdaddr_type);
      |  ^~~~~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:10248: src/bluetoothd-device.o] Error 1
make: *** [Makefile:4310: all] Error 2


##############################
Test: Build w/ext ELL - Make - FAIL
Desc: Build BlueZ source with '--enable-external-ell' configuration
Output:
tools/mgmt-tester.c: In function ‘main’:
tools/mgmt-tester.c:12364:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
12364 | int main(int argc, char *argv[])
      |     ^~~~
unit/test-avdtp.c: In function ‘main’:
unit/test-avdtp.c:766:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
  766 | int main(int argc, char *argv[])
      |     ^~~~
unit/test-avrcp.c: In function ‘main’:
unit/test-avrcp.c:989:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
  989 | int main(int argc, char *argv[])
      |     ^~~~
src/device.c: In function ‘device_set_bonded’:
src/device.c:6137:2: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
 6137 |  struct bearer_state *state = get_state(device, bdaddr_type);
      |  ^~~~~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:10248: src/bluetoothd-device.o] Error 1
make: *** [Makefile:4310: all] Error 2


##############################
Test: Incremental Build with patches - FAIL
Desc: Incremental build per patch in the series
Output:
tools/mgmt-tester.c: In function ‘main’:
tools/mgmt-tester.c:12364:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
12364 | int main(int argc, char *argv[])
      |     ^~~~
unit/test-avdtp.c: In function ‘main’:
unit/test-avdtp.c:766:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
  766 | int main(int argc, char *argv[])
      |     ^~~~
unit/test-avrcp.c: In function ‘main’:
unit/test-avrcp.c:989:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
  989 | int main(int argc, char *argv[])
      |     ^~~~
src/device.c: In function ‘device_set_bonded’:
src/device.c:6137:2: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
 6137 |  struct bearer_state *state = get_state(device, bdaddr_type);
      |  ^~~~~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:10248: src/bluetoothd-device.o] Error 1
make: *** [Makefile:4310: all] Error 2




---
Regards,
Linux Bluetooth