Add a test that validates python type parsing matches to chip-types.x…

…ml (#30711)

* Add unit test for supported types to have python match spec

* Restyle

* Add test to list of runnable tests

* Add semtag handling

* Add enum24 since chip-types defines it

* Update imports

* Remove unused imports

* zap regen

* Fix todo comment

scripts/py_matter_idl/BUILD.gn

@@ -68,6 +68,7 @@ pw_python_package("matter_idl") {
     "matter_idl/test_matter_idl_parser.py",
     "matter_idl/test_generators.py",
     "matter_idl/test_idl_generator.py",
+    "matter_idl/test_supported_types.py",
     "matter_idl/test_zapxml.py",
   ]
 

scripts/py_matter_idl/matter_idl/generators/type_definitions.py

@@ -169,6 +169,7 @@ def is_struct(self) -> bool:
     "bitmap64": BasicInteger(idl_name="bitmap64", byte_count=8, is_signed=False),
     "bitmap8": BasicInteger(idl_name="bitmap8", byte_count=1, is_signed=False),
     "enum16": BasicInteger(idl_name="enum16", byte_count=2, is_signed=False),
+    "enum24": BasicInteger(idl_name="enum24", byte_count=3, is_signed=False),
     "enum32": BasicInteger(idl_name="enum32", byte_count=4, is_signed=False),
     "enum8": BasicInteger(idl_name="enum8", byte_count=1, is_signed=False),
     "int16s": BasicInteger(idl_name="int16s", byte_count=2, is_signed=True),
@@ -188,7 +189,7 @@ def is_struct(self) -> bool:
     "int8s": BasicInteger(idl_name="int8s", byte_count=1, is_signed=True),
     "int8u": BasicInteger(idl_name="int8u", byte_count=1, is_signed=False),
     # Derived types
-    # Specification describes them in section '7.18.2. Derived Data Types'
+    # Specification describes them in section '7.19.2. Derived Data Types'
     "action_id": BasicInteger(idl_name="action_id", byte_count=1, is_signed=False),
     "attrib_id": BasicInteger(idl_name="attrib_id", byte_count=4, is_signed=False),
     "cluster_id": BasicInteger(idl_name="cluster_id", byte_count=4, is_signed=False),
@@ -213,7 +214,8 @@ def is_struct(self) -> bool:
     "percent100ths": BasicInteger(idl_name="percent100ths", byte_count=2, is_signed=False),
     "posix_ms": BasicInteger(idl_name="posix_ms", byte_count=8, is_signed=False),
     "priority": BasicInteger(idl_name="priority", byte_count=1, is_signed=False),
-    "status": BasicInteger(idl_name="status", byte_count=2, is_signed=False),
+    "semtag": BasicInteger(idl_name="semtag", byte_count=4, is_signed=False),
+    "status": BasicInteger(idl_name="status", byte_count=1, is_signed=False),
     "systime_ms": BasicInteger(idl_name="systime_ms", byte_count=8, is_signed=False),
     "systime_us": BasicInteger(idl_name="systime_us", byte_count=8, is_signed=False),
     "tag": BasicInteger(idl_name="tag", byte_count=1, is_signed=False),
@@ -334,7 +336,7 @@ def is_enum_type(self, name: str):
         Handles both standard names (like enum8) as well as enumerations defined
         within the current lookup context.
         """
-        if name.lower() in ["enum8", "enum16", "enum32"]:
+        if name.lower() in ["enum8", "enum16", "enum24", "enum32"]:
             return True
         return any(map(lambda e: e.name == name, self.all_enums))
 
@@ -384,9 +386,9 @@ def ParseDataType(data_type: DataType, lookup: TypeLookupContext) -> Union[Basic
         return BasicString(idl_name=lowercase_name, is_binary=False, max_length=data_type.max_length)
     elif lowercase_name in ['octet_string', 'long_octet_string']:
         return BasicString(idl_name=lowercase_name, is_binary=True, max_length=data_type.max_length)
-    elif lowercase_name in ['enum8', 'enum16', 'enum32']:
+    elif lowercase_name in ['enum8', 'enum16', 'enum24', 'enum32']:
         return IdlEnumType(idl_name=lowercase_name, base_type=__CHIP_SIZED_TYPES__[lowercase_name])
-    elif lowercase_name in ['bitmap8', 'bitmap16', 'bitmap24', 'bitmap32']:
+    elif lowercase_name in ['bitmap8', 'bitmap16', 'bitmap24', 'bitmap32', 'bitmap64']:
         return IdlBitmapType(idl_name=lowercase_name, base_type=__CHIP_SIZED_TYPES__[lowercase_name])
 
     int_type = __CHIP_SIZED_TYPES__.get(lowercase_name, None)

scripts/py_matter_idl/matter_idl/test_supported_types.py

@@ -0,0 +1,103 @@
+#!/usr/bin/env python3
+# Copyright (c) 2023 Project CHIP Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import os
+import unittest
+import xml.etree.ElementTree as ET
+
+try:
+    from matter_idl.generators.type_definitions import ParseDataType
+except ImportError:
+    import sys
+
+    sys.path.append(os.path.abspath(
+        os.path.join(os.path.dirname(__file__), '..')))
+    from matter_idl.generators.type_definitions import ParseDataType
+
+from matter_idl.generators.type_definitions import BasicInteger, TypeLookupContext
+from matter_idl.matter_idl_types import DataType, Idl
+
+
+class TestSupportedTypes(unittest.TestCase):
+
+    def __init__(self, *args, **kargs):
+        super().__init__(*args, **kargs)
+        self.maxDiff = None
+
+    def testAllTypesSupported(self):
+        # ALL types defined in chip-types.xml should be understandable
+        # by the generator type parsing
+        path = "src/app/zap-templates/zcl/data-model/chip/chip-types.xml"
+        path = os.path.join(os.path.dirname(__file__), "../../..", path)
+        dom = ET.parse(path).getroot()
+
+        # Format we expect:
+        #   - configurator/atomic/type
+        self.assertEqual(dom.tag, "configurator")
+        types = dom.findall("./atomic/type")
+
+        # Arbitrary non-empty assumption to make sure we
+        # test something and did not mess up our XPath query
+        self.assertTrue(len(types) > 10)
+
+        empty_lookup = TypeLookupContext(idl=Idl(), cluster=None)
+
+        for t in types:
+            # every type has the following intersting attributes:
+            #  - name (to be validated)
+            #  - size (in bytes, but may not be power of two)
+            #  - one of discrete/analog/composite
+
+            if "composite" in t.attrib and t.attrib["composite"] == "true":
+                # struct, array, octet_string and such
+                continue
+
+            data_type = DataType(name=t.attrib["name"])
+
+            # filter some know things
+            if data_type.name in {
+                    "no_data",  # intentionally skipped
+                    # handled as a non-integer type
+                    "boolean", "single", "double",
+                    # handled as specific bitmaps
+                    "bitmap8", "bitmap16", "bitmap24", "bitmap32", "bitmap64",
+                    # handled as specific enums
+                    "enum8", "enum16", "enum24", "enum32",
+
+                    # TODO: these may be bugs to fix
+                    "unknown"
+            }:
+                continue
+
+            parsed = ParseDataType(data_type, empty_lookup)
+
+            self.assertTrue(parsed is not None)  # this should always pass.
+            fail_message = f"{data_type.name} was parsed as {parsed}"
+
+            self.assertIs(type(parsed), BasicInteger, fail_message)
+
+            # check that types match
+            if "signed" in t.attrib and t.attrib["signed"] == "true":
+                # Oddly enough, we have no sign info for int8s and int8u
+                # Only temperature really has a signed component
+                self.assertTrue(parsed.is_signed, fail_message)
+
+            if "size" in t.attrib:
+                self.assertEqual(parsed.byte_count, int(
+                    t.attrib["size"]), fail_message)
+
+
+if __name__ == '__main__':
+    unittest.main()

src/app/zap-templates/zcl/data-model/chip/chip-types.xml

@@ -102,6 +102,7 @@ limitations under the License.
     <type id="0xF5" description="IPv6 Prefix"              name="ipv6pre"                     composite="true"/>
     <type id="0xF6" description="Hardware Address"         name="hwadr"                       composite="true"/>
 
+    <!-- TODO: Spec marks these as "compositte" yet they are set "discrete" here -->
     <type id="0xC9" description="Semantic Tag"             name="semtag"            size="4"  discrete="true" />
     <type id="0xCA" description="Namespace"                name="namespace"         size="1"  discrete="true" />
     <type id="0xCB" description="Tag"                      name="tag"               size="1"  discrete="true" />

src/controller/java/generated/java/matter/devicecontroller/cluster/clusters/ScenesCluster.kt

@@ -21,34 +21,34 @@ import matter.controller.MatterController
 import matter.devicecontroller.cluster.structs.*
 
 class ScenesCluster(private val controller: MatterController, private val endpointId: UShort) {
-  class AddSceneResponse(val status: UShort, val groupID: UShort, val sceneID: UByte)
+  class AddSceneResponse(val status: UByte, val groupID: UShort, val sceneID: UByte)
 
   class ViewSceneResponse(
-    val status: UShort,
+    val status: UByte,
     val groupID: UShort,
     val sceneID: UByte,
     val transitionTime: UShort?,
     val sceneName: String?,
     val extensionFieldSets: List<ScenesClusterExtensionFieldSet>?
   )
 
-  class RemoveSceneResponse(val status: UShort, val groupID: UShort, val sceneID: UByte)
+  class RemoveSceneResponse(val status: UByte, val groupID: UShort, val sceneID: UByte)
 
-  class RemoveAllScenesResponse(val status: UShort, val groupID: UShort)
+  class RemoveAllScenesResponse(val status: UByte, val groupID: UShort)
 
-  class StoreSceneResponse(val status: UShort, val groupID: UShort, val sceneID: UByte)
+  class StoreSceneResponse(val status: UByte, val groupID: UShort, val sceneID: UByte)
 
   class GetSceneMembershipResponse(
-    val status: UShort,
+    val status: UByte,
     val capacity: UByte?,
     val groupID: UShort,
     val sceneList: List<UByte>?
   )
 
-  class EnhancedAddSceneResponse(val status: UShort, val groupID: UShort, val sceneID: UByte)
+  class EnhancedAddSceneResponse(val status: UByte, val groupID: UShort, val sceneID: UByte)
 
   class EnhancedViewSceneResponse(
-    val status: UShort,
+    val status: UByte,
     val groupID: UShort,
     val sceneID: UByte,
     val transitionTime: UShort?,
@@ -57,7 +57,7 @@ class ScenesCluster(private val controller: MatterController, private val endpoi
   )
 
   class CopySceneResponse(
-    val status: UShort,
+    val status: UByte,
     val groupIdentifierFrom: UShort,
     val sceneIdentifierFrom: UByte
   )