Optional/Nullable support to Python Cluster Objects (#11515)

* Optional/Nullable support to Python Cluster Objects

This amongst other things, adds support for optional and nullable types
to the Python cluster objects. It does so by leveraging typing.Union and
typing.Optional to do so. To represent nulls, a new Nullable type has
been created to encapsulate that.

Consequently, 'None' indicates an optional field that is not present.
'NullValue' indicates a null-value.

Consequently, the following representations map to the various
combinations:

typing.Union[base-type, Types.Nullable] => a nullable base-type.
typing.Optional[base-type] => an optional base-type
typing.Union[base-type, Types.Nullable, None] => an optional, nullable,
base-type.

In addition, the generation helpers for Python have been cleaned up to
better align with how it's done for the other languages.

Finally, a unit-test for the generated objects has been added which was
crucial to fix some critical bugs in the implementation.

Tests:
- Validated using test_generated_clusterobjects.py.
- Validated by sending commands to the test cluster to a device.

* Review feedback

* Fixes the equality comparison in Python for Nones

* Re-gen cluster objects

* Adding debug mode to the newly added unit test

src/app/zap-templates/templates/app/helper.js

@@ -415,50 +415,50 @@ function zapTypeToDecodableClusterObjectType(type, options)
   return zapTypeToClusterObjectType.call(this, type, true, options)
 }
 
-function zapTypeToPythonClusterObjectType(type, options)
+async function _zapTypeToPythonClusterObjectType(type, options)
 {
-  if (StringHelper.isCharString(type)) {
-    return 'str';
-  }
-
-  if (StringHelper.isOctetString(type)) {
-    return 'bytes';
-  }
-
-  if ([ 'single', 'double' ].includes(type.toLowerCase())) {
-    return 'float';
-  }
-
-  if (type.toLowerCase() == 'boolean') {
-    return 'bool'
-  }
-
-  // #10748: asUnderlyingZclType will emit wrong types for int{48|56|64}(u), so we process all int values here.
-  if (type.toLowerCase().match(/^int\d+$/)) {
-    return 'int'
-  }
-
-  if (type.toLowerCase().match(/^int\d+u$/)) {
-    return 'uint'
-  }
-
   async function fn(pkgId)
   {
-    const ns          = asUpperCamelCase(options.hash.ns);
+    const ns          = options.hash.ns;
     const typeChecker = async (method) => zclHelper[method](this.global.db, type, pkgId).then(zclType => zclType != 'unknown');
 
     if (await typeChecker('isEnum')) {
       return ns + '.Enums.' + type;
     }
 
     if (await typeChecker('isBitmap')) {
-      return 'int';
+      return 'uint';
     }
 
     if (await typeChecker('isStruct')) {
       return ns + '.Structs.' + type;
     }
 
+    if (StringHelper.isCharString(type)) {
+      return 'str';
+    }
+
+    if (StringHelper.isOctetString(type)) {
+      return 'bytes';
+    }
+
+    if ([ 'single', 'double' ].includes(type.toLowerCase())) {
+      return 'float';
+    }
+
+    if (type.toLowerCase() == 'boolean') {
+      return 'bool'
+    }
+
+    // #10748: asUnderlyingZclType will emit wrong types for int{48|56|64}(u), so we process all int values here.
+    if (type.toLowerCase().match(/^int\d+$/)) {
+      return 'int'
+    }
+
+    if (type.toLowerCase().match(/^int\d+u$/)) {
+      return 'uint'
+    }
+
     resolvedType = await zclHelper.asUnderlyingZclType.call({ global : this.global }, type, options);
     {
       basicType = ChipTypesHelper.asBasicType(resolvedType);
@@ -469,14 +469,32 @@ function zapTypeToPythonClusterObjectType(type, options)
         return 'uint'
       }
     }
+  }
 
-    throw "Unhandled type " + resolvedType + " (from " + type + ")"
+  let promise = templateUtil.ensureZclPackageId(this).then(fn.bind(this));
+  if ((this.isList || this.isArray || this.entryType) && !options.hash.forceNotList) {
+    promise = promise.then(typeStr => `typing.List[${typeStr}]`);
+  }
+
+  const isNull     = (this.isNullable && !options.hash.forceNotNullable);
+  const isOptional = (this.isOptional && !options.hash.forceNotOptional);
+
+  if (isNull && isOptional) {
+    promise = promise.then(typeStr => `typing.Union[None, Nullable, ${typeStr}]`);
+  } else if (isNull) {
+    promise = promise.then(typeStr => `typing.Union[Nullable, ${typeStr}]`);
+  } else if (isOptional) {
+    promise = promise.then(typeStr => `typing.Optional[${typeStr}]`);
   }
 
