Merge branch 'fabric_admin_add_rpc' into fabric_admin_populate

Author: andy31415

.github/workflows/tests.yaml

@@ -24,6 +24,7 @@
 #include "StatusIB.h"
 
 #include "MessageDefHelper.h"
+#include "protocols/interaction_model/StatusCode.h"
 
 #include <inttypes.h>
 #include <stdarg.h>
@@ -149,31 +150,6 @@ CHIP_ERROR StatusIB::ToChipError() const
     return ChipError(ChipError::SdkPart::kIMGlobalStatus, to_underlying(mStatus));
 }
 
-StatusIB::StatusIB(CHIP_ERROR aError)
-{
-    if (aError.IsPart(ChipError::SdkPart::kIMClusterStatus))
-    {
-        mStatus        = Status::Failure;
-        mClusterStatus = MakeOptional(aError.GetSdkCode());
-        return;
-    }
-
-    mClusterStatus = NullOptional;
-    if (aError == CHIP_NO_ERROR)
-    {
-        mStatus = Status::Success;
-        return;
-    }
-
-    if (aError.IsPart(ChipError::SdkPart::kIMGlobalStatus))
-    {
-        mStatus = static_cast<Status>(aError.GetSdkCode());
-        return;
-    }
-
-    mStatus = Status::Failure;
-}
-
 namespace {
 bool FormatStatusIBError(char * buf, uint16_t bufSize, CHIP_ERROR err)
 {

src/app/MessageDef/StatusIB.cpp

@@ -60,7 +60,7 @@ struct StatusIB
         }
     }
 
