| Message ID | 20200831161140.Bluez.v1.1.If16fd16b4a629ec4d4093a974256225a95b58044@changeid (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Luiz Von Dentz |
| Headers | show |
| Series | [Bluez,v1] device: don't wait for timeout if RemoveDevice is called | expand |
Hi Archie, On Mon, Aug 31, 2020 at 1:12 AM 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> > --- > > 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 5e896a9f0..d6c65ff69 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 bb8e07e8f..cee0ddfd2 100644 > --- a/src/device.c > +++ b/src/device.c > @@ -3001,6 +3001,7 @@ void device_add_connection(struct btd_device *dev, uint8_t bdaddr_type) > void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type) > { > struct bearer_state *state = get_state(device, bdaddr_type); > + bool remove_device = false; > > if (!state->connected) > return; > @@ -3018,6 +3019,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); > @@ -3043,6 +3048,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, > @@ -4457,6 +4465,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 Does not apply: Applying: device: don't wait for timeout if RemoveDevice is called error: patch failed: src/device.c:3001 error: src/device.c: patch does not apply
Hi Luiz, On Tue, 1 Sep 2020 at 01:35, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > Hi Archie, > > On Mon, Aug 31, 2020 at 1:12 AM 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> > > --- > > > > 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 5e896a9f0..d6c65ff69 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 bb8e07e8f..cee0ddfd2 100644 > > --- a/src/device.c > > +++ b/src/device.c > > @@ -3001,6 +3001,7 @@ void device_add_connection(struct btd_device *dev, uint8_t bdaddr_type) > > void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type) > > { > > struct bearer_state *state = get_state(device, bdaddr_type); > > + bool remove_device = false; > > > > if (!state->connected) > > return; > > @@ -3018,6 +3019,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); > > @@ -3043,6 +3048,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, > > @@ -4457,6 +4465,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 > > Does not apply: > > Applying: device: don't wait for timeout if RemoveDevice is called > error: patch failed: src/device.c:3001 > error: src/device.c: patch does not apply Oops, let me rebase and resubmit. > > > > -- > Luiz Augusto von Dentz Thanks, Archie
diff --git a/src/adapter.c b/src/adapter.c index 5e896a9f0..d6c65ff69 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 bb8e07e8f..cee0ddfd2 100644 --- a/src/device.c +++ b/src/device.c @@ -3001,6 +3001,7 @@ void device_add_connection(struct btd_device *dev, uint8_t bdaddr_type) void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type) { struct bearer_state *state = get_state(device, bdaddr_type); + bool remove_device = false; if (!state->connected) return; @@ -3018,6 +3019,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); @@ -3043,6 +3048,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, @@ -4457,6 +4465,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;