Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200915110347.Bluez.v3.1.If16fd16b4a629ec4d4093a974256225a95b58044@changeid>
In-Reply-To: <20200915110347.Bluez.v3.1.If16fd16b4a629ec4d4093a974256225a95b58044@changeid>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Tue, 15 Sep 2020 09:51:12 -0700
Message-ID: <CABBYNZKQhnQDaTQ_e-FA6hCEW5ZChaOdOQ-qCx3gpQXSPBD29g@mail.gmail.com>
Subject: Re: [Bluez PATCH v3] device: don't wait for timeout if RemoveDevice
 is called
To:     Archie Pusaka <apusaka@google.com>
Cc:     linux-bluetooth <linux-bluetooth@vger.kernel.org>,
        CrosBT Upstreaming <chromeos-bluetooth-upstreaming@chromium.org>,
        Archie Pusaka <apusaka@chromium.org>,
        Daniel Winkler <danielwinkler@google.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

Hi Archie,

On Mon, Sep 14, 2020 at 8:04 PM 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>
> ---
>
> Changes in v3:
> * Rebasing again
>
> 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 df628a7fd..4e27bd74b 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -80,8 +80,6 @@
>  #include "adv_monitor.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 c70a7b0da..2f1e4b737 100644
> --- a/src/adapter.h
> +++ b/src/adapter.h
> @@ -29,6 +29,8 @@
>  #include <lib/bluetooth.h>
>  #include <lib/sdp.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 8f73ce4d3..3e7784034 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,
> @@ -4482,6 +4490,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.618.gf4bc123cb7-goog
>

Applied, thanks.

-- 
Luiz Augusto von Dentz