[zephyr] Added several improvements to the BLEMgr implementation (#33189)
* [zephyr] Enabled support for bonding in the BLEManager
Currently the Zephyr BLE Manager rotates Bluetooth addresses
on every boot and it does this by creating new Bluetooth identity.
Because of that the Zephyr stack does not create default Bluetooth
ID, which is required e.g. for the bonding purposes.
Added creating two separate Bluetooth identities - for the Matter
service advertising and for the bonding purposes.
Signed-off-by: Kamil Kasperczyk <kamil.kasperczyk@nordicsemi.no>
* [zephyr] Added check to drop handling callbacks for BT central
Matter BLEManager handles all BT connect and disconnect callbacks
no matter if these are Matter related ones or not. It collides
with other not Matter-related services that trigger Matter
CHIPoBLE service advertising changes. Added role check that
allows to at least drop all callbacks related to BT central role.
Signed-off-by: Kamil Kasperczyk <kamil.kasperczyk@nordicsemi.no>
* [zephyr]: allow BLE advertising restarts
* allow BLE advertising restarts in case of failures
* which can be triggered by calling SetBLEAdvertisingEnabled(true)
ConnectivityMgr public API from the application code
* do not register CHIPoBLE GATT services when the advertising
cannot be started
* this allows the disconnection handler to filter out non-Matter
BLE connections that were terminated
* fix possible underflow of connection counters
Signed-off-by: Marcin Kajor <marcin.kajor@nordicsemi.no>
---------
Signed-off-by: Kamil Kasperczyk <kamil.kasperczyk@nordicsemi.no>
Signed-off-by: Marcin Kajor <marcin.kajor@nordicsemi.no>
Co-authored-by: Marcin Kajor <marcin.kajor@nordicsemi.no>
diff --git a/src/platform/Zephyr/BLEAdvertisingArbiter.cpp b/src/platform/Zephyr/BLEAdvertisingArbiter.cpp
index 29f5516..4356ba9 100644
--- a/src/platform/Zephyr/BLEAdvertisingArbiter.cpp
+++ b/src/platform/Zephyr/BLEAdvertisingArbiter.cpp
@@ -29,6 +29,9 @@
// List of advertising requests ordered by priority
sys_slist_t sRequests;
+bool sIsInitialized = false;
+uint8_t sBtId = 0;
+
// Cast an intrusive list node to the containing request object
const BLEAdvertisingArbiter::Request & ToRequest(const sys_snode_t * node)
{
@@ -55,8 +58,9 @@
ReturnErrorOnFailure(System::MapErrorZephyr(bt_le_adv_stop()));
ReturnErrorCodeIf(sys_slist_is_empty(&sRequests), CHIP_NO_ERROR);
- const Request & top = ToRequest(sys_slist_peek_head(&sRequests));
- const bt_le_adv_param params = BT_LE_ADV_PARAM_INIT(top.options, top.minInterval, top.maxInterval, nullptr);
+ const Request & top = ToRequest(sys_slist_peek_head(&sRequests));
+ bt_le_adv_param params = BT_LE_ADV_PARAM_INIT(top.options, top.minInterval, top.maxInterval, nullptr);
+ params.id = sBtId;
const int result = bt_le_adv_start(¶ms, top.advertisingData.data(), top.advertisingData.size(), top.scanResponseData.data(),
top.scanResponseData.size());
@@ -70,8 +74,26 @@
} // namespace
+CHIP_ERROR Init(uint8_t btId)
+{
+ if (sIsInitialized)
+ {
+ return CHIP_ERROR_INCORRECT_STATE;
+ }
+
+ sBtId = btId;
+ sIsInitialized = true;
+
+ return CHIP_NO_ERROR;
+}
+
CHIP_ERROR InsertRequest(Request & request)
{
+ if (!sIsInitialized)
+ {
+ return CHIP_ERROR_INCORRECT_STATE;
+ }
+
CancelRequest(request);
sys_snode_t * prev = nullptr;
@@ -109,6 +131,11 @@
void CancelRequest(Request & request)
{
+ if (!sIsInitialized)
+ {
+ return;
+ }
+
const bool isTopPriority = (sys_slist_peek_head(&sRequests) == &request);
VerifyOrReturn(sys_slist_find_and_remove(&sRequests, &request));
diff --git a/src/platform/Zephyr/BLEAdvertisingArbiter.h b/src/platform/Zephyr/BLEAdvertisingArbiter.h
index 6176c6c..a336b1c 100644
--- a/src/platform/Zephyr/BLEAdvertisingArbiter.h
+++ b/src/platform/Zephyr/BLEAdvertisingArbiter.h
@@ -62,6 +62,18 @@
};
/**
+ * @brief Initialize BLE advertising arbiter
+ *
+ * @note This method must be called before trying to insert or cancel any requests.
+ *
+ * @param btId Local Bluetooth LE identifier to be used for the advertising parameters. Currently Bluetooth LE identifier used in
+ * this method will be used for all advertising requests and changing it dynamically is not supported.
+ * @return error If the module is already initialized.
+ * @return success Otherwise.
+ */
+CHIP_ERROR Init(uint8_t btId);
+
+/**
* @brief Request BLE advertising
*
* Add the request to the internal list of competing requests. If the request
@@ -74,6 +86,9 @@
* @note This method does not take ownership of the request object so the object
* must not get destroyed before it is cancelled.
*
+ * @note The arbiter module has to be initialized using Init() method before
+ * invoking this method.
+ *
* @param request Reference to advertising request that contains priority and
* other advertising parameters.
* @return error If the request is top-priority and failed to restart the
@@ -94,6 +109,9 @@
* An attempt to cancel a request that has not been registered at the
* advertising arbiter is a no-op. That is, it returns immediately.
*
+ * @note The arbiter module has to be initialized using Init() method before
+ * invoking this method.
+ *
* @param request Reference to advertising request that contains priority and
* other advertising parameters.
*/
diff --git a/src/platform/Zephyr/BLEManagerImpl.cpp b/src/platform/Zephyr/BLEManagerImpl.cpp
index 59898c6..09bb9c0 100644
--- a/src/platform/Zephyr/BLEManagerImpl.cpp
+++ b/src/platform/Zephyr/BLEManagerImpl.cpp
@@ -45,6 +45,10 @@
#include <zephyr/sys/byteorder.h>
#include <zephyr/sys/util.h>
+#ifdef CONFIG_BT_BONDABLE
+#include <zephyr/settings/settings.h>
+#endif // CONFIG_BT_BONDABLE
+
#include <array>
using namespace ::chip;
@@ -108,7 +112,13 @@
// This value should be adjusted accordingly if the service declaration changes.
constexpr int kCHIPoBLE_CCC_AttributeIndex = 3;
-CHIP_ERROR InitRandomStaticAddress()
+#ifdef CONFIG_BT_BONDABLE
+constexpr uint8_t kMatterBleIdentity = 1;
+#else
+constexpr uint8_t kMatterBleIdentity = 0;
+#endif // CONFIG_BT_BONDABLE
+
+int InitRandomStaticAddress(bool idPresent, int & id)
{
// Generate a random static address for the default identity.
// This must be done before bt_enable() as after that updating the default identity is not possible.
@@ -123,20 +133,29 @@
if (error)
{
ChipLogError(DeviceLayer, "Failed to create BLE address: %d", error);
- return System::MapErrorZephyr(error);
+ return error;
}
- error = bt_id_create(&addr, nullptr);
+ if (!idPresent)
+ {
+ id = bt_id_create(&addr, nullptr);
+ }
+#if CONFIG_BT_ID_MAX == 2
+ else
+ {
+ id = bt_id_reset(1, &addr, nullptr);
+ }
+#endif // CONFIG_BT_BONDABLE
- if (error < 0)
+ if (id < 0)
{
ChipLogError(DeviceLayer, "Failed to create BLE identity: %d", error);
- return System::MapErrorZephyr(error);
+ return id;
}
ChipLogProgress(DeviceLayer, "BLE address: %02X:%02X:%02X:%02X:%02X:%02X", addr.a.val[5], addr.a.val[4], addr.a.val[3],
addr.a.val[2], addr.a.val[1], addr.a.val[0]);
- return CHIP_NO_ERROR;
+ return 0;
}
} // unnamed namespace
@@ -145,17 +164,42 @@
CHIP_ERROR BLEManagerImpl::_Init()
{
+ int err = 0;
+ int id = 0;
+
mServiceMode = ConnectivityManager::kCHIPoBLEServiceMode_Enabled;
mFlags.ClearAll().Set(Flags::kAdvertisingEnabled, CHIP_DEVICE_CONFIG_CHIPOBLE_ENABLE_ADVERTISING_AUTOSTART);
mFlags.Set(Flags::kFastAdvertisingEnabled, true);
- mGAPConns = 0;
+ mMatterConnNum = 0;
+ mTotalConnNum = 0;
memset(mSubscribedConns, 0, sizeof(mSubscribedConns));
- ReturnErrorOnFailure(InitRandomStaticAddress());
- int err = bt_enable(NULL);
+#ifdef CONFIG_BT_BONDABLE
+ bt_addr_le_t idsAddr[CONFIG_BT_ID_MAX];
+ size_t idsCount = CONFIG_BT_ID_MAX;
+
+ err = bt_enable(nullptr);
+
VerifyOrReturnError(err == 0, MapErrorZephyr(err));
+ settings_load();
+
+ bt_id_get(idsAddr, &idsCount);
+
+ err = InitRandomStaticAddress(idsCount > 1, id);
+
+ VerifyOrReturnError(err == 0 && id == kMatterBleIdentity, MapErrorZephyr(err));
+
+#else
+ err = InitRandomStaticAddress(false, id);
+ VerifyOrReturnError(err == 0 && id == kMatterBleIdentity, MapErrorZephyr(err));
+ err = bt_enable(nullptr);
+ VerifyOrReturnError(err == 0, MapErrorZephyr(err));
+#endif // CONFIG_BT_BONDABLE
+
+ BLEAdvertisingArbiter::Init(static_cast<uint8_t>(id));
+
memset(&mConnCallbacks, 0, sizeof(mConnCallbacks));
mConnCallbacks.connected = HandleConnect;
mConnCallbacks.disconnected = HandleDisconnect;
@@ -200,7 +244,13 @@
{
mFlags.Clear(Flags::kAdvertisingRefreshNeeded);
err = StartAdvertising();
- SuccessOrExit(err);
+ if (err != CHIP_NO_ERROR)
+ {
+ // Return prematurely but keep the CHIPoBLE service mode enabled to allow advertising retries
+ mServiceMode = ConnectivityManager::kCHIPoBLEServiceMode_Enabled;
+ ChipLogError(DeviceLayer, "Could not start CHIPoBLE service due to error: %" CHIP_ERROR_FORMAT, err.Format());
+ return;
+ }
}
}
else
@@ -208,7 +258,12 @@
if (mFlags.Has(Flags::kAdvertising))
{
err = StopAdvertising();
- SuccessOrExit(err);
+ if (err != CHIP_NO_ERROR)
+ {
+ ChipLogError(DeviceLayer, "Disabling CHIPoBLE service due to error: %" CHIP_ERROR_FORMAT, err.Format());
+ mServiceMode = ConnectivityManager::kCHIPoBLEServiceMode_Disabled;
+ return;
+ }
}
// If no connections are active unregister also CHIPoBLE GATT service
@@ -225,13 +280,6 @@
}
}
}
-
-exit:
- if (err != CHIP_NO_ERROR)
- {
- ChipLogError(DeviceLayer, "Disabling CHIPoBLE service due to error: %" CHIP_ERROR_FORMAT, err.Format());
- mServiceMode = ConnectivityManager::kCHIPoBLEServiceMode_Disabled;
- }
}
struct BLEManagerImpl::ServiceData
@@ -297,29 +345,53 @@
else
{
ChipLogError(DeviceLayer, "Failed to start CHIPoBLE advertising: %d", rc);
+ BLEManagerImpl().StopAdvertising();
}
};
return CHIP_NO_ERROR;
}
+CHIP_ERROR BLEManagerImpl::RegisterGattService()
+{
+ // Register CHIPoBLE GATT service
+ if (!mFlags.Has(Flags::kChipoBleGattServiceRegister))
+ {
+ int err = bt_gatt_service_register(&sChipoBleService);
+ if (err != 0)
+ {
+ ChipLogError(DeviceLayer, "Failed to register CHIPoBLE GATT service: %d", err);
+ }
+
+ VerifyOrReturnError(err == 0, MapErrorZephyr(err));
+ mFlags.Set(Flags::kChipoBleGattServiceRegister);
+ }
+ return CHIP_NO_ERROR;
+}
+
+CHIP_ERROR BLEManagerImpl::UnregisterGattService()
+{
+ // Unregister CHIPoBLE GATT service
+ if (mFlags.Has(Flags::kChipoBleGattServiceRegister))
+ {
+ int err = bt_gatt_service_unregister(&sChipoBleService);
+ if (err != 0)
+ {
+ ChipLogError(DeviceLayer, "Failed to unregister CHIPoBLE GATT service: %d", err);
+ }
+
+ VerifyOrReturnError(err == 0, MapErrorZephyr(err));
+ mFlags.Clear(Flags::kChipoBleGattServiceRegister);
+ }
+ return CHIP_NO_ERROR;
+}
+
CHIP_ERROR BLEManagerImpl::StartAdvertising()
{
// Prepare advertising request
ReturnErrorOnFailure(PrepareAdvertisingRequest());
-
- // Register dynamically CHIPoBLE GATT service
- if (!mFlags.Has(Flags::kChipoBleGattServiceRegister))
- {
- int err = bt_gatt_service_register(&sChipoBleService);
-
- if (err != 0)
- ChipLogError(DeviceLayer, "Failed to register CHIPoBLE GATT service");
-
- VerifyOrReturnError(err == 0, MapErrorZephyr(err));
-
- mFlags.Set(Flags::kChipoBleGattServiceRegister);
- }
+ // We need to register GATT service before issuing the advertising to start
+ ReturnErrorOnFailure(RegisterGattService());
// Initialize C3 characteristic data
#if CHIP_ENABLE_ADDITIONAL_DATA_ADVERTISING
@@ -327,7 +399,13 @@
#endif
// Request advertising
- ReturnErrorOnFailure(BLEAdvertisingArbiter::InsertRequest(mAdvertisingRequest));
+ CHIP_ERROR err = BLEAdvertisingArbiter::InsertRequest(mAdvertisingRequest);
+ if (CHIP_NO_ERROR != err)
+ {
+ // It makes not sense to keep GATT services registered after the advertising request failed
+ (void) UnregisterGattService();
+ return err;
+ }
// Transition to the Advertising state...
if (!mFlags.Has(Flags::kAdvertising))
@@ -389,22 +467,23 @@
DeviceLayer::SystemLayer().CancelTimer(HandleSlowBLEAdvertisementInterval, this);
DeviceLayer::SystemLayer().CancelTimer(HandleExtendedBLEAdvertisementInterval, this);
}
+ else
+ {
+ ChipLogProgress(DeviceLayer, "CHIPoBLE advertising already stopped");
+ }
return CHIP_NO_ERROR;
}
CHIP_ERROR BLEManagerImpl::_SetAdvertisingEnabled(bool val)
{
- if (mFlags.Has(Flags::kAdvertisingEnabled) != val)
- {
- ChipLogDetail(DeviceLayer, "CHIPoBLE advertising set to %s", val ? "on" : "off");
+ ChipLogDetail(DeviceLayer, "CHIPoBLE advertising set to %s", val ? "on" : "off");
- mFlags.Set(Flags::kAdvertisingEnabled, val);
- // Ensure that each enabling/disabling of the standard advertising clears
- // the extended mode flag.
- mFlags.Set(Flags::kExtendedAdvertisingEnabled, false);
- PlatformMgr().ScheduleWork(DriveBLEState, 0);
- }
+ mFlags.Set(Flags::kAdvertisingEnabled, val);
+ // Ensure that each enabling/disabling of the general advertising clears
+ // the extended mode, to make sure we always start fresh in the regular mode
+ mFlags.Set(Flags::kExtendedAdvertisingEnabled, false);
+ PlatformMgr().ScheduleWork(DriveBLEState, 0);
return CHIP_NO_ERROR;
}
@@ -453,15 +532,13 @@
if (connEvent->HciResult == BT_HCI_ERR_SUCCESS)
{
ChipLogProgress(DeviceLayer, "BLE connection established (ConnId: 0x%02x)", bt_conn_index(connEvent->BtConn));
- mGAPConns++;
+ mMatterConnNum++;
}
else
{
ChipLogError(DeviceLayer, "BLE connection failed (reason: 0x%02x)", connEvent->HciResult);
}
- ChipLogProgress(DeviceLayer, "Current number of connections: %u/%u", NumConnections(), CONFIG_BT_MAX_CONN);
-
mFlags.Set(Flags::kAdvertisingRefreshNeeded);
PlatformMgr().ScheduleWork(DriveBLEState, 0);
@@ -476,7 +553,10 @@
ChipLogProgress(DeviceLayer, "BLE GAP connection terminated (reason 0x%02x)", connEvent->HciResult);
- mGAPConns--;
+ if (mMatterConnNum > 0)
+ {
+ mMatterConnNum--;
+ }
// If indications were enabled for this connection, record that they are now disabled and
// notify the BLE Layer of a disconnect.
@@ -504,8 +584,6 @@
// Unref bt_conn before scheduling DriveBLEState.
bt_conn_unref(connEvent->BtConn);
- ChipLogProgress(DeviceLayer, "Current number of connections: %u/%u", NumConnections(), CONFIG_BT_MAX_CONN);
-
ChipDeviceEvent disconnectEvent;
disconnectEvent.Type = DeviceEventType::kCHIPoBLEConnectionClosed;
ReturnErrorOnFailure(PlatformMgr().PostEvent(&disconnectEvent));
@@ -665,7 +743,7 @@
uint16_t BLEManagerImpl::_NumConnections(void)
{
- return mGAPConns;
+ return mMatterConnNum;
}
bool BLEManagerImpl::CloseConnection(BLE_CONNECTION_OBJECT conId)
@@ -841,9 +919,16 @@
void BLEManagerImpl::HandleConnect(struct bt_conn * conId, uint8_t err)
{
ChipDeviceEvent event;
+ bt_conn_info bt_info;
PlatformMgr().LockChipStack();
+ sInstance.mTotalConnNum++;
+ ChipLogProgress(DeviceLayer, "Current number of connections: %u/%u", sInstance.mTotalConnNum, CONFIG_BT_MAX_CONN);
+
+ VerifyOrExit(bt_conn_get_info(conId, &bt_info) == 0, );
+ // Drop all callbacks incoming for the role other than peripheral, required by the Matter accessory
+ VerifyOrExit(bt_info.role == BT_CONN_ROLE_PERIPHERAL, );
// Don't handle BLE connecting events when it is not related to CHIPoBLE
VerifyOrExit(sInstance.mFlags.Has(Flags::kChipoBleGattServiceRegister), );
@@ -860,9 +945,20 @@
void BLEManagerImpl::HandleDisconnect(struct bt_conn * conId, uint8_t reason)
{
ChipDeviceEvent event;
+ bt_conn_info bt_info;
PlatformMgr().LockChipStack();
+ if (sInstance.mTotalConnNum > 0)
+ {
+ sInstance.mTotalConnNum--;
+ }
+
+ ChipLogProgress(DeviceLayer, "Current number of connections: %u/%u", sInstance.mTotalConnNum, CONFIG_BT_MAX_CONN);
+
+ VerifyOrExit(bt_conn_get_info(conId, &bt_info) == 0, );
+ // Drop all callbacks incoming for the role other than peripheral, required by the Matter accessory
+ VerifyOrExit(bt_info.role == BT_CONN_ROLE_PERIPHERAL, );
// Don't handle BLE disconnecting events when it is not related to CHIPoBLE
VerifyOrExit(sInstance.mFlags.Has(Flags::kChipoBleGattServiceRegister), );
diff --git a/src/platform/Zephyr/BLEManagerImpl.h b/src/platform/Zephyr/BLEManagerImpl.h
index 4fb3352..10ecdf0 100644
--- a/src/platform/Zephyr/BLEManagerImpl.h
+++ b/src/platform/Zephyr/BLEManagerImpl.h
@@ -97,7 +97,7 @@
struct ServiceData;
BitFlags<Flags> mFlags;
- uint16_t mGAPConns;
+ uint16_t mMatterConnNum;
CHIPoBLEServiceMode mServiceMode;
bool mSubscribedConns[CONFIG_BT_MAX_CONN];
bt_gatt_indicate_params mIndicateParams[CONFIG_BT_MAX_CONN];
@@ -106,6 +106,8 @@
#if CHIP_ENABLE_ADDITIONAL_DATA_ADVERTISING
PacketBufferHandle c3CharDataBufferHandle;
#endif
+ // The summarized number of Bluetooth LE connections related to the device (including these not related to Matter service).
+ uint16_t mTotalConnNum;
void DriveBLEState(void);
CHIP_ERROR PrepareAdvertisingRequest();
@@ -123,6 +125,8 @@
bool SetSubscribed(bt_conn * conn);
bool UnsetSubscribed(bt_conn * conn);
uint32_t GetAdvertisingInterval();
+ CHIP_ERROR RegisterGattService();
+ CHIP_ERROR UnregisterGattService();
static void DriveBLEState(intptr_t arg);
