Use BitMask instead of BitFlags for data model encoding of bitmaps (

#19008)

* Add a BitMask class that adds more functionality beyond bit flags

* Use bit-mask class in zap

* Zap regen

* Update to make things compile

* Add BitMask constructors and more support in generic test validation types

* More updates for compilation - chip tool seems to compile now

* Restyle

* Fix all clusters compile

* Fix compilation for direct value assignment to a mask

* Add default copy constructor for BitMask

* Undo pigweed update

* Convert one more flag to mask for compile on TI

* Review comments: better documentation, add unit tests

* Simplify NumericAttributeTraits for BitMask

* Remove unused asserts

* Slight comment update

* Slight comment update

* Zap regen

examples/chip-tool/commands/clusters/ComplexArgument.h

@@ -102,6 +102,16 @@ class ComplexArgumentParser
         return CHIP_NO_ERROR;
     }
 
+    template <typename T>
+    static CHIP_ERROR Setup(const char * label, chip::BitMask<T> & request, Json::Value & value)
+    {
+        T requestValue;
+        ReturnErrorOnFailure(ComplexArgumentParser::Setup(label, requestValue, value));
+
+        request = chip::BitMask<T>(requestValue);
+        return CHIP_NO_ERROR;
+    }
+
     template <typename T>
     static CHIP_ERROR Setup(const char * label, chip::Optional<T> & request, Json::Value & value)
     {

examples/chip-tool/commands/common/Command.h

@@ -199,6 +199,15 @@ class Command
         return AddArgument(name, min, max, reinterpret_cast<T *>(out), desc, flags);
     }
 