-  const promise = templateUtil.ensureZclPackageId(this).then(fn.bind(this));
   return templateUtil.templatePromise(this.global, promise)
 }
 
+function zapTypeToPythonClusterObjectType(type, options)
+{
+  return _zapTypeToPythonClusterObjectType.call(this, type, options)
+}
+
 async function getResponseCommandName(responseRef, options)
 {
   let pkgId = await templateUtil.ensureZclPackageId(this);

src/controller/python/BUILD.gn

@@ -118,6 +118,7 @@ pw_python_action("python") {
         "chip/clusters/Command.py",
         "chip/clusters/Objects.py",
         "chip/clusters/TestObjects.py",
+        "chip/clusters/Types.py",
         "chip/clusters/__init__.py",
         "chip/configuration/__init__.py",
         "chip/discovery/__init__.py",

src/controller/python/build-chip-wheel.py

@@ -118,6 +118,7 @@ def finalize_options(self):
         'construct',
         'ipython',
         'dacite',
+        'rich',
     ]
 
     if platform.system() == 'Darwin':

src/controller/python/chip/clusters/ClusterObjects.py

@@ -17,44 +17,92 @@
 
 from dataclasses import dataclass, asdict, field, make_dataclass
 from typing import ClassVar, List, Dict, Any, Mapping, Type, Union, ClassVar
+import enum
+import typing
 from chip import tlv, ChipUtility
+from chip.clusters.Types import Nullable, NullValue
 from dacite import from_dict
 
 
+def GetUnionUnderlyingType(typeToCheck, matchingType=None):
+    ''' This retrieves the underlying types behind a unioned type by appropriately
+        passing in the required matching type in the matchingType input argument.
+
+        If that is 'None' (not to be confused with NoneType), then it will retrieve
+        the 'real' type behind the union, i.e not Nullable && not None
+    '''
+    if (not(typing.get_origin(typeToCheck) == typing.Union)):
+        return None
+
+    for t in typing.get_args(typeToCheck):
+        if (matchingType is None):
+            if (t != type(None) and t != Nullable):
+                return t
+        else:
+            if (t == matchingType):
+                return t
+
+    return None
+
+
 @dataclass
 class ClusterObjectFieldDescriptor:
     Label: str = ''
     Tag: int = None
     Type: Type = None
-    IsArray: bool = False
 
-    def _PutSingleElementToTLV(self, tag, val, writer: tlv.TLVWriter, debugPath: str = '?'):
-        if issubclass(self.Type, ClusterObject):
+    def _PutSingleElementToTLV(self, tag, val, elementType, writer: tlv.TLVWriter, debugPath: str = '?'):
+        if issubclass(elementType, ClusterObject):
             if not isinstance(val, dict):
                 raise ValueError(
                     f"Field {debugPath}.{self.Label} expected a struct, but got {type(val)}")
-            self.Type.descriptor.DictToTLVWithWriter(
+            elementType.descriptor.DictToTLVWithWriter(
                 f'{debugPath}.{self.Label}', tag, val, writer)
             return
+
         try:
-            val = self.Type(val)
+            val = elementType(val)
         except Exception:
             raise ValueError(
-                f"Field {debugPath}.{self.Label} expected {self.Type}, but got {type(val)}")
+                f"Field {debugPath}.{self.Label} expected {elementType}, but got {type(val)}")
         writer.put(tag, val)
 
     def PutFieldToTLV(self, tag, val, writer: tlv.TLVWriter, debugPath: str = '?'):
-        if not self.IsArray:
-            self._PutSingleElementToTLV(tag, val, writer, debugPath)
-            return
-        if not isinstance(val, List):
-            raise ValueError(
-                f"Field {debugPath}.{self.Label} expected List[{self.Type}], but got {type(val)}")
-        writer.startArray(tag)
-        for i, v in enumerate(val):
-            self._PutSingleElementToTLV(
-                None, v, writer, debugPath + f'[{i}]')
-        writer.endContainer()
+        if (val == NullValue):
+            if (GetUnionUnderlyingType(self.Type, Nullable) is None):
+                raise ValueError(
+                    f"Field {debugPath}.{self.Label} was not nullable, but got a null")
+
+            writer.put(tag, None)
+        elif (val is None):
+            if (GetUnionUnderlyingType(self.Type, type(None)) is None):
+                raise ValueError(
+                    f"Field {debugPath}.{self.Label} was not optional, but encountered None")
+        else:
+            #
+            # If it is an optional or nullable type, it's going to be a union.
+            # So, let's get at the 'real' type within that union before proceeding,
+            # since at this point, we're guarenteed to not get None or Null as values.
+            #
+            elementType = GetUnionUnderlyingType(self.Type)
+            if (elementType is None):
+                elementType = self.Type
+
+            if not isinstance(val, List):
+                self._PutSingleElementToTLV(
+                    tag, val, elementType, writer, debugPath)
+                return
+
+            writer.startArray(tag)
+
+            # Get the type of the list. This is a generic, which has its sub-type information of the list element
+            # inside its type argument.
+            (elementType, ) = typing.get_args(elementType)
+
+            for i, v in enumerate(val):
+                self._PutSingleElementToTLV(
+                    None, v, elementType, writer, debugPath + f'[{i}]')
+            writer.endContainer()
 
 
 @dataclass
@@ -73,16 +121,19 @@ def GetFieldByLabel(self, label: str) -> ClusterObjectFieldDescriptor:
                 return field
         return None
 
-    def _ConvertNonArray(self, debugPath: str, descriptor: ClusterObjectFieldDescriptor, value: Any) -> Any:
-        if not issubclass(descriptor.Type, ClusterObject):
-            if not isinstance(value, descriptor.Type):
+    def _ConvertNonArray(self, debugPath: str, elementType, value: Any) -> Any:
+        if not issubclass(elementType, ClusterObject):
+            if (issubclass(elementType, enum.Enum)):
+                value = elementType(value)
+
+            if not isinstance(value, elementType):
                 raise ValueError(
-                    f"Failed to decode field {debugPath}, expected type {descriptor.Type}, got {type(value)}")
+                    f"Failed to decode field {debugPath}, expected type {elementType}, got {type(value)}")
             return value
         if not isinstance(value, Mapping):
             raise ValueError(
                 f"Failed to decode field {debugPath}, struct expected.")
