| Message ID | 20200901113736.Bluez.v2.1.If16fd16b4a629ec4d4093a974256225a95b58044@changeid (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Luiz Von Dentz |
| Headers | show |
| Series | [Bluez,v2] device: don't wait for timeout if RemoveDevice is called | expand |
Hi Bluez maintainers, Could you take another look at this patch? Thanks, Archie On Tue, 1 Sep 2020 at 11:37, Archie Pusaka <apusaka@google.com> wrote: > > From: Archie Pusaka <apusaka@chromium.org> > > RemoveDevice on adapter interface used to remove a device, even when > the device is connected. However, since the introduction of the new > 30 seconds timeout when setting a device as temporary, RemoveDevice > doesn't immediately remove a connected device, but only disconnects > it and waits for the timer to expire before effectively removes it. > > This patch removes the device as soon as it gets disconnected, > provided the disconnection is triggered by a call to RemoveDevice. > The regular timeout still applies for other cases. > > Tested manually by calling RemoveDevice on a connected device, > and with ChromeOS autotest setup. > > Reviewed-by: Daniel Winkler <danielwinkler@google.com> > > Signed-off-by: Archie Pusaka <apusaka@chromium.org> > --- > > Changes in v2: > * Rebasing to HEAD > > src/adapter.c | 2 -- > src/adapter.h | 2 ++ > src/device.c | 11 +++++++++++ > 3 files changed, 13 insertions(+), 2 deletions(-) > > diff --git a/src/adapter.c b/src/adapter.c > index 1435e2bd7..ffd05aa0b 100644 > --- a/src/adapter.c > +++ b/src/adapter.c > @@ -79,8 +79,6 @@ > #include "advertising.h" > #include "eir.h" > > -#define ADAPTER_INTERFACE "org.bluez.Adapter1" > - > #define MODE_OFF 0x00 > #define MODE_CONNECTABLE 0x01 > #define MODE_DISCOVERABLE 0x02 > diff --git a/src/adapter.h b/src/adapter.h > index f8ac20261..f835c984f 100644 > --- a/src/adapter.h > +++ b/src/adapter.h > @@ -26,6 +26,8 @@ > #include <dbus/dbus.h> > #include <glib.h> > > +#define ADAPTER_INTERFACE "org.bluez.Adapter1" > + > #define MAX_NAME_LENGTH 248 > > /* Invalid SSP passkey value used to indicate negative replies */ > diff --git a/src/device.c b/src/device.c > index df440ce09..f20270017 100644 > --- a/src/device.c > +++ b/src/device.c > @@ -3007,6 +3007,7 @@ void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type) > { > struct bearer_state *state = get_state(device, bdaddr_type); > DBusMessage *reply; > + bool remove_device = false; > > if (!state->connected) > return; > @@ -3036,6 +3037,10 @@ void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type) > while (device->disconnects) { > DBusMessage *msg = device->disconnects->data; > > + if (dbus_message_is_method_call(msg, ADAPTER_INTERFACE, > + "RemoveDevice")) > + remove_device = true; > + > g_dbus_send_reply(dbus_conn, msg, DBUS_TYPE_INVALID); > device->disconnects = g_slist_remove(device->disconnects, msg); > dbus_message_unref(msg); > @@ -3061,6 +3066,9 @@ void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type) > > g_dbus_emit_property_changed(dbus_conn, device->path, > DEVICE_INTERFACE, "Connected"); > + > + if (remove_device) > + btd_adapter_remove_device(device->adapter, device); > } > > guint device_add_disconnect_watch(struct btd_device *device, > @@ -4477,6 +4485,9 @@ void device_remove(struct btd_device *device, gboolean remove_stored) > disconnect_all(device); > } > > + if (device->temporary_timer > 0) > + g_source_remove(device->temporary_timer); > + > if (device->store_id > 0) { > g_source_remove(device->store_id); > device->store_id = 0; > -- > 2.28.0.402.g5ffc5be6b7-goog >
Hi Bluez maintainers, May I bother you to review this patch? Thanks, Archie On Fri, 11 Sep 2020 at 10:56, Archie Pusaka <apusaka@google.com> wrote: > > Hi Bluez maintainers, > > May I bother you to review this patch? > > Thanks, > Archie > > On Mon, 7 Sep 2020 at 10:38, Archie Pusaka <apusaka@google.com> wrote: >> >> Hi Bluez maintainers, >> >> Could you take another look at this patch? >> >> Thanks, >> Archie >> >> On Tue, 1 Sep 2020 at 11:37, Archie Pusaka <apusaka@google.com> wrote: >> > >> > From: Archie Pusaka <apusaka@chromium.org> >> > >> > RemoveDevice on adapter interface used to remove a device, even when >> > the device is connected. However, since the introduction of the new >> > 30 seconds timeout when setting a device as temporary, RemoveDevice >> > doesn't immediately remove a connected device, but only disconnects >> > it and waits for the timer to expire before effectively removes it. >> > >> > This patch removes the device as soon as it gets disconnected, >> > provided the disconnection is triggered by a call to RemoveDevice. >> > The regular timeout still applies for other cases. >> > >> > Tested manually by calling RemoveDevice on a connected device, >> > and with ChromeOS autotest setup. >> > >> > Reviewed-by: Daniel Winkler <danielwinkler@google.com> >> > >> > Signed-off-by: Archie Pusaka <apusaka@chromium.org> >> > --- >> > >> > Changes in v2: >> > * Rebasing to HEAD >> > >> > src/adapter.c | 2 -- >> > src/adapter.h | 2 ++ >> > src/device.c | 11 +++++++++++ >> > 3 files changed, 13 insertions(+), 2 deletions(-) >> > >> > diff --git a/src/adapter.c b/src/adapter.c >> > index 1435e2bd7..ffd05aa0b 100644 >> > --- a/src/adapter.c >> > +++ b/src/adapter.c >> > @@ -79,8 +79,6 @@ >> > #include "advertising.h" >> > #include "eir.h" >> > >> > -#define ADAPTER_INTERFACE "org.bluez.Adapter1" >> > - >> > #define MODE_OFF 0x00 >> > #define MODE_CONNECTABLE 0x01 >> > #define MODE_DISCOVERABLE 0x02 >> > diff --git a/src/adapter.h b/src/adapter.h >> > index f8ac20261..f835c984f 100644 >> > --- a/src/adapter.h >> > +++ b/src/adapter.h >> > @@ -26,6 +26,8 @@ >> > #include <dbus/dbus.h> >> > #include <glib.h> >> > >> > +#define ADAPTER_INTERFACE "org.bluez.Adapter1" >> > + >> > #define MAX_NAME_LENGTH 248 >> > >> > /* Invalid SSP passkey value used to indicate negative replies */ >> > diff --git a/src/device.c b/src/device.c >> > index df440ce09..f20270017 100644 >> > --- a/src/device.c >> > +++ b/src/device.c >> > @@ -3007,6 +3007,7 @@ void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type) >> > { >> > struct bearer_state *state = get_state(device, bdaddr_type); >> > DBusMessage *reply; >> > + bool remove_device = false; >> > >> > if (!state->connected) >> > return; >> > @@ -3036,6 +3037,10 @@ void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type) >> > while (device->disconnects) { >> > DBusMessage *msg = device->disconnects->data; >> > >> > + if (dbus_message_is_method_call(msg, ADAPTER_INTERFACE, >> > + "RemoveDevice")) >> > + remove_device = true; >> > + >> > g_dbus_send_reply(dbus_conn, msg, DBUS_TYPE_INVALID); >> > device->disconnects = g_slist_remove(device->disconnects, msg); >> > dbus_message_unref(msg); >> > @@ -3061,6 +3066,9 @@ void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type) >> > >> > g_dbus_emit_property_changed(dbus_conn, device->path, >> > DEVICE_INTERFACE, "Connected"); >> > + >> > + if (remove_device) >> > + btd_adapter_remove_device(device->adapter, device); >> > } >> > >> > guint device_add_disconnect_watch(struct btd_device *device, >> > @@ -4477,6 +4485,9 @@ void device_remove(struct btd_device *device, gboolean remove_stored) >> > disconnect_all(device); >> > } >> > >> > + if (device->temporary_timer > 0) >> > + g_source_remove(device->temporary_timer); >> > + >> > if (device->store_id > 0) { >> > g_source_remove(device->store_id); >> > device->store_id = 0; >> > -- >> > 2.28.0.402.g5ffc5be6b7-goog >> >
diff --git a/src/adapter.c b/src/adapter.c index 1435e2bd7..ffd05aa0b 100644 --- a/src/adapter.c +++ b/src/adapter.c @@ -79,8 +79,6 @@ #include "advertising.h" #include "eir.h" -#define ADAPTER_INTERFACE "org.bluez.Adapter1" - #define MODE_OFF 0x00 #define MODE_CONNECTABLE 0x01 #define MODE_DISCOVERABLE 0x02 diff --git a/src/adapter.h b/src/adapter.h index f8ac20261..f835c984f 100644 --- a/src/adapter.h +++ b/src/adapter.h @@ -26,6 +26,8 @@ #include <dbus/dbus.h> #include <glib.h> +#define ADAPTER_INTERFACE "org.bluez.Adapter1" + #define MAX_NAME_LENGTH 248 /* Invalid SSP passkey value used to indicate negative replies */ diff --git a/src/device.c b/src/device.c index df440ce09..f20270017 100644 --- a/src/device.c +++ b/src/device.c @@ -3007,6 +3007,7 @@ void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type) { struct bearer_state *state = get_state(device, bdaddr_type); DBusMessage *reply; + bool remove_device = false; if (!state->connected) return; @@ -3036,6 +3037,10 @@ void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type) while (device->disconnects) { DBusMessage *msg = device->disconnects->data; + if (dbus_message_is_method_call(msg, ADAPTER_INTERFACE, + "RemoveDevice")) + remove_device = true; + g_dbus_send_reply(dbus_conn, msg, DBUS_TYPE_INVALID); device->disconnects = g_slist_remove(device->disconnects, msg); dbus_message_unref(msg); @@ -3061,6 +3066,9 @@ void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type) g_dbus_emit_property_changed(dbus_conn, device->path, DEVICE_INTERFACE, "Connected"); + + if (remove_device) + btd_adapter_remove_device(device->adapter, device); } guint device_add_disconnect_watch(struct btd_device *device, @@ -4477,6 +4485,9 @@ void device_remove(struct btd_device *device, gboolean remove_stored) disconnect_all(device); } + if (device->temporary_timer > 0) + g_source_remove(device->temporary_timer); + if (device->store_id > 0) { g_source_remove(device->store_id); device->store_id = 0;