Take into consideration transitive dependencies, not just root struct…

… fields (#1501) * Take into consideration transitive dependencies, not just root struct * Apply changes addressing comments in PR * Added 2 new args in previous commit: <arg name="arg7" type="DoubleNestedStructList" /> <arg name="arg8" type="DeepNestedStructList" /> * Calculate struct_has_fabric_sensitive_fields only at the top level, not transitively * Add test for struct_contains_array * Apply suggestions from code review Co-authored-by: Boris Zbarsky <[email protected]> * Fixup references after applying suggestions from code review --------- Co-authored-by: Boris Zbarsky <[email protected]>
project-chip · Jan 10, 2025 · 8e4ff10 · 8e4ff10
1 parent 6fa3da2
commit 8e4ff10
Show file tree

Hide file tree

Showing 7 changed files with 58 additions and 26 deletions.
diff --git a/src-electron/db/db-mapping.js b/src-electron/db/db-mapping.js
@@ -378,8 +378,7 @@ exports.map = {
       isNullable: dbApi.fromDbBool(x.IS_NULLABLE),
       isOptional: dbApi.fromDbBool(x.IS_OPTIONAL),
       isFabricSensitive: dbApi.fromDbBool(x.IS_FABRIC_SENSITIVE),
-      dataTypeReference: x.TYPE,
-      dataTypeReferenceName: x.DATA_TYPE_REF_NAME,
+      dataTypeReference: x.DATA_TYPE_REF,
       discriminatorName: x.DISCRIMINATOR_NAME
     }
   },