-        return descriptor.Type.descriptor.TagDictToLabelDict(debugPath, value)
+        return elementType.descriptor.TagDictToLabelDict(debugPath, value)
 
     def TagDictToLabelDict(self, debugPath: str, tlvData: Dict[int, Any]) -> Dict[str, Any]:
         ret = {}
@@ -92,13 +143,30 @@ def TagDictToLabelDict(self, debugPath: str, tlvData: Dict[int, Any]) -> Dict[st
                 # We do not have enough infomation for this field.
                 ret[tag] = value
                 continue
-            if descriptor.IsArray:
+
+            if (value is None):
+                ret[descriptor.Label] = NullValue
+                continue
+
+            if (typing.get_origin(descriptor.Type) == typing.Union):
+                realType = GetUnionUnderlyingType(descriptor.Type)
+                if (realType is None):
+                    raise ValueError(
+                        f"Field {debugPath}.{self.Label} has no valid underlying data model type")
+
+                valueType = realType
+            else:
+                valueType = descriptor.Type
+
+            if (typing.get_origin(valueType) == list):
+                listElementType = typing.get_args(valueType)[0]
                 ret[descriptor.Label] = [
-                    self._ConvertNonArray(f'{debugPath}[{i}]', descriptor, v)
+                    self._ConvertNonArray(
+                        f'{debugPath}[{i}]', listElementType, v)
                     for i, v in enumerate(value)]
                 continue
             ret[descriptor.Label] = self._ConvertNonArray(
-                f'{debugPath}.{descriptor.Label}', descriptor, value)
+                f'{debugPath}.{descriptor.Label}', valueType, value)
         return ret
 
     def TLVToDict(self, tlvBuf: bytes) -> Dict[str, Any]:
@@ -109,9 +177,6 @@ def DictToTLVWithWriter(self, debugPath: str, tag, data: Mapping, writer: tlv.TL
         writer.startStructure(tag)
         for field in self.Fields:
             val = data.get(field.Label, None)
-            if val is None:
-                raise ValueError(
-                    f"Field {debugPath}.{field.Label} is missing in the given dict")
             field.PutFieldToTLV(field.Tag, val, writer,
                                 debugPath + f'.{field.Label}')
         writer.endContainer()
@@ -198,13 +263,13 @@ def attribute_type(cls) -> ClusterObjectFieldDescriptor:
     def _cluster_object(cls) -> ClusterObject:
         return make_dataclass('InternalClass',
                               [
-                                  ('Value', List[cls.attribute_type.Type]
-                                   if cls.attribute_type.IsArray else cls.attribute_type.Type, field(default=None)),
+                                  ('Value', cls.attribute_type.Type,
+                                   field(default=None)),
                                   ('descriptor', ClassVar[ClusterObjectDescriptor],
                                    field(
                                       default=ClusterObjectDescriptor(
                                           Fields=[ClusterObjectFieldDescriptor(
-                                              Label='Value', Tag=0, Type=cls.attribute_type.Type, IsArray=cls.attribute_type.IsArray)]
+                                              Label='Value', Tag=0, Type=cls.attribute_type.Type)]
                                       )
                                   )
                                   )