Add add/remove/set flag helpers for MessageHeader (project-chip#3054)

* Starting to add set/clear flag methods to the message header * Use the new methods in the header * Create a typesafe bitflags class and use it in MessageHeader. Mark other non-typesafe flags as deprecated * Add sizeof check and rename Get to Has * Add a "HasOnly" method to bitflags * Fix usage of static assert - message is needed
mspang · Oct 8, 2020 · f94a92d · f94a92d
1 parent 2b85b54
commit f94a92d
Show file tree

Hide file tree

Showing 23 changed files with 220 additions and 179 deletions.
diff --git a/.gitignore b/.gitignore
@@ -41,3 +41,6 @@ __pycache__
 
 # Doxygen outputs
 docs/html
+
+# VSCode java extensions
+.project
diff --git a/src/ble/BLEEndPoint.cpp b/src/ble/BLEEndPoint.cpp
@@ -36,9 +36,9 @@
 #if CONFIG_NETWORK_LAYER_BLE
 #include <core/CHIPConfig.h>
 
+#include <support/BitFlags.h>
 #include <support/CHIPFaultInjection.h>
 #include <support/CodeUtils.h>
-#include <support/FlagUtils.hpp>
 #include <support/logging/CHIPLogging.h>
 
 #include <ble/BLEEndPoint.h>

diff --git a/src/ble/BtpEngine.h b/src/ble/BtpEngine.h
@@ -38,7 +38,7 @@
 #include <ble/BleConfig.h>
 
 #include <ble/BleError.h>
-#include <support/FlagUtils.hpp>
+#include <support/BitFlags.h>
 #include <system/SystemPacketBuffer.h>
 
 namespace chip {

diff --git a/src/lib/message/CHIPFabricState.h b/src/lib/message/CHIPFabricState.h
@@ -39,9 +39,9 @@
 
 #include <core/CHIPKeyIds.h>
 #include <protocols/security/CHIPApplicationKeys.h>
+#include <support/BitFlags.h>
 #include <support/CHIPCounter.h>
 #include <support/DLLUtil.h>
-#include <support/FlagUtils.hpp>
 #include <support/PersistedCounter.h>
 
 namespace chip {

diff --git a/src/lib/message/ExchangeContext.cpp b/src/lib/message/ExchangeContext.cpp
@@ -39,9 +39,9 @@
 #include <message/CHIPSecurityMgr.h>
 #include <protocols/CHIPProtocols.h>
 #include <protocols/common/CommonProtocol.h>
+#include <support/BitFlags.h>
 #include <support/CHIPFaultInjection.h>
 #include <support/CodeUtils.h>
-#include <support/FlagUtils.hpp>
 #include <support/RandUtils.h>
 #include <support/logging/CHIPLogging.h>
 #include <system/SystemStats.h>

diff --git a/src/lib/support/BUILD.gn b/src/lib/support/BUILD.gn
@@ -43,6 +43,7 @@ source_set("chip_version_header") {
 
 support_headers = [
   "Base64.h",
+  "BitFlags.h",
   "BufBound.h",
   "CHIPCounter.h",
   "CodeUtils.h",

diff --git a/src/lib/support/BitFlags.h b/src/lib/support/BitFlags.h
@@ -0,0 +1,145 @@
+/*
+ *
+ *    Copyright (c) 2020 Project CHIP Authors
+ *    Copyright (c) 2013-2017 Nest Labs, Inc.
+ *
+ *    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.
+ */
+
+/**
+ *    @file
+ *      This file defines functions for manipulating Boolean flags in
+ *      a bitfield.
+ *
+ */
+
+#ifndef CHIP_CHIP_SUPPORT_FLAGUTILS_HPP
+#define CHIP_CHIP_SUPPORT_FLAGUTILS_HPP
+
+#include <stdint.h>
+
+#include <utility>
+
+namespace chip {
+
+/**
+ * Stores bit flags in a type safe manner.
+ *
+ * @tparam StorageType is the underlying storage type (like uint16_t, uint32_t etc.)
+ * @tparam FlagsEnum is the typesafe flags setting
+ */
+template <typename StorageType, typename FlagsEnum>
+class BitFlags
+{
+public:
+    static_assert(sizeof(StorageType) >= sizeof(FlagsEnum), "All flags should fit in the storage type");
+
+    BitFlags() {}
+    BitFlags(const BitFlags &) = default;
+    BitFlags & operator=(const BitFlags &) = default;
+
+    BitFlags & Set(FlagsEnum v)
+    {
+        mValue = static_cast<StorageType>(mValue | static_cast<StorageType>(v));
+        return *this;
+    }
+
+    BitFlags & Clear(FlagsEnum v)
+    {
+        mValue &= static_cast<StorageType>(~static_cast<StorageType>(v));
+        return *this;
+    }
+
+    BitFlags & Set(FlagsEnum v, bool isSet) { return isSet ? Set(v) : Clear(v); }
+
+    bool Has(FlagsEnum v) const { return (mValue & static_cast<StorageType>(v)) != 0; }
+
+    BitFlags & Set(const BitFlags & other)
+    {
+        mValue |= other.mValue;
+        return *this;
+    }
+
+    StorageType Raw() const { return mValue; }
+    BitFlags & SetRaw(StorageType value)
+    {
+        mValue = value;
+        return *this;
+    }
+
+    /** Check that no flags outside the arguments are set.*/
+    template <typename... Args>
+    bool HasOnly(Args &&... args) const
+    {
+        return IsZeroAfterClearing(mValue, std::forward<Args>(args)...);
+    }
+
+private:
+    StorageType mValue = 0;
+
+    template <typename... Args>
+    static bool IsZeroAfterClearing(StorageType value, FlagsEnum flagToClear, Args &&... args)
+    {
+        value &= static_cast<StorageType>(~static_cast<StorageType>(flagToClear));
+        return IsZeroAfterClearing(value, std::forward<Args>(args)...);
+    }
+
+    static bool IsZeroAfterClearing(StorageType value) { return value == 0; }
+};
+
+/**
+ * @deprecated Use typesafe BitFlags class instead.
+ */
+template <typename FlagsT, typename FlagT>
+inline bool GetFlag(const FlagsT & inFlags, const FlagT inFlag)
+{
+    return (inFlags & static_cast<FlagsT>(inFlag)) != 0;
+}
+
+/**
+ * @deprecated Use typesafe BitFlags class instead.
+ */
+template <typename FlagsT, typename FlagT>
+inline void ClearFlag(FlagsT & inFlags, const FlagT inFlag)
+{
+    inFlags &= static_cast<FlagsT>(~static_cast<FlagsT>(inFlag));
+}
+
+/**
+ * @deprecated Use typesafe BitFlags class instead.
+ */
+template <typename FlagsT, typename FlagT>
+inline void SetFlag(FlagsT & inFlags, const FlagT inFlag)
+{
+    inFlags = static_cast<FlagsT>(inFlags | static_cast<FlagsT>(inFlag));
+}
+
+/**
+ * @deprecated Use typesafe BitFlags class instead.
+ */
+template <typename FlagsT, typename FlagT>
+inline void SetFlag(FlagsT & inFlags, const FlagT inFlag, const bool inValue)
+{
+    if (inValue)
+    {
+        SetFlag(inFlags, inFlag);
+    }
+    else
+    {
+        ClearFlag(inFlags, inFlag);
+    }
+}
+
+} // namespace chip
+
+#endif // CHIP_CHIP_SUPPORT_FLAGUTILS_HPP
diff --git a/src/lib/support/FlagUtils.hpp b/src/lib/support/FlagUtils.hpp
diff --git a/src/platform/Darwin/ConnectivityManagerImpl.h b/src/platform/Darwin/ConnectivityManagerImpl.h
@@ -32,7 +32,6 @@
 #include <platform/internal/GenericConnectivityManagerImpl_NoThread.h>
 #endif
 #include <platform/internal/GenericConnectivityManagerImpl_NoWiFi.h>
-#include <support/FlagUtils.hpp>
 
 namespace chip {
 namespace Inet {

diff --git a/src/platform/EFR32/ConnectivityManagerImpl.h b/src/platform/EFR32/ConnectivityManagerImpl.h
@@ -32,7 +32,6 @@
 #include <platform/internal/GenericConnectivityManagerImpl_NoThread.h>
 #endif
 #include <platform/internal/GenericConnectivityManagerImpl_NoWiFi.h>
-#include <support/FlagUtils.hpp>
 
 namespace Inet {
 class IPAddress;

diff --git a/src/platform/ESP32/ConnectivityManagerImpl.h b/src/platform/ESP32/ConnectivityManagerImpl.h
@@ -28,7 +28,6 @@
 #include <platform/internal/GenericConnectivityManagerImpl_NoBLE.h>
 #endif
 #include <platform/internal/GenericConnectivityManagerImpl_NoThread.h>
-#include <support/FlagUtils.hpp>
 
 #include "esp_event.h"
 

diff --git a/src/platform/K32W/ConnectivityManagerImpl.h b/src/platform/K32W/ConnectivityManagerImpl.h
@@ -33,7 +33,6 @@
 #include <platform/internal/GenericConnectivityManagerImpl_NoThread.h>
 #endif
 #include <platform/internal/GenericConnectivityManagerImpl_NoWiFi.h>
-#include <support/FlagUtils.hpp>
 
 namespace chip {
 namespace DeviceLayer {

diff --git a/src/platform/Linux/ConnectivityManagerImpl.h b/src/platform/Linux/ConnectivityManagerImpl.h
@@ -36,7 +36,6 @@
 #else
 #include <platform/internal/GenericConnectivityManagerImpl_NoWiFi.h>
 #endif
-#include <support/FlagUtils.hpp>
 
 #if CHIP_DEVICE_CONFIG_ENABLE_WPA
 #include <platform/Linux/dbus/wpa/DBusWpa.h>

diff --git a/src/platform/nRF5/ConnectivityManagerImpl.h b/src/platform/nRF5/ConnectivityManagerImpl.h
@@ -32,7 +32,6 @@
 #include <platform/internal/GenericConnectivityManagerImpl_NoThread.h>
 #endif
 #include <platform/internal/GenericConnectivityManagerImpl_NoWiFi.h>
-#include <support/FlagUtils.hpp>
 
 namespace chip {
 namespace Inet {

diff --git a/src/platform/nrfconnect/ConnectivityManagerImpl.h b/src/platform/nrfconnect/ConnectivityManagerImpl.h
@@ -31,7 +31,6 @@
 #include <platform/internal/GenericConnectivityManagerImpl_NoThread.h>
 #endif
 #include <platform/internal/GenericConnectivityManagerImpl_NoWiFi.h>
-#include <support/FlagUtils.hpp>
 
 #include <support/logging/CHIPLogging.h>
 

diff --git a/src/platform/qpg6100/ConnectivityManagerImpl.h b/src/platform/qpg6100/ConnectivityManagerImpl.h
@@ -31,7 +31,6 @@
 #include <platform/internal/GenericConnectivityManagerImpl_NoThread.h>
 #endif
 #include <platform/internal/GenericConnectivityManagerImpl_NoWiFi.h>
-#include <support/FlagUtils.hpp>
 
 namespace chip {
 namespace Inet {

diff --git a/src/transport/RendezvousSession.cpp b/src/transport/RendezvousSession.cpp
@@ -123,7 +123,7 @@ CHIP_ERROR RendezvousSession::SendPairingMessage(System::PacketBuffer * msgBuf)
     SuccessOrExit(err);
 
     msgBuf->ConsumeHead(headerSize);
-    err = mTransport->SendMessage(header, Header::Flags::None(), Transport::PeerAddress::BLE(), msgBuf);
+    err = mTransport->SendMessage(header, Header::Flags(), Transport::PeerAddress::BLE(), msgBuf);
     SuccessOrExit(err);
 
 exit: