[occupancy-sensing] Follow up improvements for Occupancy Sensing Clus…

…ter updates for Matter 1.4 (#34658) * [occupancy-sensing]Follow up improvements for Occupancy Sensing Cluster updates for Matter 1.4 * Apply follow-up suggestions from review PR#34293 * Make HoldTime attribute writable * Fix array index mapping issue reported from PR#34593 in occupancy-sensing-stub.cpp Signed-off-by: Oliver Fan <oliver.fan@nxp.com> * Regen ZAP after adding FeatureMap and HoldTime to zcl.json and zcl-with-test-extensions.json file for Occupancy Sensing cluster Signed-off-by: Oliver Fan <oliver.fan@nxp.com> * Restyled by clang-format * Restyled by prettier-json * Restyled by clang-format * Add an error print in occupancy-sensing-stub.cpp Signed-off-by: Oliver Fan <oliver.fan@nxp.com> * Update all-clusters-app.zap for HoldTime and FeatureMap attributes of Occupancy Sensing cluster Signed-off-by: Oliver Fan <oliver.fan@nxp.com> * Resolve merge conflict in endpoint_config.h Signed-off-by: Oliver Fan <oliver.fan@nxp.com> --------- Signed-off-by: Oliver Fan <oliver.fan@nxp.com> Co-authored-by: Restyled.io <commits@restyled.io>
project-chip · Aug 8, 2024 · 72890bb · 72890bb
1 parent a3eb1e3
commit 72890bb
Show file tree

Hide file tree

Showing 17 changed files with 114 additions and 170 deletions.
diff --git a/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter b/examples/all-clusters-app/all-clusters-common/all-clusters-app.matter
@@ -9252,10 +9252,10 @@ endpoint 1 {
     ram      attribute occupancy;
     ram      attribute occupancySensorType;
     ram      attribute occupancySensorTypeBitmap default = 1;
-    ram      attribute holdTime default = 10;
+    callback attribute holdTime;
     callback attribute holdTimeLimits;
     ram      attribute PIROccupiedToUnoccupiedDelay default = 10;
-    ram      attribute featureMap default = 0x02;
+    callback attribute featureMap;
     ram      attribute clusterRevision default = 5;
   }
 
@@ -9730,10 +9730,10 @@ endpoint 2 {
     ram      attribute occupancy;
     ram      attribute occupancySensorType;
     ram      attribute occupancySensorTypeBitmap default = 1;
-    ram      attribute holdTime default = 20;
+    callback attribute holdTime;
     callback attribute holdTimeLimits;
     ram      attribute PIROccupiedToUnoccupiedDelay default = 10;
-    ram      attribute featureMap default = 0x02;
+    callback attribute featureMap;
     ram      attribute clusterRevision default = 5;
   }
 }

diff --git a/examples/all-clusters-app/all-clusters-common/all-clusters-app.zap b/examples/all-clusters-app/all-clusters-common/all-clusters-app.zap
@@ -19196,10 +19196,10 @@
               "side": "server",
               "type": "int16u",
               "included": 1,
-              "storageOption": "RAM",
+              "storageOption": "External",
               "singleton": 0,
               "bounded": 0,
-              "defaultValue": "10",
+              "defaultValue": null,
               "reportable": 1,
               "minInterval": 1,
               "maxInterval": 65534,
@@ -19244,10 +19244,10 @@
               "side": "server",
               "type": "bitmap32",
               "included": 1,
-              "storageOption": "RAM",
+              "storageOption": "External",
               "singleton": 0,
               "bounded": 0,
-              "defaultValue": "0x02",
+              "defaultValue": null,
               "reportable": 1,
               "minInterval": 1,
               "maxInterval": 65534,
@@ -25353,10 +25353,10 @@
               "side": "server",
               "type": "int16u",
               "included": 1,
-              "storageOption": "RAM",
+              "storageOption": "External",
               "singleton": 0,
               "bounded": 0,
-              "defaultValue": "20",
+              "defaultValue": null,
               "reportable": 1,
               "minInterval": 1,
               "maxInterval": 65534,
@@ -25401,10 +25401,10 @@
               "side": "server",
               "type": "bitmap32",
               "included": 1,
-              "storageOption": "RAM",
+              "storageOption": "External",
               "singleton": 0,
               "bounded": 0,
-              "defaultValue": "0x02",
+              "defaultValue": null,
               "reportable": 1,
               "minInterval": 1,
               "maxInterval": 65534,

