2022-04-19 09:59:59

by Zhengping Jiang

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

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


2022-04-22 19:47:03

by Zhengping Jiang

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

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 <[email protected]>
Reviewed-by: Yun-Hao Chung <[email protected]>

Signed-off-by: Zhengping Jiang <[email protected]>
---

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

2022-05-03 00:09:00

by Luiz Augusto von Dentz

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

Hi Zhengping,

On Mon, May 2, 2022 at 2:20 PM Zhengping Jiang <[email protected]> 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 <[email protected]> wrote:
>>
>> Hi Zhengping,
>>
>> On Mon, Apr 18, 2022 at 3:41 PM Luiz Augusto von Dentz
>> <[email protected]> wrote:
>> >
>> > Hi Zhengping,
>> >
>> > On Mon, Apr 18, 2022 at 10:49 AM Zhengping Jiang <[email protected]> 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 <[email protected]>
>> > > Reviewed-by: Yun-Hao Chung <[email protected]>
>> > >
>> > > Signed-off-by: Zhengping Jiang <[email protected]>
>> > > ---
>> > >
>> > > 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