Fix intermittent BLE connection failures (#23598)

* Fix intermittent BLE connection failures 1. Rename two BTP constants to match BTP_CONN_RSP_TIMEOUT and BTP_ACK_TIMEOUT from the spec. 2. Move the constants to BleConfig.h 3. Use BTP_ACK_TIMEOUT in both secure and unauthenticated session for ACK timeout calculation. Signed-off-by: Damian Krolik <[email protected]> * Code review Signed-off-by: Damian Krolik <[email protected]>
project-chip · Jul 7, 2023 · 1420702 · 1420702
1 parent e62d4c3
commit 1420702
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 30 deletions.
diff --git a/src/ble/BLEEndPoint.cpp b/src/ble/BLEEndPoint.cpp
@@ -69,17 +69,6 @@
  */
 #define BLE_CONFIG_IMMEDIATE_ACK_WINDOW_THRESHOLD                   1
 
-/**
- * @def BLE_CONNECT_TIMEOUT_MS
- *
- * @brief
- *   This is the amount of time, in milliseconds, after a BLE end point initiates a transport protocol connection
- *   or receives the initial portion of a connect request before the end point will automatically release its BLE
- *   connection and free itself if the transport connection has not been established.
- *
- */
-#define BLE_CONNECT_TIMEOUT_MS                                15000 // 15 seconds
-
 /**
  *  @def BLE_UNSUBSCRIBE_TIMEOUT_MS
  *
@@ -90,7 +79,6 @@
  */
 #define BLE_UNSUBSCRIBE_TIMEOUT_MS                            5000 // 5 seconds
 
-#define BTP_ACK_RECEIVED_TIMEOUT_MS                          15000 // 15 seconds
 #define BTP_ACK_SEND_TIMEOUT_MS                               2500 // 2.5 seconds
 
 #define BTP_WINDOW_NO_ACK_SEND_THRESHOLD                         1 // Data fragments may only be sent without piggybacked
@@ -1419,7 +1407,7 @@ bool BLEEndPoint::SendIndication(PacketBufferHandle && buf)
 CHIP_ERROR BLEEndPoint::StartConnectTimer()
 {
     const CHIP_ERROR timerErr =
-        mBle->mSystemLayer->StartTimer(System::Clock::Milliseconds32(BLE_CONNECT_TIMEOUT_MS), HandleConnectTimeout, this);
+        mBle->mSystemLayer->StartTimer(System::Clock::Milliseconds32(BTP_CONN_RSP_TIMEOUT_MS), HandleConnectTimeout, this);
     ReturnErrorOnFailure(timerErr);
     mTimerStateFlags.Set(TimerStateFlag::kConnectTimerRunning);
 
@@ -1428,8 +1416,8 @@ CHIP_ERROR BLEEndPoint::StartConnectTimer()
 
 CHIP_ERROR BLEEndPoint::StartReceiveConnectionTimer()
 {
-    const CHIP_ERROR timerErr =
-        mBle->mSystemLayer->StartTimer(System::Clock::Milliseconds32(BLE_CONNECT_TIMEOUT_MS), HandleReceiveConnectionTimeout, this);
+    const CHIP_ERROR timerErr = mBle->mSystemLayer->StartTimer(System::Clock::Milliseconds32(BTP_CONN_RSP_TIMEOUT_MS),
+                                                               HandleReceiveConnectionTimeout, this);
     ReturnErrorOnFailure(timerErr);
     mTimerStateFlags.Set(TimerStateFlag::kReceiveConnectionTimerRunning);
 
@@ -1440,8 +1428,8 @@ CHIP_ERROR BLEEndPoint::StartAckReceivedTimer()
 {
     if (!mTimerStateFlags.Has(TimerStateFlag::kAckReceivedTimerRunning))
     {
-        const CHIP_ERROR timerErr = mBle->mSystemLayer->StartTimer(System::Clock::Milliseconds32(BTP_ACK_RECEIVED_TIMEOUT_MS),
-                                                                   HandleAckReceivedTimeout, this);
+        const CHIP_ERROR timerErr =
+            mBle->mSystemLayer->StartTimer(System::Clock::Milliseconds32(BTP_ACK_TIMEOUT_MS), HandleAckReceivedTimeout, this);
         ReturnErrorOnFailure(timerErr);
 
         mTimerStateFlags.Set(TimerStateFlag::kAckReceivedTimerRunning);

diff --git a/src/ble/BleConfig.h b/src/ble/BleConfig.h
@@ -105,16 +105,6 @@
 #define BLE_CONNECTION_OBJECT void*
 #endif // BLE_CONNECTION_OBJECT
 
-/**
- *  @def BLE_CONFIG_BLUEZ_MTU_FEATURE
- *
- *  @brief
- *    This define if BLUEZ MTU FEATURE is enabled or not
- */
-#ifndef BLE_CONFIG_BLUEZ_MTU_FEATURE
-#define BLE_CONFIG_BLUEZ_MTU_FEATURE 0
-#endif // BLE_CONFIG_BLUEZ_MTU_FEATURE
-
 /**
  *  @def BLE_CONNECTION_UNINITIALIZED
  *
@@ -207,6 +197,29 @@
 #define BLE_CONFIG_ERROR(e) (BLE_CONFIG_ERROR_MIN + (e))
 #endif // BLE_CONFIG_ERROR
 
+
+/**
+ * @def BTP_CONN_RSP_TIMEOUT_MS
+ *
+ * @brief
+ *   Maximum amount of time, in milliseconds, after sending or receiving a BTP Session Handshake
+ *   request to wait for connection establishment.
+ */
+#ifndef BTP_CONN_RSP_TIMEOUT_MS
+#define BTP_CONN_RSP_TIMEOUT_MS 15000 // 15 seconds
+#endif // BTP_CONN_RSP_TIMEOUT_MS
+
+/**
+ * @def BTP_ACK_TIMEOUT_MS
+ *
+ * @brief
+ *   Maximum amount of time, in milliseconds, after sending a BTP packet to wait for
+ *   an acknowledgement. When the ack is not received within this period the BTP session is closed.
+ */
+#ifndef BTP_ACK_TIMEOUT_MS
+#define BTP_ACK_TIMEOUT_MS 15000 // 15 seconds
+#endif // BTP_ACK_TIMEOUT_MS
+
 // clang-format on
 
 #include <lib/core/CHIPConfig.h>
diff --git a/src/transport/SecureSession.h b/src/transport/SecureSession.h
@@ -22,6 +22,7 @@
 #pragma once
 
 #include <app/util/basic-types.h>
+#include <ble/BleConfig.h>
 #include <lib/core/ReferenceCounted.h>
 #include <messaging/ReliableMessageProtocolConfig.h>
 #include <transport/CryptoContext.h>
@@ -167,9 +168,7 @@ class SecureSession : public Session, public ReferenceCounted<SecureSession, Sec
         case Transport::Type::kTcp:
             return System::Clock::Seconds16(30);
         case Transport::Type::kBle:
-            // TODO: Figure out what this should be, but zero is not the right
-            // answer.
-            return System::Clock::Seconds16(5);
+            return System::Clock::Milliseconds32(BTP_ACK_TIMEOUT_MS);
         default:
             break;
         }

diff --git a/src/transport/UnauthenticatedSessionTable.h b/src/transport/UnauthenticatedSessionTable.h
@@ -16,6 +16,7 @@
  */
 #pragma once
 
+#include <ble/BleConfig.h>
 #include <lib/core/CHIPError.h>
 #include <lib/core/ReferenceCounted.h>
 #include <lib/support/CodeUtils.h>
@@ -91,6 +92,8 @@ class UnauthenticatedSession : public Session,
                                             GetLastPeerActivityTime(), kMinActiveTime);
         case Transport::Type::kTcp:
             return System::Clock::Seconds16(30);
+        case Transport::Type::kBle:
+            return System::Clock::Milliseconds32(BTP_ACK_TIMEOUT_MS);
         default:
             break;
         }