Don't embed 'enum' or min/max in the Attribute class

Author: kedars

examples/wifi-echo/server/esp32/main/wifi-echo.cpp

@@ -374,7 +374,7 @@ void PrintDataModel(ClusterServer & server)
             cluster->mAttrs.Foreach([](Attribute * attr) -> void {
                 printf("      Attribute: 0x%04x\n", attr->mAttrId);
                 char printstr[20];
-                attr->mValue.ValueToStr(printstr, sizeof(printstr));
+                attr->Get().ValueToStr(printstr, sizeof(printstr));
                 printf("              Value: %s\n", printstr);
             });
         });

src/lib/datamodel/Attribute.h

@@ -17,7 +17,7 @@
 
 /**
  *    @file
- *      This file contains definitions for working with CHIP values.
+ *      This file contains definitions for working with CHIP Attributes
  *
  */
 
@@ -45,38 +45,16 @@ class Attribute
 
 public:
     AttributeId_t mAttrId;
-    Value mValue;
-    Value mMin;
-    Value mMax;
-
-    Attribute(AttributeId_t attrId, ValueTypes type) : mDeque(this), mAttrId(attrId), mValue(type), mMin(type), mMax(type) {}
-    Attribute(AttributeId_t attrId, Value value) :
-        mDeque(this), mAttrId(attrId), mValue(value), mMin(value.mType), mMax(value.mType)
-    {}
-    Attribute(AttributeId_t attrId, ValueTypes type, uint64_t min, uint64_t max) :
-        mDeque(this), mAttrId(attrId), mValue(type), mMin(type, min), mMax(type, max)
-    {}
+
+    Attribute(AttributeId_t attrId) : mDeque(this), mAttrId(attrId) {}
 
     /**
      * @brief
      *   Set this attribute to a value
      *
      * @param value  the new value that this attribute should be updated with
      */
-    CHIP_ERROR Set(const Value & newValue)
-    {
-        /* We have to check the element type match in this case */
-        if (mValue.mType != newValue.mType)
-        {
-            return CHIP_ERROR_INTERNAL;
-        }
-        if (withinRange(newValue))
-        {
-            mValue = newValue;
-            return CHIP_NO_ERROR;
-        }
-        return CHIP_ERROR_INTERNAL;
-    }
+    virtual CHIP_ERROR Set(const Value & newValue) = 0;
 
     /* Need to define the behaviour when Value contains pointers
      * to allocated data
@@ -86,32 +64,51 @@ class Attribute
      *   Get the value of this attribute
      *
      */
-    Value Get() { return mValue; }
+    virtual Value Get(void) = 0;
+};
 
-protected:
-    bool withinRange(const uint64_t & value) { return (value >= mMin.Int64) && (value <= mMax.Int64); }
+template <typename ValueType>
+class AttributeSimple : public Attribute
+{
+private:
+    ValueType mValue;
 
-    bool withinRange(const Value value)
+public:
+    AttributeSimple(AttributeId_t attrId) : Attribute(attrId) {}
+    AttributeSimple(AttributeId_t attrId, ValueType value) : Attribute(attrId), mValue(value) {}
+
+    CHIP_ERROR Set(const Value & newValue) { return ValueToType(newValue, mValue); }
+
+    Value Get(void) { return TypeToValue(mValue); }
+};
+
+template <typename ValueType, ValueType min, ValueType max>
+class AttributeWithRange : public Attribute
+{
+private:
+    ValueType mValue;
+    const ValueType mMin = min;
+    const ValueType mMax = max;
+
+public:
+    AttributeWithRange(AttributeId_t attrId) : Attribute(attrId) {}
+    AttributeWithRange(AttributeId_t attrId, ValueType value) : Attribute(attrId), mValue(value) {}
+
+    CHIP_ERROR Set(const Value & newValue)
     {
-        switch (mValue.mType)
+        ValueType tmp;
+        if (ValueToType(newValue, tmp) == CHIP_NO_ERROR)
         {
-        case kCHIPValueType_Int8:
-        case kCHIPValueType_Int16:
-        case kCHIPValueType_Int32:
-        case kCHIPValueType_Int64:
-        case kCHIPValueType_UInt8:
-        case kCHIPValueType_UInt16:
-        case kCHIPValueType_UInt32:
-        case kCHIPValueType_UInt64:
-            return withinRange(value.Int64);
-            break;
-        case kCHIPValueType_Bool:
-            return true;
-        default:
-            return false;
+            if ((tmp >= mMin) && (tmp <= mMax))
+            {
+                mValue = tmp;
+                return CHIP_NO_ERROR;
+            }
         }
-        return false;
+        return CHIP_ERROR_INTERNAL;
     }
+
+    Value Get(void) { return TypeToValue(mValue); }
 };
 
 } // namespace DataModel

src/lib/datamodel/ClusterBasic.h

