diff --git a/Kconfig b/Kconfig index 590b37fe..bfd47962 100644 --- a/Kconfig +++ b/Kconfig @@ -197,11 +197,4 @@ config NIMBLE_CPP_DEBUG_ASSERT_ENABLED Enabling this option will add debug asserts to the NimBLE CPP library. This will use approximately 1kB of flash memory. -config NIMBLE_CPP_FREERTOS_TASK_BLOCK_BIT - int "FreeRTOS task block bit." - default 31 - help - Configure the bit to set in the task notification value when a task is blocked waiting for an event. - This should be set to a bit that is not used by other notifications in the system. - endmenu \ No newline at end of file diff --git a/src/NimBLEAddress.cpp b/src/NimBLEAddress.cpp index 2a719375..a631b25d 100644 --- a/src/NimBLEAddress.cpp +++ b/src/NimBLEAddress.cpp @@ -97,8 +97,19 @@ NimBLEAddress::NimBLEAddress(const std::string& addr, uint8_t type) { } // NimBLEAddress /** - * @brief Constructor for compatibility with bluedroid esp library using native ESP representation. - * @param [in] address A uint8_t[6] or esp_bd_addr_t containing the address. + * @brief Constructor accepting a 6-byte MSB-first BLE address (Bluedroid `esp_bd_addr_t` layout). + * @details The input bytes are stored internally in LSB-first wire + * order (NimBLE's `ble_addr_t.val` layout), so this constructor + * **reverses** the input. If you already have LSB-first wire bytes — + * e.g. straight out of a NimBLE callback's `ble_addr_t` — use the + * `NimBLEAddress(ble_addr_t)` constructor instead (it stores them + * as-is). If you have the address as a hex integer (e.g. + * `0xa4c1385def16` for `"a4:c1:38:5d:ef:16"`), use + * `NimBLEAddress(const uint64_t&, uint8_t)`. + * @param [in] address Six bytes in MSB-first order + * (`{0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff}` for + * `"aa:bb:cc:dd:ee:ff"`). Compatible with + * Bluedroid's `esp_bd_addr_t`. * @param [in] type The type of the address should be one of: * * BLE_ADDR_PUBLIC (0) * * BLE_ADDR_RANDOM (1) diff --git a/src/NimBLEAttValue.cpp b/src/NimBLEAttValue.cpp index 8dbe4866..86055f30 100644 --- a/src/NimBLEAttValue.cpp +++ b/src/NimBLEAttValue.cpp @@ -18,16 +18,49 @@ #include "NimBLEAttValue.h" #if CONFIG_BT_NIMBLE_ENABLED -# ifdef USING_NIMBLE_ARDUINO_HEADERS -# include "nimble/nimble/include/nimble/nimble_npl.h" -# else -# include "nimble/nimble_npl.h" -# endif - # include "NimBLEUtils.h" # include "NimBLELog.h" +# if defined(__ARM_ARCH_6M__) +// Cortex-M0/M0+ has no LDREX/STREX, so GCC emits libcalls for 4 byte atomic read-modify-write +// operations that are not provided by libgcc. Implement them by briefly masking interrupts. +extern "C" { +unsigned int __atomic_fetch_add_4(volatile void* ptr, unsigned int val, int) { + volatile unsigned int* p = static_cast(ptr); + unsigned int primask; + __asm volatile("mrs %0, primask\n\tcpsid i" : "=r"(primask)::"memory"); + unsigned int old = *p; + *p = old + val; + __asm volatile("msr primask, %0" ::"r"(primask) : "memory"); + return old; +} + +unsigned int __atomic_fetch_sub_4(volatile void* ptr, unsigned int val, int) { + volatile unsigned int* p = static_cast(ptr); + unsigned int primask; + __asm volatile("mrs %0, primask\n\tcpsid i" : "=r"(primask)::"memory"); + unsigned int old = *p; + *p = old - val; + __asm volatile("msr primask, %0" ::"r"(primask) : "memory"); + return old; +} + +unsigned char __atomic_exchange_1(volatile void* ptr, unsigned char val, int) { + volatile unsigned char* p = static_cast(ptr); + unsigned int primask; + __asm volatile("mrs %0, primask\n\tcpsid i" : "=r"(primask)::"memory"); + unsigned char old = *p; + *p = val; + __asm volatile("msr primask, %0" ::"r"(primask) : "memory"); + return old; +} +} +# endif + static const char* LOG_TAG = "NimBLEAttValue"; +ble_npl_mutex NimBLEAttValue::Lock::m_mutex{}; +std::atomic NimBLEAttValue::Lock::m_initialized{false}; +std::atomic NimBLEAttValue::Lock::m_activeCount{0}; // Default constructor implementation. NimBLEAttValue::NimBLEAttValue(uint16_t init_len, uint16_t max_len) @@ -64,6 +97,7 @@ NimBLEAttValue::~NimBLEAttValue() { // Move assignment operator implementation. NimBLEAttValue& NimBLEAttValue::operator=(NimBLEAttValue&& source) { if (this != &source) { + Lock lock; free(m_attr_value); m_attr_value = source.m_attr_value; m_attr_max_len = source.m_attr_max_len; @@ -85,26 +119,27 @@ NimBLEAttValue& NimBLEAttValue::operator=(const NimBLEAttValue& source) { } // Copy all the data from the source object to this object, including allocated space. -void NimBLEAttValue::deepCopy(const NimBLEAttValue& source) { +bool NimBLEAttValue::deepCopy(const NimBLEAttValue& source) { + Lock lock; uint8_t* res = static_cast(realloc(m_attr_value, source.m_capacity + 1)); NIMBLE_CPP_DEBUG_ASSERT(res); if (res == nullptr) { NIMBLE_LOGE(LOG_TAG, "Failed to realloc deepCopy"); - return; + return false; } - ble_npl_hw_enter_critical(); m_attr_value = res; m_attr_max_len = source.m_attr_max_len; m_attr_len = source.m_attr_len; m_capacity = source.m_capacity; setTimeStamp(source.getTimeStamp()); memcpy(m_attr_value, source.m_attr_value, m_attr_len + 1); - ble_npl_hw_exit_critical(0); + return true; } // Set the value of the attribute. bool NimBLEAttValue::setValue(const uint8_t* value, uint16_t len) { + Lock lock; m_attr_len = 0; // Just set the value length to 0 and append instead of repeating code. m_attr_value[0] = '\0'; // Set the first byte to 0 incase the len of the new value is 0. append(value, len); @@ -113,6 +148,7 @@ bool NimBLEAttValue::setValue(const uint8_t* value, uint16_t len) { // Append the new data, allocate as necessary. NimBLEAttValue& NimBLEAttValue::append(const uint8_t* value, uint16_t len) { + Lock lock; if (len == 0) { return *this; } @@ -124,9 +160,9 @@ NimBLEAttValue& NimBLEAttValue::append(const uint8_t* value, uint16_t len) { uint8_t* res = m_attr_value; uint16_t new_len = m_attr_len + len; - if (new_len > m_capacity) { - res = static_cast(realloc(m_attr_value, (new_len + 1))); - m_capacity = new_len; + bool grow = new_len > m_capacity; + if (grow) { + res = static_cast(realloc(m_attr_value, (new_len + 1))); } NIMBLE_CPP_DEBUG_ASSERT(res); if (res == nullptr) { @@ -134,24 +170,27 @@ NimBLEAttValue& NimBLEAttValue::append(const uint8_t* value, uint16_t len) { return *this; } + if (grow) { + m_capacity = new_len; + } + # if MYNEWT_VAL(NIMBLE_CPP_ATT_VALUE_TIMESTAMP_ENABLED) time_t t = time(nullptr); # else time_t t = 0; # endif - ble_npl_hw_enter_critical(); memcpy(res + m_attr_len, value, len); m_attr_value = res; m_attr_len = new_len; m_attr_value[m_attr_len] = '\0'; setTimeStamp(t); - ble_npl_hw_exit_critical(0); return *this; } uint8_t NimBLEAttValue::operator[](int pos) const { + Lock lock; NIMBLE_CPP_DEBUG_ASSERT(pos < m_attr_len); if (pos >= m_attr_len) { NIMBLE_LOGE(LOG_TAG, "pos >= len, pos=%u, len=%u", pos, m_attr_len); diff --git a/src/NimBLEAttValue.h b/src/NimBLEAttValue.h index 9ef92d52..a3571b53 100644 --- a/src/NimBLEAttValue.h +++ b/src/NimBLEAttValue.h @@ -21,6 +21,12 @@ #include "syscfg/syscfg.h" #if CONFIG_BT_NIMBLE_ENABLED +# ifdef USING_NIMBLE_ARDUINO_HEADERS +# include "nimble/nimble/include/nimble/nimble_npl.h" +# else +# include "nimble/nimble_npl.h" +# endif + /* Enables the use of Arduino String class for attribute values */ # ifndef NIMBLE_CPP_ARDUINO_STRING_AVAILABLE # define NIMBLE_CPP_ARDUINO_STRING_AVAILABLE (__has_include()) @@ -35,6 +41,7 @@ # include # include # include +# include # ifndef MYNEWT_VAL_NIMBLE_CPP_ATT_VALUE_TIMESTAMP_ENABLED # ifndef CONFIG_NIMBLE_CPP_ATT_VALUE_TIMESTAMP_ENABLED @@ -100,9 +107,69 @@ class NimBLEAttValue { # if MYNEWT_VAL(NIMBLE_CPP_ATT_VALUE_TIMESTAMP_ENABLED) time_t m_timestamp{}; # endif - void deepCopy(const NimBLEAttValue& source); + bool deepCopy(const NimBLEAttValue& source); public: + class Lock { + static ble_npl_mutex m_mutex; + static std::atomic m_initialized; + static std::atomic m_activeCount; + bool m_locked{false}; + + public: + Lock() { + // Increment active count to signal that a constructor is running + m_activeCount.fetch_add(1); + + if (m_initialized.load()) { + m_locked = (ble_npl_mutex_pend(&m_mutex, BLE_NPL_TIME_FOREVER) == BLE_NPL_OK); + + // Post-lock check: If deinit flipped the flag while we were waiting in line, + // we must abort, give it back, and act un-locked. + if (!m_initialized.load() && m_locked) { + ble_npl_mutex_release(&m_mutex); + m_locked = false; + } + } + + // Decrement active count to signal that the constructor has finished processing + m_activeCount.fetch_sub(1); + } + + ~Lock() { + if (m_locked) { + ble_npl_mutex_release(&m_mutex); + } + } + + static bool init() { + if (!m_initialized.load()) { + if (ble_npl_mutex_init(&m_mutex) == BLE_NPL_OK) { + m_initialized.store(true); + } + } + return m_initialized.load(); + } + + static void deinit() { + if (m_initialized.load()) { + // Prevent any new instances from acquiring the lock while we deinitialize + m_initialized.store(false); + + // Wait for any active users to finish + while (m_activeCount.load() > 0) { + ble_npl_time_delay(1); + } + + // Acquire the mutex to ensure no other threads are holding it before deinitializing. + if (ble_npl_mutex_pend(&m_mutex, BLE_NPL_TIME_FOREVER) == BLE_NPL_OK) { + // Deinitialize the mutex now that we have exclusive ownership and no active users. + ble_npl_mutex_deinit(&m_mutex); + } + } + } + }; + /** * @brief Default constructor. * @param[in] init_len The initial size in bytes. @@ -181,16 +248,22 @@ class NimBLEAttValue { /** @brief Returns the current size of the value in bytes */ uint16_t size() const { return m_attr_len; } - /** @brief Returns a pointer to the internal buffer of the value */ + /** + * @brief Returns a pointer to the internal buffer of the value + * @note The pointer is not protected by the lock and can be invalidated by a concurrent update. + * Only use the pointer getters (`data()`, `c_str()`, `begin()`, `end()`) on a copy of the value. + */ const uint8_t* data() const { return m_attr_value; } - /** @brief Returns a pointer to the internal buffer of the value as a const char* */ + /** @brief Returns a pointer to the internal buffer of the value as a const char* + * @note See data() for thread safety restrictions. + */ const char* c_str() const { return reinterpret_cast(m_attr_value); } - /** @brief Iterator begin */ + /** @brief Iterator begin; see data() for thread safety restrictions. */ const uint8_t* begin() const { return m_attr_value; } - /** @brief Iterator end */ + /** @brief Iterator end; see data() for thread safety restrictions. */ const uint8_t* end() const { return m_attr_value + m_attr_len; } # if MYNEWT_VAL(NIMBLE_CPP_ATT_VALUE_TIMESTAMP_ENABLED) @@ -250,6 +323,13 @@ class NimBLEAttValue { */ NimBLEAttValue& append(const uint8_t* value, uint16_t len); + /** + * @brief Copy the value from another NimBLEAttValue instance. + * @param[in] source The NimBLEAttValue instance to copy from. + * @returns True if the copy was successful. + */ + bool copyFrom(const NimBLEAttValue& source) { return deepCopy(source); } + /*********************** Template Functions ************************/ # if __cplusplus < 201703L @@ -262,9 +342,7 @@ class NimBLEAttValue { # ifdef _DOXYGEN_ bool # else - typename std::enable_if::value && - std::is_same::type, char>::value, - bool>::type + typename std::enable_if::value && std::is_same::type, char>::value, bool>::type # endif setValue(const T& s) { return setValue(reinterpret_cast(s), strnlen(s, sizeof(T))); @@ -281,8 +359,7 @@ class NimBLEAttValue { bool # else typename std::enable_if::value && !Has_c_str_length::value && !Has_data_size::value && - !(std::is_array::value && - std::is_same::type, char>::value), + !(std::is_array::value && std::is_same::type, char>::value), bool>::type # endif setValue(const T& v) { @@ -354,8 +431,7 @@ class NimBLEAttValue { } } else if constexpr (Has_c_str_length::value) { return setValue(reinterpret_cast(s.c_str()), s.length()); - } else if constexpr (std::is_array::value && - std::is_same::type, char>::value) { + } else if constexpr (std::is_array::value && std::is_same::type, char>::value) { return setValue(reinterpret_cast(s), strnlen(s, sizeof(s))); } else { return setValue(reinterpret_cast(&s), sizeof(s)); @@ -375,6 +451,7 @@ class NimBLEAttValue { */ template T getValue(time_t* timestamp = nullptr, bool skipSizeCheck = false) const { + Lock lock; if (timestamp != nullptr) { # if MYNEWT_VAL(NIMBLE_CPP_ATT_VALUE_TIMESTAMP_ENABLED) *timestamp = m_timestamp; @@ -395,16 +472,30 @@ class NimBLEAttValue { uint8_t operator[](int pos) const; /** @brief Operator; Get the value as a std::vector. */ - operator std::vector() const { return std::vector(m_attr_value, m_attr_value + m_attr_len); } + operator std::vector() const { + Lock lock; + return std::vector(m_attr_value, m_attr_value + m_attr_len); + } /** @brief Operator; Get the value as a std::string. */ - operator std::string() const { return std::string(reinterpret_cast(m_attr_value), m_attr_len); } + operator std::string() const { + Lock lock; + return std::string(reinterpret_cast(m_attr_value), m_attr_len); + } /** @brief Operator; Get the value as a const uint8_t*. */ operator const uint8_t*() const { return m_attr_value; } /** @brief Operator; Append another NimBLEAttValue. */ - NimBLEAttValue& operator+=(const NimBLEAttValue& source) { return append(source.data(), source.size()); } + NimBLEAttValue& operator+=(const NimBLEAttValue& source) { + Lock lock; // Recursive; keeps the source data and size consistent while appending + if (&source == this) { + // append() may reallocate the buffer, so append from a copy of our own data + std::vector copy(m_attr_value, m_attr_value + m_attr_len); + return append(copy.data(), copy.size()); + } + return append(source.data(), source.size()); + } /** @brief Operator; Set the value from a std::string source. */ NimBLEAttValue& operator=(const std::string& source) { @@ -420,6 +511,7 @@ class NimBLEAttValue { /** @brief Equality operator */ bool operator==(const NimBLEAttValue& source) const { + Lock lock; return (m_attr_len == source.size()) ? memcmp(m_attr_value, source.data(), m_attr_len) == 0 : false; } @@ -428,7 +520,10 @@ class NimBLEAttValue { # if NIMBLE_CPP_ARDUINO_STRING_AVAILABLE /** @brief Operator; Get the value as an Arduino String value. */ - operator String() const { return String(reinterpret_cast(m_attr_value)); } + operator String() const { + Lock lock; + return String(reinterpret_cast(m_attr_value)); + } # endif }; diff --git a/src/NimBLEClient.cpp b/src/NimBLEClient.cpp index f17d0489..470dbf2c 100644 --- a/src/NimBLEClient.cpp +++ b/src/NimBLEClient.cpp @@ -248,9 +248,9 @@ int NimBLEClient::startConnectionAttempt(const ble_addr_t* peerAddr) { */ bool NimBLEClient::connect(const NimBLEAddress& address, bool deleteAttributes, bool asyncConnect, bool exchangeMTU) { NIMBLE_LOGD(LOG_TAG, ">> connect(%s)", address.toString().c_str()); - NimBLETaskData taskData(this); - const ble_addr_t* peerAddr = address.getBase(); - int rc = 0; + NimBLEUtils::TaskData taskData(this); + const ble_addr_t* peerAddr = address.getBase(); + int rc = 0; if (!NimBLEDevice::m_synced) { NIMBLE_LOGE(LOG_TAG, "Host not synced with controller."); @@ -347,7 +347,7 @@ bool NimBLEClient::secureConnection(bool async) const { return true; } - NimBLETaskData taskData(const_cast(this), BLE_HS_ENOTCONN); + NimBLEUtils::TaskData taskData(const_cast(this), BLE_HS_ENOTCONN); m_pTaskData = &taskData; int retryCount = 1; do { @@ -769,8 +769,8 @@ bool NimBLEClient::retrieveServices(const NimBLEUUID* uuidFilter) { return false; } - int rc = 0; - NimBLETaskData taskData(this); + int rc = 0; + NimBLEUtils::TaskData taskData(this); if (uuidFilter == nullptr) { rc = ble_gattc_disc_all_svcs(m_connHandle, NimBLEClient::serviceDiscoveredCB, &taskData); @@ -809,8 +809,8 @@ int NimBLEClient::serviceDiscoveredCB(uint16_t connHandle, error->status, (error->status == 0) ? service->start_handle : -1); - NimBLETaskData* pTaskData = (NimBLETaskData*)arg; - NimBLEClient* pClient = (NimBLEClient*)pTaskData->m_pInstance; + NimBLEUtils::TaskData* pTaskData = (NimBLEUtils::TaskData*)arg; + NimBLEClient* pClient = (NimBLEClient*)pTaskData->m_pInstance; if (error->status == BLE_HS_ENOTCONN) { NIMBLE_LOGE(LOG_TAG, "<< Service Discovered; Disconnected"); @@ -1010,9 +1010,9 @@ void NimBLEClient::setConnectRetries(uint8_t numRetries) { * @param [in] arg A pointer to the client instance that registered for this callback. */ int NimBLEClient::handleGapEvent(struct ble_gap_event* event, void* arg) { - NimBLEClient* pClient = (NimBLEClient*)arg; - int rc = 0; - NimBLETaskData* pTaskData = pClient->m_pTaskData; // save a copy in case client is deleted + NimBLEClient* pClient = (NimBLEClient*)arg; + int rc = 0; + NimBLEUtils::TaskData* pTaskData = pClient->m_pTaskData; // save a copy in case client is deleted NIMBLE_LOGD(LOG_TAG, ">> handleGapEvent %s", NimBLEUtils::gapEventToString(event->type)); @@ -1179,34 +1179,32 @@ int NimBLEClient::handleGapEvent(struct ble_gap_event* event, void* arg) { return BLE_ATT_ERR_INVALID_HANDLE; } - auto len = event->notify_rx.om->om_len; - if (pChr->m_value.setValue(event->notify_rx.om->om_data, len)) { - os_mbuf* next; - next = SLIST_NEXT(event->notify_rx.om, om_next); - while (next != NULL) { - pChr->m_value.append(next->om_data, next->om_len); - if (pChr->m_value.length() != len + next->om_len) { - rc = BLE_ATT_ERR_INVALID_ATTR_VALUE_LEN; - break; - } - len += next->om_len; - next = SLIST_NEXT(next, om_next); - } - } else { - rc = BLE_ATT_ERR_INVALID_ATTR_VALUE_LEN; + // Build the value locally so the update of the characteristic value is atomic + // and the callback gets a stable copy without holding the value lock. + const auto totalLen = OS_MBUF_PKTLEN(event->notify_rx.om); + if (totalLen > BLE_ATT_ATTR_MAX_LEN) { + NIMBLE_LOGE(LOG_TAG, "notification value error; exceeds limit"); + return BLE_ATT_ERR_INVALID_ATTR_VALUE_LEN; } - if (rc != 0) { // This should never happen - NIMBLE_LOGE(LOG_TAG, "notification value error; exceeds limit"); - return rc; + NimBLEAttValue value(totalLen, BLE_ATT_ATTR_MAX_LEN); + if (value.data() == nullptr) { + NIMBLE_LOGE(LOG_TAG, "Failed to allocate notification value"); + return BLE_ATT_ERR_INSUFFICIENT_RES; + } + + for (os_mbuf* segment = event->notify_rx.om; segment != nullptr; segment = SLIST_NEXT(segment, om_next)) { + value.append(segment->om_data, segment->om_len); + } + + if (!pChr->m_value.copyFrom(value)) { + NIMBLE_LOGE(LOG_TAG, "Failed to copy value to characteristic"); + return BLE_ATT_ERR_INSUFFICIENT_RES; } if (pChr->m_notifyCallback != nullptr) { // TODO: change this callback to use the NimBLEAttValue class instead of raw data and length - pChr->m_notifyCallback(pChr, - const_cast(pChr->m_value.getValue().data()), - pChr->m_value.length(), - !event->notify_rx.indication); + pChr->m_notifyCallback(pChr, const_cast(value.data()), value.length(), !event->notify_rx.indication); } return 0; diff --git a/src/NimBLEClient.h b/src/NimBLEClient.h index 9b9b8538..761f9e72 100644 --- a/src/NimBLEClient.h +++ b/src/NimBLEClient.h @@ -28,6 +28,7 @@ # endif # include "NimBLEAddress.h" +# include "NimBLEUtils.h" # include # include @@ -41,7 +42,6 @@ class NimBLEAdvertisedDevice; class NimBLEAttValue; class NimBLEClientCallbacks; class NimBLEConnInfo; -struct NimBLETaskData; /** * @brief A model of a BLE client. @@ -155,7 +155,7 @@ class NimBLEClient { NimBLEAddress m_peerAddress; mutable int m_lastErr; int32_t m_connectTimeout; - mutable NimBLETaskData* m_pTaskData; + mutable NimBLEUtils::TaskData* m_pTaskData; std::vector m_svcVec; NimBLEClientCallbacks* m_pClientCallbacks; uint16_t m_connHandle; diff --git a/src/NimBLEDevice.cpp b/src/NimBLEDevice.cpp index 899242fc..15b9e5ec 100644 --- a/src/NimBLEDevice.cpp +++ b/src/NimBLEDevice.cpp @@ -54,6 +54,9 @@ # if defined(ESP_PLATFORM) && defined(CONFIG_ENABLE_ARDUINO_DEPENDS) # include "esp32-hal-bt.h" +# if __has_include("esp32-hal-bt-mem.h") +# include "esp32-hal-bt-mem.h" +# endif # endif # include "NimBLELog.h" @@ -91,9 +94,9 @@ NimBLEAdvertising* NimBLEDevice::m_bleAdvertising = nullptr; std::array NimBLEDevice::m_pClients{}; # endif -bool NimBLEDevice::m_initialized{false}; +std::atomic NimBLEDevice::m_initialized{false}; uint32_t NimBLEDevice::m_passkey{123456}; -bool NimBLEDevice::m_synced{false}; +std::atomic NimBLEDevice::m_synced{false}; ble_gap_event_listener NimBLEDevice::m_listener{}; std::vector NimBLEDevice::m_whiteList{}; uint8_t NimBLEDevice::m_ownAddrType{BLE_OWN_ADDR_PUBLIC}; @@ -254,7 +257,7 @@ NimBLEScan* NimBLEDevice::getScan() { * @details Must only be called before calling NimBLEDevice::init. */ void NimBLEDevice::setScanDuplicateCacheSize(uint16_t size) { - if (m_initialized) { + if (isInitialized()) { NIMBLE_LOGE(LOG_TAG, "Cannot change scan cache size while initialized"); return; } else { @@ -283,7 +286,7 @@ void NimBLEDevice::setScanDuplicateCacheSize(uint16_t size) { * @details Must only be called before calling NimBLEDevice::init. */ void NimBLEDevice::setScanFilterMode(uint8_t mode) { - if (m_initialized) { + if (isInitialized()) { NIMBLE_LOGE(LOG_TAG, "Cannot change scan duplicate type while initialized"); return; } else if (mode > 2) { @@ -301,7 +304,7 @@ void NimBLEDevice::setScanFilterMode(uint8_t mode) { * even if already seen in the current scan. If set to 0 the cache will never be reset. */ void NimBLEDevice::setScanDuplicateCacheResetTime(uint16_t time) { - if (m_initialized) { + if (isInitialized()) { NIMBLE_LOGE(LOG_TAG, "Cannot change scan cache reset time while initialized"); return; } else if (time > 1000) { @@ -725,7 +728,7 @@ bool NimBLEDevice::onWhiteList(const NimBLEAddress& address) { bool NimBLEDevice::whiteListAdd(const NimBLEAddress& address) { if (!NimBLEDevice::onWhiteList(address)) { m_whiteList.push_back(address); - int rc = ble_gap_wl_set(reinterpret_cast(&m_whiteList[0]), m_whiteList.size()); + int rc = ble_gap_wl_set(reinterpret_cast(m_whiteList.data()), m_whiteList.size()); if (rc != 0) { NIMBLE_LOGE(LOG_TAG, "Failed adding to whitelist rc=%d", rc); m_whiteList.pop_back(); @@ -745,7 +748,7 @@ bool NimBLEDevice::whiteListRemove(const NimBLEAddress& address) { for (auto it = m_whiteList.begin(); it < m_whiteList.end(); ++it) { if (*it == address) { m_whiteList.erase(it); - int rc = ble_gap_wl_set(reinterpret_cast(&m_whiteList[0]), m_whiteList.size()); + int rc = ble_gap_wl_set(reinterpret_cast(m_whiteList.data()), m_whiteList.size()); if (rc != 0) { m_whiteList.push_back(address); NIMBLE_LOGE(LOG_TAG, "Failed removing from whitelist rc=%d", rc); @@ -753,6 +756,10 @@ bool NimBLEDevice::whiteListRemove(const NimBLEAddress& address) { } std::vector(m_whiteList).swap(m_whiteList); + break; // `it` was invalidated by erase() and its buffer freed by the + // swap above; continuing would iterate a dangling iterator. + // Duplicates are impossible (whiteListAdd checks onWhiteList), + // so a single match is total. } } @@ -813,11 +820,11 @@ bool NimBLEDevice::setDefaultPhy(uint8_t txPhyMask, uint8_t rxPhyMask) { * @param [in] reason The reason code for the reset. */ void NimBLEDevice::onReset(int reason) { - if (!m_synced) { + if (!m_synced.load(std::memory_order_acquire)) { return; } - m_synced = false; + m_synced.store(false, std::memory_order_release); NIMBLE_LOGE(LOG_TAG, "Host reset; reason=%d, %s", reason, NimBLEUtils::returnCodeToString(reason)); } // onReset @@ -826,10 +833,10 @@ void NimBLEDevice::onReset(int reason) { * @brief Host synced with controller, all clear to make calls to the stack. */ void NimBLEDevice::onSync(void) { - NIMBLE_LOGI(LOG_TAG, "NimBle host synced."); + NIMBLE_LOGI(LOG_TAG, "NimBLE host synced."); // This check is needed due to potentially being called multiple times in succession // If this happens, the call to scan start may get stuck or cause an advertising fault. - if (m_synced) { + if (m_synced.load(std::memory_order_acquire)) { return; } @@ -854,9 +861,9 @@ void NimBLEDevice::onSync(void) { // Occasionally triggers exception without. ble_npl_time_delay(1); - m_synced = true; + m_synced.store(true, std::memory_order_release); - if (m_initialized) { + if (isInitialized()) { # if MYNEWT_VAL(BLE_ROLE_OBSERVER) if (m_pScan != nullptr) { m_pScan->onHostSync(); @@ -885,7 +892,7 @@ void NimBLEDevice::host_task(void* param) { * @param [in] deviceName The device name of the device. */ bool NimBLEDevice::init(const std::string& deviceName) { - if (!m_initialized) { + if (!isInitialized()) { NIMBLE_LOGD(LOG_TAG, "Starting %s", getVersion()); # ifdef ESP_PLATFORM @@ -997,18 +1004,24 @@ bool NimBLEDevice::init(const std::string& deviceName) { ble_hs_cfg.sm_their_key_dist |= BLE_SM_PAIR_KEY_DIST_ID; # endif + if (!NimBLEAttValue::Lock::init()) { + NIMBLE_LOGE(LOG_TAG, "Failed to initialize value lock"); + nimble_port_deinit(); + return false; + } + setDeviceName(deviceName); ble_store_config_init(); nimble_port_freertos_init(NimBLEDevice::host_task); } // Wait for host and controller to sync before returning and accepting new tasks - while (!m_synced) { + while (!m_synced.load(std::memory_order_acquire)) { ble_npl_time_delay(1); } - m_initialized = true; // Set the initialization flag to ensure we are only initialized once. - NIMBLE_LOGD(LOG_TAG, "Initialized"); + m_initialized.store(true); // Set the initialization flag to ensure we are only initialized once. + NIMBLE_LOGD(LOG_TAG, "Initialized BLE stack"); return true; } // init @@ -1021,9 +1034,18 @@ bool NimBLEDevice::init(const std::string& deviceName) { */ bool NimBLEDevice::deinit(bool clearAll) { int rc = 0; - if (m_initialized) { + if (isInitialized()) { rc = nimble_port_stop(); if (rc == 0) { + m_initialized.store(false); + m_synced.store(false, std::memory_order_release); +# if MYNEWT_VAL(BLE_ROLE_OBSERVER) + if (NimBLEDevice::m_pScan != nullptr) { + NimBLEDevice::m_pScan->onHostDeinit(); + } +# endif + NimBLEAttValue::Lock::deinit(); + NimBLEUtils::deinit(); nimble_port_deinit(); # ifndef USING_NIMBLE_ARDUINO_HEADERS # if ESP_IDF_VERSION < ESP_IDF_VERSION_VAL(5, 0, 0) @@ -1033,8 +1055,9 @@ bool NimBLEDevice::deinit(bool clearAll) { } # endif # endif - m_initialized = false; - m_synced = false; + } else { + NIMBLE_LOGE(LOG_TAG, "nimble_port_stop() failed with error: %d", rc); + return false; } } @@ -1081,7 +1104,7 @@ bool NimBLEDevice::deinit(bool clearAll) { * @return true if initialized. */ bool NimBLEDevice::isInitialized() { - return m_initialized; + return m_initialized.load(); } // getInitialized /* -------------------------------------------------------------------------- */ diff --git a/src/NimBLEDevice.h b/src/NimBLEDevice.h index fb64c83d..fc247bf7 100644 --- a/src/NimBLEDevice.h +++ b/src/NimBLEDevice.h @@ -42,6 +42,7 @@ # include # include +# include # if MYNEWT_VAL(BLE_ROLE_CENTRAL) # include @@ -214,8 +215,8 @@ class NimBLEDevice { # endif private: - static bool m_synced; - static bool m_initialized; + static std::atomic m_synced; + static std::atomic m_initialized; static uint32_t m_passkey; static ble_gap_event_listener m_listener; static uint8_t m_ownAddrType; diff --git a/src/NimBLEEddystoneTLM.cpp b/src/NimBLEEddystoneTLM.cpp index 8ee405b5..4cfd7498 100644 --- a/src/NimBLEEddystoneTLM.cpp +++ b/src/NimBLEEddystoneTLM.cpp @@ -101,10 +101,12 @@ std::string NimBLEEddystoneTLM::toString() { out += val; out += " mV\n"; - out += "Temperature "; - uint8_t intTemp = m_eddystoneData.temp / 256; - uint8_t frac = m_eddystoneData.temp % 256 * 100 / 256; - snprintf(val, sizeof(val), "%d.%d", intTemp, frac); + out += "Temperature "; + int16_t temp = ENDIAN_CHANGE_U16(m_eddystoneData.temp); + uint32_t absTemp = temp < 0 ? -static_cast(temp) : temp; + uint8_t intTemp = absTemp / 256; + uint8_t frac = absTemp % 256 * 100 / 256; + snprintf(val, sizeof(val), "%s%d.%02d", (temp < 0 ? "-" : ""), intTemp, frac); out += val; out += " C\n"; @@ -188,7 +190,7 @@ void NimBLEEddystoneTLM::setVersion(uint8_t version) { * @param [in] volt The voltage in millivolts. */ void NimBLEEddystoneTLM::setVolt(uint16_t volt) { - m_eddystoneData.volt = volt; + m_eddystoneData.volt = ENDIAN_CHANGE_U16(volt); } // setVolt /** @@ -196,7 +198,7 @@ void NimBLEEddystoneTLM::setVolt(uint16_t volt) { * @param [in] temp The temperature value in 8.8 fixed point format. */ void NimBLEEddystoneTLM::setTemp(int16_t temp) { - m_eddystoneData.temp = temp; + m_eddystoneData.temp = ENDIAN_CHANGE_U16(temp); } // setTemp /** @@ -204,7 +206,7 @@ void NimBLEEddystoneTLM::setTemp(int16_t temp) { * @param [in] advCount The advertisement number. */ void NimBLEEddystoneTLM::setCount(uint32_t advCount) { - m_eddystoneData.advCount = advCount; + m_eddystoneData.advCount = ENDIAN_CHANGE_U32(advCount); } // setCount /** @@ -212,7 +214,7 @@ void NimBLEEddystoneTLM::setCount(uint32_t advCount) { * @param [in] tmil The advertisement time in milliseconds. */ void NimBLEEddystoneTLM::setTime(uint32_t tmil) { - m_eddystoneData.tmil = tmil; + m_eddystoneData.tmil = ENDIAN_CHANGE_U32(tmil); } // setTime #endif // CONFIG_BT_NIMBLE_ENABLED && MYNEWT_VAL(BLE_ROLE_BROADCASTER) diff --git a/src/NimBLEEddystoneTLM.h b/src/NimBLEEddystoneTLM.h index f48cf94c..fdfcb841 100644 --- a/src/NimBLEEddystoneTLM.h +++ b/src/NimBLEEddystoneTLM.h @@ -37,8 +37,8 @@ class NimBLEEddystoneTLM { struct BeaconData { uint8_t frameType{EDDYSTONE_TLM_FRAME_TYPE}; uint8_t version{0}; - uint16_t volt{3300}; - uint16_t temp{23 * 256}; + uint16_t volt{0xE40C}; // Big-endian (wire format) encoding of 3300 mV; getVolt() swaps back to host order. + uint16_t temp{0x0017}; // Big-endian (wire format) encoding of 23.0C (23 * 256); getTemp() swaps back to host order. uint32_t advCount{0}; uint32_t tmil{0}; } __attribute__((packed)); diff --git a/src/NimBLEL2CAPChannel.cpp b/src/NimBLEL2CAPChannel.cpp index 2eff6a82..36b740ad 100644 --- a/src/NimBLEL2CAPChannel.cpp +++ b/src/NimBLEL2CAPChannel.cpp @@ -88,7 +88,7 @@ int NimBLEL2CAPChannel::writeFragment(std::vector::const_iterator begin if (stalled) { NIMBLE_LOGD(LOG_TAG, "L2CAP Channel waiting for unstall..."); - NimBLETaskData taskData; + NimBLEUtils::TaskData taskData; m_pTaskData = &taskData; NimBLEUtils::taskWait(taskData, BLE_NPL_TIME_FOREVER); m_pTaskData = nullptr; diff --git a/src/NimBLEL2CAPChannel.h b/src/NimBLEL2CAPChannel.h index 3c1c9507..0e196bdb 100644 --- a/src/NimBLEL2CAPChannel.h +++ b/src/NimBLEL2CAPChannel.h @@ -22,12 +22,13 @@ # undef max /**************************/ +# include "NimBLEUtils.h" + # include # include class NimBLEClient; class NimBLEL2CAPChannelCallbacks; -struct NimBLETaskData; /** * @brief Encapsulates a L2CAP channel. @@ -94,7 +95,7 @@ class NimBLEL2CAPChannel { // Runtime handling std::atomic stalled{false}; - NimBLETaskData* m_pTaskData{nullptr}; + NimBLEUtils::TaskData* m_pTaskData{nullptr}; // Allocate / deallocate NimBLE memory pool bool setupMemPool(); diff --git a/src/NimBLELocalValueAttribute.h b/src/NimBLELocalValueAttribute.h index 5fd37e55..0c4f3909 100644 --- a/src/NimBLELocalValueAttribute.h +++ b/src/NimBLELocalValueAttribute.h @@ -125,8 +125,8 @@ class NimBLELocalValueAttribute : public NimBLELocalAttribute, public NimBLEValu virtual void writeEvent(const uint8_t* val, uint16_t len, NimBLEConnInfo& connInfo) = 0; /** - * @brief Get a pointer to value of the attribute. - * @return A pointer to the value of the attribute. + * @brief Get a reference to value of the attribute. + * @return A reference to the value of the attribute. * @details This function is used by NimBLEServer when handling read/write requests. */ const NimBLEAttValue& getAttVal() const { return m_value; } diff --git a/src/NimBLERemoteCharacteristic.cpp b/src/NimBLERemoteCharacteristic.cpp index 64a1f308..30023799 100644 --- a/src/NimBLERemoteCharacteristic.cpp +++ b/src/NimBLERemoteCharacteristic.cpp @@ -26,10 +26,13 @@ # include +// Private structure used to filter descriptors during discovery and return results. struct NimBLEDescriptorFilter { - NimBLERemoteDescriptor* dsc; + NimBLEDescriptorFilter() = delete; + NimBLEDescriptorFilter(const NimBLEUUID* uuid, NimBLEUtils::TaskData* td = nullptr) : uuid(uuid), taskData(td) {} const NimBLEUUID* uuid; - void* taskData; + NimBLEUtils::TaskData* taskData; + NimBLERemoteDescriptor* dsc{nullptr}; }; static const char* LOG_TAG = "NimBLERemoteCharacteristic"; @@ -66,7 +69,7 @@ int NimBLERemoteCharacteristic::descriptorDiscCB( uint16_t connHandle, const ble_gatt_error* error, uint16_t chrHandle, const ble_gatt_dsc* dsc, void* arg) { int rc = error->status; auto filter = (NimBLEDescriptorFilter*)arg; - auto pTaskData = (NimBLETaskData*)filter->taskData; + auto pTaskData = (NimBLEUtils::TaskData*)filter->taskData; const auto pChr = (NimBLERemoteCharacteristic*)pTaskData->m_pInstance; const auto uuid = filter->uuid; // UUID to filter for NIMBLE_LOGD(LOG_TAG, "Descriptor Discovery >> status: %d handle: %d", rc, (rc == 0) ? dsc->handle : -1); @@ -116,10 +119,12 @@ bool NimBLERemoteCharacteristic::retrieveDescriptors(NimBLEDescriptorFilter* pFi return true; } - NimBLETaskData taskData(const_cast(this)); - NimBLEDescriptorFilter defaultFilter{nullptr, nullptr, &taskData}; + NimBLEUtils::TaskData taskData(const_cast(this)); + NimBLEDescriptorFilter defaultFilter(nullptr, &taskData); if (pFilter == nullptr) { pFilter = &defaultFilter; + } else { + pFilter->taskData = &taskData; } int rc = ble_gattc_disc_all_dscs(getClient()->getConnHandle(), @@ -134,7 +139,7 @@ bool NimBLERemoteCharacteristic::retrieveDescriptors(NimBLEDescriptorFilter* pFi auto prevDscCount = m_vDescriptors.size(); NimBLEUtils::taskWait(taskData, BLE_NPL_TIME_FOREVER); - rc = ((NimBLETaskData*)pFilter->taskData)->m_flags; + rc = taskData.m_flags; if (rc != BLE_HS_EDONE) { NIMBLE_LOGE(LOG_TAG, "<< retrieveDescriptors(): failed: rc=%d %s", rc, NimBLEUtils::returnCodeToString(rc)); return false; @@ -156,8 +161,7 @@ bool NimBLERemoteCharacteristic::retrieveDescriptors(NimBLEDescriptorFilter* pFi NimBLERemoteDescriptor* NimBLERemoteCharacteristic::getDescriptor(const NimBLEUUID& uuid) const { NIMBLE_LOGD(LOG_TAG, ">> getDescriptor: uuid: %s", uuid.toString().c_str()); NimBLEUUID uuidTmp{uuid}; - NimBLETaskData taskData(const_cast(this)); - NimBLEDescriptorFilter filter{nullptr, &uuidTmp, &taskData}; + NimBLEDescriptorFilter filter(&uuidTmp); for (const auto& dsc : m_vDescriptors) { if (dsc->getUUID() == uuid) { diff --git a/src/NimBLERemoteService.cpp b/src/NimBLERemoteService.cpp index f15a9a31..f5831a41 100644 --- a/src/NimBLERemoteService.cpp +++ b/src/NimBLERemoteService.cpp @@ -75,7 +75,7 @@ NimBLERemoteCharacteristic* NimBLERemoteService::getCharacteristic(const char* u */ NimBLERemoteCharacteristic* NimBLERemoteService::getCharacteristic(const NimBLEUUID& uuid) const { NIMBLE_LOGD(LOG_TAG, ">> getCharacteristic: uuid: %s", uuid.toString().c_str()); - NimBLERemoteCharacteristic* pChar = nullptr; + NimBLERemoteCharacteristic* pChar = nullptr; for (const auto& it : m_vChars) { if (it->getUUID() == uuid) { @@ -136,7 +136,7 @@ int NimBLERemoteService::characteristicDiscCB(uint16_t conn_handle, "Characteristic Discovery >> status: %d handle: %d", error->status, (error->status == 0) ? chr->def_handle : -1); - auto pTaskData = (NimBLETaskData*)arg; + auto pTaskData = (NimBLEUtils::TaskData*)arg; const auto pSvc = (NimBLERemoteService*)pTaskData->m_pInstance; if (error->status == BLE_HS_ENOTCONN) { @@ -178,8 +178,8 @@ int NimBLERemoteService::characteristicDiscCB(uint16_t conn_handle, */ bool NimBLERemoteService::retrieveCharacteristics(const NimBLEUUID* uuidFilter, NimBLERemoteCharacteristic** ppChar) const { NIMBLE_LOGD(LOG_TAG, ">> retrieveCharacteristics()"); - int rc = 0; - NimBLETaskData taskData(const_cast(this)); + int rc = 0; + NimBLEUtils::TaskData taskData(const_cast(this)); if (uuidFilter == nullptr) { rc = ble_gattc_disc_all_chrs(m_pClient->getConnHandle(), diff --git a/src/NimBLERemoteValueAttribute.cpp b/src/NimBLERemoteValueAttribute.cpp index f7021145..cb270d07 100644 --- a/src/NimBLERemoteValueAttribute.cpp +++ b/src/NimBLERemoteValueAttribute.cpp @@ -29,11 +29,11 @@ static const char* LOG_TAG = "NimBLERemoteValueAttribute"; bool NimBLERemoteValueAttribute::writeValue(const uint8_t* data, size_t length, bool response) const { NIMBLE_LOGD(LOG_TAG, ">> writeValue()"); - const NimBLEClient* pClient = getClient(); - int retryCount = 1; - int rc = 0; - uint16_t mtu = pClient->getMTU() - 3; - NimBLETaskData taskData(const_cast(this)); + const NimBLEClient* pClient = getClient(); + int retryCount = 1; + int rc = 0; + uint16_t mtu = pClient->getMTU() - 3; + NimBLEUtils::TaskData taskData(const_cast(this)); // Check if the data length is longer than we can write in one connection event. // If so we must do a long write which requires a response. @@ -98,7 +98,7 @@ bool NimBLERemoteValueAttribute::writeValue(const uint8_t* data, size_t length, * @return success == 0 or error code. */ int NimBLERemoteValueAttribute::onWriteCB(uint16_t conn_handle, const ble_gatt_error* error, ble_gatt_attr* attr, void* arg) { - auto pTaskData = static_cast(arg); + auto pTaskData = static_cast(arg); const auto pAtt = static_cast(pTaskData->m_pInstance); if (error->status == BLE_HS_ENOTCONN) { @@ -124,11 +124,11 @@ int NimBLERemoteValueAttribute::onWriteCB(uint16_t conn_handle, const ble_gatt_e NimBLEAttValue NimBLERemoteValueAttribute::readValue(time_t* timestamp) { NIMBLE_LOGD(LOG_TAG, ">> readValue()"); - NimBLEAttValue value{}; - const NimBLEClient* pClient = getClient(); - int rc = 0; - int retryCount = 1; - NimBLETaskData taskData(const_cast(this), 0, &value); + NimBLEAttValue value{}; + const NimBLEClient* pClient = getClient(); + int rc = 0; + int retryCount = 1; + NimBLEUtils::TaskData taskData(const_cast(this), 0, &value); do { rc = ble_gattc_read_long(pClient->getConnHandle(), getHandle(), 0, NimBLERemoteValueAttribute::onReadCB, &taskData); @@ -183,7 +183,7 @@ NimBLEAttValue NimBLERemoteValueAttribute::readValue(time_t* timestamp) { * @return success == 0 or error code. */ int NimBLERemoteValueAttribute::onReadCB(uint16_t conn_handle, const ble_gatt_error* error, ble_gatt_attr* attr, void* arg) { - auto pTaskData = static_cast(arg); + auto pTaskData = static_cast(arg); const auto pAtt = static_cast(pTaskData->m_pInstance); if (error->status == BLE_HS_ENOTCONN) { diff --git a/src/NimBLEScan.cpp b/src/NimBLEScan.cpp index 54028075..870e233f 100644 --- a/src/NimBLEScan.cpp +++ b/src/NimBLEScan.cpp @@ -84,7 +84,6 @@ NimBLEScan::NimBLEScan() }, m_pTaskData{nullptr}, m_maxResults{0xFF} { - ble_npl_callout_init(&m_srTimer, nimble_port_get_dflt_eventq(), NimBLEScan::srTimerCb, nullptr); ble_npl_time_ms_to_ticks(DEFAULT_SCAN_RESP_TIMEOUT_MS, &m_srTimeoutTicks); } // NimBLEScan::NimBLEScan @@ -92,7 +91,10 @@ NimBLEScan::NimBLEScan() * @brief Scan destructor, release any allocated resources. */ NimBLEScan::~NimBLEScan() { - ble_npl_callout_deinit(&m_srTimer); + if (m_srTimerInitialized) { + ble_npl_callout_deinit(&m_srTimer); + m_srTimerInitialized = false; + } for (const auto& dev : m_scanResults.m_deviceVec) { delete dev; @@ -178,7 +180,9 @@ void NimBLEScan::removeWaitingDevice(NimBLEAdvertisedDevice* pDev) { void NimBLEScan::clearWaitingList() { // Stop the timer and remove any pending timeout events since we're clearing // the list and won't be processing any more timeouts for these devices - ble_npl_callout_stop(&m_srTimer); + if (m_srTimerInitialized) { + ble_npl_callout_stop(&m_srTimer); + } ble_npl_hw_enter_critical(); NimBLEAdvertisedDevice* current = m_pWaitingListHead; while (current != nullptr) { @@ -196,10 +200,19 @@ void NimBLEScan::clearWaitingList() { */ void NimBLEScan::resetWaitingTimer() { if (m_srTimeoutTicks == 0 || m_pWaitingListHead == nullptr) { - ble_npl_callout_stop(&m_srTimer); + if (m_srTimerInitialized) { + ble_npl_callout_stop(&m_srTimer); + } return; } + if (!m_srTimerInitialized) { + m_srTimerInitialized = ble_npl_callout_init(&m_srTimer, nimble_port_get_dflt_eventq(), NimBLEScan::srTimerCb, nullptr) == 0; + if (!m_srTimerInitialized) { + return; + } + } + ble_npl_time_t now = ble_npl_time_get(); ble_npl_time_t elapsed = now - m_pWaitingListHead->m_time; ble_npl_time_t nextTime = elapsed >= m_srTimeoutTicks ? 1 : m_srTimeoutTicks - elapsed; @@ -346,7 +359,9 @@ int NimBLEScan::handleGapEvent(ble_gap_event* event, void* arg) { } case BLE_GAP_EVENT_DISC_COMPLETE: { - ble_npl_callout_stop(&pScan->m_srTimer); + if (pScan->m_srTimerInitialized) { + ble_npl_callout_stop(&pScan->m_srTimer); + } // If we have any scannable devices that haven't received a scan response, // we should trigger the callback with whatever data we have since the scan is complete @@ -392,7 +407,9 @@ int NimBLEScan::handleGapEvent(ble_gap_event* event, void* arg) { */ void NimBLEScan::setScanResponseTimeout(uint32_t timeoutMs) { if (timeoutMs == 0) { - ble_npl_callout_stop(&m_srTimer); + if (m_srTimerInitialized) { + ble_npl_callout_stop(&m_srTimer); + } m_srTimeoutTicks = 0; return; } @@ -673,6 +690,18 @@ void NimBLEScan::onHostSync() { m_pScanCallbacks->onScanEnd(m_scanResults, BLE_HS_ENOTSYNCED); } +/** + * @brief Called before host deinit so callouts don't outlive the default event queue. + */ +void NimBLEScan::onHostDeinit() { + clearWaitingList(); + + if (m_srTimerInitialized) { + ble_npl_callout_deinit(&m_srTimer); + m_srTimerInitialized = false; + } +} + /** * @brief Start scanning and block until scanning has been completed. * @param [in] duration The duration in milliseconds for which to scan. @@ -689,7 +718,7 @@ NimBLEScanResults NimBLEScan::getResults(uint32_t duration, bool is_continue) { return m_scanResults; } - NimBLETaskData taskData; + NimBLEUtils::TaskData taskData; m_pTaskData = &taskData; if (start(duration, is_continue)) { diff --git a/src/NimBLEScan.h b/src/NimBLEScan.h index 2b8a2499..89c858c8 100644 --- a/src/NimBLEScan.h +++ b/src/NimBLEScan.h @@ -129,11 +129,16 @@ class NimBLEScan { snprintf(&out[0], out.size(), "Scan stats:\n" - " Devices seen : %" PRIu32 "\n" - " Duplicate advs : %" PRIu32 "\n" - " Scan responses : %" PRIu32 "\n" - " SR timing (ms) : min=%" PRIu32 ", max=%" PRIu32 ", avg=%" PRIu64 "\n" - " Orphaned SR : %" PRIu32 "\n" + " Devices seen : %" PRIu32 + "\n" + " Duplicate advs : %" PRIu32 + "\n" + " Scan responses : %" PRIu32 + "\n" + " SR timing (ms) : min=%" PRIu32 ", max=%" PRIu32 ", avg=%" PRIu64 + "\n" + " Orphaned SR : %" PRIu32 + "\n" " Missed SR : %" PRIu32 "\n", devCount, dupCount, @@ -176,6 +181,7 @@ class NimBLEScan { ~NimBLEScan(); static int handleGapEvent(ble_gap_event* event, void* arg); void onHostSync(); + void onHostDeinit(); static void srTimerCb(ble_npl_event* event); // Linked list helpers for devices awaiting scan responses @@ -187,8 +193,9 @@ class NimBLEScan { NimBLEScanCallbacks* m_pScanCallbacks; ble_gap_disc_params m_scanParams; NimBLEScanResults m_scanResults; - NimBLETaskData* m_pTaskData; + NimBLEUtils::TaskData* m_pTaskData; ble_npl_callout m_srTimer{}; + bool m_srTimerInitialized{false}; ble_npl_time_t m_srTimeoutTicks{}; uint8_t m_maxResults; NimBLEAdvertisedDevice* m_pWaitingListHead{}; // head of linked list for devices awaiting scan responses diff --git a/src/NimBLEServer.cpp b/src/NimBLEServer.cpp index 1cab58ed..3129d113 100644 --- a/src/NimBLEServer.cpp +++ b/src/NimBLEServer.cpp @@ -54,6 +54,7 @@ NimBLEServer::NimBLEServer() : m_gattsStarted{false}, m_svcChanged{false}, m_deleteCallbacks{false}, + m_registerAppServicesFirst{false}, # if !MYNEWT_VAL(BLE_EXT_ADV) m_advertiseOnDisconnect{false}, # endif @@ -366,6 +367,20 @@ void NimBLEServer::advertiseOnDisconnect(bool enable) { } // advertiseOnDisconnect # endif +/** + * @brief Register the application's services before the standard GAP/GATT services. + * @param [in] enable true == register the application services first (they take the + * low attribute handles, starting at 0x0001, and GAP/GATT are placed after them); + * false (default) == the standard behavior, GAP/GATT register first and take the low + * handles. + * @details Must be called before the services are started (i.e. before + * NimBLEServer::start()). Useful when a peripheral must expose a fixed attribute-table + * layout that another device relies on by hardcoded handle rather than discovery. + */ +void NimBLEServer::registerAppServicesFirst(bool enable) { + m_registerAppServicesFirst = enable; +} // registerAppServicesFirst + /** * @brief Return the number of connected clients. * @return The number of connected clients. @@ -745,9 +760,8 @@ int NimBLEServer::handleGattEvent(uint16_t connHandle, uint16_t attrHandle, ble_ pAtt->readEvent(peerInfo); } - ble_npl_hw_enter_critical(); + NimBLEAttValue::Lock lock; int rc = os_mbuf_append(ctxt->om, val.data(), val.size()); - ble_npl_hw_exit_critical(0); return rc == 0 ? 0 : BLE_ATT_ERR_INSUFFICIENT_RES; } @@ -889,9 +903,32 @@ bool NimBLEServer::resetGATT() { NimBLEDevice::stopAdvertising(); # endif + // esp-idf nimble clears the device name and + // appearance during ble_gatts_reset() +#ifndef CONFIG_USING_NIMBLE_COMPONENT + std::string name(ble_svc_gap_device_name()); + uint16_t appearance = ble_svc_gap_device_appearance(); +#endif + ble_gatts_reset(); - ble_svc_gap_init(); - ble_svc_gatt_init(); + + // Register the standard GAP (0x1800) and GATT (0x1801) services, restoring the + // device name/appearance that ble_gatts_reset() clears. By default this happens + // first, so GAP/GATT take the low attribute handles (0x0001+). When + // registerAppServicesFirst() is enabled, it is deferred until after the application + // services below, so those take the low handles instead. + auto initGapGattServices = [&]() { + ble_svc_gap_init(); +#ifndef CONFIG_USING_NIMBLE_COMPONENT + ble_svc_gap_device_name_set(name.c_str()); + ble_svc_gap_device_appearance_set(appearance); +#endif + ble_svc_gatt_init(); + }; + + if (!m_registerAppServicesFirst) { + initGapGattServices(); + } for (auto svcIt = m_svcVec.begin(); svcIt != m_svcVec.end();) { auto* pSvc = *svcIt; @@ -928,6 +965,12 @@ bool NimBLEServer::resetGATT() { if (pSvc->getRemoved() == 0) { if (!pSvc->start_internal()) { NIMBLE_LOGE(LOG_TAG, "Failed to start service: %s", pSvc->getUUID().toString().c_str()); + // When deferring GAP/GATT (registerAppServicesFirst), still register + // them on the failure path so the mandatory GAP/GATT services (and + // the restored name/appearance) are never left out of the database. + if (m_registerAppServicesFirst) { + initGapGattServices(); + } return false; } } @@ -936,6 +979,10 @@ bool NimBLEServer::resetGATT() { ++svcIt; } + if (m_registerAppServicesFirst) { + initGapGattServices(); + } + return true; } // resetGATT diff --git a/src/NimBLEServer.h b/src/NimBLEServer.h index b2b79e1e..cd07b171 100644 --- a/src/NimBLEServer.h +++ b/src/NimBLEServer.h @@ -81,6 +81,7 @@ class NimBLEServer { NimBLEConnInfo getPeerInfo(const NimBLEAddress& address) const; NimBLEConnInfo getPeerInfoByHandle(uint16_t connHandle) const; void advertiseOnDisconnect(bool enable); + void registerAppServicesFirst(bool enable); void setDataLen(uint16_t connHandle, uint16_t tx_octets) const; bool updatePhy(uint16_t connHandle, uint8_t txPhysMask, uint8_t rxPhysMask, uint16_t phyOptions); bool getPhy(uint16_t connHandle, uint8_t* txPhy, uint8_t* rxPhy); @@ -129,6 +130,7 @@ class NimBLEServer { bool m_gattsStarted : 1; bool m_svcChanged : 1; bool m_deleteCallbacks : 1; + bool m_registerAppServicesFirst : 1; # if !MYNEWT_VAL(BLE_EXT_ADV) && MYNEWT_VAL(BLE_ROLE_BROADCASTER) bool m_advertiseOnDisconnect : 1; # endif diff --git a/src/NimBLEStream.cpp b/src/NimBLEStream.cpp index 7ed0c908..853cef22 100644 --- a/src/NimBLEStream.cpp +++ b/src/NimBLEStream.cpp @@ -356,8 +356,8 @@ size_t NimBLEStream::write(const uint8_t* data, size_t len) { * @brief Get the available free space in the stream's TX buffer. * @return the number of bytes that can be written to the stream without blocking. */ -size_t NimBLEStream::availableForWrite() const { - return m_txBuf ? m_txBuf->freeSize() : 0; +int NimBLEStream::availableForWrite() { + return m_txBuf ? static_cast(m_txBuf->freeSize()) : 0; } /** diff --git a/src/NimBLEStream.h b/src/NimBLEStream.h index 47ccb83d..5285294b 100644 --- a/src/NimBLEStream.h +++ b/src/NimBLEStream.h @@ -55,6 +55,7 @@ class Stream : public Print { virtual int available() = 0; virtual int read() = 0; virtual int peek() = 0; + virtual int availableForWrite() { return 0; } virtual void flush() {} void setTimeout(unsigned long timeout) { m_timeout = timeout; } unsigned long getTimeout() const { return m_timeout; } @@ -86,7 +87,7 @@ class NimBLEStream : public Stream { return write(static_cast(data)); } - size_t availableForWrite() const; + int availableForWrite() override; // Read up to len bytes into buffer (non-blocking) size_t read(uint8_t* buffer, size_t len); diff --git a/src/NimBLEUtils.cpp b/src/NimBLEUtils.cpp index 75969a59..683a05a6 100644 --- a/src/NimBLEUtils.cpp +++ b/src/NimBLEUtils.cpp @@ -18,12 +18,12 @@ #include "NimBLEUtils.h" #if CONFIG_BT_NIMBLE_ENABLED -# include "NimBLEAddress.h" +# include "NimBLEDevice.h" # include "NimBLELog.h" -#ifdef USING_NIMBLE_ARDUINO_HEADERS +# ifdef USING_NIMBLE_ARDUINO_HEADERS # include "nimble/nimble/host/include/host/ble_hs.h" -#else +# else # include "host/ble_hs.h" # endif @@ -59,57 +59,20 @@ # endif # endif -# if defined INC_FREERTOS_H -# ifndef MYNEWT_VAL_NIMBLE_CPP_FREERTOS_TASK_BLOCK_BIT -# ifndef CONFIG_NIMBLE_CPP_FREERTOS_TASK_BLOCK_BIT -# define MYNEWT_VAL_NIMBLE_CPP_FREERTOS_TASK_BLOCK_BIT 31 -# else -# define MYNEWT_VAL_NIMBLE_CPP_FREERTOS_TASK_BLOCK_BIT CONFIG_NIMBLE_CPP_FREERTOS_TASK_BLOCK_BIT -# endif -# endif -# endif +static const char* LOG_TAG = "NimBLEUtils"; -constexpr uint32_t TASK_BLOCK_BIT = (1 << MYNEWT_VAL(NIMBLE_CPP_FREERTOS_TASK_BLOCK_BIT)); -static const char* LOG_TAG = "NimBLEUtils"; +std::vector NimBLEUtils::m_taskSemEntries{}; +std::atomic NimBLEUtils::m_semUserCount{0}; +std::atomic_flag NimBLEUtils::m_taskSemLock = ATOMIC_FLAG_INIT; /** - * @brief Construct a NimBLETaskData instance. + * @brief Construct a NimBLEUtils::TaskData instance. * @param [in] pInstance An instance of the class that will be waiting. * @param [in] flags General purpose flags for the caller. * @param [in] buf A buffer for data. */ -NimBLETaskData::NimBLETaskData(void* pInstance, int flags, void* buf) - : m_pInstance{pInstance}, - m_flags{flags}, - m_pBuf{buf} -# ifdef INC_FREERTOS_H - , - m_pHandle{xTaskGetCurrentTaskHandle()} { -} -# else -{ - ble_npl_sem* sem = new ble_npl_sem; - if (ble_npl_sem_init(sem, 0) != BLE_NPL_OK) { - NIMBLE_LOGE(LOG_TAG, "Failed to init semaphore"); - delete sem; - m_pHandle = nullptr; - } else { - m_pHandle = sem; - } -} -# endif - -/** - * @brief Destructor. - */ -NimBLETaskData::~NimBLETaskData() { -# ifndef INC_FREERTOS_H - if (m_pHandle != nullptr) { - ble_npl_sem_deinit(static_cast(m_pHandle)); - delete static_cast(m_pHandle); - } -# endif -} +NimBLEUtils::TaskData::TaskData(void* pInstance, int flags, void* buf) + : m_pInstance{pInstance}, m_flags{flags}, m_pBuf{buf} {} /** * @brief Blocks the calling task until released or timeout. @@ -117,7 +80,16 @@ NimBLETaskData::~NimBLETaskData() { * @param [in] timeout The time to wait in milliseconds. * @return True if the task completed, false if the timeout was reached. */ -bool NimBLEUtils::taskWait(const NimBLETaskData& taskData, uint32_t timeout) { +bool NimBLEUtils::taskWait(const TaskData& taskData, uint32_t timeout) { + m_semUserCount.fetch_add(1); + + if (!NimBLEDevice::isInitialized()) { + NIMBLE_LOGE(LOG_TAG, "NimBLE stack is not initialized"); + taskData.m_flags = BLE_HS_EOS; + m_semUserCount.fetch_sub(1); + return false; + } + ble_npl_time_t ticks; if (timeout == BLE_NPL_TIME_FOREVER) { ticks = BLE_NPL_TIME_FOREVER; @@ -125,18 +97,49 @@ bool NimBLEUtils::taskWait(const NimBLETaskData& taskData, uint32_t timeout) { ble_npl_time_ms_to_ticks(timeout, &ticks); } -# ifdef INC_FREERTOS_H - uint32_t notificationValue; - xTaskNotifyWait(0, TASK_BLOCK_BIT, ¬ificationValue, 0); - if (notificationValue & TASK_BLOCK_BIT) { - return true; + taskData.m_pSem = nullptr; + + // find a semaphore that is not currently in use, or create a new one + { + SpinGuard guard(m_taskSemLock); + for (auto& entry : m_taskSemEntries) { + if (!entry->inUse.exchange(true, std::memory_order_acq_rel)) { + taskData.m_pSem = entry; + break; + } + } } - return xTaskNotifyWait(0, TASK_BLOCK_BIT, nullptr, ticks) == pdTRUE; + if (taskData.m_pSem == nullptr) { + auto* sem = new ble_npl_sem{}; + if (ble_npl_sem_init(sem, 0) != BLE_NPL_OK) { + NIMBLE_LOGE(LOG_TAG, "Failed to initialize semaphore for taskWait"); + delete sem; + m_semUserCount.fetch_sub(1); + return false; + } -# else - return ble_npl_sem_pend(static_cast(taskData.m_pHandle), ticks) == BLE_NPL_OK; -# endif + TaskSemEntry* entry = new TaskSemEntry; + entry->sem = sem; + entry->inUse = true; + { + SpinGuard guard(m_taskSemLock); + m_taskSemEntries.push_back(entry); + } + taskData.m_pSem = entry; + } + + taskData.m_pSem->inUse.store(true); + m_semUserCount.fetch_sub(1); + NIMBLE_LOGD(LOG_TAG, "Task waiting with timeout %" PRIu32 "ms", timeout); + if (ble_npl_sem_pend(taskData.m_pSem->sem, ticks) == BLE_NPL_OK) { + taskData.m_pSem->inUse.store(false); + return true; + } + + NIMBLE_LOGE(LOG_TAG, "Task wait timed out after %" PRIu32 "ms", timeout); + taskData.m_pSem->inUse.store(false); + return false; } // taskWait /** @@ -144,17 +147,45 @@ bool NimBLEUtils::taskWait(const NimBLETaskData& taskData, uint32_t timeout) { * @param [in] taskData A pointer to the task data structure. * @param [in] flags A return value to set in the task data structure. */ -void NimBLEUtils::taskRelease(const NimBLETaskData& taskData, int flags) { +void NimBLEUtils::taskRelease(const TaskData& taskData, int flags) { taskData.m_flags = flags; - if (taskData.m_pHandle != nullptr) { -# ifdef INC_FREERTOS_H - xTaskNotify(static_cast(taskData.m_pHandle), TASK_BLOCK_BIT, eSetBits); -# else - ble_npl_sem_release(static_cast(taskData.m_pHandle)); -# endif + if (taskData.m_pSem == nullptr) { + return; + } + + auto rc = ble_npl_sem_release(taskData.m_pSem->sem); + if (rc != BLE_NPL_OK) { + NIMBLE_LOGE(LOG_TAG, "Failed to release semaphore: rc=%d %s", rc, returnCodeToString(rc)); + return; } } // taskRelease +/** + * @brief Deinitializes the NimBLEUtils class and deletes all semaphores used for task waiting. + * @details This should be called when the NimBLE stack is deinitialized to clean up any resources used by waiting tasks. + */ +void NimBLEUtils::deinit() { + // wait for any active users of the semaphore vector to finish + while (m_semUserCount.load() > 0) { + ble_npl_time_delay(1); + } + + for (auto entry : m_taskSemEntries) { + if (entry->inUse.load()) { + ble_npl_sem_release(entry->sem); + // wait for the pend to complete before deinitializing the semaphore + while (entry->inUse.load()) { + ble_npl_time_delay(1); + } + } + + ble_npl_sem_deinit(entry->sem); + delete entry->sem; + delete entry; + } + m_taskSemEntries.clear(); +} + /** * @brief Converts a return code from the NimBLE stack to a text string. * @param [in] rc The return code to convert. @@ -563,7 +594,7 @@ const char* NimBLEUtils::gapEventToString(uint8_t eventType) { NIMBLE_LOGD(LOG_TAG, "Unknown event type %d 0x%.2x", eventType, eventType); return "Unknown event type"; } -# else // MYNEWT_VAL(NIMBLE_CPP_ENABLE_GAP_EVENT_CODE_TEXT) +# else // MYNEWT_VAL(NIMBLE_CPP_ENABLE_GAP_EVENT_CODE_TEXT) (void)eventType; return ""; # endif // MYNEWT_VAL(NIMBLE_CPP_ENABLE_GAP_EVENT_CODE_TEXT) @@ -604,4 +635,24 @@ NimBLEAddress NimBLEUtils::generateAddr(bool nrpa) { return NimBLEAddress{addr}; } // generateAddr +/** + * @brief Construct a SpinGuard and acquire the lock. + * @param [in] flag The atomic_flag to guard. + * @details Spins until the lock is acquired. + */ +NimBLEUtils::SpinGuard::SpinGuard(std::atomic_flag& flag) : m_flag(flag) { + while (m_flag.test_and_set(std::memory_order_acquire)) { + // spin until the lock is acquired + ble_npl_time_delay(1); + } +} + +/** + * @brief Destruct a SpinGuard and release the lock. + * @details Releases the lock acquired by the constructor. + */ +NimBLEUtils::SpinGuard::~SpinGuard() { + m_flag.clear(std::memory_order_release); +} + #endif // CONFIG_BT_NIMBLE_ENABLED diff --git a/src/NimBLEUtils.h b/src/NimBLEUtils.h index 63b7f199..0bf780c2 100644 --- a/src/NimBLEUtils.h +++ b/src/NimBLEUtils.h @@ -29,51 +29,78 @@ # endif # endif -#if MYNEWT_VAL(NIMBLE_CPP_DEBUG_ASSERT_ENABLED) && !defined NDEBUG -void nimble_cpp_assert(const char *file, unsigned line) __attribute((weak, noreturn)); -# define NIMBLE_ATT_VAL_FILE (__builtin_strrchr(__FILE__, '/') ? \ - __builtin_strrchr (__FILE__, '/') + 1 : __FILE__) -# define NIMBLE_CPP_DEBUG_ASSERT(cond) \ - if (!(cond)) { \ - nimble_cpp_assert(NIMBLE_ATT_VAL_FILE, __LINE__); \ - } -#else -# define NIMBLE_CPP_DEBUG_ASSERT(cond) (void(0)) -#endif +# if MYNEWT_VAL(NIMBLE_CPP_DEBUG_ASSERT_ENABLED) && !defined NDEBUG +void nimble_cpp_assert(const char* file, unsigned line) __attribute((weak, noreturn)); +# define NIMBLE_ATT_VAL_FILE (__builtin_strrchr(__FILE__, '/') ? __builtin_strrchr(__FILE__, '/') + 1 : __FILE__) +# define NIMBLE_CPP_DEBUG_ASSERT(cond) \ + if (!(cond)) { \ + nimble_cpp_assert(NIMBLE_ATT_VAL_FILE, __LINE__); \ + } +# else +# define NIMBLE_CPP_DEBUG_ASSERT(cond) (void(0)) +# endif # include +# include +# include class NimBLEAddress; - -/** - * @brief A structure to hold data for a task that is waiting for a response. - * @details This structure is used in conjunction with NimBLEUtils::taskWait() and NimBLEUtils::taskRelease(). - * All items are optional, the m_pHandle will be set in taskWait(). - */ -struct NimBLETaskData { - NimBLETaskData(void* pInstance = nullptr, int flags = 0, void* buf = nullptr); - ~NimBLETaskData(); - void* m_pInstance{nullptr}; - mutable int m_flags{0}; - void* m_pBuf{nullptr}; - - private: - mutable void* m_pHandle{nullptr}; // semaphore or task handle - friend class NimBLEUtils; -}; +struct ble_npl_sem; /** * @brief A BLE Utility class with methods for debugging and general purpose use. */ class NimBLEUtils { + struct TaskSemEntry { + ble_npl_sem* sem; + std::atomic inUse{false}; + }; + public: + /** + * @brief A simple spinlock guard for atomic_flag. + * @details Acquires the lock on construction and releases it on destruction. + */ + class SpinGuard { + public: + explicit SpinGuard(std::atomic_flag& flag); + ~SpinGuard(); + + private: + std::atomic_flag& m_flag; + }; + + /** + * @brief A structure to hold data for a task that is waiting for a response. + * @details This structure is used in conjunction with NimBLEUtils::taskWait() and NimBLEUtils::taskRelease(). + * All items are optional; the internal semaphore entry is assigned in taskWait(). + */ + struct TaskData { + TaskData(void* pInstance = nullptr, int flags = 0, void* buf = nullptr); + ~TaskData() = default; + void* m_pInstance{nullptr}; + mutable int m_flags{0}; + void* m_pBuf{nullptr}; + + private: + friend class NimBLEUtils; + mutable TaskSemEntry* m_pSem{nullptr}; + }; + static const char* gapEventToString(uint8_t eventType); static std::string dataToHexString(const uint8_t* source, uint8_t length); static const char* advTypeToString(uint8_t advType); static const char* returnCodeToString(int rc); static NimBLEAddress generateAddr(bool nrpa); - static bool taskWait(const NimBLETaskData& taskData, uint32_t timeout); - static void taskRelease(const NimBLETaskData& taskData, int rc = 0); + static bool taskWait(const TaskData& taskData, uint32_t timeout); + static void taskRelease(const TaskData& taskData, int rc = 0); + + private: + friend class NimBLEDevice; + static void deinit(); + static std::vector m_taskSemEntries; + static std::atomic m_semUserCount; + static std::atomic_flag m_taskSemLock; }; #endif // CONFIG_BT_NIMBLE_ENABLED