Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix intermittent BLE connection failures #23598

Merged
merged 2 commits into from
Nov 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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