Strongly Typed Bitmaps

Switching some of the attributes in the test cluster of type BITMAP* to actually using the enum-like definition of the bitmap resulted in all kinds of compiler errors. This adds support for strongly typed BITMAP* fields in the XML that in turn result in the use of the BitFlags type in the generated C++ code. To correctly work this type for attributes, a specialization of NumericAttributeTraits has been added for BitFlags<T> to correctly handle the underlying storage type as well the special rule for nullability. Some of other fix-ups to the chip-tool helpers were needed as well. Tests: Ensured the YAML tests for bitmaps still works.
project-chip · Mar 9, 2022 · 31c4cd8 · 31c4cd8
1 parent cd44f9b
commit 31c4cd8
Show file tree

Hide file tree

Showing 27 changed files with 1,248 additions and 426 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
@@ -2816,6 +2816,34 @@ server cluster TestCluster = 1295 {
     kValueC = 3;
   }
 
+  bitmap Bitmap16MaskMap : BITMAP16 {
+    kMaskVal1 = 0x1;
+    kMaskVal2 = 0x2;
+    kMaskVal3 = 0x4;
+    kMaskVal4 = 0x4000;
+  }
+
+  bitmap Bitmap32MaskMap : BITMAP32 {
+    kMaskVal1 = 0x1;
+    kMaskVal2 = 0x2;
+    kMaskVal3 = 0x4;
+    kMaskVal4 = 0x40000000;
+  }
+
+  bitmap Bitmap64MaskMap : BITMAP64 {
+    kMaskVal1 = 0x1;
+    kMaskVal2 = 0x2;
+    kMaskVal3 = 0x4;
+    kMaskVal4 = 0x4000000000000000;
+  }
+
+  bitmap Bitmap8MaskMap : BITMAP8 {
+    kMaskVal1 = 0x1;
+    kMaskVal2 = 0x2;
+    kMaskVal3 = 0x4;
+    kMaskVal4 = 0x40;
+  }
+
   bitmap SimpleBitmap : BITMAP8 {
     kValueA = 0x1;
     kValueB = 0x2;
@@ -2894,10 +2922,10 @@ server cluster TestCluster = 1295 {
   }
 
   attribute boolean boolean = 0;
-  attribute bitmap8 bitmap8 = 1;
-  attribute bitmap16 bitmap16 = 2;
-  attribute bitmap32 bitmap32 = 3;
-  attribute bitmap64 bitmap64 = 4;
+  attribute Bitmap8MaskMap bitmap8 = 1;
+  attribute Bitmap16MaskMap bitmap16 = 2;
+  attribute Bitmap32MaskMap bitmap32 = 3;
+  attribute Bitmap64MaskMap bitmap64 = 4;
   attribute int8u int8u = 5;
   attribute int16u int16u = 6;
   attribute int24u int24u = 7;
@@ -2941,10 +2969,10 @@ server cluster TestCluster = 1295 {
   attribute boolean generalErrorBoolean = 49;
   attribute boolean clusterErrorBoolean = 50;
   attribute nullable boolean nullableBoolean = 32768;
-  attribute nullable bitmap8 nullableBitmap8 = 32769;
-  attribute nullable bitmap16 nullableBitmap16 = 32770;
-  attribute nullable bitmap32 nullableBitmap32 = 32771;
-  attribute nullable bitmap64 nullableBitmap64 = 32772;
+  attribute nullable Bitmap8MaskMap nullableBitmap8 = 32769;
+  attribute nullable Bitmap16MaskMap nullableBitmap16 = 32770;
+  attribute nullable Bitmap32MaskMap nullableBitmap32 = 32771;
+  attribute nullable Bitmap64MaskMap nullableBitmap64 = 32772;
   attribute nullable int8u nullableInt8u = 32773;
   attribute nullable int16u nullableInt16u = 32774;
   attribute nullable int24u nullableInt24u = 32775;

diff --git a/examples/chip-tool/templates/tests/partials/command_value.zapt b/examples/chip-tool/templates/tests/partials/command_value.zapt
@@ -48,7 +48,7 @@
       chip::ByteSpan(chip::Uint8::from_const_char("{{octetStringEscapedForCLiteral definedValue}}garbage: not in length on purpose"), {{definedValue.length}});
     {{else}}
       {{#if_is_bitmap type}}
-      static_cast<{{zapTypeToEncodableClusterObjectType type ns=ns}}>({{definedValue}});
+      static_cast<{{zapTypeToEncodableClusterObjectType type ns=ns}}>({{asTypedLiteral definedValue type}});
       {{else if (chip_tests_config_has definedValue)}}
       m{{asUpperCamelCase definedValue}}.HasValue() ? m{{asUpperCamelCase definedValue}}.Value() : {{asTypedLiteral (chip_tests_config_get_default_value definedValue) (chip_tests_config_get_type definedValue)}};
       {{else}}

diff --git a/examples/tv-casting-app/tv-casting-common/tv-casting-app.matter b/examples/tv-casting-app/tv-casting-common/tv-casting-app.matter
@@ -2416,6 +2416,34 @@ server cluster TestCluster = 1295 {
     kValueC = 3;
   }
 
+  bitmap Bitmap16MaskMap : BITMAP16 {
+    kMaskVal1 = 0x1;
+    kMaskVal2 = 0x2;
+    kMaskVal3 = 0x4;
+    kMaskVal4 = 0x4000;
+  }
+
+  bitmap Bitmap32MaskMap : BITMAP32 {
+    kMaskVal1 = 0x1;
+    kMaskVal2 = 0x2;
+    kMaskVal3 = 0x4;
+    kMaskVal4 = 0x40000000;
+  }
+
+  bitmap Bitmap64MaskMap : BITMAP64 {
+    kMaskVal1 = 0x1;
+    kMaskVal2 = 0x2;
+    kMaskVal3 = 0x4;
+    kMaskVal4 = 0x4000000000000000;
+  }
+
+  bitmap Bitmap8MaskMap : BITMAP8 {
+    kMaskVal1 = 0x1;
+    kMaskVal2 = 0x2;
+    kMaskVal3 = 0x4;
+    kMaskVal4 = 0x40;
+  }
+
   bitmap SimpleBitmap : BITMAP8 {
     kValueA = 0x1;
     kValueB = 0x2;
@@ -2441,10 +2469,10 @@ server cluster TestCluster = 1295 {
   }
 
   attribute boolean boolean = 0;
-  attribute bitmap8 bitmap8 = 1;
-  attribute bitmap16 bitmap16 = 2;
-  attribute bitmap32 bitmap32 = 3;
-  attribute bitmap64 bitmap64 = 4;
+  attribute Bitmap8MaskMap bitmap8 = 1;
+  attribute Bitmap16MaskMap bitmap16 = 2;
+  attribute Bitmap32MaskMap bitmap32 = 3;
+  attribute Bitmap64MaskMap bitmap64 = 4;
   attribute int8u int8u = 5;
   attribute int16u int16u = 6;
   attribute int32u int32u = 8;

diff --git a/src/app/tests/suites/include/ConstraintsChecker.h b/src/app/tests/suites/include/ConstraintsChecker.h
@@ -257,6 +257,18 @@ class ConstraintsChecker
         return true;
     }
 
+    template <typename T>
+    bool CheckConstraintNotValue(const char * itemName, chip::BitFlags<T> current, chip::BitFlags<T> expected)
+    {
+        if (current == expected)
+        {
+            Exit(std::string(itemName) + " got unexpected value: " + std::to_string(current.Raw()));
+            return false;
+        }
+
+        return true;
+    }
+
     template <typename T, typename U>
     bool CheckConstraintNotValue(const char * itemName, const chip::app::DataModel::Nullable<T> & current, U expected)
     {

diff --git a/src/app/tests/suites/include/ValueChecker.h b/src/app/tests/suites/include/ValueChecker.h
@@ -83,6 +83,12 @@ class ValueChecker
         return CheckValue(itemName, current.Raw(), expected);
     }
 
+    template <typename T>
+    bool CheckValue(const char * itemName, chip::BitFlags<T> current, chip::BitFlags<T> expected)
+    {
+        return CheckValue(itemName, current.Raw(), expected.Raw());
+    }
+
     template <typename T>
     bool CheckValuePresent(const char * itemName, const chip::Optional<T> & value)
     {

diff --git a/src/app/util/attribute-storage-null-handling.h b/src/app/util/attribute-storage-null-handling.h
@@ -21,6 +21,7 @@
 #include <lib/support/TypeTraits.h>
 
 #include <limits>
+#include <type_traits>
 
 namespace chip {
 namespace app {
@@ -125,6 +126,56 @@ struct NumericAttributeTraits
     static uint8_t * ToAttributeStoreRepresentation(StorageType & value) { return reinterpret_cast<uint8_t *>(&value); }
 };
 
+template <typename T>
+struct NumericAttributeTraits<BitFlags<T>>
+{
+    using StorageType = T;
+    using WorkingType = BitFlags<T>;
+
+    static constexpr void WorkingToStorage(WorkingType workingValue, StorageType & storageValue)
+    {
+        storageValue = static_cast<StorageType>(workingValue.Raw());
+    }
+
+    static constexpr WorkingType StorageToWorking(StorageType storageValue) { return WorkingType(storageValue); }
+
+    static constexpr void SetNull(StorageType & value)
+    {
+        //
+        // When setting to null, store a value that has all bits set. This aliases to the same behavior
+        // we do for other integral types, ensuring consistency across all underlying integral types in the data store as well as
+        // re-using logic for serialization/de-serialization of that data in the IM.
+        //
+        value = static_cast<StorageType>(std::numeric_limits<std::underlying_type_t<T>>::max());
+    }
+
+    static constexpr bool IsNullValue(StorageType value)
+    {
+        //
+        // While we store a nullable bitmap value by setting all its bits, we can be a bit more conservative when actually
+        // testing for null since the spec only mandates that the MSB be reserved for nullable bitmaps.
+        //
+        auto msbSetValue = std::underlying_type_t<T>(1) << (std::numeric_limits<std::underlying_type_t<T>>::digits - 1);
+        return !!(std::underlying_type_t<T>(value) & msbSetValue);
+    }
+
+    static constexpr bool CanRepresentValue(bool isNullable, StorageType value)
+    {
+        //
+        // We permit the full-range of the underlying StorageType if !isNullable,
+        // and the restricted range otherwise.
+        //
+        return !isNullable || !IsNullValue(value);
+    }
+
+    static CHIP_ERROR Encode(TLV::TLVWriter & writer, TLV::Tag tag, StorageType value)
+    {
+        return writer.Put(tag, static_cast<bool>(value));
+    }
+
+    static uint8_t * ToAttributeStoreRepresentation(StorageType & value) { return reinterpret_cast<uint8_t *>(&value); }
+};
+
 template <>
 struct NumericAttributeTraits<bool>
 {

diff --git a/src/app/zap-templates/zcl/data-model/chip/test-cluster.xml b/src/app/zap-templates/zcl/data-model/chip/test-cluster.xml
@@ -105,6 +105,38 @@ limitations under the License.
           array="true" optional="true"/>
   </struct>
 
+    <bitmap name="Bitmap8MaskMap" type="BITMAP8">
+        <cluster code="0x050F" />
+        <field mask="0x01" name="MaskVal1" />
+        <field mask="0x02" name="MaskVal2" />
+        <field mask="0x04" name="MaskVal3" />
+        <field mask="0x40" name="MaskVal4" />
+    </bitmap>
+
+    <bitmap name="Bitmap16MaskMap" type="BITMAP16">
+        <cluster code="0x050F" />
+        <field mask="0x01" name="MaskVal1" />
+        <field mask="0x02" name="MaskVal2" />
+        <field mask="0x04" name="MaskVal3" />
+        <field mask="0x4000" name="MaskVal4" />
+    </bitmap>
+
+    <bitmap name="Bitmap32MaskMap" type="BITMAP32">
+        <cluster code="0x050F" />
+        <field mask="0x01" name="MaskVal1" />
+        <field mask="0x02" name="MaskVal2" />
+        <field mask="0x04" name="MaskVal3" />
+        <field mask="0x40000000" name="MaskVal4" />
+    </bitmap>
+
+    <bitmap name="Bitmap64MaskMap" type="BITMAP64">
+        <cluster code="0x050F" />
+        <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>Test Cluster</name>
@@ -113,10 +145,10 @@ limitations under the License.
     <description>The Test Cluster is meant to validate the generated code</description>
     <!-- Base data types -->
     <attribute side="server" code="0x0000" define="BOOLEAN" type="BOOLEAN" writable="true" default="false" optional="false">boolean</attribute>
-    <attribute side="server" code="0x0001" define="BITMAP8" type="BITMAP8" writable="true" default="0" optional="false">bitmap8</attribute>
-    <attribute side="server" code="0x0002" define="BITMAP16" type="BITMAP16" writable="true" default="0" optional="false">bitmap16</attribute>
-    <attribute side="server" code="0x0003" define="BITMAP32" type="BITMAP32" writable="true" default="0" optional="false">bitmap32</attribute>
-    <attribute side="server" code="0x0004" define="BITMAP64" type="BITMAP64" writable="true" default="0" optional="false">bitmap64</attribute>
+    <attribute side="server" code="0x0001" define="BITMAP8" type="Bitmap8MaskMap" writable="true" default="0" optional="false">bitmap8</attribute>
+    <attribute side="server" code="0x0002" define="BITMAP16" type="Bitmap16MaskMap" writable="true" default="0" optional="false">bitmap16</attribute>
+    <attribute side="server" code="0x0003" define="BITMAP32" type="Bitmap32MaskMap" writable="true" default="0" optional="false">bitmap32</attribute>
+    <attribute side="server" code="0x0004" define="BITMAP64" type="Bitmap64MaskMap" writable="true" default="0" optional="false">bitmap64</attribute>
     <attribute side="server" code="0x0005" define="INT8U" type="INT8U" writable="true" default="0" optional="false">int8u</attribute>
     <attribute side="server" code="0x0006" define="INT16U" type="INT16U" writable="true" default="0" optional="false">int16u</attribute>
     <attribute side="server" code="0x0007" define="INT24U" type="INT24U" writable="true" default="0" optional="false">int24u</attribute>
@@ -168,10 +200,10 @@ limitations under the License.
                type="BOOLEAN" writable="true" optional="false">cluster_error_boolean</attribute>
 
     <attribute side="server" code="0x8000" define="NULLABLE_BOOLEAN" type="BOOLEAN" writable="true" default="false" isNullable="true" optional="false">nullable_boolean</attribute>
-    <attribute side="server" code="0x8001" define="NULLABLE_BITMAP8" type="BITMAP8" writable="true" default="0" isNullable="true" optional="false">nullable_bitmap8</attribute>
-    <attribute side="server" code="0x8002" define="NULLABLE_BITMAP16" type="BITMAP16" writable="true" default="0" isNullable="true" optional="false">nullable_bitmap16</attribute>
-    <attribute side="server" code="0x8003" define="NULLABLE_BITMAP32" type="BITMAP32" writable="true" default="0" isNullable="true" optional="false">nullable_bitmap32</attribute>
-    <attribute side="server" code="0x8004" define="NULLABLE_BITMAP64" type="BITMAP64" writable="true" default="0" isNullable="true" optional="false">nullable_bitmap64</attribute>
+    <attribute side="server" code="0x8001" define="NULLABLE_BITMAP8" type="Bitmap8MaskMap" writable="true" default="0" isNullable="true" optional="false">nullable_bitmap8</attribute>
+    <attribute side="server" code="0x8002" define="NULLABLE_BITMAP16" type="Bitmap16MaskMap" writable="true" default="0" isNullable="true" optional="false">nullable_bitmap16</attribute>
+    <attribute side="server" code="0x8003" define="NULLABLE_BITMAP32" type="Bitmap32MaskMap" writable="true" default="0" isNullable="true" optional="false">nullable_bitmap32</attribute>
+    <attribute side="server" code="0x8004" define="NULLABLE_BITMAP64" type="Bitmap64MaskMap" writable="true" default="0" isNullable="true" optional="false">nullable_bitmap64</attribute>
     <attribute side="server" code="0x8005" define="NULLABLE_INT8U" type="INT8U" writable="true" default="0" isNullable="true" optional="false">nullable_int8u</attribute>
     <attribute side="server" code="0x8006" define="NULLABLE_INT16U" type="INT16U" writable="true" default="0" isNullable="true" optional="false">nullable_int16u</attribute>
     <attribute side="server" code="0x8007" define="NULLABLE_INT24U" type="INT24U" writable="true" default="0" isNullable="true" optional="false">nullable_int24u</attribute>

diff --git a/src/controller/data_model/controller-clusters.matter b/src/controller/data_model/controller-clusters.matter
@@ -3279,6 +3279,34 @@ client cluster TestCluster = 1295 {
     kValueC = 3;
   }
 
+  bitmap Bitmap16MaskMap : BITMAP16 {
+    kMaskVal1 = 0x1;
+    kMaskVal2 = 0x2;
+    kMaskVal3 = 0x4;
+    kMaskVal4 = 0x4000;
+  }
+
+  bitmap Bitmap32MaskMap : BITMAP32 {
+    kMaskVal1 = 0x1;
+    kMaskVal2 = 0x2;
+    kMaskVal3 = 0x4;
+    kMaskVal4 = 0x40000000;
+  }
+
+  bitmap Bitmap64MaskMap : BITMAP64 {
+    kMaskVal1 = 0x1;
+    kMaskVal2 = 0x2;
+    kMaskVal3 = 0x4;
+    kMaskVal4 = 0x4000000000000000;
+  }
+
+  bitmap Bitmap8MaskMap : BITMAP8 {
+    kMaskVal1 = 0x1;
+    kMaskVal2 = 0x2;
+    kMaskVal3 = 0x4;
+    kMaskVal4 = 0x40;
+  }
+
   bitmap SimpleBitmap : BITMAP8 {
     kValueA = 0x1;
     kValueB = 0x2;
@@ -3357,10 +3385,10 @@ client cluster TestCluster = 1295 {
   }
 
   attribute boolean boolean = 0;
-  attribute bitmap8 bitmap8 = 1;
-  attribute bitmap16 bitmap16 = 2;
-  attribute bitmap32 bitmap32 = 3;
-  attribute bitmap64 bitmap64 = 4;
+  attribute Bitmap8MaskMap bitmap8 = 1;
+  attribute Bitmap16MaskMap bitmap16 = 2;
+  attribute Bitmap32MaskMap bitmap32 = 3;
+  attribute Bitmap64MaskMap bitmap64 = 4;
   attribute int8u int8u = 5;
   attribute int16u int16u = 6;
   attribute int24u int24u = 7;
@@ -3405,10 +3433,10 @@ client cluster TestCluster = 1295 {
   attribute boolean clusterErrorBoolean = 50;
   attribute boolean unsupported = 255;
   attribute nullable boolean nullableBoolean = 32768;
-  attribute nullable bitmap8 nullableBitmap8 = 32769;
-  attribute nullable bitmap16 nullableBitmap16 = 32770;
-  attribute nullable bitmap32 nullableBitmap32 = 32771;
-  attribute nullable bitmap64 nullableBitmap64 = 32772;
+  attribute nullable Bitmap8MaskMap nullableBitmap8 = 32769;
+  attribute nullable Bitmap16MaskMap nullableBitmap16 = 32770;
+  attribute nullable Bitmap32MaskMap nullableBitmap32 = 32771;
+  attribute nullable Bitmap64MaskMap nullableBitmap64 = 32772;
   attribute nullable int8u nullableInt8u = 32773;
   attribute nullable int16u nullableInt16u = 32774;
   attribute nullable int24u nullableInt24u = 32775;