From 7d3f5bb1a66bee018b5fc0b205d6fb2cafb89849 Mon Sep 17 00:00:00 2001 From: Damian Krolik Date: Mon, 14 Nov 2022 16:57:51 +0100 Subject: [PATCH 1/2] 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 --- src/ble/BLEEndPoint.cpp | 20 +++---------- src/ble/BleConfig.h | 33 ++++++++++++++------- src/transport/SecureSession.h | 5 ++-- src/transport/UnauthenticatedSessionTable.h | 3 ++ 4 files changed, 32 insertions(+), 29 deletions(-) diff --git a/src/ble/BLEEndPoint.cpp b/src/ble/BLEEndPoint.cpp index 1b35f0777b2fcd..669888f776dd54 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), HandleConnectTimeout, this); ReturnErrorOnFailure(timerErr); mTimerStateFlags.Set(TimerStateFlag::kConnectTimerRunning); @@ -1429,7 +1417,7 @@ CHIP_ERROR BLEEndPoint::StartConnectTimer() CHIP_ERROR BLEEndPoint::StartReceiveConnectionTimer() { const CHIP_ERROR timerErr = - mBle->mSystemLayer->StartTimer(System::Clock::Milliseconds32(BLE_CONNECT_TIMEOUT_MS), HandleReceiveConnectionTimeout, this); + mBle->mSystemLayer->StartTimer(System::Clock::Milliseconds32(BTP_CONN_RSP_TIMEOUT), 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), HandleAckReceivedTimeout, this); ReturnErrorOnFailure(timerErr); mTimerStateFlags.Set(TimerStateFlag::kAckReceivedTimerRunning); diff --git a/src/ble/BleConfig.h b/src/ble/BleConfig.h index c0fde2854f68ff..1b64535fc55101 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 + * + * @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 +#define BTP_CONN_RSP_TIMEOUT 15000 // 15 seconds +#endif // BTP_CONN_RSP_TIMEOUT + +/** + * @def BTP_ACK_TIMEOUT + * + * @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 +#define BTP_ACK_TIMEOUT 15000 // 15 seconds +#endif // BTP_ACK_TIMEOUT + // clang-format on #include diff --git a/src/transport/SecureSession.h b/src/transport/SecureSession.h index c7800b10ce736a..f7be7b84abc65b 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); default: break; } From 326cbe1df8f638a4a7a5cfd8bb29ac45495aae47 Mon Sep 17 00:00:00 2001 From: Damian Krolik Date: Tue, 15 Nov 2022 12:33:16 +0100 Subject: [PATCH 2/2] Code review --- src/ble/BLEEndPoint.cpp | 8 ++++---- src/ble/BleConfig.h | 16 ++++++++-------- src/transport/SecureSession.h | 2 +- src/transport/UnauthenticatedSessionTable.h | 2 +- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/ble/BLEEndPoint.cpp b/src/ble/BLEEndPoint.cpp index 669888f776dd54..2f59291efcd4cd 100644 --- a/src/ble/BLEEndPoint.cpp +++ b/src/ble/BLEEndPoint.cpp @@ -1407,7 +1407,7 @@ bool BLEEndPoint::SendIndication(PacketBufferHandle && buf) CHIP_ERROR BLEEndPoint::StartConnectTimer() { const CHIP_ERROR timerErr = - mBle->mSystemLayer->StartTimer(System::Clock::Milliseconds32(BTP_CONN_RSP_TIMEOUT), HandleConnectTimeout, this); + mBle->mSystemLayer->StartTimer(System::Clock::Milliseconds32(BTP_CONN_RSP_TIMEOUT_MS), HandleConnectTimeout, this); ReturnErrorOnFailure(timerErr); mTimerStateFlags.Set(TimerStateFlag::kConnectTimerRunning); @@ -1416,8 +1416,8 @@ CHIP_ERROR BLEEndPoint::StartConnectTimer() CHIP_ERROR BLEEndPoint::StartReceiveConnectionTimer() { - const CHIP_ERROR timerErr = - mBle->mSystemLayer->StartTimer(System::Clock::Milliseconds32(BTP_CONN_RSP_TIMEOUT), 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); @@ -1429,7 +1429,7 @@ CHIP_ERROR BLEEndPoint::StartAckReceivedTimer() if (!mTimerStateFlags.Has(TimerStateFlag::kAckReceivedTimerRunning)) { const CHIP_ERROR timerErr = - mBle->mSystemLayer->StartTimer(System::Clock::Milliseconds32(BTP_ACK_TIMEOUT), HandleAckReceivedTimeout, this); + 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 1b64535fc55101..91005a20882fc1 100644 --- a/src/ble/BleConfig.h +++ b/src/ble/BleConfig.h @@ -199,26 +199,26 @@ /** - * @def BTP_CONN_RSP_TIMEOUT + * @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 -#define BTP_CONN_RSP_TIMEOUT 15000 // 15 seconds -#endif // BTP_CONN_RSP_TIMEOUT +#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 + * @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 -#define BTP_ACK_TIMEOUT 15000 // 15 seconds -#endif // BTP_ACK_TIMEOUT +#ifndef BTP_ACK_TIMEOUT_MS +#define BTP_ACK_TIMEOUT_MS 15000 // 15 seconds +#endif // BTP_ACK_TIMEOUT_MS // clang-format on diff --git a/src/transport/SecureSession.h b/src/transport/SecureSession.h index f7be7b84abc65b..f1482ca77ae422 100644 --- a/src/transport/SecureSession.h +++ b/src/transport/SecureSession.h @@ -168,7 +168,7 @@ class SecureSession : public Session, public ReferenceCounted