Skip to content

Commit

Permalink
Changed BLE notifications to indications (#19051)
Browse files Browse the repository at this point in the history 
According to the most recent spec changes, BTP shall use GATT
indications instead of notifications (4.17.3.2).

Changed BLE notifications to indications for nRFConnect
and Android platform.

Tested manually that commissioning over BLE works with nRFConnect
and Linux/Android CHIPTool.
  • Loading branch information
@kkasperczyk-no @pull
kkasperczyk-no authored and pull[bot] committed Jun 8, 2022
1 parent 85a0570 commit 76c8fde
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 46 deletions.
75 changes: 39 additions & 36 deletions src/platform/Zephyr/BLEManagerImpl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ struct bt_gatt_attr sChipoBleAttributes[] = {
BT_GATT_PERM_READ | BT_GATT_PERM_WRITE,
nullptr, BLEManagerImpl::HandleRXWrite, nullptr),
BT_GATT_CHARACTERISTIC(&UUID128_CHIPoBLEChar_TX.uuid,
BT_GATT_CHRC_NOTIFY,
BT_GATT_CHRC_INDICATE,
BT_GATT_PERM_NONE,
nullptr, nullptr, nullptr),
BT_GATT_CCC_MANAGED(&CHIPoBLEChar_TX_CCC, BT_GATT_PERM_READ | BT_GATT_PERM_WRITE),
Expand Down Expand Up @@ -470,7 +470,7 @@ CHIP_ERROR BLEManagerImpl::HandleGAPDisconnect(const ChipDeviceEvent * event)

mGAPConns--;

// If notifications were enabled for this connection, record that they are now disabled and
// If indications were enabled for this connection, record that they are now disabled and
// notify the BLE Layer of a disconnect.
if (UnsetSubscribed(connEvent->BtConn))
{
Expand Down Expand Up @@ -516,8 +516,8 @@ CHIP_ERROR BLEManagerImpl::HandleTXCharCCCDWrite(const ChipDeviceEvent * event)

ChipLogDetail(DeviceLayer, "ConnId: 0x%02x, New CCCD value: 0x%04x", bt_conn_index(writeEvent->BtConn), writeEvent->Value);

// If the client has requested to enable notifications and if it is not yet subscribed
if (writeEvent->Value != 0 && SetSubscribed(writeEvent->BtConn))
// If the client has requested to enable indications and if it is not yet subscribed
if (writeEvent->Value == BT_GATT_CCC_INDICATE && SetSubscribed(writeEvent->BtConn))
{
// Alert the BLE layer that CHIPoBLE "subscribe" has been received and increment the bt_conn reference counter.
HandleSubscribeReceived(writeEvent->BtConn, &CHIP_BLE_SVC_ID, &chipUUID_CHIPoBLEChar_TX);
Expand Down Expand Up @@ -547,26 +547,28 @@ CHIP_ERROR BLEManagerImpl::HandleTXCharCCCDWrite(const ChipDeviceEvent * event)

CHIP_ERROR BLEManagerImpl::HandleRXCharWrite(const ChipDeviceEvent * event)
{
const BleRXWriteEventType * writeEvent = &event->Platform.BleRXWriteEvent;
const BleC1WriteEventType * c1WriteEvent = &event->Platform.BleC1WriteEvent;

ChipLogDetail(DeviceLayer, "Write request received for CHIPoBLE RX (ConnId 0x%02x)", bt_conn_index(writeEvent->BtConn));
ChipLogDetail(DeviceLayer, "Write request received for CHIPoBLE RX characteristic (ConnId 0x%02x)",
bt_conn_index(c1WriteEvent->BtConn));

HandleWriteReceived(writeEvent->BtConn, &CHIP_BLE_SVC_ID, &chipUUID_CHIPoBLEChar_RX,
PacketBufferHandle::Adopt(writeEvent->Data));
bt_conn_unref(writeEvent->BtConn);
HandleWriteReceived(c1WriteEvent->BtConn, &CHIP_BLE_SVC_ID, &chipUUID_CHIPoBLEChar_RX,
PacketBufferHandle::Adopt(c1WriteEvent->Data));
bt_conn_unref(c1WriteEvent->BtConn);

return CHIP_NO_ERROR;
}

CHIP_ERROR BLEManagerImpl::HandleTXCharComplete(const ChipDeviceEvent * event)
{
const BleTXCompleteEventType * completeEvent = &event->Platform.BleTXCompleteEvent;
const BleC2IndDoneEventType * c2IndDoneEvent = &event->Platform.BleC2IndDoneEvent;

ChipLogDetail(DeviceLayer, "Notification for CHIPoBLE TX done (ConnId 0x%02x)", bt_conn_index(completeEvent->BtConn));
ChipLogDetail(DeviceLayer, "Indication for CHIPoBLE TX characteristic done (ConnId 0x%02x, result 0x%02x)",
bt_conn_index(c2IndDoneEvent->BtConn), c2IndDoneEvent->Result);

// Signal the BLE Layer that the outstanding notification is complete.
HandleIndicationConfirmation(completeEvent->BtConn, &CHIP_BLE_SVC_ID, &chipUUID_CHIPoBLEChar_TX);
bt_conn_unref(completeEvent->BtConn);
// Signal the BLE Layer that the outstanding indication is complete.
HandleIndicationConfirmation(c2IndDoneEvent->BtConn, &CHIP_BLE_SVC_ID, &chipUUID_CHIPoBLEChar_TX);
bt_conn_unref(c2IndDoneEvent->BtConn);

return CHIP_NO_ERROR;
}
Expand Down Expand Up @@ -633,11 +635,11 @@ void BLEManagerImpl::_OnPlatformEvent(const ChipDeviceEvent * event)
err = HandleTXCharCCCDWrite(event);
break;

case DeviceEventType::kPlatformZephyrBleRXWrite:
case DeviceEventType::kPlatformZephyrBleC1WriteEvent:
err = HandleRXCharWrite(event);
break;

case DeviceEventType::kPlatformZephyrBleTXComplete:
case DeviceEventType::kPlatformZephyrBleC2IndDoneEvent:
err = HandleTXCharComplete(event);
break;

Expand Down Expand Up @@ -706,29 +708,29 @@ bool BLEManagerImpl::UnsubscribeCharacteristic(BLE_CONNECTION_OBJECT conId, cons
bool BLEManagerImpl::SendIndication(BLE_CONNECTION_OBJECT conId, const ChipBleUUID * svcId, const ChipBleUUID * charId,
PacketBufferHandle pBuf)
{
CHIP_ERROR err = CHIP_NO_ERROR;
int status = 0;
uint8_t index = bt_conn_index(conId);
bt_gatt_notify_params * params = &mNotifyParams[index];
CHIP_ERROR err = CHIP_NO_ERROR;
int status = 0;
uint8_t index = bt_conn_index(conId);
bt_gatt_indicate_params * params = &mIndicateParams[index];

VerifyOrExit(IsSubscribed(conId) == true, err = CHIP_ERROR_INVALID_ARGUMENT);

ChipLogDetail(DeviceLayer, "Sending notification for CHIPoBLE TX (ConnId %02x, len %u)", index, pBuf->DataLength());
ChipLogDetail(DeviceLayer, "Sending indication for CHIPoBLE TX characteristic (ConnId %02x, len %u)", index,
pBuf->DataLength());

params->uuid = nullptr;
params->attr = &sChipoBleAttributes[kCHIPoBLE_CCC_AttributeIndex];
params->data = pBuf->Start();
params->len = pBuf->DataLength();
params->func = HandleTXCompleted;
params->user_data = nullptr;
params->uuid = nullptr;
params->attr = &sChipoBleAttributes[kCHIPoBLE_CCC_AttributeIndex];
params->func = HandleTXIndicated;
params->data = pBuf->Start();
params->len = pBuf->DataLength();

status = bt_gatt_notify_cb(conId, params);
status = bt_gatt_indicate(conId, params);
VerifyOrExit(status == 0, err = MapErrorZephyr(status));

exit:
if (err != CHIP_NO_ERROR)
{
ChipLogError(DeviceLayer, "Sending notification for CHIPoBLE TX failed: %s", ErrorStr(err));
ChipLogError(DeviceLayer, "BLEManagerImpl::SendIndication() failed: %s", ErrorStr(err));
}

return err == CHIP_NO_ERROR;
Expand Down Expand Up @@ -804,9 +806,9 @@ ssize_t BLEManagerImpl::HandleRXWrite(struct bt_conn * conId, const struct bt_ga
if (!packetBuf.IsNull())
{
// Arrange to post a CHIPoBLERXWriteEvent event to the CHIP queue.
event.Type = DeviceEventType::kPlatformZephyrBleRXWrite;
event.Platform.BleRXWriteEvent.BtConn = bt_conn_ref(conId);
event.Platform.BleRXWriteEvent.Data = std::move(packetBuf).UnsafeRelease();
event.Type = DeviceEventType::kPlatformZephyrBleC1WriteEvent;
event.Platform.BleC1WriteEvent.BtConn = bt_conn_ref(conId);
event.Platform.BleC1WriteEvent.Data = std::move(packetBuf).UnsafeRelease();
}

// If we failed to allocate a buffer, post a kPlatformZephyrBleOutOfBuffersEvent event.
Expand All @@ -824,7 +826,7 @@ ssize_t BLEManagerImpl::HandleTXCCCWrite(struct bt_conn * conId, const struct bt
{
ChipDeviceEvent event;

if (value != BT_GATT_CCC_NOTIFY && value != 0)
if (value != BT_GATT_CCC_INDICATE && value != 0)
{
return BT_GATT_ERR(BT_ATT_ERR_VALUE_NOT_ALLOWED);
}
Expand All @@ -838,12 +840,13 @@ ssize_t BLEManagerImpl::HandleTXCCCWrite(struct bt_conn * conId, const struct bt
return sizeof(value);
}

void BLEManagerImpl::HandleTXCompleted(struct bt_conn * conId, void * /* param */)
void BLEManagerImpl::HandleTXIndicated(struct bt_conn * conId, IndicationAttrType, uint8_t err)
{
ChipDeviceEvent event;

event.Type = DeviceEventType::kPlatformZephyrBleTXComplete;
event.Platform.BleTXCompleteEvent.BtConn = bt_conn_ref(conId);
event.Type = DeviceEventType::kPlatformZephyrBleC2IndDoneEvent;
event.Platform.BleC2IndDoneEvent.BtConn = bt_conn_ref(conId);
event.Platform.BleC2IndDoneEvent.Result = err;

PlatformMgr().PostEventOrDie(&event);
}
Expand Down
13 changes: 11 additions & 2 deletions src/platform/Zephyr/BLEManagerImpl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@

#include <lib/support/logging/CHIPLogging.h>

#include <type_traits>

namespace chip {
namespace DeviceLayer {
namespace Internal {
Expand All @@ -47,6 +49,13 @@ class BLEManagerImpl final : public BLEManager, private BleLayer, private BlePla
friend BLEManager;

private:
// As a result of https://github.com/zephyrproject-rtos/zephyr/issues/29357, BLE indication
// callback parameter type has changed in recent Zephyr revisions. Select the compatible type
// below to support both versions for now.
using IndicationAttrType =
std::conditional_t<std::is_same<bt_gatt_indicate_func_t, void (*)(bt_conn *, bt_gatt_indicate_params *, uint8_t)>::value,
bt_gatt_indicate_params *, const bt_gatt_attr *>;

// ===== Members that implement the BLEManager internal interface.

CHIP_ERROR _Init(void);
Expand Down Expand Up @@ -101,7 +110,7 @@ class BLEManagerImpl final : public BLEManager, private BleLayer, private BlePla
uint16_t mGAPConns;
CHIPoBLEServiceMode mServiceMode;
bool mSubscribedConns[CONFIG_BT_MAX_CONN];
bt_gatt_notify_params mNotifyParams[CONFIG_BT_MAX_CONN];
bt_gatt_indicate_params mIndicateParams[CONFIG_BT_MAX_CONN];
bt_conn_cb mConnCallbacks;
#if CHIP_ENABLE_ADDITIONAL_DATA_ADVERTISING
PacketBufferHandle c3CharDataBufferHandle;
Expand All @@ -127,7 +136,7 @@ class BLEManagerImpl final : public BLEManager, private BleLayer, private BlePla
static void DriveBLEState(intptr_t arg);

// Below callbacks run from the system workqueue context and have a limited stack capacity.
static void HandleTXCompleted(bt_conn * conn, void * param);
static void HandleTXIndicated(bt_conn * conn, IndicationAttrType attr, uint8_t err);
static void HandleConnect(bt_conn * conn, uint8_t err);
static void HandleDisconnect(bt_conn * conn, uint8_t reason);
static void HandleBLEAdvertisementTimeout(System::Layer * layer, void * param);
Expand Down
13 changes: 7 additions & 6 deletions src/platform/Zephyr/CHIPDevicePlatformEvent.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ enum InternalPlatformSpecificEventTypes
kPlatformZephyrBleConnected,
kPlatformZephyrBleDisconnected,
kPlatformZephyrBleCCCWrite,
kPlatformZephyrBleRXWrite,
kPlatformZephyrBleTXComplete,
kPlatformZephyrBleC1WriteEvent,
kPlatformZephyrBleC2IndDoneEvent,
kPlatformZephyrBleOutOfBuffersEvent,
};

Expand All @@ -68,15 +68,16 @@ struct BleCCCWriteEventType
uint16_t Value;
};

struct BleRXWriteEventType
struct BleC1WriteEventType
{
bt_conn * BtConn;
::chip::System::PacketBuffer * Data;
};

struct BleTXCompleteEventType
struct BleC2IndDoneEventType
{
bt_conn * BtConn;
uint8_t Result;
};

/**
Expand All @@ -88,8 +89,8 @@ struct ChipDevicePlatformEvent final
{
BleConnEventType BleConnEvent;
BleCCCWriteEventType BleCCCWriteEvent;
BleRXWriteEventType BleRXWriteEvent;
BleTXCompleteEventType BleTXCompleteEvent;
BleC1WriteEventType BleC1WriteEvent;
BleC2IndDoneEventType BleC2IndDoneEvent;
};
};

Expand Down
4 changes: 2 additions & 2 deletions src/platform/android/java/chip/platform/AndroidBleManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ public void onDescriptorWrite(
return;
}

if (desc.getValue() == BluetoothGattDescriptor.ENABLE_NOTIFICATION_VALUE) {
if (desc.getValue() == BluetoothGattDescriptor.ENABLE_INDICATION_VALUE) {
mPlatform.handleSubscribeComplete(
connId, svcIdBytes, charIdBytes, status == BluetoothGatt.GATT_SUCCESS);
} else if (desc.getValue() == BluetoothGattDescriptor.DISABLE_NOTIFICATION_VALUE) {
Expand Down Expand Up @@ -283,7 +283,7 @@ public boolean onSubscribeCharacteristic(int connId, byte[] svcId, byte[] charId

BluetoothGattDescriptor descriptor =
subscribeChar.getDescriptor(UUID.fromString(CLIENT_CHARACTERISTIC_CONFIG));
descriptor.setValue(BluetoothGattDescriptor.ENABLE_NOTIFICATION_VALUE);
descriptor.setValue(BluetoothGattDescriptor.ENABLE_INDICATION_VALUE);
if (!bluetoothGatt.writeDescriptor(descriptor)) {
Log.e(TAG, "writeDescriptor failed");
return false;
Expand Down

0 comments on commit 76c8fde

Please sign in to comment.