diff --git a/src-electron/db/query-zcl.js b/src-electron/db/query-zcl.js
@@ -342,7 +342,8 @@ async function selectStructsWithItemsImpl(db, packageIds, clusterId) {
       SI.IS_WRITABLE AS ITEM_IS_WRITABLE,
       SI.IS_NULLABLE AS ITEM_IS_NULLABLE,
       SI.IS_OPTIONAL AS ITEM_IS_OPTIONAL,
-      SI.IS_FABRIC_SENSITIVE AS ITEM_IS_FABRIC_SENSITIVE
+      SI.IS_FABRIC_SENSITIVE AS ITEM_IS_FABRIC_SENSITIVE,
+      SI.DATA_TYPE_REF AS ITEM_DATA_TYPE_REF
     FROM
       STRUCT AS S
     INNER JOIN
@@ -374,7 +375,8 @@ async function selectStructsWithItemsImpl(db, packageIds, clusterId) {
       SI.IS_WRITABLE AS ITEM_IS_WRITABLE,
       SI.IS_NULLABLE AS ITEM_IS_NULLABLE,
       SI.IS_OPTIONAL AS ITEM_IS_OPTIONAL,
-      SI.IS_FABRIC_SENSITIVE AS ITEM_IS_FABRIC_SENSITIVE
+      SI.IS_FABRIC_SENSITIVE AS ITEM_IS_FABRIC_SENSITIVE,
+      SI.DATA_TYPE_REF AS ITEM_DATA_TYPE_REF
     FROM
       STRUCT AS S
     INNER JOIN
@@ -428,7 +430,8 @@ async function selectStructsWithItemsImpl(db, packageIds, clusterId) {
       isWritable: dbApi.fromDbBool(value.ITEM_IS_WRITABLE),
       isNullable: dbApi.fromDbBool(value.ITEM_IS_NULLABLE),
       isOptional: dbApi.fromDbBool(value.ITEM_IS_OPTIONAL),
-      isFabricSensitive: dbApi.fromDbBool(value.ITEM_IS_FABRIC_SENSITIVE)
+      isFabricSensitive: dbApi.fromDbBool(value.ITEM_IS_FABRIC_SENSITIVE),
+      dataTypeReference: value.ITEM_DATA_TYPE_REF
     })
     objectToActOn.itemCnt++
     return acc
@@ -449,8 +452,7 @@ async function selectAllStructItemsById(db, id) {
 SELECT
   STRUCT_ITEM.FIELD_IDENTIFIER,
   STRUCT_ITEM.NAME,
-  (SELECT DATA_TYPE.NAME FROM DATA_TYPE WHERE DATA_TYPE.DATA_TYPE_ID = STRUCT_ITEM.DATA_TYPE_REF) AS TYPE,
-  DATA_TYPE.NAME AS DATA_TYPE_REF_NAME,
+  DATA_TYPE.NAME AS TYPE,
   DISCRIMINATOR.NAME AS DISCRIMINATOR_NAME,
   STRUCT_ITEM.STRUCT_REF,
   STRUCT_ITEM.IS_ARRAY,
@@ -460,7 +462,8 @@ SELECT
   STRUCT_ITEM.IS_WRITABLE,
   STRUCT_ITEM.IS_NULLABLE,
   STRUCT_ITEM.IS_OPTIONAL,
-  STRUCT_ITEM.IS_FABRIC_SENSITIVE
+  STRUCT_ITEM.IS_FABRIC_SENSITIVE,
+  STRUCT_ITEM.DATA_TYPE_REF
 FROM
   STRUCT_ITEM
 INNER JOIN
@@ -498,19 +501,19 @@ async function selectAllStructItemsByStructName(
   let clusterJoinQuery = ''
   let clusterWhereQuery = ''
   if (clusterName) {
-    clusterJoinQuery = ` 
+    clusterJoinQuery = `
     INNER JOIN
       DATA_TYPE_CLUSTER
     ON
       DATA_TYPE_CLUSTER.DATA_TYPE_REF = DT.DATA_TYPE_ID
     INNER JOIN
       CLUSTER
     ON
-      DATA_TYPE_CLUSTER.CLUSTER_REF = CLUSTER.CLUSTER_ID 
+      DATA_TYPE_CLUSTER.CLUSTER_REF = CLUSTER.CLUSTER_ID
       `
-    clusterWhereQuery = ` 
+    clusterWhereQuery = `
     AND
-      CLUSTER.NAME = "${clusterName}" 
+      CLUSTER.NAME = "${clusterName}"
       `
   }
   return dbApi
@@ -531,7 +534,8 @@ SELECT
   SI.IS_WRITABLE,
   SI.IS_NULLABLE,
   SI.IS_OPTIONAL,
-  SI.IS_FABRIC_SENSITIVE
+  SI.IS_FABRIC_SENSITIVE,
+  SI.DATA_TYPE_REF
 FROM
   STRUCT_ITEM AS SI
 INNER JOIN

diff --git a/src-electron/generator/helper-zcl.js b/src-electron/generator/helper-zcl.js
@@ -144,21 +144,37 @@ async function zcl_structs(options) {
     )
   }
   structs = await zclUtil.sortStructsByDependency(structs)
-  structs.forEach((st) => {
+  for (const st of structs) {
     st.struct_contains_array = false
     st.struct_has_fabric_sensitive_fields = false
     st.has_no_clusters = st.struct_cluster_count < 1
     st.has_one_cluster = st.struct_cluster_count == 1
     st.has_more_than_one_cluster = st.struct_cluster_count > 1
-    st.items.forEach((i) => {
-      if (i.isArray) {
-        st.struct_contains_array = true
+    const checkForArraysTransitively = async (items) => {
+      let promises = []
+      for (const i of items) {
+        if (i.isArray) {
+          st.struct_contains_array = true
+        }
+        if (!st.struct_contains_array) {
+          promises.push(
+            queryZcl
+              .selectAllStructItemsById(this.global.db, i.dataTypeReference)
+              .then(checkForArraysTransitively)
+          )
+        }
+      }
+      if (promises.length != 0) {
+        await Promise.all(promises)
       }
+    }
+    for (const i of st.items) {
       if (i.isFabricSensitive) {
         st.struct_has_fabric_sensitive_fields = true
       }
-    })
-  })
+    }
+    await checkForArraysTransitively(st.items)
+  }
   if (checkForDoubleNestedArray) {
     // If this is set to true in a template, then we populate the
     // struct_contains_nested_array variable with true
@@ -169,10 +185,9 @@ async function zcl_structs(options) {
       for (const i of st.items) {
         if (i.isArray) {
           // Found an array. Now let's check if it points to a struct that also contains an array.
-          let sis = await queryZcl.selectAllStructItemsByStructName(
+          let sis = await queryZcl.selectAllStructItemsById(
             this.global.db,
-            i.type,
-            packageIds
+            i.dataTypeReference
           )
           if (sis.length > 0) {
             for (const ss of sis) {

diff --git a/test/gen-matter-3-1.test.js b/test/gen-matter-3-1.test.js
@@ -476,6 +476,9 @@ test(
     expect(ept).toContain(
       'TargetStruct item 4 from Binding cluster: FabricIndex'
     )
+
+    expect(ept).toContain('Struct with array: NestedStructList')
+    expect(ept).toContain('Struct with array: DoubleNestedStructList')
   },
   testUtil.timeout.long()
 )

diff --git a/test/gen-template/matter3/miscellaneous_helper_tests.zapt b/test/gen-template/matter3/miscellaneous_helper_tests.zapt
@@ -28,4 +28,9 @@ TargetStruct item {{index}} from Access Control cluster: {{name}}
 
 {{#zcl_struct_items_by_struct_and_cluster_name "TargetStruct" "Binding"}}
 TargetStruct item {{index}} from Binding cluster: {{name}}
-{{/zcl_struct_items_by_struct_and_cluster_name}}
+{{/zcl_struct_items_by_struct_and_cluster_name}}
+
+{{#zcl_structs}}
+  {{#if struct_contains_array}} Struct with array: {{name}}
+  {{/if}}
+{{/zcl_structs}}
diff --git a/test/test-util.js b/test/test-util.js
@@ -189,7 +189,7 @@ exports.totalDotDotEnumItems = 637
 
 exports.totalMatterClusters = 72
 exports.totalMatterDeviceTypes = 119
-exports.totalMatterCommandArgs = 595
+exports.totalMatterCommandArgs = 597
 exports.totalMatterCommands = 248
 exports.totalMatterAttributes = 784
 exports.totalMatterTags = 17

diff --git a/zcl-builtin/matter/data-model/chip/test-cluster.xml b/zcl-builtin/matter/data-model/chip/test-cluster.xml
@@ -83,6 +83,10 @@ limitations under the License.
     <item name="a" type="NestedStructList" array="true" optional="false"/>
   </struct>
 
+  <struct name="DeepNestedStructList">
+    <cluster code="0xFFF1FC05"/>
+    <item name="a" type="NestedStructList" optional="false"/>
+  </struct>
 
   <struct name="NullablesAndOptionalsStruct">
     <cluster code="0xFFF1FC05"/>
@@ -127,15 +131,15 @@ limitations under the License.
         <field mask="0x04" name="MaskVal3" />
         <field mask="0x40000000" name="MaskVal4" />
     </bitmap>
-    
+
     <bitmap name="Bitmap64MaskMap" type="BITMAP64">
         <cluster code="0xFFF1FC05" />
         <field mask="0x01" name="MaskVal1" />
         <field mask="0x02" name="MaskVal2" />
         <field mask="0x04" name="MaskVal3" />
         <field mask="0x4000000000000000" name="MaskVal4" />
     </bitmap>
-    
+
   <cluster>
     <domain>CHIP</domain>
     <name>Unit Testing</name>
@@ -287,6 +291,8 @@ limitations under the License.
       <arg name="arg4" type="BOOLEAN" array="true"/>
       <arg name="arg5" type="SimpleEnum"/>
       <arg name="arg6" type="BOOLEAN"/>
+      <arg name="arg7" type="DoubleNestedStructList" />
+      <arg name="arg8" type="DeepNestedStructList" />
     </command>
 
     <command source="client" code="0x07" name="TestStructArgumentRequest"