diff --git a/examples/all-clusters-app/all-clusters-common/src/occupancy-sensing-stub.cpp b/examples/all-clusters-app/all-clusters-common/src/occupancy-sensing-stub.cpp
@@ -18,6 +18,7 @@
 #include <app/CommandHandler.h>
 #include <app/clusters/occupancy-sensor-server/occupancy-hal.h>
 #include <app/clusters/occupancy-sensor-server/occupancy-sensor-server.h>
+#include <app/util/attribute-storage.h>
 #include <platform/CHIPDeviceLayer.h>
 
 using namespace chip;
@@ -28,30 +29,48 @@ using namespace chip::DeviceLayer;
 
 using chip::Protocols::InteractionModel::Status;
 
-static std::unique_ptr<OccupancySensingAttrAccess>
-    gAttrAccess[MATTER_DM_OCCUPANCY_SENSING_CLUSTER_SERVER_ENDPOINT_COUNT + CHIP_DEVICE_CONFIG_DYNAMIC_ENDPOINT_COUNT];
+namespace {
+
+static constexpr size_t kOccupancySensingClusterTableSize =
+    MATTER_DM_OCCUPANCY_SENSING_CLUSTER_SERVER_ENDPOINT_COUNT + CHIP_DEVICE_CONFIG_DYNAMIC_ENDPOINT_COUNT;
+
+static_assert(kOccupancySensingClusterTableSize <= kEmberInvalidEndpointIndex, "Occupancy Sensing Cluster table size error");
+
+static std::unique_ptr<Instance> gOccupancySensingClusterInstances[kOccupancySensingClusterTableSize];
+
+} // namespace
 
 void emberAfOccupancySensingClusterInitCallback(EndpointId endpointId)
 {
-    VerifyOrDie(!gAttrAccess[endpointId]);
+    uint16_t epIndex = emberAfGetClusterServerEndpointIndex(endpointId, chip::app::Clusters::OccupancySensing::Id,
+                                                            MATTER_DM_OCCUPANCY_SENSING_CLUSTER_SERVER_ENDPOINT_COUNT);
 
-    gAttrAccess[endpointId] = std::make_unique<OccupancySensingAttrAccess>(
-        BitMask<OccupancySensing::Feature, uint32_t>(OccupancySensing::Feature::kPassiveInfrared));
+    if (epIndex < kOccupancySensingClusterTableSize)
+    {
+        VerifyOrDie(!gOccupancySensingClusterInstances[epIndex]);
 
-    OccupancySensing::Structs::HoldTimeLimitsStruct::Type holdTimeLimits = {
-        .holdTimeMin     = 1,
-        .holdTimeMax     = 300,
-        .holdTimeDefault = 10,
-    };
+        gOccupancySensingClusterInstances[epIndex] =
+            std::make_unique<Instance>(BitMask<OccupancySensing::Feature, uint32_t>(OccupancySensing::Feature::kPassiveInfrared));
 
-    uint16_t holdTime = 10;
+        OccupancySensing::Structs::HoldTimeLimitsStruct::Type holdTimeLimits = {
+            .holdTimeMin     = 1,
+            .holdTimeMax     = 300,
+            .holdTimeDefault = 10,
+        };
 
-    if (gAttrAccess[endpointId])
-    {
-        gAttrAccess[endpointId]->Init();
+        uint16_t holdTime = 10;
+
+        if (gOccupancySensingClusterInstances[epIndex])
+        {
+            gOccupancySensingClusterInstances[epIndex]->Init();
 
-        SetHoldTimeLimits(endpointId, holdTimeLimits);
+            SetHoldTimeLimits(endpointId, holdTimeLimits);
 
-        SetHoldTime(endpointId, holdTime);
+            SetHoldTime(endpointId, holdTime);
+        }
+    }
+    else
+    {
+        ChipLogError(AppServer, "Error: invalid/unexpected OccupancySensing Cluster endpoint index.");
     }
 }
diff --git a/examples/all-clusters-minimal-app/all-clusters-common/all-clusters-minimal-app.matter b/examples/all-clusters-minimal-app/all-clusters-common/all-clusters-minimal-app.matter
@@ -6922,7 +6922,7 @@ endpoint 1 {
     ram      attribute occupancy;
     ram      attribute occupancySensorType;
     ram      attribute occupancySensorTypeBitmap;
-    ram      attribute featureMap default = 0;
+    callback attribute featureMap;
     ram      attribute clusterRevision default = 5;
   }
 
@@ -7275,7 +7275,7 @@ endpoint 2 {
     ram      attribute occupancy;
     ram      attribute occupancySensorType;
     ram      attribute occupancySensorTypeBitmap;
-    ram      attribute featureMap default = 0;
+    callback attribute featureMap;
     ram      attribute clusterRevision default = 5;
   }
 }

