[Bugfix] Attribute getValue fails with some data types

Author: h2zero

src/NimBLEAttValue.h

@@ -313,14 +313,10 @@ class NimBLEAttValue {
 # endif
         }
 
-        if (std::is_convertible<NimBLEAttValue, T>::value) {
-            return *this;
-        } else {
-            if (!skipSizeCheck && size() < sizeof(T)) {
-                return T();
-            }
-            return *(reinterpret_cast<const T*>(m_attr_value));
+        if (!skipSizeCheck && size() < sizeof(T)) {
+            return T();
         }
+        return *(reinterpret_cast<const T*>(m_attr_value));
     }
 
     /*********************** Operators ************************/

src/NimBLELocalValueAttribute.h

@@ -48,30 +48,18 @@ typedef enum {
 } NIMBLE_PROPERTY;
 
 # include "NimBLELocalAttribute.h"
+# include "NimBLEValueAttribute.h"
 # include "NimBLEAttValue.h"
 # include <vector>
 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<T>(val);
     }
 
-    /**
-     * @brief Template to convert the data to <type\>.
-     * @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 <tt>sizeof(<type\>)</tt>.
-     * @return The data converted to <type\> or NULL if skipSizeCheck is false and the data is less than <tt>sizeof(<type\>)</tt>.
-     * @details <b>Use:</b> <tt>getValue<type>(&timestamp, skipSizeCheck);</tt>
-     */
-    template <typename T>
-    T getValue(time_t* timestamp = nullptr, bool skipSizeCheck = false) const {
-        return m_value.getValue<T>(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

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{};

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 <type\>.
-     * @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 <tt>sizeof(<type\>)</tt>.
-     * @return The data converted to <type\> or NULL if skipSizeCheck is false and the data is
-     * less than <tt>sizeof(<type\>)</tt>.
-     * @details <b>Use:</b> <tt>getValue<type>(&timestamp, skipSizeCheck);</tt>
-     */
-    template <typename T>
-    T getValue(time_t* timestamp = nullptr, bool skipSizeCheck = false) const {
-        return m_value.getValue<T>(timestamp, skipSizeCheck);
-    }
-
     /**
      * @brief Template to convert the remote characteristic data to <type\>.
      * @tparam T The type to convert the data to.
@@ -177,16 +150,16 @@ class NimBLERemoteValueAttribute : public NimBLEAttribute {
      * @details <b>Use:</b> <tt>readValue<type>(&timestamp, skipSizeCheck);</tt>
      */
     template <typename T>
-    T readValue(time_t* timestamp = nullptr, bool skipSizeCheck = false) const {
+    T readValue(time_t* timestamp = nullptr, bool skipSizeCheck = false) {
         readValue();
-        return m_value.getValue<T>(timestamp, skipSizeCheck);
+        return getValue<T>(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 */

src/NimBLEValueAttribute.h

@@ -0,0 +1,86 @@
+/*
+ * Copyright 2020-2025 Ryan Powell <ryan@nable-embedded.io> 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 <type\>.
+     * @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 <tt>sizeof(<type\>)</tt>.
+     * @return The data converted to <type\> or NULL if skipSizeCheck is false and the data is less than <tt>sizeof(<type\>)</tt>.
+     * @details <b>Use:</b> <tt>getValue<type>(&timestamp, skipSizeCheck);</tt>
+     * Used for types that are trivially copyable and convertible to NimBLEAttValue.
+     */
+    template <typename T>
+    typename std::enable_if<std::is_trivially_copyable<T>::value, T>::type
+    getValue(time_t* timestamp = nullptr, bool skipSizeCheck = false) const {
+        return m_value.getValue<T>(timestamp, skipSizeCheck);
+    }
+
+    /**
+     * @brief Template to convert the data to <type\>.
+     * @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 <tt>sizeof(<type\>)</tt>.
+     * @return The data converted to <type\> or NULL if skipSizeCheck is false and the data is less than <tt>sizeof(<type\>)</tt>.
+     * @details <b>Use:</b> <tt>getValue<type>(&timestamp, skipSizeCheck);</tt>
+     * Used for types that are not trivially copyable and convertible to NimBLEAttValue via it's operators.
+     */
+    template <typename T>
+    typename std::enable_if<!std::is_trivially_copyable<T>::value && std::is_convertible<T, NimBLEAttValue>::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_