-    explicit StatusIB(CHIP_ERROR error);
+    explicit StatusIB(CHIP_ERROR error) : StatusIB(Protocols::InteractionModel::ClusterStatusCode(error)) {}
 
     enum class Tag : uint8_t
     {

src/app/MessageDef/StatusIB.h

@@ -282,11 +282,11 @@ CHIP_ERROR EcosystemInformationServer::ReadAttribute(const ConcreteReadAttribute
     switch (aPath.mAttributeId)
     {
     case Attributes::RemovedOn::Id:
-        return EcosystemInformationServer::Instance().EncodeRemovedOnAttribute(aPath.mEndpointId, aEncoder);
+        return EncodeRemovedOnAttribute(aPath.mEndpointId, aEncoder);
     case Attributes::DeviceDirectory::Id:
-        return EcosystemInformationServer::Instance().EncodeDeviceDirectoryAttribute(aPath.mEndpointId, aEncoder);
+        return EncodeDeviceDirectoryAttribute(aPath.mEndpointId, aEncoder);
     case Attributes::LocationDirectory::Id:
-        return EcosystemInformationServer::Instance().EncodeLocationStructAttribute(aPath.mEndpointId, aEncoder);
+        return EncodeLocationStructAttribute(aPath.mEndpointId, aEncoder);
     case Attributes::ClusterRevision::Id: {
         uint16_t rev = ZCL_ECOSYSTEM_INFORMATION_CLUSTER_REVISION;
         return aEncoder.Encode(rev);

src/app/clusters/ecosystem-information-server/ecosystem-information-server.cpp

@@ -149,45 +149,42 @@ bool ActionReturnStatus::IsOutOfSpaceEncodingResponse() const
     return false;
 }
 
-const char * ActionReturnStatus::c_str() const
+const char * ActionReturnStatus::c_str(ActionReturnStatus::StringStorage & storage) const
 {
-
-    // Generally size should be sufficient.
-    // len("Status<123>, Code 255") == 21 (and then 22 for null terminator. We have slack.)
-    static chip::StringBuilder<32> sFormatBuffer;
-
     if (const CHIP_ERROR * err = std::get_if<CHIP_ERROR>(&mReturnStatus))
     {
 #if CHIP_CONFIG_ERROR_FORMAT_AS_STRING
         return err->Format(); // any length
 #else
-        sFormatBuffer.Reset().AddFormat("%" CHIP_ERROR_FORMAT, err->Format());
-        return sFormatBuffer.c_str();
+        storage.formatBuffer.Reset().AddFormat("%" CHIP_ERROR_FORMAT, err->Format());
+        return storage.formatBuffer.c_str();
 #endif
     }
 
     if (const ClusterStatusCode * status = std::get_if<ClusterStatusCode>(&mReturnStatus))
     {
+        storage.formatBuffer.Reset();
+
 #if CHIP_CONFIG_IM_STATUS_CODE_VERBOSE_FORMAT
-        sFormatBuffer.AddFormat("%s(%d)", Protocols::InteractionModel::StatusName(status->GetStatus()),
-                                static_cast<int>(status->GetStatus()));
+        storage.formatBuffer.AddFormat("%s(%d)", Protocols::InteractionModel::StatusName(status->GetStatus()),
+                                       static_cast<int>(status->GetStatus()));
 #else
         if (status->IsSuccess())
         {
-            sFormatBuffer.Add("Success");
+            storage.formatBuffer.Add("Success");
         }
         else
         {
-            sFormatBuffer.AddFormat("Status<%d>", static_cast<int>(status->GetStatus()));
+            storage.formatBuffer.AddFormat("Status<%d>", static_cast<int>(status->GetStatus()));
         }
 #endif
 
         chip::Optional<ClusterStatus> clusterCode = status->GetClusterSpecificCode();
         if (clusterCode.HasValue())
         {
-            sFormatBuffer.AddFormat(", Code %d", static_cast<int>(clusterCode.Value()));
+            storage.formatBuffer.AddFormat(", Code %d", static_cast<int>(clusterCode.Value()));
         }
-        return sFormatBuffer.c_str();
+        return storage.formatBuffer.c_str();
     }
 
     // all std::variant cases exhausted

src/app/data-model-provider/ActionReturnStatus.cpp

@@ -44,6 +44,21 @@ namespace DataModel {
 class ActionReturnStatus
 {
 public:
+    /// Provides storage for the c_str() call for the action status.
+    struct StringStorage
+    {
+        // Generally size should be sufficient.
+        // The longest status code from StatusCodeList is NO_UPSTREAM_SUBSCRIPTION(197)
+        // so we need space for one of:
+        //    "NO_UPSTREAM_SUBSCRIPTION(197)\0" = 30   // explicit verbose status code
+        //    "FAILURE(1), Code 255\0")                // cluster failure, verbose
+        //    "SUCCESS(0), Code 255\0")                // cluster success, verbose
+        //    "Status<197>, Code 255\0")               // Cluster failure, non-verbose
+        //
+        // CHIP_ERROR has its own (global/static!) storage
+        chip::StringBuilder<32> formatBuffer;
+    };
+
     ActionReturnStatus(CHIP_ERROR error) : mReturnStatus(error) {}
     ActionReturnStatus(Protocols::InteractionModel::Status status) :
         mReturnStatus(Protocols::InteractionModel::ClusterStatusCode(status))
@@ -81,12 +96,8 @@ class ActionReturnStatus
 
     /// Get the formatted string of this status.
     ///
-    /// NOTE: this is NOT thread safe in the general case, however the safety guarantees
-    ///       are similar to chip::ErrorStr which also assumes a static buffer.
-    ///
-    /// Use this in the chip main event loop (and since that is a single thread,
-    /// there should be no races)
-    const char * c_str() const;
+    /// May use `storage` for storying the actual underlying character string.
+    const char * c_str(StringStorage & storage) const;
 
 private:
     std::variant<CHIP_ERROR, Protocols::InteractionModel::ClusterStatusCode> mReturnStatus;

src/app/data-model-provider/ActionReturnStatus.h

@@ -23,7 +23,8 @@ template <>
 StatusWithSize ToString<chip::app::DataModel::ActionReturnStatus>(const chip::app::DataModel::ActionReturnStatus & status,
                                                                   pw::span<char> buffer)
 {
-    return pw::string::Format(buffer, "ActionReturnStatus<%s>", status.c_str());
+    chip::app::DataModel::ActionReturnStatus::StringStorage storage;
+    return pw::string::Format(buffer, "ActionReturnStatus<%s>", status.c_str(storage));
 }
 
 } // namespace pw

src/app/data-model-provider/StringBuilderAdapters.cpp

@@ -111,3 +111,45 @@ TEST(TestActionReturnStatus, TestStatusCode)
     ASSERT_EQ(ActionReturnStatus(CHIP_IM_CLUSTER_STATUS(0x12)).GetStatusCode(), ClusterStatusCode::ClusterSpecificFailure(0x12));
     ASSERT_EQ(ActionReturnStatus(CHIP_IM_GLOBAL_STATUS(Timeout)).GetStatusCode(), ClusterStatusCode(Status::Timeout));
 }
+
+TEST(TestActionReturnStatus, TestCString)
+{
+    /// only tests the strings that we build and NOT the CHIP_ERROR ones which
+    /// are tested separately. for chip_error we just say it should not be empty.
+    ActionReturnStatus::StringStorage buffer;
+    ActionReturnStatus status(Status::Success);
+
+    // chip-error returns something non-empty
+    status = CHIP_ERROR_NOT_FOUND;
+    ASSERT_STRNE(status.c_str(buffer), "");
+
+    status = CHIP_NO_ERROR;
+    ASSERT_STRNE(status.c_str(buffer), "");
+
+    // the items below we control
+#if CHIP_CONFIG_IM_STATUS_CODE_VERBOSE_FORMAT
+    status = Status::Success;
+    ASSERT_STREQ(status.c_str(buffer), "SUCCESS(0)");
+
+    status = Status::UnsupportedCommand;
+    ASSERT_STREQ(status.c_str(buffer), "UNSUPPORTED_COMMAND(129)");
+
+    status = ClusterStatusCode::ClusterSpecificSuccess(31);
+    ASSERT_STREQ(status.c_str(buffer), "SUCCESS(0), Code 31");
+
+    status = ClusterStatusCode::ClusterSpecificFailure(32);
+    ASSERT_STREQ(status.c_str(buffer), "FAILURE(1), Code 32");
+#else
+    status = Status::Success;
+    ASSERT_STREQ(status.c_str(buffer), "Success");
+
+    status = Status::UnsupportedCommand;
+    ASSERT_STREQ(status.c_str(buffer), "Status<129>");
+
+    status = ClusterStatusCode::ClusterSpecificSuccess(31);
+    ASSERT_STREQ(status.c_str(buffer), "Success, Code 31");
+
+    status = ClusterStatusCode::ClusterSpecificFailure(32);
+    ASSERT_STREQ(status.c_str(buffer), "Status<1>, Code 32");
+#endif
+}

src/app/data-model-provider/tests/TestActionReturnStatus.cpp

@@ -82,12 +82,13 @@ ActionReturnStatus RetrieveClusterData(DataModel::Provider * dataModel, const Ac
 
     if (statusEmber != statusDm)
     {
-        StringBuilder<128> buffer;
+        ActionReturnStatus::StringStorage buffer;
+
         // Note log + chipDie instead of VerifyOrDie so that breakpoints (and usage of rr)
         // is easier to debug.
         ChipLogError(Test, "Different return codes between ember and DM");
-        ChipLogError(Test, "  Ember status: %s", statusEmber.c_str());
-        ChipLogError(Test, "  DM status:    %s", statusDm.c_str());
+        ChipLogError(Test, "  Ember status: %s", statusEmber.c_str(buffer));
+        ChipLogError(Test, "  DM status:    %s", statusDm.c_str(buffer));
 
         // For time-dependent data, we may have size differences here: one data fitting in buffer
         // while another not, resulting in different errors (success vs out of space).

src/app/reporting/Read-Checked.cpp

@@ -95,7 +95,8 @@ DataModel::ActionReturnStatus RetrieveClusterData(DataModel::Provider * dataMode
     // and will be sent to the client as well).
     if (!status.IsOutOfSpaceEncodingResponse())
     {
-        ChipLogError(DataManagement, "Failed to read attribute: %s", status.c_str());
+        DataModel::ActionReturnStatus::StringStorage storage;
+        ChipLogError(DataManagement, "Failed to read attribute: %s", status.c_str(storage));
     }
     return status;
 }

src/app/reporting/Read-DataModel.cpp

@@ -164,6 +164,7 @@ typedef NS_ENUM(uint8_t, MTRTransportType) {
  *
  * nil is used to represent wildcards.
  */
+NS_SWIFT_SENDABLE
 MTR_AVAILABLE(ios(17.0), macos(14.0), watchos(10.0), tvos(17.0))
 @interface MTRAttributeRequestPath : NSObject <NSCopying>
 @property (nonatomic, readonly, copy, nullable) NSNumber * endpoint MTR_AVAILABLE(ios(17.0), macos(14.0), watchos(10.0), tvos(17.0));
@@ -182,6 +183,7 @@ MTR_AVAILABLE(ios(17.0), macos(14.0), watchos(10.0), tvos(17.0))
  *
  * nil is used to represent wildcards.
  */
+NS_SWIFT_SENDABLE
 MTR_AVAILABLE(ios(17.0), macos(14.0), watchos(10.0), tvos(17.0))
 @interface MTREventRequestPath : NSObject <NSCopying>
 @property (nonatomic, readonly, copy, nullable) NSNumber * endpoint MTR_AVAILABLE(ios(17.0), macos(14.0), watchos(10.0), tvos(17.0));
@@ -572,6 +574,7 @@ MTR_AVAILABLE(ios(16.1), macos(13.0), watchos(9.1), tvos(16.1))
  * A path indicating a specific cluster on a device (i.e. without any
  * wildcards).
  */
+NS_SWIFT_SENDABLE
 MTR_AVAILABLE(ios(16.4), macos(13.3), watchos(9.4), tvos(16.4))
 @interface MTRClusterPath : NSObject <NSCopying, NSSecureCoding>
 
@@ -588,6 +591,7 @@ MTR_AVAILABLE(ios(16.4), macos(13.3), watchos(9.4), tvos(16.4))
  * A path indicating a specific attribute on a device (i.e. without any
  * wildcards).
  */
+NS_SWIFT_SENDABLE
 MTR_AVAILABLE(ios(16.1), macos(13.0), watchos(9.1), tvos(16.1))
 @interface MTRAttributePath : MTRClusterPath <NSSecureCoding>
 
@@ -604,6 +608,7 @@ MTR_AVAILABLE(ios(16.1), macos(13.0), watchos(9.1), tvos(16.1))
  * (i.e. without any wildcards).  There can be multiple instances of actual
  * events for a given event path.
  */
+NS_SWIFT_SENDABLE
 MTR_AVAILABLE(ios(16.1), macos(13.0), watchos(9.1), tvos(16.1))
 @interface MTREventPath : MTRClusterPath
 
@@ -618,6 +623,7 @@ MTR_AVAILABLE(ios(16.1), macos(13.0), watchos(9.1), tvos(16.1))
  * A path indicating a specific command on a device (i.e. without any
  * wildcards).
  */
+NS_SWIFT_SENDABLE
 MTR_AVAILABLE(ios(16.1), macos(13.0), watchos(9.1), tvos(16.1))
 @interface MTRCommandPath : MTRClusterPath
 
@@ -628,6 +634,7 @@ MTR_AVAILABLE(ios(16.1), macos(13.0), watchos(9.1), tvos(16.1))
                                     commandID:(NSNumber *)commandID MTR_AVAILABLE(ios(16.4), macos(13.3), watchos(9.4), tvos(16.4));
 @end
 
+NS_SWIFT_SENDABLE
 MTR_AVAILABLE(ios(16.1), macos(13.0), watchos(9.1), tvos(16.1))
 @interface MTRAttributeReport : NSObject
 
@@ -640,7 +647,7 @@ MTR_AVAILABLE(ios(16.1), macos(13.0), watchos(9.1), tvos(16.1))
  * * The attribute is nullable and the value of the attribute is null.
  *
  * If value is not nil, the actual type of value will depend on the
- * schema-defined (typically defiend in the Matter specification) type of the
+ * schema-defined (typically defined in the Matter specification) type of the
  * attribute as follows:
  *
  * * list: NSArray of whatever type the list entries are.
@@ -697,6 +704,7 @@ typedef NS_ENUM(NSUInteger, MTREventPriority) {
     MTREventPriorityCritical = 2
 } MTR_AVAILABLE(ios(16.5), macos(13.4), watchos(9.5), tvos(16.5));
 
+NS_SWIFT_SENDABLE
 MTR_AVAILABLE(ios(16.1), macos(13.0), watchos(9.1), tvos(16.1))
 @interface MTREventReport : NSObject
 

src/darwin/Framework/CHIP/MTRBaseDevice.h

@@ -14,6 +14,16 @@
 #    See the License for the specific language governing permissions and
 #    limitations under the License.
 #
+
+# === BEGIN CI TEST ARGUMENTS ===
+# test-runner-runs: run1
+# test-runner-run/run1/app: ${ALL_CLUSTERS_APP}
+# test-runner-run/run1/factoryreset: True
+# test-runner-run/run1/quiet: True
+# test-runner-run/run1/app-args: --discriminator 1234 --KVS kvs1 --trace-to json:${TRACE_APP}.json
+# test-runner-run/run1/script-args: --storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021 --trace-to json:${TRACE_TEST_JSON}.json --trace-to perfetto:${TRACE_TEST_PERFETTO}.perfetto
+# === END CI TEST ARGUMENTS ===
+
 import chip.clusters as Clusters
 from matter_testing_support import MatterBaseTest, TestStep, async_test_body, default_matter_test_main
 from mobly import asserts

src/python_testing/TC_ACL_2_2.py

@@ -15,6 +15,15 @@
 #    limitations under the License.
 #
 
+# === BEGIN CI TEST ARGUMENTS ===
+# test-runner-runs: run1
+# test-runner-run/run1/app: ${ALL_CLUSTERS_APP}
+# test-runner-run/run1/factoryreset: True
+# test-runner-run/run1/quiet: True
+# test-runner-run/run1/app-args: --discriminator 1234 --KVS kvs1 --trace-to json:${TRACE_APP}.json
+# test-runner-run/run1/script-args: --storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021 --trace-to json:${TRACE_TEST_JSON}.json --trace-to perfetto:${TRACE_TEST_PERFETTO}.perfetto
+# === END CI TEST ARGUMENTS ===
+
 import functools
 import logging
 from operator import ior

src/python_testing/TC_BOOLCFG_2_1.py

@@ -15,6 +15,15 @@
 #    limitations under the License.
 #
 
+# === BEGIN CI TEST ARGUMENTS ===
+# test-runner-runs: run1
+# test-runner-run/run1/app: ${ALL_CLUSTERS_APP}
+# test-runner-run/run1/factoryreset: True
+# test-runner-run/run1/quiet: True
+# test-runner-run/run1/app-args: --discriminator 1234 --KVS kvs1 --trace-to json:${TRACE_APP}.json
+# test-runner-run/run1/script-args: --storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021 --trace-to json:${TRACE_TEST_JSON}.json --trace-to perfetto:${TRACE_TEST_PERFETTO}.perfetto
+# === END CI TEST ARGUMENTS ===
+
 import logging
 from random import choice
 

src/python_testing/TC_BOOLCFG_3_1.py

@@ -15,6 +15,15 @@
 #    limitations under the License.
 #
 
+# === BEGIN CI TEST ARGUMENTS ===
+# test-runner-runs: run1
+# test-runner-run/run1/app: ${ALL_CLUSTERS_APP}
+# test-runner-run/run1/factoryreset: True
+# test-runner-run/run1/quiet: True
+# test-runner-run/run1/app-args: --discriminator 1234 --KVS kvs1 --trace-to json:${TRACE_APP}.json
+# test-runner-run/run1/script-args: --storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021 --trace-to json:${TRACE_TEST_JSON}.json --trace-to perfetto:${TRACE_TEST_PERFETTO}.perfetto
+# === END CI TEST ARGUMENTS ===
+
 import logging
 
 import chip.clusters as Clusters

src/python_testing/TC_BOOLCFG_4_1.py

@@ -15,6 +15,15 @@
 #    limitations under the License.
 #
 
+# === BEGIN CI TEST ARGUMENTS ===
+# test-runner-runs: run1
+# test-runner-run/run1/app: ${ALL_CLUSTERS_APP}
+# test-runner-run/run1/factoryreset: True
+# test-runner-run/run1/quiet: True
+# test-runner-run/run1/app-args: --discriminator 1234 --KVS kvs1 --enable-key 000102030405060708090a0b0c0d0e0f --trace-to json:${TRACE_APP}.json
+# test-runner-run/run1/script-args: --storage-path admin_storage.json --commissioning-method on-network --discriminator 1234 --passcode 20202021 --hex-arg PIXIT.BOOLCFG.TEST_EVENT_TRIGGER_KEY:000102030405060708090a0b0c0d0e0f --trace-to json:${TRACE_TEST_JSON}.json --trace-to perfetto:${TRACE_TEST_PERFETTO}.perfetto
+# === END CI TEST ARGUMENTS ===
+
 import logging
 
 import chip.clusters as Clusters