Fix the XML for ModeOptionStruct to match spec. (#18637)

Also fixes some ZAP templates that were generating incorrect code for lists in struct fields, noticed while looking at the codegen from these changes. Fixes #14077 Fixes #11748
project-chip · Sep 22, 2023 · 4333681 · 4333681
1 parent 07eddd4
commit 4333681
Show file tree

Hide file tree

Showing 26 changed files with 760 additions and 553 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
@@ -2104,7 +2104,12 @@ server cluster ModeSelect = 80 {
   struct ModeOptionStruct {
     char_string<32> label = 0;
     int8u mode = 1;
-    int32u semanticTag = 2;
+    SemanticTag semanticTags[] = 2;
+  }
+
+  struct SemanticTag {
+    enum16 mfgCode = 0;
+    enum16 value = 1;
   }
 
   readonly attribute char_string<32> description = 0;

diff --git a/examples/all-clusters-app/all-clusters-common/src/static-supported-modes-manager.cpp b/examples/all-clusters-app/all-clusters-common/src/static-supported-modes-manager.cpp
@@ -6,22 +6,32 @@ using namespace chip::app::Clusters;
 using namespace chip::app::Clusters::ModeSelect;
 
 using ModeOptionStructType = Structs::ModeOptionStruct::Type;
-using storage_value_type   = const ModeOptionStructType;
+using SemanticTag          = Structs::SemanticTag::Type;
+template <typename T>
+using List               = app::DataModel::List<T>;
+using storage_value_type = const ModeOptionStructType;
 namespace {
-Structs::ModeOptionStruct::Type buildModeOptionStruct(const char * label, uint8_t mode, uint32_t semanticTag)
+Structs::ModeOptionStruct::Type buildModeOptionStruct(const char * label, uint8_t mode,
+                                                      const List<const SemanticTag> & semanticTags)
 {
     Structs::ModeOptionStruct::Type option;
-    option.label       = CharSpan::fromCharString(label);
-    option.mode        = mode;
-    option.semanticTag = semanticTag;
+    option.label        = CharSpan::fromCharString(label);
+    option.mode         = mode;
+    option.semanticTags = semanticTags;
     return option;
 }
 } // namespace
 
+constexpr SemanticTag semanticTagsBlack[]     = { { .value = 0 } };
+constexpr SemanticTag semanticTagsCappucino[] = { { .value = 0 } };
+constexpr SemanticTag semanticTagsEspresso[]  = { { .value = 0 } };
+
 // TODO: Configure your options for each endpoint
-storage_value_type StaticSupportedModesManager::coffeeOptions[] = { buildModeOptionStruct("Black", 0, 0),
-                                                                    buildModeOptionStruct("Cappuccino", 4, 0),
-                                                                    buildModeOptionStruct("Espresso", 7, 0) };
+storage_value_type StaticSupportedModesManager::coffeeOptions[] = {
+    buildModeOptionStruct("Black", 0, List<const SemanticTag>(semanticTagsBlack)),
+    buildModeOptionStruct("Cappuccino", 4, List<const SemanticTag>(semanticTagsCappucino)),
+    buildModeOptionStruct("Espresso", 7, List<const SemanticTag>(semanticTagsEspresso))
+};
 const StaticSupportedModesManager::EndpointSpanPair
     StaticSupportedModesManager::supportedOptionsByEndpoints[EMBER_AF_MODE_SELECT_CLUSTER_SERVER_ENDPOINT_COUNT] = {
         EndpointSpanPair(1, Span<storage_value_type>(StaticSupportedModesManager::coffeeOptions)) // Options for Endpoint 1

diff --git a/examples/chip-tool/templates/tests/partials/value_equals.zapt b/examples/chip-tool/templates/tests/partials/value_equals.zapt
@@ -26,14 +26,14 @@
       auto iter_{{depth}} = {{actual}}.begin();
       {{#each expected}}
         VerifyOrReturn(CheckNextListItemDecodes<decltype({{../actual}})>("{{../label}}", iter_{{../depth}}, {{@index}}));
-        {{>valueEquals label=(concat ../label "[" @index "]") actual=(concat "iter_" ../depth ".GetValue()") expected=this isArray=false type=../type chipType=../chipType depth=(incrementDepth depth) parent=../parent}}
+        {{>valueEquals label=(concat ../label "[" @index "]") actual=(concat "iter_" ../depth ".GetValue()") expected=this isArray=false type=../type chipType=../chipType depth=(incrementDepth ../depth) parent=../parent}}
       {{/each}}
       VerifyOrReturn(CheckNoMoreListItems<decltype({{actual}})>("{{label}}", iter_{{depth}}, {{expected.length}}));
     }
 {{else if isEvent}}
     {{#zcl_events_fields_by_event_name type}}
       {{#if (hasProperty ../expected label)}}
-        {{>valueEquals label=(concat ../label "." (asLowerCamelCase label)) actual=(concat ../actual "." (asLowerCamelCase label)) expected=(lookup ../expected label) depth=(incrementDepth depth)}}
+        {{>valueEquals label=(concat ../label "." (asLowerCamelCase label)) actual=(concat ../actual "." (asLowerCamelCase label)) expected=(lookup ../expected label) depth=(incrementDepth ../depth)}}
       {{/if}}
     {{/zcl_events_fields_by_event_name}}
 {{else}}
@@ -43,7 +43,7 @@
         call. }}
     {{#zcl_struct_items_by_struct_name type}}
       {{#if (hasProperty ../expected label)}}
-        {{>valueEquals label=(concat ../label "." (asLowerCamelCase label)) actual=(concat ../actual "." (asLowerCamelCase label)) expected=(lookup ../expected label) depth=(incrementDepth depth)}}
+        {{>valueEquals label=(concat ../label "." (asLowerCamelCase label)) actual=(concat ../actual "." (asLowerCamelCase label)) expected=(lookup ../expected label) depth=(incrementDepth ../depth)}}
       {{/if}}
     {{/zcl_struct_items_by_struct_name}}
     {{! Maybe we should add a check for properties in the expected object (other

diff --git a/examples/placeholder/linux/apps/app1/config.matter b/examples/placeholder/linux/apps/app1/config.matter
@@ -1203,7 +1203,12 @@ client cluster ModeSelect = 80 {
   struct ModeOptionStruct {
     char_string<32> label = 0;
     int8u mode = 1;
-    int32u semanticTag = 2;
+    SemanticTag semanticTags[] = 2;
+  }
+
+  struct SemanticTag {
+    enum16 mfgCode = 0;
+    enum16 value = 1;
   }
 
   readonly attribute char_string<32> description = 0;
@@ -1232,7 +1237,12 @@ server cluster ModeSelect = 80 {
   struct ModeOptionStruct {
     char_string<32> label = 0;
     int8u mode = 1;
-    int32u semanticTag = 2;
+    SemanticTag semanticTags[] = 2;
+  }
+
+  struct SemanticTag {
+    enum16 mfgCode = 0;
+    enum16 value = 1;
   }
 
   readonly attribute char_string<32> description = 0;

diff --git a/examples/placeholder/linux/apps/app2/config.matter b/examples/placeholder/linux/apps/app2/config.matter
@@ -1203,7 +1203,12 @@ client cluster ModeSelect = 80 {
   struct ModeOptionStruct {
     char_string<32> label = 0;
     int8u mode = 1;
-    int32u semanticTag = 2;
+    SemanticTag semanticTags[] = 2;
+  }
+
+  struct SemanticTag {
+    enum16 mfgCode = 0;
+    enum16 value = 1;
   }
 
   readonly attribute char_string<32> description = 0;
@@ -1232,7 +1237,12 @@ server cluster ModeSelect = 80 {
   struct ModeOptionStruct {
     char_string<32> label = 0;
     int8u mode = 1;
-    int32u semanticTag = 2;
+    SemanticTag semanticTags[] = 2;
+  }
+
+  struct SemanticTag {
+    enum16 mfgCode = 0;
+    enum16 value = 1;
   }
 
   readonly attribute char_string<32> description = 0;

diff --git a/examples/placeholder/linux/static-supported-modes-manager.cpp b/examples/placeholder/linux/static-supported-modes-manager.cpp
@@ -6,22 +6,32 @@ using namespace chip::app::Clusters;
 using namespace chip::app::Clusters::ModeSelect;
 
 using ModeOptionStructType = Structs::ModeOptionStruct::Type;
-using storage_value_type   = const ModeOptionStructType;
+using SemanticTag          = Structs::SemanticTag::Type;
+template <typename T>
+using List               = app::DataModel::List<T>;
+using storage_value_type = const ModeOptionStructType;
 namespace {
-Structs::ModeOptionStruct::Type buildModeOptionStruct(const char * label, uint8_t mode, uint32_t semanticTag)
+Structs::ModeOptionStruct::Type buildModeOptionStruct(const char * label, uint8_t mode,
+                                                      const List<const SemanticTag> & semanticTags)
 {
     Structs::ModeOptionStruct::Type option;
-    option.label       = CharSpan::fromCharString(label);
-    option.mode        = mode;
-    option.semanticTag = semanticTag;
+    option.label        = CharSpan::fromCharString(label);
+    option.mode         = mode;
+    option.semanticTags = semanticTags;
     return option;
 }
 } // namespace
 
+constexpr SemanticTag semanticTagsBlack[]     = { { .value = 0 } };
+constexpr SemanticTag semanticTagsCappucino[] = { { .value = 0 } };
+constexpr SemanticTag semanticTagsEspresso[]  = { { .value = 0 } };
+
 // TODO: Configure your options for each endpoint
-storage_value_type StaticSupportedModesManager::coffeeOptions[] = { buildModeOptionStruct("Black", 0, 0),
-                                                                    buildModeOptionStruct("Cappuccino", 4, 0),
-                                                                    buildModeOptionStruct("Espresso", 7, 0) };
+storage_value_type StaticSupportedModesManager::coffeeOptions[] = {
+    buildModeOptionStruct("Black", 0, List<const SemanticTag>(semanticTagsBlack)),
+    buildModeOptionStruct("Cappuccino", 4, List<const SemanticTag>(semanticTagsCappucino)),
+    buildModeOptionStruct("Espresso", 7, List<const SemanticTag>(semanticTagsEspresso))
+};
 const StaticSupportedModesManager::EndpointSpanPair
     StaticSupportedModesManager::supportedOptionsByEndpoints[EMBER_AF_MODE_SELECT_CLUSTER_SERVER_ENDPOINT_COUNT] = {
         EndpointSpanPair(1, Span<storage_value_type>(StaticSupportedModesManager::coffeeOptions)) // Options for Endpoint 1

diff --git a/src/app/tests/suites/TestModeSelectCluster.yaml b/src/app/tests/suites/TestModeSelectCluster.yaml
@@ -45,9 +45,17 @@ tests:
       response:
           value:
               [
-                  { Label: "Black", Mode: 0, SemanticTag: 0 },
-                  { Label: "Cappuccino", Mode: 4, SemanticTag: 0 },
-                  { Label: "Espresso", Mode: 7, SemanticTag: 0 },
+                  { Label: "Black", Mode: 0, SemanticTags: [{ "value": 0 }] },
+                  {
+                      Label: "Cappuccino",
+                      Mode: 4,
+                      SemanticTags: [{ "value": 0 }],
+                  },
+                  {
+                      Label: "Espresso",
+                      Mode: 7,
+                      SemanticTags: [{ "value": 0 }],
+                  },
               ]
           constraints:
               - type: list

diff --git a/src/app/zap-templates/zcl/data-model/chip/mode-select-cluster.xml b/src/app/zap-templates/zcl/data-model/chip/mode-select-cluster.xml
@@ -27,9 +27,7 @@ limitations under the License.
     <cluster code="0x0050"/>
     <item name="Label" type="CHAR_STRING" optional="false" length="32"/>
     <item name="Mode" type="INT8U" optional="false"/>
-    <!-- TODO: A list of list is not supported yet. Otherwise this should be a list of SemanticTags -->
-    <!-- <item name="SemanticTags" type="SemanticTag" array="true" optional="false"/> -->
-    <item name="SemanticTag" type="INT32U" optional="false"/>
+    <item name="SemanticTags" type="SemanticTag" array="true" optional="false"/>
   </struct>
 
   <cluster>

diff --git a/src/controller/data_model/controller-clusters.matter b/src/controller/data_model/controller-clusters.matter
@@ -2478,7 +2478,12 @@ client cluster ModeSelect = 80 {
   struct ModeOptionStruct {
     char_string<32> label = 0;
     int8u mode = 1;
-    int32u semanticTag = 2;
+    SemanticTag semanticTags[] = 2;
+  }
+
+  struct SemanticTag {
+    enum16 mfgCode = 0;
+    enum16 value = 1;
   }
 
   readonly attribute char_string<32> description = 0;

diff --git a/src/controller/java/templates/partials/decode_value.zapt b/src/controller/java/templates/partials/decode_value.zapt
@@ -28,7 +28,7 @@ if ({{source}}.IsNull()) {
 {{else}}
   {{#if_is_struct type}}
     {{#zcl_struct_items_by_struct_name type}}
-      {{>decode_value target=(concat ../target "_" (asLowerCamelCase label)) source=(concat ../source "." (asLowerCamelCase label)) cluster=../cluster depth=(incrementDepth depth) omitDeclaration=false earlyReturn=../earlyReturn}}
+      {{>decode_value target=(concat ../target "_" (asLowerCamelCase label)) source=(concat ../source "." (asLowerCamelCase label)) cluster=../cluster depth=(incrementDepth ../depth) omitDeclaration=false earlyReturn=../earlyReturn}}
     {{/zcl_struct_items_by_struct_name}}
 
     jclass {{asLowerCamelCase type}}StructClass;