Revert "[binding] Make binding table an attribute (#14874)" (#14907)

This reverts commit 5834518. The PR made an unnecessary API mass change that should be reverted no matter what. It also introduces a null-dereference crash. Apart from that, the new code does not follow the spec in various ways: 1) Incorrect encoding of the binding target structs. 2) Does not preserve order, as the spec requires. 3) Will fail on legal writes of a list that completely replaces an existing list if the _sum_ of the lengths does not fit in our storage. 5) Does not do required error-checking on inputs.
project-chip · Sep 6, 2023 · 27afe40 · 27afe40
1 parent f127711
commit 27afe40
Show file tree

Hide file tree

Showing 101 changed files with 2,948 additions and 3,208 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
@@ -235,15 +235,24 @@ server cluster BinaryInputBasic = 15 {
 }
 
 server cluster Binding = 30 {
-  struct BindingEntry {
+  readonly global attribute int16u clusterRevision = 65533;
+
+  request struct BindRequest {
     NODE_ID nodeId = 0;
     GROUP_ID groupId = 1;
     ENDPOINT_NO endpointId = 2;
     CLUSTER_ID clusterId = 3;
   }
 
-  attribute BindingEntry bindingList[] = 0;
-  readonly global attribute int16u clusterRevision = 65533;
+  request struct UnbindRequest {
+    NODE_ID nodeId = 0;
+    GROUP_ID groupId = 1;
+    ENDPOINT_NO endpointId = 2;
+    CLUSTER_ID clusterId = 3;
+  }
+
+  command Bind(BindRequest): DefaultSuccess = 0;
+  command Unbind(UnbindRequest): DefaultSuccess = 1;
 }
 
 server cluster BooleanState = 69 {

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
@@ -877,7 +877,24 @@
           "define": "BINDING_CLUSTER",
           "side": "client",
           "enabled": 0,
-          "commands": [],
+          "commands": [
+            {
+              "name": "Bind",
+              "code": 0,
+              "mfgCode": null,
+              "source": "client",
+              "incoming": 1,
+              "outgoing": 1
+            },
+            {
+              "name": "Unbind",
+              "code": 1,
+              "mfgCode": null,
+              "source": "client",
+              "incoming": 1,
+              "outgoing": 1
+            }
+          ],
           "attributes": [
             {
               "name": "ClusterRevision",
@@ -905,21 +922,6 @@
           "enabled": 1,
           "commands": [],
           "attributes": [
-            {
-              "name": "binding list",
-              "code": 0,
-              "mfgCode": null,
-              "side": "server",
-              "included": 1,
-              "storageOption": "RAM",
-              "singleton": 0,
-              "bounded": 0,
-              "defaultValue": "",
-              "reportable": 0,
-              "minInterval": 1,
-              "maxInterval": 65534,
-              "reportableChange": 0
-            },
             {
               "name": "ClusterRevision",
               "code": 65533,
@@ -8704,7 +8706,24 @@
           "define": "BINDING_CLUSTER",
           "side": "client",
           "enabled": 0,
-          "commands": [],
+          "commands": [
+            {
+              "name": "Bind",
+              "code": 0,
+              "mfgCode": null,
+              "source": "client",
+              "incoming": 1,
+              "outgoing": 1
+            },
+            {
+              "name": "Unbind",
+              "code": 1,
+              "mfgCode": null,
+              "source": "client",
+              "incoming": 1,
+              "outgoing": 1
+            }
+          ],
           "attributes": [
             {
               "name": "ClusterRevision",
@@ -8732,21 +8751,6 @@
           "enabled": 1,
           "commands": [],
           "attributes": [
-            {
-              "name": "binding list",
-              "code": 0,
-              "mfgCode": null,
-              "side": "server",
-              "included": 1,
-              "storageOption": "RAM",
-              "singleton": 0,
-              "bounded": 0,
-              "defaultValue": "",
-              "reportable": 0,
-              "minInterval": 1,
-              "maxInterval": 65534,
-              "reportableChange": 0
-            },
             {
               "name": "ClusterRevision",
               "code": 65533,
@@ -21099,4 +21103,4 @@
       "deviceIdentifier": 256
     }
   ]
-}
+}
diff --git a/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp b/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp
@@ -37,7 +37,7 @@ class BridgedActionsAttrAccess : public AttributeAccessInterface
     // Register for the Bridged Actions cluster on all endpoints.
     BridgedActionsAttrAccess() : AttributeAccessInterface(Optional<EndpointId>::Missing(), BridgedActions::Id) {}
 
-    CHIP_ERROR Read(FabricIndex fabricIndex, const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override;
+    CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override;
 
 private:
     static constexpr uint16_t ClusterRevision = 1;
@@ -75,8 +75,7 @@ CHIP_ERROR BridgedActionsAttrAccess::ReadClusterRevision(EndpointId endpoint, At
 
 BridgedActionsAttrAccess gAttrAccess;
 
-CHIP_ERROR BridgedActionsAttrAccess::Read(FabricIndex fabricIndex, const ConcreteReadAttributePath & aPath,
-                                          AttributeValueEncoder & aEncoder)
+CHIP_ERROR BridgedActionsAttrAccess::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder)
 {
     VerifyOrDie(aPath.mClusterId == BridgedActions::Id);
 

diff --git a/examples/light-switch-app/light-switch-common/light-switch-app.matter b/examples/light-switch-app/light-switch-common/light-switch-app.matter
@@ -143,6 +143,23 @@ server cluster Basic = 40 {
 
 server cluster Binding = 30 {
   readonly global attribute int16u clusterRevision = 65533;
+
+  request struct BindRequest {
+    NODE_ID nodeId = 0;
+    GROUP_ID groupId = 1;
+    ENDPOINT_NO endpointId = 2;
+    CLUSTER_ID clusterId = 3;
+  }
+
+  request struct UnbindRequest {
+    NODE_ID nodeId = 0;
+    GROUP_ID groupId = 1;
+    ENDPOINT_NO endpointId = 2;
+    CLUSTER_ID clusterId = 3;
+  }
+
+  command Bind(BindRequest): DefaultSuccess = 0;
+  command Unbind(UnbindRequest): DefaultSuccess = 1;
 }
 
 client cluster ColorControl = 768 {

diff --git a/examples/thermostat/thermostat-common/thermostat.matter b/examples/thermostat/thermostat-common/thermostat.matter
@@ -142,15 +142,24 @@ server cluster Basic = 40 {
 }
 
 server cluster Binding = 30 {
-  struct BindingEntry {
+  readonly global attribute int16u clusterRevision = 65533;
+
+  request struct BindRequest {
     NODE_ID nodeId = 0;
     GROUP_ID groupId = 1;
     ENDPOINT_NO endpointId = 2;
     CLUSTER_ID clusterId = 3;
   }
 
-  attribute BindingEntry bindingList[] = 0;
-  readonly global attribute int16u clusterRevision = 65533;
+  request struct UnbindRequest {
+    NODE_ID nodeId = 0;
+    GROUP_ID groupId = 1;
+    ENDPOINT_NO endpointId = 2;
+    CLUSTER_ID clusterId = 3;
+  }
+
+  command Bind(BindRequest): DefaultSuccess = 0;
+  command Unbind(UnbindRequest): DefaultSuccess = 1;
 }
 
 server cluster Descriptor = 29 {

diff --git a/examples/thermostat/thermostat-common/thermostat.zap b/examples/thermostat/thermostat-common/thermostat.zap
@@ -854,7 +854,24 @@
           "define": "BINDING_CLUSTER",
           "side": "client",
           "enabled": 0,
-          "commands": [],
+          "commands": [
+            {
+              "name": "Bind",
+              "code": 0,
+              "mfgCode": null,
+              "source": "client",
+              "incoming": 1,
+              "outgoing": 1
+            },
+            {
+              "name": "Unbind",
+              "code": 1,
+              "mfgCode": null,
+              "source": "client",
+              "incoming": 1,
+              "outgoing": 1
+            }
+          ],
           "attributes": [
             {
               "name": "ClusterRevision",
@@ -882,21 +899,6 @@
           "enabled": 1,
           "commands": [],
           "attributes": [
-            {
-              "name": "binding list",
-              "code": 0,
-              "mfgCode": null,
-              "side": "server",
-              "included": 1,
-              "storageOption": "RAM",
-              "singleton": 0,
-              "bounded": 0,
-              "defaultValue": "",
-              "reportable": 0,
-              "minInterval": 1,
-              "maxInterval": 65534,
-              "reportableChange": 0
-            },
             {
               "name": "ClusterRevision",
               "code": 65533,
@@ -8156,7 +8158,24 @@
           "define": "BINDING_CLUSTER",
           "side": "client",
           "enabled": 0,
-          "commands": [],
+          "commands": [
+            {
+              "name": "Bind",
+              "code": 0,
+              "mfgCode": null,
+              "source": "client",
+              "incoming": 1,
+              "outgoing": 1
+            },
+            {
+              "name": "Unbind",
+              "code": 1,
+              "mfgCode": null,
+              "source": "client",
+              "incoming": 1,
+              "outgoing": 1
+            }
+          ],
           "attributes": [
             {
               "name": "ClusterRevision",
@@ -8184,21 +8203,6 @@
           "enabled": 0,
           "commands": [],
           "attributes": [
-            {
-              "name": "binding list",
-              "code": 0,
-              "mfgCode": null,
-              "side": "server",
-              "included": 1,
-              "storageOption": "RAM",
-              "singleton": 0,
-              "bounded": 0,
-              "defaultValue": "",
-              "reportable": 0,
-              "minInterval": 1,
-              "maxInterval": 65534,
-              "reportableChange": 0
-            },
             {
               "name": "ClusterRevision",
               "code": 65533,

diff --git a/examples/tv-app/linux/include/cluster-init.cpp b/examples/tv-app/linux/include/cluster-init.cpp
@@ -42,8 +42,7 @@ class TvAttrAccess : public app::AttributeAccessInterface
 public:
     TvAttrAccess() : app::AttributeAccessInterface(Optional<EndpointId>::Missing(), AttrTypeInfo::GetClusterId()) {}
 
-    CHIP_ERROR Read(FabricIndex fabricIndex, const app::ConcreteReadAttributePath & aPath,
-                    app::AttributeValueEncoder & aEncoder) override
+    CHIP_ERROR Read(const app::ConcreteReadAttributePath & aPath, app::AttributeValueEncoder & aEncoder) override
     {
         if (aPath.mAttributeId == AttrTypeInfo::GetAttributeId())
         {

diff --git a/examples/tv-app/linux/main.cpp b/examples/tv-app/linux/main.cpp
@@ -135,7 +135,7 @@ class MyPostCommissioningListener : public PostCommissioningListener
     }
 
     /* Callback when command results in success */
-    static void OnSuccessResponse(void * context)
+    static void OnSuccessResponse(void * context, const chip::app::DataModel::NullObjectType &)
     {
         ChipLogProgress(Controller, "OnSuccessResponse - Binding Add Successfully");
         CommissionerDiscoveryController * cdc = GetCommissionerDiscoveryController();

diff --git a/examples/tv-app/tv-common/tv-app.matter b/examples/tv-app/tv-common/tv-app.matter
@@ -277,27 +277,45 @@ server cluster Basic = 40 {
 }
 
 client cluster Binding = 30 {
-  struct BindingEntry {
+  readonly global attribute int16u clusterRevision = 65533;
+
+  request struct BindRequest {
     NODE_ID nodeId = 0;
     GROUP_ID groupId = 1;
     ENDPOINT_NO endpointId = 2;
     CLUSTER_ID clusterId = 3;
   }
 
-  attribute BindingEntry bindingList[] = 0;
-  readonly global attribute int16u clusterRevision = 65533;
+  request struct UnbindRequest {
+    NODE_ID nodeId = 0;
+    GROUP_ID groupId = 1;
+    ENDPOINT_NO endpointId = 2;
+    CLUSTER_ID clusterId = 3;
+  }
+
+  command Bind(BindRequest): DefaultSuccess = 0;
+  command Unbind(UnbindRequest): DefaultSuccess = 1;
 }
 
 server cluster Binding = 30 {
-  struct BindingEntry {
+  readonly global attribute int16u clusterRevision = 65533;
+
+  request struct BindRequest {
     NODE_ID nodeId = 0;
     GROUP_ID groupId = 1;
     ENDPOINT_NO endpointId = 2;
     CLUSTER_ID clusterId = 3;
   }
 
-  attribute BindingEntry bindingList[] = 0;
-  readonly global attribute int16u clusterRevision = 65533;
+  request struct UnbindRequest {
+    NODE_ID nodeId = 0;
+    GROUP_ID groupId = 1;
+    ENDPOINT_NO endpointId = 2;
+    CLUSTER_ID clusterId = 3;
+  }
+
+  command Bind(BindRequest): DefaultSuccess = 0;
+  command Unbind(UnbindRequest): DefaultSuccess = 1;
 }
 
 server cluster Channel = 1284 {