Re: [PATCH] adapter: Add retry when bonding device returns connection failure

[Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx>]

Hi,

On Mon, Jul 1, 2024 at 6:13 AM Chengyi Zhao <zhaochengyi@xxxxxxxxxxxxx> wrote:
>
> When a user initiates pairing with a BLE Bluetooth mouse,
> MGMT_STATUS_CONNECT_FAILED(0x04) is returned with a low
> probability, resulting in pairing failure. To improve
> user experience, retry bonding is performed when
> MGMT_STATUS_CONNECT_FAILED is returned.
>
> Just retry once when MGMT_STATUS_CONNECT_FAILED occurs
> because this status may be continuously returned.
>
> Debug log:
> bluetoothd[1539]: src/adapter.c:pair_device_complete() Connect Failed
> (0x04)
> bluetoothd[1539]: src/adapter.c:bonding_attempt_complete() hci0 bdaddr
> DD:EC:0F:57:A9:2E type 2 status 0x4
> bluetoothd[1539]: src/device.c:device_bonding_complete() bonding
> 0x5591f87230 status 0x04
> bluetoothd[1539]: src/device.c:btd_device_set_temporary() temporary 1
> bluetoothd[1539]: src/device.c:device_bonding_failed() status 4
>
> HCI package:
> Frame 2969: 7 bytes on wire (56 bits), 7 bytes captured (56 bits)
> Bluetooth
> Bluetooth HCI H4
> Bluetooth HCI Event - Disconnect Complete
> Event Code: Disconnect Complete (0x05)
> Parameter Total Length: 4
> Status: Success (0x00)
> Connection Handle: 0x0040
> Reason: Connection Failed to be Established (0x3e)
> ---
>  src/adapter.c |  4 ++++
>  src/device.c  | 24 ++++++++++++++++++++++++
>  src/device.h  |  2 ++
>  3 files changed, 30 insertions(+)
>
> diff --git a/src/adapter.c b/src/adapter.c
> index bb49a1eca..574fa7665 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -8371,6 +8371,10 @@ static void bonding_attempt_complete(struct btd_adapter *adapter,
>                 }
>         }
>
> +       /* Retry once when status is MGMT_STATUS_CONNECT_FAILED */
> +       if (device && device_bonding_check_connection(device, status))
> +               return;
> +
>         /* Ignore disconnects during retry. */
>         if (status == MGMT_STATUS_DISCONNECTED &&
>                                         device && device_is_retrying(device))
> diff --git a/src/device.c b/src/device.c
> index 097b1fbba..12fabbff1 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -290,6 +290,8 @@ struct btd_device {
>         time_t          name_resolve_failed_time;
>
>         int8_t          volume;
> +
> +       uint8_t bonding_status;
>  };
>
>  static const uint16_t uuid_list[] = {
> @@ -6559,6 +6561,28 @@ bool device_remove_svc_complete_callback(struct btd_device *dev,
>         return false;
>  }
>
> +gboolean device_bonding_check_connection(struct btd_device *device,
> +                                                               uint8_t status)
> +{
> +       if (status == MGMT_STATUS_CONNECT_FAILED) {
> +
> +               if (device->bonding_status != MGMT_STATUS_CONNECT_FAILED) {
> +                       device->bonding_status = MGMT_STATUS_CONNECT_FAILED;
> +
> +                       DBG("status is 0x%x, retry once.", status);
> +
> +                       if (device_bonding_attempt_retry(device) == 0)
> +                               return TRUE;
> +               }
> +       } else {
> +               device->bonding_status = status;
> +
> +               DBG("device->bonding_status is 0x%x.", device->bonding_status);
> +       }
> +
> +       return FALSE;
> +}
> +
>  gboolean device_is_bonding(struct btd_device *device, const char *sender)
>  {
>         struct bonding_req *bonding = device->bonding;
> diff --git a/src/device.h b/src/device.h
> index 0794f92d0..7c269cc4d 100644
> --- a/src/device.h
> +++ b/src/device.h
> @@ -111,6 +111,8 @@ uint8_t btd_device_get_bdaddr_type(struct btd_device *dev);
>  bool device_is_retrying(struct btd_device *device);
>  void device_bonding_complete(struct btd_device *device, uint8_t bdaddr_type,
>                                                         uint8_t status);
> +gboolean device_bonding_check_connection(struct btd_device *device,
> +                                                       uint8_t status);
>  gboolean device_is_bonding(struct btd_device *device, const char *sender);
>  void device_bonding_attempt_failed(struct btd_device *device, uint8_t status);
>  void device_bonding_failed(struct btd_device *device, uint8_t status);
> --
> 2.20.1

Here is what Ive actually had in mind:

diff --git a/src/adapter.c b/src/adapter.c
index bb49a1ecad23..f1cc4f2ed25a 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -8363,12 +8363,17 @@ static void bonding_attempt_complete(struct
btd_adapter *adapter,
        else
                device = btd_adapter_find_device(adapter, bdaddr, addr_type);

-       if (status == MGMT_STATUS_AUTH_FAILED && adapter->pincode_requested) {
-               /* On faliure, issue a bonding_retry if possible. */
+       switch (status) {
+       case MGMT_STATUS_AUTH_FAILED:
+               if (!adapter->pincode_requested)
+                       break;
+       /* fall through */
+       case MGMT_STATUS_CONNECT_FAILED:
                if (device != NULL) {
                        if (device_bonding_attempt_retry(device) == 0)
                                return;
                }
+               break;
        }

        /* Ignore disconnects during retry. */


-- 
Luiz Augusto von Dentz