From 3820f57076f88366827978dd637dadd2d27a7f0f Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Mon, 14 Oct 2024 18:02:21 -0500 Subject: [PATCH] fix: #200 Enable use of `data()`/`size()` before trying `c_str()`/`length()` (#201) * fix: #200 Use `data()`/`size()` instead of `c_str()`/`length()` * Reduce duplication, only allow template function in characteristic and remote value attr if the type is not a pointer (otherwise sizeof is useless). add appropriate notes * clean up AttValue::setValue to remove unnecessary length parameter enabling requirement of non-pointer type --- src/NimBLEAttValue.h | 53 ++++++++++++-------------------- src/NimBLECharacteristic.cpp | 20 ------------ src/NimBLECharacteristic.h | 51 +++++++++++++++++------------- src/NimBLEHIDDevice.cpp | 6 ++-- src/NimBLEHIDDevice.h | 2 +- src/NimBLERemoteValueAttribute.h | 42 ++++++------------------- 6 files changed, 63 insertions(+), 111 deletions(-) diff --git a/src/NimBLEAttValue.h b/src/NimBLEAttValue.h index 853ae06..0416781 100644 --- a/src/NimBLEAttValue.h +++ b/src/NimBLEAttValue.h @@ -38,12 +38,20 @@ # error CONFIG_NIMBLE_CPP_ATT_VALUE_INIT_LENGTH cannot be less than 1; Range = 1 : 512 # endif -/* Used to determine if the type passed to a template has a c_str() and length() method. */ +/* Used to determine if the type passed to a template has a data() and size() method. */ template -struct Has_c_str_len : std::false_type {}; +struct Has_data_size : std::false_type {}; template -struct Has_c_str_len().c_str())), decltype(void(std::declval().length()))> +struct Has_data_size().data())), decltype(void(std::declval().size()))> + : std::true_type {}; + +/* Used to determine if the type passed to a template has a c_str() and length() method. */ +template +struct Has_c_str_length : std::false_type {}; + +template +struct Has_c_str_length().c_str())), decltype(void(std::declval().length()))> : std::true_type {}; /** @@ -216,39 +224,18 @@ class NimBLEAttValue { /** * @brief Template to set value to the value of val. * @param [in] s The value to set. - * @param [in] len The length of the value in bytes, defaults to sizeof(T). - * @details Only used for types without a `c_str()` method. + * @note This function is only availabe if the type T is not a pointer. */ template -# ifdef _DOXYGEN_ - bool -# else - typename std::enable_if::value, bool>::type -# endif - setValue(const T& s, uint16_t len = 0) { - if (len == 0) { - len = sizeof(T); + typename std::enable_if::value, bool>::type + setValue(const T& s) { + if constexpr (Has_data_size::value) { + return setValue(reinterpret_cast(s.data()), s.size()); + } else if constexpr (Has_c_str_length::value) { + return setValue(reinterpret_cast(s.c_str()), s.length()); + } else { + return setValue(reinterpret_cast(&s), sizeof(s)); } - return setValue(reinterpret_cast(&s), len); - } - - /** - * @brief Template to set value to the value of val. - * @param [in] s The value to set. - * @param [in] len The length of the value in bytes, defaults to string.length(). - * @details Only used if the has a `c_str()` method. - */ - template -# ifdef _DOXYGEN_ - bool -# else - typename std::enable_if::value, bool>::type -# endif - setValue(const T& s, uint16_t len = 0) { - if (len == 0) { - len = s.length(); - } - return setValue(reinterpret_cast(s.c_str()), len); } /** diff --git a/src/NimBLECharacteristic.cpp b/src/NimBLECharacteristic.cpp index 6a7b07c..e2380e3 100644 --- a/src/NimBLECharacteristic.cpp +++ b/src/NimBLECharacteristic.cpp @@ -266,16 +266,6 @@ void NimBLECharacteristic::indicate(const uint8_t* value, size_t length, uint16_ sendValue(value, length, false, conn_handle); } // indicate -/** - * @brief Send an indication. - * @param[in] value A std::vector containing the value to send as the notification value. - * @param[in] conn_handle Connection handle to send an individual indication, or BLE_HS_CONN_HANDLE_NONE to send - * the indication to all subscribed clients. - */ -void NimBLECharacteristic::indicate(const std::vector& value, uint16_t conn_handle) const { - sendValue(value.data(), value.size(), false, conn_handle); -} // indicate - /** * @brief Send a notification. * @param[in] conn_handle Connection handle to send an individual notification, or BLE_HS_CONN_HANDLE_NONE to send @@ -296,16 +286,6 @@ void NimBLECharacteristic::notify(const uint8_t* value, size_t length, uint16_t sendValue(value, length, true, conn_handle); } // indicate -/** - * @brief Send a notification. - * @param[in] value A std::vector containing the value to send as the notification value. - * @param[in] conn_handle Connection handle to send an individual notification, or BLE_HS_CONN_HANDLE_NONE to send - * the notification to all subscribed clients. - */ -void NimBLECharacteristic::notify(const std::vector& value, uint16_t conn_handle) const { - sendValue(value.data(), value.size(), true, conn_handle); -} // notify - /** * @brief Sends a notification or indication. * @param[in] value A pointer to the data to send. diff --git a/src/NimBLECharacteristic.h b/src/NimBLECharacteristic.h index c43054c..a787ebe 100644 --- a/src/NimBLECharacteristic.h +++ b/src/NimBLECharacteristic.h @@ -56,10 +56,8 @@ class NimBLECharacteristic : public NimBLELocalValueAttribute { void setCallbacks(NimBLECharacteristicCallbacks* pCallbacks); void indicate(uint16_t conn_handle = BLE_HS_CONN_HANDLE_NONE) const; void indicate(const uint8_t* value, size_t length, uint16_t conn_handle = BLE_HS_CONN_HANDLE_NONE) const; - void indicate(const std::vector& value, uint16_t conn_handle = BLE_HS_CONN_HANDLE_NONE) const; void notify(uint16_t conn_handle = BLE_HS_CONN_HANDLE_NONE) const; void notify(const uint8_t* value, size_t length, uint16_t conn_handle = BLE_HS_CONN_HANDLE_NONE) const; - void notify(const std::vector& value, uint16_t conn_handle = BLE_HS_CONN_HANDLE_NONE) const; NimBLEDescriptor* createDescriptor(const char* uuid, uint32_t properties = NIMBLE_PROPERTY::READ | NIMBLE_PROPERTY::WRITE, @@ -77,36 +75,47 @@ class NimBLECharacteristic : public NimBLELocalValueAttribute { /*********************** Template Functions ************************/ /** - * @brief Template to send a notification from a class type that has a c_str() and length() method. + * @brief Template to send a notification for classes which may have + * data()/size() or c_str()/length() methods. Falls back to sending + * the data by casting the first element of the array to a uint8_t + * pointer and getting the length of the array using sizeof. * @tparam T The a reference to a class containing the data to send. * @param[in] value The value to set. - * @param[in] is_notification if true sends a notification, false sends an indication. - * @details Only used if the has a `c_str()` method. + * @param[in] conn_handle The connection handle to send the notification to. + * @note This function is only available if the type T is not a pointer. */ template -# ifdef _DOXYGEN_ - void -# else - typename std::enable_if::value, void>::type -# endif - notify(const T& value, bool is_notification = true) const { - notify(reinterpret_cast(value.c_str()), value.length(), is_notification); + typename std::enable_if::value, void>::type + notify(const T& value, uint16_t conn_handle = BLE_HS_CONN_HANDLE_NONE) const { + if constexpr (Has_data_size::value) { + notify(reinterpret_cast(value.data()), value.size(), conn_handle); + } else if constexpr (Has_c_str_length::value) { + notify(reinterpret_cast(value.c_str()), value.length(), conn_handle); + } else { + notify(reinterpret_cast(&value), sizeof(value), conn_handle); + } } /** - * @brief Template to send an indication from a class type that has a c_str() and length() method. + * @brief Template to send an indication for classes which may have + * data()/size() or c_str()/length() methods. Falls back to sending + * the data by casting the first element of the array to a uint8_t + * pointer and getting the length of the array using sizeof. * @tparam T The a reference to a class containing the data to send. * @param[in] value The value to set. - * @details Only used if the has a `c_str()` method. + * @param[in] conn_handle The connection handle to send the indication to. + * @note This function is only available if the type T is not a pointer. */ template -# ifdef _DOXYGEN_ - void -# else - typename std::enable_if::value, void>::type -# endif - indicate(const T& value) const { - indicate(reinterpret_cast(value.c_str()), value.length()); + typename std::enable_if::value, void>::type + indicate(const T& value, uint16_t conn_handle = BLE_HS_CONN_HANDLE_NONE) const { + if constexpr (Has_data_size::value) { + indicate(reinterpret_cast(value.data()), value.size(), conn_handle); + } else if constexpr (Has_c_str_length::value) { + indicate(reinterpret_cast(value.c_str()), value.length(), conn_handle); + } else { + indicate(reinterpret_cast(&value), sizeof(value), conn_handle); + } } private: diff --git a/src/NimBLEHIDDevice.cpp b/src/NimBLEHIDDevice.cpp index 12c14de..ad04197 100644 --- a/src/NimBLEHIDDevice.cpp +++ b/src/NimBLEHIDDevice.cpp @@ -225,12 +225,12 @@ NimBLECharacteristic* NimBLEHIDDevice::batteryLevel() { return m_batteryLevelCharacteristic; } -/* - -BLECharacteristic* BLEHIDDevice::reportMap() { +NimBLECharacteristic* NimBLEHIDDevice::reportMap() { return m_reportMapCharacteristic; } +/* + BLECharacteristic* BLEHIDDevice::pnp() { return m_pnpCharacteristic; } diff --git a/src/NimBLEHIDDevice.h b/src/NimBLEHIDDevice.h index 6461a4f..f0da994 100644 --- a/src/NimBLEHIDDevice.h +++ b/src/NimBLEHIDDevice.h @@ -60,7 +60,7 @@ public: void setBatteryLevel(uint8_t level); - //NimBLECharacteristic* reportMap(); + NimBLECharacteristic* reportMap(); NimBLECharacteristic* hidControl(); NimBLECharacteristic* inputReport(uint8_t reportID); NimBLECharacteristic* outputReport(uint8_t reportID); diff --git a/src/NimBLERemoteValueAttribute.h b/src/NimBLERemoteValueAttribute.h index 6eabb27..90e4c00 100644 --- a/src/NimBLERemoteValueAttribute.h +++ b/src/NimBLERemoteValueAttribute.h @@ -63,16 +63,6 @@ class NimBLERemoteValueAttribute : public NimBLEAttribute { */ bool writeValue(const uint8_t* data, size_t length, bool response = false) const; - /** - * @brief Write a new value to the remote characteristic from a std::vector. - * @param [in] vec A std::vector value to write to the remote characteristic. - * @param [in] response Whether we require a response from the write. - * @return false if not connected or otherwise cannot perform write. - */ - bool writeValue(const std::vector& v, bool response = false) const { - return writeValue(&v[0], v.size(), response); - } - /** * @brief Write a new value to the remote characteristic from a const char*. * @param [in] str A character string to write to the remote characteristic. @@ -88,32 +78,18 @@ class NimBLERemoteValueAttribute : public NimBLEAttribute { * @brief Template to set the remote characteristic value to val. * @param [in] s The value to write. * @param [in] response True == request write response. - * @details Only used for non-arrays and types without a `c_str()` method. + * @note This function is only available if the type T is not a pointer. */ template -# ifdef _DOXYGEN_ - bool -# else - typename std::enable_if::value && !Has_c_str_len::value, bool>::type -# endif + typename std::enable_if::value, bool>::type writeValue(const T& v, bool response = false) const { - return writeValue(reinterpret_cast(&v), sizeof(T), response); - } - - /** - * @brief Template to set the remote characteristic value to val. - * @param [in] s The value to write. - * @param [in] response True == request write response. - * @details Only used if the has a `c_str()` method. - */ - template -# ifdef _DOXYGEN_ - bool -# else - typename std::enable_if::value, bool>::type -# endif - writeValue(const T& s, bool response = false) const { - return writeValue(reinterpret_cast(s.c_str()), s.length(), response); + if constexpr (Has_data_size::value) { + return writeValue(reinterpret_cast(v.data()), v.size(), response); + } else if constexpr (Has_c_str_length::value) { + return writeValue(reinterpret_cast(v.c_str()), v.length(), response); + } else { + return writeValue(reinterpret_cast(&v), sizeof(v), response); + } } /**