From 4af5d03cd596cc0594f9bdaafb6420905f1be287 Mon Sep 17 00:00:00 2001 From: Tim Date: Tue, 29 Sep 2015 15:04:59 +0100 Subject: [PATCH 1/8] Error check number of characteristics Currently it just blindly writes beyond the end of the array, leading to impossible-to-find bugs. It doesn't help that the limit on the number of characteristics doesn't seem to be documented anywhere. --- source/nRF5xGattServer.cpp | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/source/nRF5xGattServer.cpp b/source/nRF5xGattServer.cpp index 8e4f7fa..3280639 100644 --- a/source/nRF5xGattServer.cpp +++ b/source/nRF5xGattServer.cpp @@ -45,7 +45,6 @@ nRF5xGattServer &nRF5xGattServer::getInstance(void) { /**************************************************************************/ ble_error_t nRF5xGattServer::addService(GattService &service) { - /* ToDo: Make sure we don't overflow the array, etc. */ /* ToDo: Make sure this service UUID doesn't already exist (?) */ /* ToDo: Basic validation */ @@ -63,7 +62,10 @@ ble_error_t nRF5xGattServer::addService(GattService &service) /* Add characteristics to the service */ for (uint8_t i = 0; i < service.getCharacteristicCount(); i++) { - GattCharacteristic *p_char = service.getCharacteristic(i); + if (characteristicCount >= BLE_TOTAL_CHARACTERISTICS) { + return BLE_ERROR_NO_MEM; + } + GattCharacteristic *p_char = service.getCharacteristic(i); /* Skip any incompletely defined, read-only characteristics. */ if ((p_char->getValueAttribute().getValuePtr() == NULL) && @@ -108,9 +110,12 @@ ble_error_t nRF5xGattServer::addService(GattService &service) characteristicCount++; /* Add optional descriptors if any */ - /* ToDo: Make sure we don't overflow the array */ for (uint8_t j = 0; j < p_char->getDescriptorCount(); j++) { - GattAttribute *p_desc = p_char->getDescriptor(j); + if (descriptorCount >= BLE_TOTAL_DESCRIPTORS) { + return BLE_ERROR_NO_MEM; + } + + GattAttribute *p_desc = p_char->getDescriptor(j); /* skip the user-description-descriptor here; this has already been handled when adding the characteristic (above). */ if (p_desc->getUUID() == BLE_UUID_DESCRIPTOR_CHAR_USER_DESC) { continue; From eab6631cb67076e62b62f4c4e075f8bcd33fb812 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Brucker Date: Mon, 5 Oct 2015 13:15:35 +0100 Subject: [PATCH 2/8] Update S110 detection macros again The mbed SDK actually prefixes all labels from targets.py with "TARGET_". Update our detection macros accordingly. --- source/btle/btle.cpp | 4 ++-- source/btle/btle_discovery.cpp | 2 +- source/nRF5xGap.h | 2 +- source/nRF5xGattClient.cpp | 2 +- source/nRF5xGattClient.h | 2 +- .../nordic-sdk/components/softdevice/s130/include/ble_gap.h | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/source/btle/btle.cpp b/source/btle/btle.cpp index cb6e454..a1d8efb 100644 --- a/source/btle/btle.cpp +++ b/source/btle/btle.cpp @@ -133,7 +133,7 @@ static void btle_handler(ble_evt_t *p_ble_evt) dm_ble_evt_handler(p_ble_evt); -#if !defined(MCU_NRF51_16K_S110) && !defined(MCU_NRF51_32K_S110) +#if !defined(TARGET_MCU_NRF51_16K_S110) && !defined(TARGET_MCU_NRF51_32K_S110) bleGattcEventHandler(p_ble_evt); #endif @@ -141,7 +141,7 @@ static void btle_handler(ble_evt_t *p_ble_evt) switch (p_ble_evt->header.evt_id) { case BLE_GAP_EVT_CONNECTED: { Gap::Handle_t handle = p_ble_evt->evt.gap_evt.conn_handle; -#if defined(MCU_NRF51_16K_S110) || defined(MCU_NRF51_32K_S110) +#if defined(TARGET_MCU_NRF51_16K_S110) || defined(TARGET_MCU_NRF51_32K_S110) /* Only peripheral role is supported by S110 */ Gap::Role_t role = Gap::PERIPHERAL; #else diff --git a/source/btle/btle_discovery.cpp b/source/btle/btle_discovery.cpp index d737f94..2329c97 100644 --- a/source/btle/btle_discovery.cpp +++ b/source/btle/btle_discovery.cpp @@ -17,7 +17,7 @@ #include "nRF5xServiceDiscovery.h" #include "nRF5xGattClient.h" -#if !defined(MCU_NRF51_16K_S110) && !defined(MCU_NRF51_32K_S110) +#if !defined(TARGET_MCU_NRF51_16K_S110) && !defined(TARGET_MCU_NRF51_32K_S110) void bleGattcEventHandler(const ble_evt_t *p_ble_evt) { nRF5xServiceDiscovery &sdSingleton = nRF5xGattClient::getInstance().discovery; diff --git a/source/nRF5xGap.h b/source/nRF5xGap.h index b272604..a138a6f 100644 --- a/source/nRF5xGap.h +++ b/source/nRF5xGap.h @@ -81,7 +81,7 @@ public: } /* Observer role is not supported by S110, return BLE_ERROR_NOT_IMPLEMENTED */ -#if !defined(MCU_NRF51_16K_S110) && !defined(MCU_NRF51_32K_S110) +#if !defined(TARGET_MCU_NRF51_16K_S110) && !defined(TARGET_MCU_NRF51_32K_S110) virtual ble_error_t startRadioScan(const GapScanningParams &scanningParams) { ble_gap_scan_params_t scanParams = { .active = scanningParams.getActiveScanning(), /**< If 1, perform active scanning (scan requests). */ diff --git a/source/nRF5xGattClient.cpp b/source/nRF5xGattClient.cpp index b29a4f3..f257f48 100644 --- a/source/nRF5xGattClient.cpp +++ b/source/nRF5xGattClient.cpp @@ -22,7 +22,7 @@ nRF5xGattClient::getInstance(void) { return nRFGattClientSingleton; } -#if !defined(MCU_NRF51_16K_S110) && !defined(MCU_NRF51_32K_S110) +#if !defined(TARGET_MCU_NRF51_16K_S110) && !defined(TARGET_MCU_NRF51_32K_S110) ble_error_t nRF5xGattClient::launchServiceDiscovery(Gap::Handle_t connectionHandle, ServiceDiscovery::ServiceCallback_t sc, diff --git a/source/nRF5xGattClient.h b/source/nRF5xGattClient.h index 76a015e..5974f18 100644 --- a/source/nRF5xGattClient.h +++ b/source/nRF5xGattClient.h @@ -29,7 +29,7 @@ public: * When using S110, all Gatt client features will return * BLE_ERROR_NOT_IMPLEMENTED */ -#if !defined(MCU_NRF51_16K_S110) && !defined(MCU_NRF51_32K_S110) +#if !defined(TARGET_MCU_NRF51_16K_S110) && !defined(TARGET_MCU_NRF51_32K_S110) /** * Launch service discovery. Once launched, service discovery will remain diff --git a/source/nordic-sdk/components/softdevice/s130/include/ble_gap.h b/source/nordic-sdk/components/softdevice/s130/include/ble_gap.h index a41f655..7c21684 100644 --- a/source/nordic-sdk/components/softdevice/s130/include/ble_gap.h +++ b/source/nordic-sdk/components/softdevice/s130/include/ble_gap.h @@ -547,7 +547,7 @@ typedef struct { ble_gap_addr_t peer_addr; /**< Bluetooth address of the peer device. */ ble_gap_addr_t own_addr; /**< Bluetooth address of the local device used during connection setup. */ -#if !defined(MCU_NRF51_16K_S110) && !defined(MCU_NRF51_32K_S110) +#if !defined(TARGET_MCU_NRF51_16K_S110) && !defined(TARGET_MCU_NRF51_32K_S110) uint8_t role; /**< BLE role for this connection, see @ref BLE_GAP_ROLES */ #endif uint8_t irk_match :1; /**< If 1, peer device's address resolved using an IRK. */ From 816f4559f1ecddfd07cf928c65b2ee04edb05fc5 Mon Sep 17 00:00:00 2001 From: Rohit Grover Date: Tue, 13 Oct 2015 12:54:46 +0100 Subject: [PATCH 3/8] white space diffs; convert tabs to spaces --- source/nRF5xGattServer.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/source/nRF5xGattServer.cpp b/source/nRF5xGattServer.cpp index 3280639..03eddf4 100644 --- a/source/nRF5xGattServer.cpp +++ b/source/nRF5xGattServer.cpp @@ -62,10 +62,10 @@ ble_error_t nRF5xGattServer::addService(GattService &service) /* Add characteristics to the service */ for (uint8_t i = 0; i < service.getCharacteristicCount(); i++) { - if (characteristicCount >= BLE_TOTAL_CHARACTERISTICS) { - return BLE_ERROR_NO_MEM; - } - GattCharacteristic *p_char = service.getCharacteristic(i); + if (characteristicCount >= BLE_TOTAL_CHARACTERISTICS) { + return BLE_ERROR_NO_MEM; + } + GattCharacteristic *p_char = service.getCharacteristic(i); /* Skip any incompletely defined, read-only characteristics. */ if ((p_char->getValueAttribute().getValuePtr() == NULL) && @@ -111,11 +111,11 @@ ble_error_t nRF5xGattServer::addService(GattService &service) /* Add optional descriptors if any */ for (uint8_t j = 0; j < p_char->getDescriptorCount(); j++) { - if (descriptorCount >= BLE_TOTAL_DESCRIPTORS) { - return BLE_ERROR_NO_MEM; - } + if (descriptorCount >= BLE_TOTAL_DESCRIPTORS) { + return BLE_ERROR_NO_MEM; + } - GattAttribute *p_desc = p_char->getDescriptor(j); + GattAttribute *p_desc = p_char->getDescriptor(j); /* skip the user-description-descriptor here; this has already been handled when adding the characteristic (above). */ if (p_desc->getUUID() == BLE_UUID_DESCRIPTOR_CHAR_USER_DESC) { continue; From 6082c76ab57ba534c3e67a320d79d36f6c23583d Mon Sep 17 00:00:00 2001 From: Marcus Chang Date: Fri, 16 Oct 2015 11:44:03 +0100 Subject: [PATCH 4/8] When connecting, if no scanning parameters are passed, use values from Gap parent. --- source/nRF5xGap.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/source/nRF5xGap.cpp b/source/nRF5xGap.cpp index 62da90c..a46de0c 100644 --- a/source/nRF5xGap.cpp +++ b/source/nRF5xGap.cpp @@ -233,17 +233,18 @@ ble_error_t nRF5xGap::connect(const Address_t peerAddr, } ble_gap_scan_params_t scanParams; - scanParams.active = 0; /**< If 1, perform active scanning (scan requests). */ scanParams.selective = 0; /**< If 1, ignore unknown devices (non whitelisted). */ scanParams.p_whitelist = NULL; /**< Pointer to whitelist, NULL if none is given. */ if (scanParamsIn != NULL) { - scanParams.interval = scanParamsIn->getInterval(); /**< Scan interval between 0x0004 and 0x4000 in 0.625ms units (2.5ms to 10.24s). */ - scanParams.window = scanParamsIn->getWindow(); /**< Scan window between 0x0004 and 0x4000 in 0.625ms units (2.5ms to 10.24s). */ - scanParams.timeout = scanParamsIn->getTimeout(); /**< Scan timeout between 0x0001 and 0xFFFF in seconds, 0x0000 disables timeout. */ + scanParams.active = scanParamsIn->getActiveScanning(); /**< If 1, perform active scanning (scan requests). */ + scanParams.interval = scanParamsIn->getInterval(); /**< Scan interval between 0x0004 and 0x4000 in 0.625ms units (2.5ms to 10.24s). */ + scanParams.window = scanParamsIn->getWindow(); /**< Scan window between 0x0004 and 0x4000 in 0.625ms units (2.5ms to 10.24s). */ + scanParams.timeout = scanParamsIn->getTimeout(); /**< Scan timeout between 0x0001 and 0xFFFF in seconds, 0x0000 disables timeout. */ } else { - scanParams.interval = 500; /**< Scan interval between 0x0004 and 0x4000 in 0.625ms units (2.5ms to 10.24s). */ - scanParams.window = 200; /**< Scan window between 0x0004 and 0x4000 in 0.625ms units (2.5ms to 10.24s). */ - scanParams.timeout = 0; /**< Scan timeout between 0x0001 and 0xFFFF in seconds, 0x0000 disables timeout. */ + scanParams.active = _scanningParams.getActiveScanning(); /**< If 1, perform active scanning (scan requests). */ + scanParams.interval = _scanningParams.getInterval(); /**< Scan interval between 0x0004 and 0x4000 in 0.625ms units (2.5ms to 10.24s). */ + scanParams.window = _scanningParams.getWindow(); /**< Scan window between 0x0004 and 0x4000 in 0.625ms units (2.5ms to 10.24s). */ + scanParams.timeout = _scanningParams.getTimeout(); /**< Scan timeout between 0x0001 and 0xFFFF in seconds, 0x0000 disables timeout. */ } uint32_t rc = sd_ble_gap_connect(&addr, &scanParams, &connParams); From 97a65815ed5e32a98c0f93362c3fb72cc3f49d43 Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Thu, 29 Oct 2015 11:39:00 +0000 Subject: [PATCH 5/8] Introduced changes for memory savings Moved GattSecurityManager and GattClient to be allocated dynamically and reduced the size of some arrays to increase memory savings. --- source/btle/custom/custom_helper.cpp | 2 +- source/nRF5xGattClient.cpp | 7 +++++-- source/nRF5xSecurityManager.cpp | 7 +++++-- .../ble/device_manager/config/device_manager_cnfg.h | 2 +- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/source/btle/custom/custom_helper.cpp b/source/btle/custom/custom_helper.cpp index ba4b303..81d7d0c 100644 --- a/source/btle/custom/custom_helper.cpp +++ b/source/btle/custom/custom_helper.cpp @@ -26,7 +26,7 @@ typedef struct { UUID::LongUUIDBytes_t uuid; uint8_t type; } converted_uuid_table_entry_t; -static const unsigned UUID_TABLE_MAX_ENTRIES = 8; /* This is the maximum number of 128-bit UUIDs with distinct bases that +static const unsigned UUID_TABLE_MAX_ENTRIES = 4; /* This is the maximum number of 128-bit UUIDs with distinct bases that * we expect to be in use; increase this limit if needed. */ static unsigned uuidTableEntries = 0; /* current usage of the table */ converted_uuid_table_entry_t convertedUUIDTable[UUID_TABLE_MAX_ENTRIES]; diff --git a/source/nRF5xGattClient.cpp b/source/nRF5xGattClient.cpp index f257f48..3a4b1e6 100644 --- a/source/nRF5xGattClient.cpp +++ b/source/nRF5xGattClient.cpp @@ -18,8 +18,11 @@ nRF5xGattClient & nRF5xGattClient::getInstance(void) { - static nRF5xGattClient nRFGattClientSingleton; - return nRFGattClientSingleton; + static nRF5xGattClient* nRFGattClientSingleton = NULL; + if (nRFGattClientSingleton == NULL) { + nRFGattClientSingleton = new nRF5xGattClient(); + } + return *nRFGattClientSingleton; } #if !defined(TARGET_MCU_NRF51_16K_S110) && !defined(TARGET_MCU_NRF51_32K_S110) diff --git a/source/nRF5xSecurityManager.cpp b/source/nRF5xSecurityManager.cpp index f1e5682..a4ccd4f 100644 --- a/source/nRF5xSecurityManager.cpp +++ b/source/nRF5xSecurityManager.cpp @@ -17,6 +17,9 @@ #include "nRF5xSecurityManager.h" nRF5xSecurityManager &nRF5xSecurityManager::getInstance(void) { - static nRF5xSecurityManager m_instance; - return m_instance; + static nRF5xSecurityManager* m_instance = NULL; + if (m_instance == NULL) { + m_instance = new nRF5xSecurityManager(); + } + return *m_instance; } diff --git a/source/nordic-sdk/components/ble/device_manager/config/device_manager_cnfg.h b/source/nordic-sdk/components/ble/device_manager/config/device_manager_cnfg.h index 4bf89da..5189db2 100644 --- a/source/nordic-sdk/components/ble/device_manager/config/device_manager_cnfg.h +++ b/source/nordic-sdk/components/ble/device_manager/config/device_manager_cnfg.h @@ -85,7 +85,7 @@ * be stored. In such cases, application will be notified with DM_DEVICE_CONTEXT_FULL * as event result at the completion of the security procedure. */ -#define DEVICE_MANAGER_MAX_BONDS 4 +#define DEVICE_MANAGER_MAX_BONDS 2 /** From 20b07e38bc2eca6bc689f3d04d244f78fb821f68 Mon Sep 17 00:00:00 2001 From: Rohit Grover Date: Wed, 28 Oct 2015 14:05:27 +0000 Subject: [PATCH 6/8] update init() to match the chagnes around initializationCompleteCallback. refer to https://github.com/ARMmbed/ble/pull/91 and https://github.com/ARMmbed/ble/issues/90 --- source/nRF5xn.cpp | 17 +++++++++++++++-- source/nRF5xn.h | 15 ++++++++++++--- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/source/nRF5xn.cpp b/source/nRF5xn.cpp index 66499b9..ce8ed82 100644 --- a/source/nRF5xn.cpp +++ b/source/nRF5xn.cpp @@ -38,7 +38,7 @@ createBLEInstance(void) return (&deviceInstance); } -nRF5xn::nRF5xn(void) +nRF5xn::nRF5xn(void) : initialized(false), instanceID(BLE::DEFAULT_INSTANCE) { } @@ -72,11 +72,24 @@ const char *nRF5xn::getVersion(void) return versionString; } -ble_error_t nRF5xn::init(void) +ble_error_t nRF5xn::init(BLE::InstanceID_t instanceID, BLE::InitializationCompleteCallback_t callback) { + if (initialized) { + if (callback) { + callback(BLE::Instance(instanceID), BLE_ERROR_ALREADY_INITIALIZED); + } + return BLE_ERROR_ALREADY_INITIALIZED; + } + + instanceID = instanceID; + /* ToDo: Clear memory contents, reset the SD, etc. */ btle_init(); + initialized = true; + if (callback) { + callback(BLE::Instance(instanceID), BLE_ERROR_NONE); + } return BLE_ERROR_NONE; } diff --git a/source/nRF5xn.h b/source/nRF5xn.h index 387eb6e..105c0af 100644 --- a/source/nRF5xn.h +++ b/source/nRF5xn.h @@ -17,13 +17,15 @@ #ifndef __NRF51822_H__ #define __NRF51822_H__ -#include "mbed.h" -#include "ble/blecommon.h" #include "ble/BLE.h" +#include "ble/blecommon.h" +#include "ble/BLEInstanceBase.h" + #include "nRF5xGap.h" #include "nRF5xGattServer.h" #include "nRF5xGattClient.h" #include "nRF5xSecurityManager.h" + #include "btle.h" class nRF5xn : public BLEInstanceBase @@ -32,7 +34,10 @@ public: nRF5xn(void); virtual ~nRF5xn(void); - virtual ble_error_t init(void); + virtual ble_error_t init(BLE::InstanceID_t instanceID, BLE::InitializationCompleteCallback_t); + virtual bool hasInitialized(void) const { + return initialized; + } virtual ble_error_t shutdown(void); virtual const char *getVersion(void); @@ -58,6 +63,10 @@ public: return nRF5xSecurityManager::getInstance(); } virtual void waitForEvent(void); + +private: + bool initialized; + BLE::InstanceID_t instanceID; }; #endif From c5dc7e52a60eb14602a880d7e451ad8d177f6b1a Mon Sep 17 00:00:00 2001 From: Rohit Grover Date: Fri, 30 Oct 2015 10:18:37 +0000 Subject: [PATCH 7/8] add init guards for some top level APIs --- source/nRF5xn.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/source/nRF5xn.cpp b/source/nRF5xn.cpp index ce8ed82..de7e707 100644 --- a/source/nRF5xn.cpp +++ b/source/nRF5xn.cpp @@ -16,6 +16,7 @@ #include "mbed.h" #include "nRF5xn.h" +#include "ble/blecommon.h" #include "nrf_soc.h" #include "btle/btle.h" @@ -48,6 +49,10 @@ nRF5xn::~nRF5xn(void) const char *nRF5xn::getVersion(void) { + if (!initialized) { + return "INITIALIZATION_INCOMPLETE"; + } + static char versionString[32]; static bool versionFetched = false; @@ -95,6 +100,10 @@ ble_error_t nRF5xn::init(BLE::InstanceID_t instanceID, BLE::InitializationComple ble_error_t nRF5xn::shutdown(void) { + if (!initialized) { + return BLE_ERROR_INITIALIZATION_INCOMPLETE; + } + return (softdevice_handler_sd_disable() == NRF_SUCCESS) ? BLE_ERROR_NONE : BLE_STACK_BUSY; } From 847c27cfe9c9cbd17cde747043f485f04efb8d40 Mon Sep 17 00:00:00 2001 From: Rohit Grover Date: Fri, 30 Oct 2015 13:56:02 +0000 Subject: [PATCH 8/8] Version 2.0.0 ============= * Update init() to match the changes around initializationCompleteCallback. Also implemented hasInitialized(). Refer to https://github.com/ARMmbed/ble/pull/91 and https://github.com/ARMmbed/ble/issues/90. * Some changes for memory savings. Certain singletons are now allocated dynamically; so some memory may not be needed if the application exercises limited functionality. Also reduced the size of some global tables for memory savings; affected tables/constants: UUID_TABLE_MAX_ENTRIES (down to 4), DEVICE_MANAGER_MAX_BONDS (down to 2). --- module.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/module.json b/module.json index 39bab34..b525337 100644 --- a/module.json +++ b/module.json @@ -1,6 +1,6 @@ { "name": "ble-nrf51822", - "version": "1.0.0", + "version": "2.0.0", "description": "Nordic stack and drivers for the mbed BLE API.", "keywords": [ "Bluetooth", @@ -21,7 +21,7 @@ } ], "dependencies": { - "ble": "^1.0.0" + "ble": "^2.0.0" }, "extraIncludes": [ "source/btle",