From 6f19dea656edc1dee06a418a75e378b7c1024987 Mon Sep 17 00:00:00 2001 From: Vincent Coubard Date: Mon, 27 Nov 2017 18:56:51 +0000 Subject: [PATCH] BLE: Fix GattServer::write on Nordic targets. GattServer::write on Nordic's targets use sd_ble_gatts_hvx to send an handle value Notification or Indication; This function can fail if the connection handle is invalid or if Updates are not enabled for this connection. This patch workaround those limitations. --- .../source/nRF5xGattServer.cpp | 120 +++++++++++------- .../source/nRF5xGattServer.h | 9 ++ .../TARGET_NRF5/source/nRF5xGattServer.cpp | 111 +++++++++------- .../TARGET_NRF5/source/nRF5xGattServer.h | 9 ++ 4 files changed, 162 insertions(+), 87 deletions(-) diff --git a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_MCU_NRF51822/source/nRF5xGattServer.cpp b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_MCU_NRF51822/source/nRF5xGattServer.cpp index 26f53848f8..d675bdfddc 100644 --- a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_MCU_NRF51822/source/nRF5xGattServer.cpp +++ b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_MCU_NRF51822/source/nRF5xGattServer.cpp @@ -26,6 +26,34 @@ #include "nRF5xn.h" +namespace { + +static ble_error_t set_attribute_value( + Gap::Handle_t connectionHandle, + GattAttribute::Handle_t attributeHandle, + ble_gatts_value_t *value +) { + uint32_t err = sd_ble_gatts_value_set(connectionHandle, attributeHandle, value); + switch(err) { + case NRF_SUCCESS: + return BLE_ERROR_NONE; + case NRF_ERROR_INVALID_ADDR: + case NRF_ERROR_INVALID_PARAM: + return BLE_ERROR_INVALID_PARAM; + case NRF_ERROR_NOT_FOUND: + case NRF_ERROR_DATA_SIZE: + case BLE_ERROR_INVALID_CONN_HANDLE: + case BLE_ERROR_GATTS_INVALID_ATTR_TYPE: + return BLE_ERROR_PARAM_OUT_OF_RANGE; + case NRF_ERROR_FORBIDDEN: + return BLE_ERROR_OPERATION_NOT_PERMITTED; + default: + return BLE_ERROR_UNSPECIFIED; + } + } + +} // end of anonymous namespace + /**************************************************************************/ /*! @brief Adds a new service to the GATT table on the peripheral @@ -244,53 +272,52 @@ ble_error_t nRF5xGattServer::write(Gap::Handle_t connectionHandle, GattAttribute nRF5xGap &gap = (nRF5xGap &) nRF5xn::Instance(BLE::DEFAULT_INSTANCE).getGap(); connectionHandle = gap.getConnectionHandle(); } - error_t error = (error_t) sd_ble_gatts_hvx(connectionHandle, &hvx_params); - if (error != ERROR_NONE) { - switch (error) { - case ERROR_BLE_NO_TX_BUFFERS: /* Notifications consume application buffers. The return value can be used for resending notifications. */ - case ERROR_BUSY: - returnValue = BLE_STACK_BUSY; - break; - case ERROR_INVALID_STATE: - case ERROR_BLEGATTS_SYS_ATTR_MISSING: - returnValue = BLE_ERROR_INVALID_STATE; - break; - - default : - ASSERT_INT( ERROR_NONE, - sd_ble_gatts_value_set(connectionHandle, attributeHandle, &value), - BLE_ERROR_PARAM_OUT_OF_RANGE ); - - /* Notifications consume application buffers. The return value can - * be used for resending notifications. */ - returnValue = BLE_STACK_BUSY; - break; + bool updatesEnabled = false; + if (connectionHandle != BLE_CONN_HANDLE_INVALID) { + ble_error_t err = areUpdatesEnabled(connectionHandle, attributeHandle, &updatesEnabled); + // FIXME: The softdevice allocates and populates CCCD when the client + // interract with them. Checking for updates may return an out of + // range error in such case. + if(err && err != BLE_ERROR_PARAM_OUT_OF_RANGE) { + return err; } } - } else { - uint32_t err = sd_ble_gatts_value_set(connectionHandle, attributeHandle, &value); - switch(err) { - case NRF_SUCCESS: - returnValue = BLE_ERROR_NONE; - break; - case NRF_ERROR_INVALID_ADDR: - case NRF_ERROR_INVALID_PARAM: - returnValue = BLE_ERROR_INVALID_PARAM; - break; - case NRF_ERROR_NOT_FOUND: - case NRF_ERROR_DATA_SIZE: - case BLE_ERROR_INVALID_CONN_HANDLE: - case BLE_ERROR_GATTS_INVALID_ATTR_TYPE: - returnValue = BLE_ERROR_PARAM_OUT_OF_RANGE; - break; - case NRF_ERROR_FORBIDDEN: - returnValue = BLE_ERROR_OPERATION_NOT_PERMITTED; - break; - default: - returnValue = BLE_ERROR_UNSPECIFIED; - break; + + if (updatesEnabled) { + error_t error = (error_t) sd_ble_gatts_hvx(connectionHandle, &hvx_params); + if (error != ERROR_NONE) { + switch (error) { + case ERROR_BLE_NO_TX_BUFFERS: /* Notifications consume application buffers. The return value can be used for resending notifications. */ + case ERROR_BUSY: + returnValue = BLE_STACK_BUSY; + break; + + case ERROR_INVALID_STATE: + case ERROR_BLEGATTS_SYS_ATTR_MISSING: + returnValue = BLE_ERROR_INVALID_STATE; + break; + + default : + ASSERT_INT( ERROR_NONE, + sd_ble_gatts_value_set(connectionHandle, attributeHandle, &value), + BLE_ERROR_PARAM_OUT_OF_RANGE ); + + if (connectionHandle == BLE_CONN_HANDLE_INVALID) { + returnValue = BLE_ERROR_NONE; + } else { + /* Notifications consume application buffers. The return value can + * be used for resending notifications. */ + returnValue = BLE_STACK_BUSY; + } + break; + } + } + } else { + returnValue = set_attribute_value(connectionHandle, attributeHandle, &value); } + } else { + returnValue = set_attribute_value(connectionHandle, attributeHandle, &value); } return returnValue; @@ -305,7 +332,12 @@ ble_error_t nRF5xGattServer::areUpdatesEnabled(const GattCharacteristic &charact ble_error_t nRF5xGattServer::areUpdatesEnabled(Gap::Handle_t connectionHandle, const GattCharacteristic &characteristic, bool *enabledP) { - int characteristicIndex = resolveValueHandleToCharIndex(characteristic.getValueHandle()); + return areUpdatesEnabled(connectionHandle, characteristic.getValueHandle(), enabledP); +} + +ble_error_t nRF5xGattServer::areUpdatesEnabled(Gap::Handle_t connectionHandle, GattAttribute::Handle_t attributeHandle, bool *enabledP) +{ + int characteristicIndex = resolveValueHandleToCharIndex(attributeHandle); if (characteristicIndex == -1) { return BLE_ERROR_INVALID_PARAM; } diff --git a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_MCU_NRF51822/source/nRF5xGattServer.h b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_MCU_NRF51822/source/nRF5xGattServer.h index 1a1ad86d89..b2e5f87dea 100644 --- a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_MCU_NRF51822/source/nRF5xGattServer.h +++ b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_MCU_NRF51822/source/nRF5xGattServer.h @@ -79,6 +79,15 @@ private: return -1; } + /** + * Query if updates of a characteristics are enabled for a given connection. + */ + ble_error_t areUpdatesEnabled( + Gap::Handle_t connectionHandle, + GattAttribute::Handle_t valueHandle, + bool *enabledP + ); + private: GattCharacteristic *p_characteristics[BLE_TOTAL_CHARACTERISTICS]; ble_gatts_char_handles_t nrfCharacteristicHandles[BLE_TOTAL_CHARACTERISTICS]; diff --git a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xGattServer.cpp b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xGattServer.cpp index 2a1c0af1a6..c3daf0e7b4 100644 --- a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xGattServer.cpp +++ b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xGattServer.cpp @@ -65,8 +65,32 @@ static const ble_gatts_rw_authorize_reply_params_t write_auth_invalid_reply = { } }; +static ble_error_t set_attribute_value( + Gap::Handle_t connectionHandle, + GattAttribute::Handle_t attributeHandle, + ble_gatts_value_t *value +) { + uint32_t err = sd_ble_gatts_value_set(connectionHandle, attributeHandle, value); + switch(err) { + case NRF_SUCCESS: + return BLE_ERROR_NONE; + case NRF_ERROR_INVALID_ADDR: + case NRF_ERROR_INVALID_PARAM: + return BLE_ERROR_INVALID_PARAM; + case NRF_ERROR_NOT_FOUND: + case NRF_ERROR_DATA_SIZE: + case BLE_ERROR_INVALID_CONN_HANDLE: + case BLE_ERROR_GATTS_INVALID_ATTR_TYPE: + return BLE_ERROR_PARAM_OUT_OF_RANGE; + case NRF_ERROR_FORBIDDEN: + return BLE_ERROR_OPERATION_NOT_PERMITTED; + default: + return BLE_ERROR_UNSPECIFIED; + } } +} // end of anonymous namespace + /**************************************************************************/ /*! @brief Adds a new service to the GATT table on the peripheral @@ -285,53 +309,49 @@ ble_error_t nRF5xGattServer::write(Gap::Handle_t connectionHandle, GattAttribute nRF5xGap &gap = (nRF5xGap &) nRF5xn::Instance(BLE::DEFAULT_INSTANCE).getGap(); connectionHandle = gap.getConnectionHandle(); } - error_t error = (error_t) sd_ble_gatts_hvx(connectionHandle, &hvx_params); - if (error != ERROR_NONE) { - switch (error) { - case ERROR_BLE_NO_TX_BUFFERS: /* Notifications consume application buffers. The return value can be used for resending notifications. */ - case ERROR_BUSY: - returnValue = BLE_STACK_BUSY; - break; - case ERROR_INVALID_STATE: - case ERROR_BLEGATTS_SYS_ATTR_MISSING: - returnValue = BLE_ERROR_INVALID_STATE; - break; + bool updatesEnabled = false; + if (connectionHandle != BLE_CONN_HANDLE_INVALID) { + ble_error_t err = areUpdatesEnabled(connectionHandle, attributeHandle, &updatesEnabled); - default : - ASSERT_INT( ERROR_NONE, - sd_ble_gatts_value_set(connectionHandle, attributeHandle, &value), - BLE_ERROR_PARAM_OUT_OF_RANGE ); - - /* Notifications consume application buffers. The return value can - * be used for resending notifications. */ - returnValue = BLE_STACK_BUSY; - break; + // FIXME: The softdevice allocates and populates CCCD when the client + // interract with them. Checking for updates may return an out of + // range error in such case. + if(err && err != BLE_ERROR_PARAM_OUT_OF_RANGE) { + return err; } } - } else { - uint32_t err = sd_ble_gatts_value_set(connectionHandle, attributeHandle, &value); - switch(err) { - case NRF_SUCCESS: - returnValue = BLE_ERROR_NONE; - break; - case NRF_ERROR_INVALID_ADDR: - case NRF_ERROR_INVALID_PARAM: - returnValue = BLE_ERROR_INVALID_PARAM; - break; - case NRF_ERROR_NOT_FOUND: - case NRF_ERROR_DATA_SIZE: - case BLE_ERROR_INVALID_CONN_HANDLE: - case BLE_ERROR_GATTS_INVALID_ATTR_TYPE: - returnValue = BLE_ERROR_PARAM_OUT_OF_RANGE; - break; - case NRF_ERROR_FORBIDDEN: - returnValue = BLE_ERROR_OPERATION_NOT_PERMITTED; - break; - default: - returnValue = BLE_ERROR_UNSPECIFIED; - break; + + if (updatesEnabled) { + error_t error = (error_t) sd_ble_gatts_hvx(connectionHandle, &hvx_params); + if (error != ERROR_NONE) { + switch (error) { + case ERROR_BLE_NO_TX_BUFFERS: /* Notifications consume application buffers. The return value can be used for resending notifications. */ + case ERROR_BUSY: + returnValue = BLE_STACK_BUSY; + break; + + case ERROR_INVALID_STATE: + case ERROR_BLEGATTS_SYS_ATTR_MISSING: + returnValue = BLE_ERROR_INVALID_STATE; + break; + + default : + ASSERT_INT( ERROR_NONE, + sd_ble_gatts_value_set(connectionHandle, attributeHandle, &value), + BLE_ERROR_PARAM_OUT_OF_RANGE ); + + /* Notifications consume application buffers. The return value can + * be used for resending notifications. */ + returnValue = BLE_STACK_BUSY; + break; + } + } + } else { + returnValue = set_attribute_value(connectionHandle, attributeHandle, &value); } + } else { + returnValue = set_attribute_value(connectionHandle, attributeHandle, &value); } return returnValue; @@ -346,7 +366,12 @@ ble_error_t nRF5xGattServer::areUpdatesEnabled(const GattCharacteristic &charact ble_error_t nRF5xGattServer::areUpdatesEnabled(Gap::Handle_t connectionHandle, const GattCharacteristic &characteristic, bool *enabledP) { - int characteristicIndex = resolveValueHandleToCharIndex(characteristic.getValueHandle()); + return areUpdatesEnabled(connectionHandle, characteristic.getValueHandle(), enabledP); +} + +ble_error_t nRF5xGattServer::areUpdatesEnabled(Gap::Handle_t connectionHandle, GattAttribute::Handle_t attributeHandle, bool *enabledP) +{ + int characteristicIndex = resolveValueHandleToCharIndex(attributeHandle); if (characteristicIndex == -1) { return BLE_ERROR_INVALID_PARAM; } diff --git a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xGattServer.h b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xGattServer.h index 77bfd26d90..ca4e069d80 100644 --- a/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xGattServer.h +++ b/features/FEATURE_BLE/targets/TARGET_NORDIC/TARGET_NRF5/source/nRF5xGattServer.h @@ -133,6 +133,15 @@ private: */ void releaseAllWriteRequests(); + /** + * Query if updates of a characteristics are enabled for a given connection. + */ + ble_error_t areUpdatesEnabled( + Gap::Handle_t connectionHandle, + GattAttribute::Handle_t valueHandle, + bool *enabledP + ); + private: GattCharacteristic *p_characteristics[BLE_TOTAL_CHARACTERISTICS]; ble_gatts_char_handles_t nrfCharacteristicHandles[BLE_TOTAL_CHARACTERISTICS];