@@ -45,18 +45,17 @@ static const AttributeId_t kAttributeIdHWVersion          = 0x0003;
 class ClusterBasic : public Cluster
 {
 private:
-    Attribute mZCLVersion;
-    Attribute mApplicationVersion;
-    Attribute mStackVersion;
-    Attribute mHWVersion;
+    AttributeSimple<uint8_t> mZCLVersion;
+    AttributeSimple<uint8_t> mApplicationVersion;
+    AttributeSimple<uint8_t> mStackVersion;
+    AttributeSimple<uint8_t> mHWVersion;
 
 public:
     ClusterBasic(uint8_t ZCLVersion, uint8_t applicationVersion, uint8_t stackVersion, uint8_t HWVersion) :
         Cluster(kClusterIdBase),
         /* Attributes */
-        mZCLVersion(kAttributeIdZCLVersion, ValueUInt8(ZCLVersion)),
-        mApplicationVersion(kAttributeIdApplicationVersion, ValueUInt8(applicationVersion)),
-        mStackVersion(kAttributeIdStackVersion, ValueUInt8(stackVersion)), mHWVersion(kAttributeIdHWVersion, ValueUInt8(HWVersion))
+        mZCLVersion(kAttributeIdZCLVersion, ZCLVersion), mApplicationVersion(kAttributeIdApplicationVersion, applicationVersion),
+        mStackVersion(kAttributeIdStackVersion, stackVersion), mHWVersion(kAttributeIdHWVersion, HWVersion)
     {
         AddAttribute(&mZCLVersion);
         AddAttribute(&mApplicationVersion);

src/lib/datamodel/ClusterOnOff.h

@@ -46,16 +46,15 @@ class ClusterOnOff : public Cluster
 {
     // TODO: these should
 private:
-    Attribute mOnOff;
-    Attribute mGlobalSceneControl;
-    Attribute mOnTime;
-    Attribute mOffWaitTime;
+    AttributeSimple<bool> mOnOff;
+    AttributeSimple<bool> mGlobalSceneControl;
+    AttributeSimple<uint16_t> mOnTime;
+    AttributeSimple<uint16_t> mOffWaitTime;
 
 public:
     ClusterOnOff() :
-        Cluster(kClusterIdOnOff), mOnOff(kAttributeIdOnOff, kCHIPValueType_Bool),
-        mGlobalSceneControl(kAttributeIdGlobalSceneControl, kCHIPValueType_Bool),
-        mOnTime(kAttributeIdOnTime, kCHIPValueType_UInt16), mOffWaitTime(kAttributeIdOffWaitTime, kCHIPValueType_UInt16)
+        Cluster(kClusterIdOnOff), mOnOff(kAttributeIdOnOff), mGlobalSceneControl(kAttributeIdGlobalSceneControl),
+        mOnTime(kAttributeIdOnTime), mOffWaitTime(kAttributeIdOffWaitTime)
     {
         AddAttribute(&mOnOff);
         AddAttribute(&mGlobalSceneControl);

src/lib/datamodel/Value.h

@@ -24,6 +24,7 @@
 #ifndef CHIP_VALUE_H_
 #define CHIP_VALUE_H_
 
+#include <core/CHIPError.h>
 #include <inttypes.h>
 #include <stdbool.h>
 #include <stdint.h>
@@ -117,6 +118,67 @@ static inline uint8_t ValueToUInt8(Value v)
     return (uint8_t) v.Int64;
 }
 
+/* TODO: Once we use this, I don't think we will need the above convenience API */
+static inline Value TypeToValue(bool b)
+{
+    return Value(kCHIPValueType_Bool, (uint64_t) b);
+}
+
+static inline Value TypeToValue(uint8_t i)
+{
+    return Value(kCHIPValueType_UInt8, (uint64_t) i);
+}
+
+static inline Value TypeToValue(uint16_t i)
+{
+    return Value(kCHIPValueType_UInt16, (uint64_t) i);
+}
+
+static inline Value TypeToValue(uint32_t i)
+{
+    return Value(kCHIPValueType_UInt32, (uint64_t) i);
+}
+
+static inline CHIP_ERROR ValueToType(Value v, bool & b)
+{
+    if (v.mType == kCHIPValueType_Bool)
+    {
+        b = (bool) v.Int64;
+        return CHIP_NO_ERROR;
+    }
+    return CHIP_ERROR_INTERNAL;
+}
+
+static inline CHIP_ERROR ValueToType(Value v, uint8_t & i)
+{
+    if (v.mType == kCHIPValueType_UInt8)
+    {
+        i = (uint8_t) v.Int64;
+        return CHIP_NO_ERROR;
+    }
+    return CHIP_ERROR_INTERNAL;
+}
+
+static inline CHIP_ERROR ValueToType(Value v, uint16_t & i)
+{
+    if (v.mType == kCHIPValueType_UInt16)
+    {
+        i = (uint16_t) v.Int64;
+        return CHIP_NO_ERROR;
+    }
+    return CHIP_ERROR_INTERNAL;
+}
+
+static inline CHIP_ERROR ValueToType(Value v, uint32_t & i)
+{
+    if (v.mType == kCHIPValueType_UInt32)
+    {
+        i = (uint32_t) v.Int64;
+        return CHIP_NO_ERROR;
+    }
+    return CHIP_ERROR_INTERNAL;
+}
+
 } // namespace DataModel
 } // namespace chip