From 1420702ac0c85e1c50c4b1c51c1e0870dc6ede83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20Kr=C3=B3lik?= <66667989+Damian-Nordic@users.noreply.github.com> Date: Fri, 18 Nov 2022 13:33:49 +0100 Subject: [PATCH] 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 * Code review Signed-off-by: Damian Krolik --- src/ble/BLEEndPoint.cpp | 22 ++++---------- src/ble/BleConfig.h | 33 ++++++++++++++------- src/transport/SecureSession.h | 5 ++-- src/transport/UnauthenticatedSessionTable.h | 3 ++ 4 files changed, 33 insertions(+), 30 deletions(-) diff --git a/src/ble/BLEEndPoint.cpp b/src/ble/BLEEndPoint.cpp index 1b35f0777b2fcd..2f59291efcd4cd 100644 --- 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 index c0fde2854f68ff..91005a20882fc1 100644 --- 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 diff --git a/src/transport/SecureSession.h b/src/transport/SecureSession.h index c7800b10ce736a..f1482ca77ae422 100644 --- a/src/transport/SecureSession.h +++ b/src/transport/SecureSession.h @@ -22,6 +22,7 @@ #pragma once #include +#include #include #include #include @@ -167,9 +168,7 @@ class SecureSession : public Session, public ReferenceCounted #include #include #include @@ -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; }