diff --git a/examples/chef/devices/rootnode_occupancysensor_iHyVgifZuo.matter b/examples/chef/devices/rootnode_occupancysensor_iHyVgifZuo.matter
@@ -1932,7 +1932,7 @@ endpoint 1 {
     callback attribute generatedCommandList;
     callback attribute acceptedCommandList;
     callback attribute attributeList;
-    ram      attribute featureMap default = 0;
+    callback attribute featureMap;
     ram      attribute clusterRevision default = 5;
   }
 }

diff --git a/examples/contact-sensor-app/contact-sensor-common/contact-sensor-app.matter b/examples/contact-sensor-app/contact-sensor-common/contact-sensor-app.matter
@@ -2369,7 +2369,7 @@ endpoint 1 {
     ram      attribute occupancy;
     ram      attribute occupancySensorType;
     ram      attribute occupancySensorTypeBitmap;
-    ram      attribute featureMap default = 0;
+    callback attribute featureMap;
     ram      attribute clusterRevision default = 5;
   }
 }

diff --git a/examples/lighting-app/lighting-common/lighting-app.matter b/examples/lighting-app/lighting-common/lighting-app.matter
@@ -3193,7 +3193,7 @@ endpoint 1 {
     ram      attribute occupancy;
     ram      attribute occupancySensorType;
     ram      attribute occupancySensorTypeBitmap;
-    ram      attribute featureMap default = 0;
+    callback attribute featureMap;
     ram      attribute clusterRevision default = 5;
   }
 }

diff --git a/examples/placeholder/linux/apps/app1/config.matter b/examples/placeholder/linux/apps/app1/config.matter
@@ -9589,7 +9589,7 @@ endpoint 1 {
     ram      attribute physicalContactOccupiedToUnoccupiedDelay default = 0x00;
     ram      attribute physicalContactUnoccupiedToOccupiedDelay default = 0x00;
     ram      attribute physicalContactUnoccupiedToOccupiedThreshold default = 1;
-    ram      attribute featureMap default = 0;
+    callback attribute featureMap;
     callback attribute clusterRevision default = 5;
   }
 }

diff --git a/examples/placeholder/linux/apps/app2/config.matter b/examples/placeholder/linux/apps/app2/config.matter
@@ -9528,7 +9528,7 @@ endpoint 1 {
     ram      attribute physicalContactOccupiedToUnoccupiedDelay default = 0x00;
     ram      attribute physicalContactUnoccupiedToOccupiedDelay default = 0x00;
     ram      attribute physicalContactUnoccupiedToOccupiedThreshold default = 1;
-    ram      attribute featureMap default = 0;
+    callback attribute featureMap;
     callback attribute clusterRevision default = 5;
   }
 }

diff --git a/scripts/tools/zap/tests/outputs/all-clusters-app/app-templates/endpoint_config.h b/scripts/tools/zap/tests/outputs/all-clusters-app/app-templates/endpoint_config.h
@@ -1575,11 +1575,11 @@
             { ZAP_SIMPLE_DEFAULT(3), 0x0000FFFD, 2, ZAP_TYPE(INT16U), 0 },                                 /* ClusterRevision */   \
                                                                                                                                    \
             /* Endpoint: 1, Cluster: Occupancy Sensing (server) */                                                                 \
