diff mbox series

adapter: Add retry when bonding device returns connection failure

Message ID 20240701101243.2902-1-zhaochengyi@uniontech.com
State New
Headers show
Series adapter: Add retry when bonding device returns connection failure | expand

Commit Message

赵成义 July 1, 2024, 10:12 a.m. UTC 
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(+)

Comments

Luiz Augusto von Dentz July 1, 2024, 4:19 p.m. UTC | #1 
Hi,

On Mon, Jul 1, 2024 at 6:13 AM Chengyi Zhao <zhaochengyi@uniontech.com> 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. */
赵成义 July 2, 2024, 3:10 a.m. UTC | #2 
Hi,

> 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. */

Great, I think you are right if kernel does not continue to return MGMT_STATUS_CONNECT_FAILED status.

My idea is to retry only once after returning MGMT_STATUS_CONNECT_FAILED,
it can avoid repeated retry when continuing to return MGMT_STATUS_CONNECT_FAILED.

Cheers,
Chengyi
赵成义 July 3, 2024, 2:48 a.m. UTC | #3 
Hi guys,

I would like to ask if the patch I submitted is valuable. 
Maybe its effect is weak, but do you agree to merge it? Thanks.

>
>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);

Cheers,
Chengyi
diff mbox series

Patch

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);