From 68dd0b37b512b0c47c7f4b90b624e4697ac5aa1e Mon Sep 17 00:00:00 2001 From: h2zero Date: Fri, 9 May 2025 13:02:27 -0600 Subject: [PATCH] [Bugfix] Attribute getValue fails with some data types --- src/NimBLEAttValue.h | 10 ++-- src/NimBLELocalValueAttribute.h | 35 ++---------- src/NimBLERemoteValueAttribute.cpp | 2 +- src/NimBLERemoteValueAttribute.h | 41 +++----------- src/NimBLEValueAttribute.h | 86 ++++++++++++++++++++++++++++++ 5 files changed, 100 insertions(+), 74 deletions(-) create mode 100644 src/NimBLEValueAttribute.h diff --git a/src/NimBLEAttValue.h b/src/NimBLEAttValue.h index 67a2cea..117b8e0 100644 --- a/src/NimBLEAttValue.h +++ b/src/NimBLEAttValue.h @@ -313,14 +313,10 @@ class NimBLEAttValue { # endif } - if (std::is_convertible::value) { - return *this; - } else { - if (!skipSizeCheck && size() < sizeof(T)) { - return T(); - } - return *(reinterpret_cast(m_attr_value)); + if (!skipSizeCheck && size() < sizeof(T)) { + return T(); } + return *(reinterpret_cast(m_attr_value)); } /*********************** Operators ************************/ diff --git a/src/NimBLELocalValueAttribute.h b/src/NimBLELocalValueAttribute.h index 55b03fc..d720880 100644 --- a/src/NimBLELocalValueAttribute.h +++ b/src/NimBLELocalValueAttribute.h @@ -48,30 +48,18 @@ typedef enum { } NIMBLE_PROPERTY; # include "NimBLELocalAttribute.h" +# include "NimBLEValueAttribute.h" # include "NimBLEAttValue.h" # include class NimBLEConnInfo; -class NimBLELocalValueAttribute : public NimBLELocalAttribute { +class NimBLELocalValueAttribute : public NimBLELocalAttribute, public NimBLEValueAttribute { public: /** * @brief Get the properties of the attribute. */ uint16_t getProperties() const { return m_properties; } - /** - * @brief Get the length of the attribute value. - * @return The length of the attribute value. - */ - size_t getLength() const { return m_value.size(); } - - /** - * @brief Get a copy of the value of the attribute value. - * @param [in] timestamp (Optional) A pointer to a time_t struct to get the time the value set. - * @return A copy of the attribute value. - */ - NimBLEAttValue getValue(time_t* timestamp = nullptr) const { return m_value; } - /** * @brief Set the value of the attribute value. * @param [in] data The data to set the value to. @@ -100,19 +88,6 @@ class NimBLELocalValueAttribute : public NimBLELocalAttribute { m_value.setValue(val); } - /** - * @brief Template to convert the data to . - * @tparam T The type to convert the data to. - * @param [in] timestamp (Optional) A pointer to a time_t struct to get the time the value set. - * @param [in] skipSizeCheck (Optional) If true it will skip checking if the data size is less than sizeof(). - * @return The data converted to or NULL if skipSizeCheck is false and the data is less than sizeof(). - * @details Use: getValue(×tamp, skipSizeCheck); - */ - template - T getValue(time_t* timestamp = nullptr, bool skipSizeCheck = false) const { - return m_value.getValue(timestamp, skipSizeCheck); - } - protected: friend class NimBLEServer; @@ -127,8 +102,7 @@ class NimBLELocalValueAttribute : public NimBLELocalAttribute { uint16_t handle, uint16_t maxLen, uint16_t initLen = CONFIG_NIMBLE_CPP_ATT_VALUE_INIT_LENGTH) - : NimBLELocalAttribute(uuid, handle), m_value(initLen, maxLen) {} - + : NimBLELocalAttribute(uuid, handle), NimBLEValueAttribute(maxLen, initLen) {} /** * @brief Destroy the NimBLELocalValueAttribute object. */ @@ -163,8 +137,7 @@ class NimBLELocalValueAttribute : public NimBLELocalAttribute { */ void setProperties(uint16_t properties) { m_properties = properties; } - NimBLEAttValue m_value{}; - uint16_t m_properties{0}; + uint16_t m_properties{0}; }; #endif // CONFIG_BT_ENABLED && CONFIG_BT_NIMBLE_ROLE_PERIPHERAL diff --git a/src/NimBLERemoteValueAttribute.cpp b/src/NimBLERemoteValueAttribute.cpp index 1de2940..b983371 100644 --- a/src/NimBLERemoteValueAttribute.cpp +++ b/src/NimBLERemoteValueAttribute.cpp @@ -122,7 +122,7 @@ int NimBLERemoteValueAttribute::onWriteCB(uint16_t conn_handle, const ble_gatt_e * @param [in] timestamp A pointer to a time_t struct to store the time the value was read. * @return The value of the remote characteristic. */ -NimBLEAttValue NimBLERemoteValueAttribute::readValue(time_t* timestamp) const { +NimBLEAttValue NimBLERemoteValueAttribute::readValue(time_t* timestamp) { NIMBLE_LOGD(LOG_TAG, ">> readValue()"); NimBLEAttValue value{}; diff --git a/src/NimBLERemoteValueAttribute.h b/src/NimBLERemoteValueAttribute.h index b722c74..5d0ff2b 100644 --- a/src/NimBLERemoteValueAttribute.h +++ b/src/NimBLERemoteValueAttribute.h @@ -32,32 +32,19 @@ # undef max /**************************/ -# include "NimBLEAttribute.h" +# include "NimBLEValueAttribute.h" # include "NimBLEAttValue.h" class NimBLEClient; -class NimBLERemoteValueAttribute : public NimBLEAttribute { +class NimBLERemoteValueAttribute : public NimBLEValueAttribute, public NimBLEAttribute { public: /** * @brief Read the value of the remote attribute. * @param [in] timestamp A pointer to a time_t struct to store the time the value was read. * @return The value of the remote attribute. */ - NimBLEAttValue readValue(time_t* timestamp = nullptr) const; - - /** - * @brief Get the length of the remote attribute value. - * @return The length of the remote attribute value. - */ - size_t getLength() const { return m_value.size(); } - - /** - * @brief Get the value of the remote attribute. - * @return The value of the remote attribute. - * @details This returns a copy of the value to avoid potential race conditions. - */ - NimBLEAttValue getValue() const { return m_value; } + NimBLEAttValue readValue(time_t* timestamp = nullptr); /** * Get the client instance that owns this attribute. @@ -153,20 +140,6 @@ class NimBLERemoteValueAttribute : public NimBLEAttribute { } # endif - /** - * @brief Template to convert the remote characteristic data to . - * @tparam T The type to convert the data to. - * @param [in] timestamp A pointer to a time_t struct to store the time the value was read. - * @param [in] skipSizeCheck If true it will skip checking if the data size is less than sizeof(). - * @return The data converted to or NULL if skipSizeCheck is false and the data is - * less than sizeof(). - * @details Use: getValue(×tamp, skipSizeCheck); - */ - template - T getValue(time_t* timestamp = nullptr, bool skipSizeCheck = false) const { - return m_value.getValue(timestamp, skipSizeCheck); - } - /** * @brief Template to convert the remote characteristic data to . * @tparam T The type to convert the data to. @@ -177,16 +150,16 @@ class NimBLERemoteValueAttribute : public NimBLEAttribute { * @details Use: readValue(×tamp, skipSizeCheck); */ template - T readValue(time_t* timestamp = nullptr, bool skipSizeCheck = false) const { + T readValue(time_t* timestamp = nullptr, bool skipSizeCheck = false) { readValue(); - return m_value.getValue(timestamp, skipSizeCheck); + return getValue(timestamp, skipSizeCheck); } protected: /** * @brief Construct a new NimBLERemoteValueAttribute object. */ - NimBLERemoteValueAttribute(const ble_uuid_any_t& uuid, uint16_t handle) : NimBLEAttribute(uuid, handle) {} + NimBLERemoteValueAttribute(const ble_uuid_any_t& uuid, uint16_t handle) : NimBLEAttribute{uuid, handle} {} /** * @brief Destroy the NimBLERemoteValueAttribute object. @@ -195,8 +168,6 @@ class NimBLERemoteValueAttribute : public NimBLEAttribute { static int onReadCB(uint16_t conn_handle, const ble_gatt_error* error, ble_gatt_attr* attr, void* arg); static int onWriteCB(uint16_t conn_handle, const ble_gatt_error* error, ble_gatt_attr* attr, void* arg); - - mutable NimBLEAttValue m_value{}; }; #endif /* CONFIG_BT_ENABLED && CONFIG_BT_NIMBLE_ROLE_CENTRAL */ diff --git a/src/NimBLEValueAttribute.h b/src/NimBLEValueAttribute.h new file mode 100644 index 0000000..2e9775a --- /dev/null +++ b/src/NimBLEValueAttribute.h @@ -0,0 +1,86 @@ +/* + * Copyright 2020-2025 Ryan Powell and + * esp-nimble-cpp, NimBLE-Arduino contributors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef NIMBLE_CPP_VALUE_ATTRIBUTE_H_ +#define NIMBLE_CPP_VALUE_ATTRIBUTE_H_ + +#include "nimconfig.h" +#if defined(CONFIG_BT_ENABLED) && (defined(CONFIG_BT_NIMBLE_ROLE_PERIPHERAL) || defined(CONFIG_BT_NIMBLE_ROLE_CENTRAL)) + +# include "NimBLEAttribute.h" +# include "NimBLEAttValue.h" + +class NimBLEValueAttribute { + public: + NimBLEValueAttribute(uint16_t maxLen = BLE_ATT_ATTR_MAX_LEN, uint16_t initLen = CONFIG_NIMBLE_CPP_ATT_VALUE_INIT_LENGTH) + : m_value(initLen, maxLen) {} + + /** + * @brief Get a copy of the value of the attribute value. + * @param [in] timestamp (Optional) A pointer to a time_t struct to get the time the value set. + * @return A copy of the attribute value. + */ + NimBLEAttValue getValue(time_t* timestamp) const { return m_value.getValue(timestamp); } + + /** + * @brief Get a copy of the value of the attribute value. + * @return A copy of the attribute value. + */ + NimBLEAttValue getValue() const { return m_value; } + + /** + * @brief Get the length of the attribute value. + * @return The length of the attribute value. + */ + size_t getLength() const { return m_value.size(); } + + /** + * @brief Template to convert the data to . + * @tparam T The type to convert the data to. + * @param [in] timestamp (Optional) A pointer to a time_t struct to get the time the value set. + * @param [in] skipSizeCheck (Optional) If true it will skip checking if the data size is less than sizeof(). + * @return The data converted to or NULL if skipSizeCheck is false and the data is less than sizeof(). + * @details Use: getValue(×tamp, skipSizeCheck); + * Used for types that are trivially copyable and convertible to NimBLEAttValue. + */ + template + typename std::enable_if::value, T>::type + getValue(time_t* timestamp = nullptr, bool skipSizeCheck = false) const { + return m_value.getValue(timestamp, skipSizeCheck); + } + + /** + * @brief Template to convert the data to . + * @tparam T The type to convert the data to. + * @param [in] timestamp (Optional) A pointer to a time_t struct to get the time the value set. + * @param [in] skipSizeCheck (Optional) If true it will skip checking if the data size is less than sizeof(). + * @return The data converted to or NULL if skipSizeCheck is false and the data is less than sizeof(). + * @details Use: getValue(×tamp, skipSizeCheck); + * Used for types that are not trivially copyable and convertible to NimBLEAttValue via it's operators. + */ + template + typename std::enable_if::value && std::is_convertible::value, T>::type + getValue(time_t* timestamp = nullptr, bool skipSizeCheck = false) const { + return m_value; + } + + protected: + NimBLEAttValue m_value{}; +}; + +#endif // CONFIG_BT_ENABLED && (CONFIG_BT_NIMBLE_ROLE_PERIPHERAL || CONFIG_BT_NIMBLE_ROLE_CENTRAL) +#endif // NIMBLE_CPP_ATTRIBUTE_H_