-            { ZAP_EMPTY_DEFAULT(), 0x00000000, 1, ZAP_TYPE(BITMAP8), 0 },    /* Occupancy */                                       \
-            { ZAP_EMPTY_DEFAULT(), 0x00000001, 1, ZAP_TYPE(ENUM8), 0 },      /* OccupancySensorType */                             \
-            { ZAP_EMPTY_DEFAULT(), 0x00000002, 1, ZAP_TYPE(BITMAP8), 0 },    /* OccupancySensorTypeBitmap */                       \
-            { ZAP_SIMPLE_DEFAULT(0), 0x0000FFFC, 4, ZAP_TYPE(BITMAP32), 0 }, /* FeatureMap */                                      \
-            { ZAP_SIMPLE_DEFAULT(4), 0x0000FFFD, 2, ZAP_TYPE(INT16U), 0 },   /* ClusterRevision */                                 \
+            { ZAP_EMPTY_DEFAULT(), 0x00000000, 1, ZAP_TYPE(BITMAP8), 0 }, /* Occupancy */                                          \
+            { ZAP_EMPTY_DEFAULT(), 0x00000001, 1, ZAP_TYPE(ENUM8), 0 },   /* OccupancySensorType */                                \
+            { ZAP_EMPTY_DEFAULT(), 0x00000002, 1, ZAP_TYPE(BITMAP8), 0 }, /* OccupancySensorTypeBitmap */                          \
+            { ZAP_EMPTY_DEFAULT(), 0x0000FFFC, 4, ZAP_TYPE(BITMAP32), ZAP_ATTRIBUTE_MASK(EXTERNAL_STORAGE) }, /* FeatureMap */     \
+            { ZAP_SIMPLE_DEFAULT(4), 0x0000FFFD, 2, ZAP_TYPE(INT16U), 0 }, /* ClusterRevision */                                   \
                                                                                                                                    \
             /* Endpoint: 1, Cluster: Carbon Monoxide Concentration Measurement (server) */                                         \
             { ZAP_EMPTY_DEFAULT(), 0x00000000, 4, ZAP_TYPE(SINGLE),                                                                \
@@ -2016,11 +2016,11 @@
             { ZAP_SIMPLE_DEFAULT(1), 0x0000FFFD, 2, ZAP_TYPE(INT16U), 0 },                                 /* ClusterRevision */   \
                                                                                                                                    \
             /* Endpoint: 2, Cluster: Occupancy Sensing (server) */                                                                 \
-            { ZAP_EMPTY_DEFAULT(), 0x00000000, 1, ZAP_TYPE(BITMAP8), 0 },    /* Occupancy */                                       \
-            { ZAP_EMPTY_DEFAULT(), 0x00000001, 1, ZAP_TYPE(ENUM8), 0 },      /* OccupancySensorType */                             \
-            { ZAP_EMPTY_DEFAULT(), 0x00000002, 1, ZAP_TYPE(BITMAP8), 0 },    /* OccupancySensorTypeBitmap */                       \
-            { ZAP_SIMPLE_DEFAULT(0), 0x0000FFFC, 4, ZAP_TYPE(BITMAP32), 0 }, /* FeatureMap */                                      \
-            { ZAP_SIMPLE_DEFAULT(4), 0x0000FFFD, 2, ZAP_TYPE(INT16U), 0 },   /* ClusterRevision */                                 \
+            { ZAP_EMPTY_DEFAULT(), 0x00000000, 1, ZAP_TYPE(BITMAP8), 0 }, /* Occupancy */                                          \
+            { ZAP_EMPTY_DEFAULT(), 0x00000001, 1, ZAP_TYPE(ENUM8), 0 },   /* OccupancySensorType */                                \
+            { ZAP_EMPTY_DEFAULT(), 0x00000002, 1, ZAP_TYPE(BITMAP8), 0 }, /* OccupancySensorTypeBitmap */                          \
+            { ZAP_EMPTY_DEFAULT(), 0x0000FFFC, 4, ZAP_TYPE(BITMAP32), ZAP_ATTRIBUTE_MASK(EXTERNAL_STORAGE) }, /* FeatureMap */     \
+            { ZAP_SIMPLE_DEFAULT(4), 0x0000FFFD, 2, ZAP_TYPE(INT16U), 0 }, /* ClusterRevision */                                   \
                                                                                                                                    \
             /* Endpoint: 65534, Cluster: Descriptor (server) */                                                                    \
             { ZAP_EMPTY_DEFAULT(), 0x00000000, 0, ZAP_TYPE(ARRAY), ZAP_ATTRIBUTE_MASK(EXTERNAL_STORAGE) },    /* DeviceTypeList */ \
@@ -3884,7 +3884,7 @@
       .clusterId = 0x00000406, \
       .attributes = ZAP_ATTRIBUTE_INDEX(761), \
       .attributeCount = 5, \
-      .clusterSize = 9, \
+      .clusterSize = 5, \
       .mask = ZAP_CLUSTER_MASK(SERVER) | ZAP_CLUSTER_MASK(INIT_FUNCTION), \
       .functions = chipFuncArrayOccupancySensingServer, \
       .acceptedCommandList = nullptr, \
@@ -4157,7 +4157,7 @@
       .clusterId = 0x00000406, \
       .attributes = ZAP_ATTRIBUTE_INDEX(1033), \
       .attributeCount = 5, \
-      .clusterSize = 9, \
+      .clusterSize = 5, \
       .mask = ZAP_CLUSTER_MASK(SERVER) | ZAP_CLUSTER_MASK(INIT_FUNCTION), \
       .functions = chipFuncArrayOccupancySensingServer, \
       .acceptedCommandList = nullptr, \
@@ -4200,7 +4200,7 @@
 // This is an array of EmberAfEndpointType structures.
 #define GENERATED_ENDPOINT_TYPES                                                                                                   \
     {                                                                                                                              \
-        { ZAP_CLUSTER_INDEX(0), 29, 349 }, { ZAP_CLUSTER_INDEX(29), 74, 3521 }, { ZAP_CLUSTER_INDEX(103), 7, 126 },                \
+        { ZAP_CLUSTER_INDEX(0), 29, 349 }, { ZAP_CLUSTER_INDEX(29), 74, 3517 }, { ZAP_CLUSTER_INDEX(103), 7, 122 },                \
             { ZAP_CLUSTER_INDEX(110), 2, 0 },                                                                                      \
     }
 
@@ -4213,7 +4213,7 @@ static_assert(ATTRIBUTE_LARGEST <= CHIP_CONFIG_MAX_ATTRIBUTE_STORE_ELEMENT_SIZE,
 #define ATTRIBUTE_SINGLETONS_SIZE (36)
 
 // Total size of attribute storage
-#define ATTRIBUTE_MAX_SIZE (3996)
+#define ATTRIBUTE_MAX_SIZE (3988)
 
 // Number of fixed endpoints
 #define FIXED_ENDPOINT_COUNT (4)

diff --git a/scripts/tools/zap/tests/outputs/lighting-app/app-templates/endpoint_config.h b/scripts/tools/zap/tests/outputs/lighting-app/app-templates/endpoint_config.h
@@ -536,11 +536,11 @@
             { ZAP_SIMPLE_DEFAULT(7), 0x0000FFFD, 2, ZAP_TYPE(INT16U), 0 },      /* ClusterRevision */                              \
                                                                                                                                    \
             /* Endpoint: 1, Cluster: Occupancy Sensing (server) */                                                                 \
-            { ZAP_EMPTY_DEFAULT(), 0x00000000, 1, ZAP_TYPE(BITMAP8), 0 },    /* Occupancy */                                       \
-            { ZAP_EMPTY_DEFAULT(), 0x00000001, 1, ZAP_TYPE(ENUM8), 0 },      /* OccupancySensorType */                             \
-            { ZAP_EMPTY_DEFAULT(), 0x00000002, 1, ZAP_TYPE(BITMAP8), 0 },    /* OccupancySensorTypeBitmap */                       \
-            { ZAP_SIMPLE_DEFAULT(0), 0x0000FFFC, 4, ZAP_TYPE(BITMAP32), 0 }, /* FeatureMap */                                      \
-            { ZAP_SIMPLE_DEFAULT(4), 0x0000FFFD, 2, ZAP_TYPE(INT16U), 0 },   /* ClusterRevision */                                 \
+            { ZAP_EMPTY_DEFAULT(), 0x00000000, 1, ZAP_TYPE(BITMAP8), 0 }, /* Occupancy */                                          \
+            { ZAP_EMPTY_DEFAULT(), 0x00000001, 1, ZAP_TYPE(ENUM8), 0 },   /* OccupancySensorType */                                \
+            { ZAP_EMPTY_DEFAULT(), 0x00000002, 1, ZAP_TYPE(BITMAP8), 0 }, /* OccupancySensorTypeBitmap */                          \
+            { ZAP_EMPTY_DEFAULT(), 0x0000FFFC, 4, ZAP_TYPE(BITMAP32), ZAP_ATTRIBUTE_MASK(EXTERNAL_STORAGE) }, /* FeatureMap */     \
+            { ZAP_SIMPLE_DEFAULT(4), 0x0000FFFD, 2, ZAP_TYPE(INT16U), 0 }, /* ClusterRevision */                                   \
     }
 
 // clang-format off
@@ -1170,7 +1170,7 @@
       .clusterId = 0x00000406, \
       .attributes = ZAP_ATTRIBUTE_INDEX(271), \
       .attributeCount = 5, \
-      .clusterSize = 9, \
+      .clusterSize = 5, \
       .mask = ZAP_CLUSTER_MASK(SERVER) | ZAP_CLUSTER_MASK(INIT_FUNCTION), \
       .functions = chipFuncArrayOccupancySensingServer, \
       .acceptedCommandList = nullptr, \
@@ -1187,7 +1187,7 @@
 // This is an array of EmberAfEndpointType structures.
 #define GENERATED_ENDPOINT_TYPES                                                                                                   \
     {                                                                                                                              \
-        { ZAP_CLUSTER_INDEX(0), 21, 223 }, { ZAP_CLUSTER_INDEX(21), 8, 121 },                                                      \
+        { ZAP_CLUSTER_INDEX(0), 21, 223 }, { ZAP_CLUSTER_INDEX(21), 8, 117 },                                                      \
     }
 
 // Largest attribute size is needed for various buffers
@@ -1199,7 +1199,7 @@ static_assert(ATTRIBUTE_LARGEST <= CHIP_CONFIG_MAX_ATTRIBUTE_STORE_ELEMENT_SIZE,
 #define ATTRIBUTE_SINGLETONS_SIZE (36)
 
 // Total size of attribute storage
-#define ATTRIBUTE_MAX_SIZE (344)
+#define ATTRIBUTE_MAX_SIZE (340)
 
 // Number of fixed endpoints
 #define FIXED_ENDPOINT_COUNT (2)

diff --git a/src/app/clusters/occupancy-sensor-server/occupancy-sensor-server.cpp b/src/app/clusters/occupancy-sensor-server/occupancy-sensor-server.cpp
@@ -39,18 +39,18 @@ Structs::HoldTimeLimitsStruct::Type
 uint16_t sHoldTime[MATTER_DM_OCCUPANCY_SENSING_CLUSTER_SERVER_ENDPOINT_COUNT + CHIP_DEVICE_CONFIG_DYNAMIC_ENDPOINT_COUNT];
 } // namespace
 
-CHIP_ERROR OccupancySensingAttrAccess::Init()
+CHIP_ERROR Instance::Init()
 {
     VerifyOrReturnError(registerAttributeAccessOverride(this), CHIP_ERROR_INCORRECT_STATE);
     return CHIP_NO_ERROR;
 }
 
-void OccupancySensingAttrAccess::Shutdown()
+void Instance::Shutdown()
 {
     unregisterAttributeAccessOverride(this);
 }
 
-CHIP_ERROR OccupancySensingAttrAccess::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder)
+CHIP_ERROR Instance::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder)
 {
     VerifyOrDie(aPath.mClusterId == app::Clusters::OccupancySensing::Id);
 
@@ -88,7 +88,34 @@ CHIP_ERROR OccupancySensingAttrAccess::Read(const ConcreteReadAttributePath & aP
     return CHIP_NO_ERROR;
 }
 
-bool OccupancySensingAttrAccess::HasFeature(Feature aFeature) const
+CHIP_ERROR Instance::Write(const ConcreteDataAttributePath & aPath, AttributeValueDecoder & aDecoder)
+{
+    VerifyOrDie(aPath.mClusterId == app::Clusters::OccupancySensing::Id);
+
+    switch (aPath.mAttributeId)
+    {
+    case Attributes::HoldTime::Id: {
+
+        uint16_t newHoldTime;
+
+        ReturnErrorOnFailure(aDecoder.Decode(newHoldTime));
+
+        Structs::HoldTimeLimitsStruct::Type * currHoldTimeLimits = GetHoldTimeLimitsForEndpoint(aPath.mEndpointId);
+        VerifyOrReturnError(currHoldTimeLimits != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
+        VerifyOrReturnError(newHoldTime >= currHoldTimeLimits->holdTimeMin, CHIP_IM_GLOBAL_STATUS(ConstraintError));
+        VerifyOrReturnError(newHoldTime <= currHoldTimeLimits->holdTimeMax, CHIP_IM_GLOBAL_STATUS(ConstraintError));
+
+        return SetHoldTime(aPath.mEndpointId, newHoldTime);
+    }
+    default: {
+        break;
+    }
+    }
+
+    return CHIP_NO_ERROR;
+}
+
+bool Instance::HasFeature(Feature aFeature) const
 {
     return mFeature.Has(aFeature);
 }