Skip to content

Commit

Permalink
Fix intermittent BLE connection failures (#23598)
Browse files Browse the repository at this point in the history
* 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 <damian.krolik@nordicsemi.no>

* Code review

Signed-off-by: Damian Krolik <damian.krolik@nordicsemi.no>
  • Loading branch information
Damian-Nordic authored Nov 18, 2022
1 parent e241184 commit 54d8dd0
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 30 deletions.
22 changes: 5 additions & 17 deletions src/ble/BLEEndPoint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand All @@ -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
Expand Down Expand Up @@ -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);

Expand All @@ -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);

Expand All @@ -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);
Expand Down
33 changes: 23 additions & 10 deletions src/ble/BleConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down Expand Up @@ -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>
5 changes: 2 additions & 3 deletions src/transport/SecureSession.h
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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;
}
Expand Down
3 changes: 3 additions & 0 deletions src/transport/UnauthenticatedSessionTable.h
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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;
}
Expand Down

0 comments on commit 54d8dd0

Please sign in to comment.