+    template <typename T>
+    size_t AddArgument(const char * name, int64_t min, uint64_t max, chip::BitMask<T> * out, const char * desc = "",
+                       uint8_t flags = 0)
+    {
+        // This is a terrible hack that relies on BitMask only having the one
+        // mValue member.
+        return AddArgument(name, min, max, reinterpret_cast<T *>(out), desc, flags);
+    }
+
     template <typename T>
     size_t AddArgument(const char * name, chip::Optional<T> * value, const char * desc = "")
     {

examples/pump-app/cc13x2x7_26x2x7/main/AppTask.cpp

@@ -467,7 +467,7 @@ void AppTask::UpdateClusterState(void)
 void AppTask::UpdateCluster(intptr_t context)
 {
     EmberStatus status;
-    BitFlags<PumpConfigurationAndControl::PumpStatus> pumpStatus;
+    BitMask<PumpConfigurationAndControl::PumpStatus> pumpStatus;
 
     ChipLogProgress(NotSpecified, "Update Cluster State");
 

examples/window-app/common/src/WindowApp.cpp

@@ -294,8 +294,8 @@ void WindowApp::DispatchEvent(const WindowApp::Event & event)
 void WindowApp::DispatchEventAttributeChange(chip::EndpointId endpoint, chip::AttributeId attribute)
 {
     Cover * cover = GetCover(endpoint);
-    chip::BitFlags<Mode> mode;
-    chip::BitFlags<ConfigStatus> configStatus;
+    chip::BitMask<Mode> mode;
+    chip::BitMask<ConfigStatus> configStatus;
 
     if (nullptr == cover)
     {
@@ -426,7 +426,7 @@ void WindowApp::Cover::Init(chip::EndpointId endpoint)
     TypeSet(endpoint, Type::kTiltBlindLiftAndTilt);
 
     // Attribute: Id  7 ConfigStatus
-    chip::BitFlags<ConfigStatus> configStatus = ConfigStatusGet(endpoint);
+    chip::BitMask<ConfigStatus> configStatus = ConfigStatusGet(endpoint);
     configStatus.Set(ConfigStatus::kLiftEncoderControlled);
     configStatus.Set(ConfigStatus::kTiltEncoderControlled);
     configStatus.Set(ConfigStatus::kOnlineReserved);
@@ -438,7 +438,7 @@ void WindowApp::Cover::Init(chip::EndpointId endpoint)
     EndProductTypeSet(endpoint, EndProductType::kInteriorBlind);
 
     // Attribute: Id 24 Mode
-    chip::BitFlags<Mode> mode;
+    chip::BitMask<Mode> mode;
     mode.Clear(Mode::kMotorDirectionReversed);
     mode.Clear(Mode::kMaintenanceMode);
     mode.Clear(Mode::kCalibrationMode);

src/app/clusters/door-lock-server/door-lock-server.cpp

@@ -2395,7 +2395,7 @@ void DoorLockServer::sendGetWeekDayScheduleResponse(chip::app::CommandHandler *
     response.status       = status;
     if (DlStatus::kSuccess == status)
     {
-        response.daysMask    = Optional<chip::BitFlags<DlDaysMaskMap>>(daysMask);
+        response.daysMask    = Optional<chip::BitMask<DlDaysMaskMap>>(daysMask);
         response.startHour   = Optional<uint8_t>(startHour);
         response.startMinute = Optional<uint8_t>(startMinute);
         response.endHour     = Optional<uint8_t>(endHour);

src/app/clusters/pump-configuration-and-control-server/pump-configuration-and-control-server.cpp

@@ -22,7 +22,6 @@
 #include <app-common/zap-generated/cluster-enums.h>
 #include <app-common/zap-generated/ids/Attributes.h>
 #include <app/AttributeAccessInterface.h>
-//#include <app/CommandHandler.h>
 #include <app/ConcreteAttributePath.h>
 #include <app/ConcreteCommandPath.h>
 #include <app/InteractionModelEngine.h>
@@ -73,7 +72,7 @@ static void updateAttributeLinks(EndpointId endpoint)
 {
     PumpControlMode controlMode;
     PumpOperationMode operationMode;
-    BitFlags<PumpStatus> pumpStatus;
+    BitMask<PumpStatus> pumpStatus;
     bool isControlModeAvailable = true;
     bool isPumpStatusAvailable  = true;
 

src/app/clusters/window-covering-server/window-covering-server.cpp

@@ -165,7 +165,7 @@ Type TypeGet(chip::EndpointId endpoint)
     return value;
 }
 
-void ConfigStatusPrint(const chip::BitFlags<ConfigStatus> & configStatus)
+void ConfigStatusPrint(const chip::BitMask<ConfigStatus> & configStatus)
 {
     emberAfWindowCoveringClusterPrint("ConfigStatus 0x%02X Operational=%u OnlineReserved=%u", configStatus.Raw(),
                                       configStatus.Has(ConfigStatus::kOperational),
@@ -177,22 +177,22 @@ void ConfigStatusPrint(const chip::BitFlags<ConfigStatus> & configStatus)
         configStatus.Has(ConfigStatus::kTiltPositionAware), configStatus.Has(ConfigStatus::kTiltEncoderControlled));
 }
 
-void ConfigStatusSet(chip::EndpointId endpoint, const chip::BitFlags<ConfigStatus> & configStatus)
+void ConfigStatusSet(chip::EndpointId endpoint, const chip::BitMask<ConfigStatus> & configStatus)
 {
     Attributes::ConfigStatus::Set(endpoint, configStatus);
 }
 
-chip::BitFlags<ConfigStatus> ConfigStatusGet(chip::EndpointId endpoint)
+chip::BitMask<ConfigStatus> ConfigStatusGet(chip::EndpointId endpoint)
 {
-    chip::BitFlags<ConfigStatus> configStatus;
+    chip::BitMask<ConfigStatus> configStatus;
     Attributes::ConfigStatus::Get(endpoint, &configStatus);
 
     return configStatus;
 }
 
 void ConfigStatusUpdateFeatures(chip::EndpointId endpoint)
 {
-    chip::BitFlags<ConfigStatus> configStatus = ConfigStatusGet(endpoint);
+    chip::BitMask<ConfigStatus> configStatus = ConfigStatusGet(endpoint);
 
     configStatus.Set(ConfigStatus::kLiftPositionAware, HasFeaturePaLift(endpoint));
     configStatus.Set(ConfigStatus::kTiltPositionAware, HasFeaturePaTilt(endpoint));
@@ -255,19 +255,19 @@ EndProductType EndProductTypeGet(chip::EndpointId endpoint)
     return value;
 }
 
-void ModePrint(const chip::BitFlags<Mode> & mode)
+void ModePrint(const chip::BitMask<Mode> & mode)
 {
     emberAfWindowCoveringClusterPrint("Mode 0x%02X MotorDirReversed=%u LedFeedback=%u Maintenance=%u Calibration=%u", mode.Raw(),
                                       mode.Has(Mode::kMotorDirectionReversed), mode.Has(Mode::kLedFeedback),
                                       mode.Has(Mode::kMaintenanceMode), mode.Has(Mode::kCalibrationMode));
 }
 
-void ModeSet(chip::EndpointId endpoint, chip::BitFlags<Mode> & newMode)
+void ModeSet(chip::EndpointId endpoint, chip::BitMask<Mode> & newMode)
 {
-    chip::BitFlags<ConfigStatus> newStatus;
+    chip::BitMask<ConfigStatus> newStatus;
 
-    chip::BitFlags<ConfigStatus> oldStatus = ConfigStatusGet(endpoint);
-    chip::BitFlags<Mode> oldMode           = ModeGet(endpoint);
+    chip::BitMask<ConfigStatus> oldStatus = ConfigStatusGet(endpoint);
+    chip::BitMask<Mode> oldMode           = ModeGet(endpoint);
 
     newStatus = oldStatus;
 
@@ -288,9 +288,9 @@ void ModeSet(chip::EndpointId endpoint, chip::BitFlags<Mode> & newMode)
         ConfigStatusSet(endpoint, newStatus);
 }
 
-chip::BitFlags<Mode> ModeGet(chip::EndpointId endpoint)
+chip::BitMask<Mode> ModeGet(chip::EndpointId endpoint)
 {
-    chip::BitFlags<Mode> mode;
+    chip::BitMask<Mode> mode;
 
     Attributes::Mode::Get(endpoint, &mode);
     return mode;
@@ -591,8 +591,8 @@ void PostAttributeChange(chip::EndpointId endpoint, chip::AttributeId attributeI
 {
     // all-cluster-app: simulation for the CI testing
     // otherwise it is defined for manufacturer specific implementation */
-    BitFlags<Mode> mode;
-    BitFlags<ConfigStatus> configStatus;
+    BitMask<Mode> mode;
+    BitMask<ConfigStatus> configStatus;
     NPercent100ths current, target;
     OperationalStatus prevOpStatus = OperationalStatusGet(endpoint);
     OperationalStatus opStatus     = prevOpStatus;
@@ -659,8 +659,8 @@ void PostAttributeChange(chip::EndpointId endpoint, chip::AttributeId attributeI
 
 EmberAfStatus GetMotionLockStatus(chip::EndpointId endpoint)
 {
-    BitFlags<Mode> mode                 = ModeGet(endpoint);
-    BitFlags<ConfigStatus> configStatus = ConfigStatusGet(endpoint);
+    BitMask<Mode> mode                 = ModeGet(endpoint);
+    BitMask<ConfigStatus> configStatus = ConfigStatusGet(endpoint);
 
     // Is the device locked?
     if (!configStatus.Has(ConfigStatus::kOperational))

src/app/clusters/window-covering-server/window-covering-server.h

@@ -99,9 +99,9 @@ bool HasFeaturePaTilt(chip::EndpointId endpoint);
 void TypeSet(chip::EndpointId endpoint, Type type);
 Type TypeGet(chip::EndpointId endpoint);
 
-void ConfigStatusPrint(const chip::BitFlags<ConfigStatus> & configStatus);
-void ConfigStatusSet(chip::EndpointId endpoint, const chip::BitFlags<ConfigStatus> & status);
-chip::BitFlags<ConfigStatus> ConfigStatusGet(chip::EndpointId endpoint);
+void ConfigStatusPrint(const chip::BitMask<ConfigStatus> & configStatus);
+void ConfigStatusSet(chip::EndpointId endpoint, const chip::BitMask<ConfigStatus> & status);
+chip::BitMask<ConfigStatus> ConfigStatusGet(chip::EndpointId endpoint);
 void ConfigStatusUpdateFeatures(chip::EndpointId endpoint);
 
 void OperationalStatusSet(chip::EndpointId endpoint, const OperationalStatus & status);
@@ -115,9 +115,9 @@ Percent100ths ComputePercent100thsStep(OperationalState direction, Percent100ths
 void EndProductTypeSet(chip::EndpointId endpoint, EndProductType type);
 EndProductType EndProductTypeGet(chip::EndpointId endpoint);
 
-void ModePrint(const chip::BitFlags<Mode> & mode);
-void ModeSet(chip::EndpointId endpoint, chip::BitFlags<Mode> & mode);
-chip::BitFlags<Mode> ModeGet(chip::EndpointId endpoint);
+void ModePrint(const chip::BitMask<Mode> & mode);
+void ModeSet(chip::EndpointId endpoint, chip::BitMask<Mode> & mode);
+chip::BitMask<Mode> ModeGet(chip::EndpointId endpoint);
 
 void SafetyStatusSet(chip::EndpointId endpoint, SafetyStatus & status);
 const SafetyStatus SafetyStatusGet(chip::EndpointId endpoint);

src/app/data-model/BasicTypes.h

@@ -19,6 +19,7 @@
 #pragma once
 
 #include <lib/support/BitFlags.h>
+#include <lib/support/BitMask.h>
 #include <lib/support/Span.h>
 
 #include <type_traits>
@@ -43,6 +44,12 @@ struct IsBasicType<BitFlags<X>>
     static constexpr bool value = true;
 };
 
+template <typename X>
+struct IsBasicType<BitMask<X>>
+{
+    static constexpr bool value = true;
+};
+
 template <>
 struct IsBasicType<ByteSpan>
 {

src/app/tests/suites/include/ConstraintsChecker.h

@@ -219,6 +219,18 @@ class ConstraintsChecker
         return true;
     }
 
+    template <typename T, typename U>
+    bool CheckConstraintMinValue(const char * itemName, chip::BitMask<T> current, U expected)
+    {
+        if (current.Raw() < expected)
+        {
+            Exit(std::string(itemName) + " value < minValue: " + std::to_string(current.Raw()) + " < " + std::to_string(expected));
+            return false;
+        }
+
+        return true;
+    }
+
     template <typename T, typename U>
     bool CheckConstraintMinValue(const char * itemName, const chip::app::DataModel::Nullable<T> & current, U expected)
     {
@@ -270,6 +282,18 @@ class ConstraintsChecker
         return true;
     }
 
+    template <typename T, typename U>
+    bool CheckConstraintMaxValue(const char * itemName, chip::BitMask<T> current, U expected)
+    {
+        if (current.Raw() > expected)
+        {
+            Exit(std::string(itemName) + " value > maxValue: " + std::to_string(current.Raw()) + " > " + std::to_string(expected));
+            return false;
+        }
+
+        return true;
+    }
+
     template <typename T, typename U>
     bool CheckConstraintMaxValue(const char * itemName, const chip::app::DataModel::Nullable<T> & current, U expected)
     {
@@ -345,6 +369,18 @@ class ConstraintsChecker
         return true;
     }
 
+    template <typename T>
+    bool CheckConstraintNotValue(const char * itemName, chip::BitMask<T> current, chip::BitMask<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, chip::BitFlags<T> current, U expected)
     {
@@ -358,6 +394,19 @@ class ConstraintsChecker
         return true;
     }
 
+    template <typename T, typename U>
+    bool CheckConstraintNotValue(const char * itemName, chip::BitMask<T> current, U expected)
+    {
+        if (current.Raw() == 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)
     {

src/app/tests/suites/include/ValueChecker.h

@@ -94,6 +94,12 @@ class ValueChecker
         return CheckValue(itemName, current.Raw(), expected);
     }
 
+    template <typename T, typename U>
+    bool CheckValue(const char * itemName, chip::BitMask<T> current, U expected)
+    {
+        return CheckValue(itemName, current.Raw(), expected);
+    }
+
     // Allow an expected value that is a nullable wrapped around the actual
     // value (e.g. a SaveAs from reading a different attribute that has a value
     // space that includes null).  In that case we check that:
@@ -114,6 +120,12 @@ class ValueChecker
         return CheckValue(itemName, current.Raw(), expected.Raw());
     }
 
+    template <typename T>
+    bool CheckValue(const char * itemName, chip::BitMask<T> current, chip::BitMask<T> expected)
+    {
+        return CheckValue(itemName, current.Raw(), expected.Raw());
+    }
+
     template <typename T>
     bool CheckValuePresent(const char * itemName, const chip::Optional<T> & value)
     {

src/app/util/attribute-storage-null-handling.h

@@ -18,6 +18,8 @@
 #pragma once
 
 #include <lib/core/CHIPTLV.h>
+#include <lib/support/BitFlags.h>
+#include <lib/support/BitMask.h>
 #include <lib/support/TypeTraits.h>
 
 #include <limits>
@@ -171,6 +173,15 @@ struct NumericAttributeTraits<BitFlags<T>>
     static uint8_t * ToAttributeStoreRepresentation(StorageType & value) { return reinterpret_cast<uint8_t *>(&value); }
 };
 
+template <typename T>
+struct NumericAttributeTraits<BitMask<T>> : public NumericAttributeTraits<BitFlags<T>>
+{
+    using StorageType = T;
+    using WorkingType = BitMask<T>;
+
+    static constexpr WorkingType StorageToWorking(StorageType storageValue) { return WorkingType(storageValue); }
+};
+
 template <>
 struct NumericAttributeTraits<bool>
 {

src/app/zap-templates/templates/app/cluster-objects.zapt

@@ -11,7 +11,7 @@
 #include <app/EventLoggingTypes.h>
 #include <app/util/basic-types.h>
 #include <lib/core/ClusterEnums.h>
-#include <lib/support/BitFlags.h>
+#include <lib/support/BitMask.h>
 #include <protocols/interaction_model/Constants.h>
 #include <app-common/zap-generated/enums.h>
 #include <app-common/zap-generated/ids/Attributes.h>

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

@@ -491,7 +491,7 @@ async function zapTypeToClusterObjectType(type, isDecodable, options)
       if (s) {
         return 'uint' + s[1] + '_t';
       }
-      return 'chip::BitFlags<' + ns + type + '>';
+      return 'chip::BitMask<' + ns + type + '>';
     }
 
     if (types.isStruct) {