| Message ID | 20220418174914.Bluez.v2.1.I6ab300fa4999c9310f4cb6fc09b1290edb6b2c2b@changeid (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Adding bonded flag to D-Bus property | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/checkpatch | success | Checkpatch PASS |
| tedd_an/gitlint | success | Gitlint PASS |
| tedd_an/setupell | success | Setup ELL PASS |
| tedd_an/buildprep | success | Build Prep PASS |
| tedd_an/build | success | Build Configuration PASS |
| tedd_an/makecheck | success | Make Check PASS |
| tedd_an/makecheckvalgrind | success | Make Check PASS |
| tedd_an/makedistcheck | success | Make Distcheck PASS |
| tedd_an/build_extell | success | Build External ELL PASS |
| tedd_an/build_extell_make | success | Build Make with External ELL PASS |
| tedd_an/incremental_build | success | Pass |
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
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 >
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
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
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)