Introduce a Status + Cluster Code abstraction (#31121)
* Introduce a Status + Cluster Code abstraction
- Existing code always has to split IM status and
cluster-specific status codes, which causes clumsy
implementation of clusters (see issue #31120).
- This PR introduces a value that encapsulates both
the IM status and cluster-specific status, in an
easy-to-pass-around abstraction, which can be directly
used in place of Status.
- Subsequent PR will implement usage in IM with overloads
for common cases, such as for AddStatus.
Issue #31120
Testing done:
- Added unit tests
- Other tests still pass
* Restyled by clang-format
* Restyled by gn
* Address review comments
* Rename ClusterStatus to ClusterStatusCode
* Fix build deps
* Restyled by gn
---------
Co-authored-by: Restyled.io <commits@restyled.io>
diff --git a/examples/darwin-framework-tool/BUILD.gn b/examples/darwin-framework-tool/BUILD.gn
index e6b1a3e..7ee396d 100644
--- a/examples/darwin-framework-tool/BUILD.gn
+++ b/examples/darwin-framework-tool/BUILD.gn
@@ -248,7 +248,7 @@
# pics is needed by tests
"${chip_root}/src/app/tests/suites/pics",
- "${chip_root}/src/protocols:im_status",
+ "${chip_root}/src/protocols/interaction_model",
]
defines = []
diff --git a/src/BUILD.gn b/src/BUILD.gn
index 532da2a..2d0204d 100644
--- a/src/BUILD.gn
+++ b/src/BUILD.gn
@@ -60,6 +60,7 @@
"${chip_root}/src/lib/core/tests",
"${chip_root}/src/messaging/tests",
"${chip_root}/src/protocols/bdx/tests",
+ "${chip_root}/src/protocols/interaction_model/tests",
"${chip_root}/src/protocols/user_directed_commissioning/tests",
"${chip_root}/src/transport/retransmit/tests",
]
diff --git a/src/app/BUILD.gn b/src/app/BUILD.gn
index e053756..a96dc22 100644
--- a/src/app/BUILD.gn
+++ b/src/app/BUILD.gn
@@ -250,6 +250,7 @@
"${chip_root}/src/lib/address_resolve",
"${chip_root}/src/lib/support",
"${chip_root}/src/messaging",
+ "${chip_root}/src/protocols/interaction_model",
"${chip_root}/src/protocols/secure_channel",
"${chip_root}/src/system",
"${nlio_root}:nlio",
diff --git a/src/app/common/BUILD.gn b/src/app/common/BUILD.gn
index fee9795..c6c350e 100644
--- a/src/app/common/BUILD.gn
+++ b/src/app/common/BUILD.gn
@@ -27,6 +27,7 @@
public_deps = [
"${chip_root}/src/lib/core",
"${chip_root}/src/lib/support",
+ "${chip_root}/src/protocols/interaction_model",
]
defines = []
diff --git a/src/app/tests/TestStatusIB.cpp b/src/app/tests/TestStatusIB.cpp
index 4ff97e4..3eb47f4 100644
--- a/src/app/tests/TestStatusIB.cpp
+++ b/src/app/tests/TestStatusIB.cpp
@@ -22,6 +22,7 @@
#include <lib/core/ErrorStr.h>
#include <lib/support/CHIPMem.h>
#include <lib/support/UnitTestRegistration.h>
+#include <protocols/interaction_model/StatusCode.h>
#include <nlunit-test.h>
diff --git a/src/app/tests/suites/commands/interaction_model/BUILD.gn b/src/app/tests/suites/commands/interaction_model/BUILD.gn
index e132971..ecefa4a 100644
--- a/src/app/tests/suites/commands/interaction_model/BUILD.gn
+++ b/src/app/tests/suites/commands/interaction_model/BUILD.gn
@@ -16,7 +16,7 @@
import("//build_overrides/chip.gni")
static_library("interaction_model") {
- output_name = "libInteractionModel"
+ output_name = "libInteractionModelCommands"
sources = [
"InteractionModel.cpp",
diff --git a/src/protocols/BUILD.gn b/src/protocols/BUILD.gn
index 10b48fb..2895334 100644
--- a/src/protocols/BUILD.gn
+++ b/src/protocols/BUILD.gn
@@ -38,27 +38,12 @@
cflags = [ "-Wconversion" ]
public_deps = [
- ":im_status",
":type_definitions",
"${chip_root}/src/lib/core",
"${chip_root}/src/lib/support",
"${chip_root}/src/messaging",
"${chip_root}/src/protocols/bdx",
+ "${chip_root}/src/protocols/interaction_model",
"${chip_root}/src/protocols/secure_channel",
]
}
-
-static_library("im_status") {
- sources = [
- "interaction_model/StatusCode.cpp",
- "interaction_model/StatusCode.h",
- ]
-
- cflags = [ "-Wconversion" ]
-
- public_deps = [
- ":type_definitions",
- "${chip_root}/src/lib/core",
- "${chip_root}/src/lib/support",
- ]
-}
diff --git a/src/protocols/interaction_model/BUILD.gn b/src/protocols/interaction_model/BUILD.gn
new file mode 100644
index 0000000..b8c3bd7
--- /dev/null
+++ b/src/protocols/interaction_model/BUILD.gn
@@ -0,0 +1,34 @@
+# Copyright (c) 2020 Project CHIP Authors
+#
+# 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.
+
+import("//build_overrides/chip.gni")
+
+static_library("interaction_model") {
+ output_name = "libInteractionModel"
+
+ sources = [
+ "Constants.h",
+ "StatusCode.cpp",
+ "StatusCode.h",
+ "StatusCodeList.h",
+ ]
+
+ cflags = [ "-Wconversion" ]
+
+ public_deps = [
+ "${chip_root}/src/lib/core",
+ "${chip_root}/src/lib/support",
+ "${chip_root}/src/protocols:type_definitions",
+ ]
+}
diff --git a/src/protocols/interaction_model/StatusCode.h b/src/protocols/interaction_model/StatusCode.h
index 5d0453e..3161567 100644
--- a/src/protocols/interaction_model/StatusCode.h
+++ b/src/protocols/interaction_model/StatusCode.h
@@ -17,9 +17,12 @@
#pragma once
+#include <limits>
#include <stdint.h>
#include <lib/core/CHIPConfig.h>
+#include <lib/core/DataModelTypes.h>
+#include <lib/core/Optional.h>
#include <lib/support/TypeTraits.h>
#if CHIP_CONFIG_IM_STATUS_CODE_VERBOSE_FORMAT
@@ -48,6 +51,112 @@
const char * StatusName(Status status);
#endif // CHIP_CONFIG_IM_STATUS_CODE_VERBOSE_FORMAT
+/**
+ * @brief Class to encapsulate a Status code, including possibly a
+ * cluster-specific code for generic SUCCESS/FAILURE.
+ *
+ * This abstractions joins together Status and ClusterStatus, which
+ * are the components of a StatusIB, used in many IM actions, in a
+ * way which allows both of them to carry together.
+ *
+ * This can be used everywhere a `Status` is used, but it is lossy
+ * to the cluster-specific code if used in place of `Status` when
+ * the cluster-specific code is set.
+ *
+ * This class can only be directly constructed from a `Status`. To
+ * attach a cluster-specific-code, please use the `ClusterSpecificFailure()`
+ * and `ClusterSpecificSuccess()` factory methods.
+ */
+class ClusterStatusCode
+{
+public:
+ explicit ClusterStatusCode(Status status) : mStatus(status) {}
+
+ // We only have simple copyable members, so we should be trivially copyable.
+ ClusterStatusCode(const ClusterStatusCode & other) = default;
+ ClusterStatusCode & operator=(const ClusterStatusCode & other) = default;
+
+ bool operator==(const ClusterStatusCode & other)
+ {
+ return (this->mStatus == other.mStatus) && (this->HasClusterSpecificCode() == other.HasClusterSpecificCode()) &&
+ (this->GetClusterSpecificCode() == other.GetClusterSpecificCode());
+ }
+
+ bool operator!=(const ClusterStatusCode & other) { return !(*this == other); }
+
+ ClusterStatusCode & operator=(const Status & status)
+ {
+ this->mStatus = status;
+ this->mClusterSpecificCode = chip::NullOptional;
+ return *this;
+ }
+
+ /**
+ * @brief Builder for a cluster-specific failure status code.
+ *
+ * @tparam T - enum type for the cluster-specific status code
+ * (e.g. chip::app::Clusters::AdministratorCommissioning::CommissioningWindowStatusEnum)
+ * @param cluster_specific_code - cluster-specific code to record with the failure
+ * (e.g. chip::app::Clusters::AdministratorCommissioning::CommissioningWindowStatusEnum::kWindowNotOpen)
+ * @return a ClusterStatusCode instance properly configured.
+ */
+ template <typename T>
+ static ClusterStatusCode ClusterSpecificFailure(T cluster_specific_code)
+ {
+ static_assert(std::numeric_limits<T>::max() <= std::numeric_limits<ClusterStatus>::max(), "Type used must fit in uint8_t");
+ return ClusterStatusCode(Status::Failure, chip::to_underlying(cluster_specific_code));
+ }
+
+ /**
+ * @brief Builder for a cluster-specific success status code.
+ *
+ * @tparam T - enum type for the cluster-specific status code
+ * (e.g. chip::app::Clusters::AdministratorCommissioning::CommissioningWindowStatusEnum)
+ * @param cluster_specific_code - cluster-specific code to record with the success
+ * (e.g. chip::app::Clusters::AdministratorCommissioning::CommissioningWindowStatusEnum::kBasicWindowOpen)
+ * @return a ClusterStatusCode instance properly configured.
+ */
+ template <typename T>
+ static ClusterStatusCode ClusterSpecificSuccess(T cluster_specific_code)
+ {
+ static_assert(std::numeric_limits<T>::max() <= std::numeric_limits<ClusterStatus>::max(), "Type used must fit in uint8_t");
+ return ClusterStatusCode(Status::Success, chip::to_underlying(cluster_specific_code));
+ }
+
+ /// @return true if the core Status associated with this ClusterStatusCode is the one for success.
+ bool IsSuccess() const { return mStatus == Status::Success; }
+
+ /// @return the core Status code associated withi this ClusterStatusCode.
+ Status GetStatus() const { return mStatus; }
+
+ /// @return true if a cluster-specific code is associated with the ClusterStatusCode.
+ bool HasClusterSpecificCode() const { return mClusterSpecificCode.HasValue(); }
+
+ /// @return the cluster-specific code associated with this ClusterStatusCode or chip::NullOptional if none is associated.
+ chip::Optional<ClusterStatus> GetClusterSpecificCode() const
+ {
+ if ((mStatus != Status::Failure) && (mStatus != Status::Success))
+ {
+ return chip::NullOptional;
+ }
+ return mClusterSpecificCode;
+ }
+
+ // Automatic conversions to common types, using the status code alone.
+ operator Status() const { return mStatus; }
+
+private:
+ ClusterStatusCode() = delete;
+ ClusterStatusCode(Status status, ClusterStatus cluster_specific_code) :
+ mStatus(status), mClusterSpecificCode(chip::MakeOptional(cluster_specific_code))
+ {}
+
+ Status mStatus;
+ chip::Optional<ClusterStatus> mClusterSpecificCode;
+};
+
+static_assert(sizeof(ClusterStatusCode) <= sizeof(uint32_t), "ClusterStatusCode must not grow to be larger than a uint32_t");
+
} // namespace InteractionModel
} // namespace Protocols
} // namespace chip
diff --git a/src/protocols/interaction_model/tests/BUILD.gn b/src/protocols/interaction_model/tests/BUILD.gn
new file mode 100644
index 0000000..1679200
--- /dev/null
+++ b/src/protocols/interaction_model/tests/BUILD.gn
@@ -0,0 +1,35 @@
+# Copyright (c) 2023 Project CHIP Authors
+#
+# 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.
+
+import("//build_overrides/build.gni")
+import("//build_overrides/chip.gni")
+import("//build_overrides/nlunit_test.gni")
+
+import("${chip_root}/build/chip/chip_test_suite.gni")
+
+chip_test_suite_using_nltest("tests") {
+ output_name = "libInteractionModelTests"
+
+ test_sources = [ "TestStatusCode.cpp" ]
+
+ public_deps = [
+ "${chip_root}/src/lib/core",
+ "${chip_root}/src/lib/support",
+ "${chip_root}/src/lib/support:testing",
+ "${chip_root}/src/protocols/interaction_model",
+ "${nlunit_test_root}:nlunit-test",
+ ]
+
+ cflags = [ "-Wconversion" ]
+}
diff --git a/src/protocols/interaction_model/tests/TestStatusCode.cpp b/src/protocols/interaction_model/tests/TestStatusCode.cpp
new file mode 100644
index 0000000..4b89e0f
--- /dev/null
+++ b/src/protocols/interaction_model/tests/TestStatusCode.cpp
@@ -0,0 +1,135 @@
+/*
+ *
+ * Copyright (c) 2023 Project CHIP Authors
+ * All rights reserved.
+ *
+ * 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.
+ */
+
+#include <stdint.h>
+
+#include <lib/core/Optional.h>
+#include <lib/support/UnitTestExtendedAssertions.h>
+#include <lib/support/UnitTestRegistration.h>
+#include <protocols/interaction_model/StatusCode.h>
+
+#include <nlunit-test.h>
+
+using namespace ::chip;
+using namespace ::chip::Protocols::InteractionModel;
+
+namespace {
+
+void TestStatusBasicValues(nlTestSuite * inSuite, void * inContext)
+{
+ NL_TEST_ASSERT_EQUALS(inSuite, static_cast<int>(Status::Success), 0);
+ NL_TEST_ASSERT_EQUALS(inSuite, static_cast<int>(Status::Failure), 1);
+ NL_TEST_ASSERT_EQUALS(inSuite, static_cast<int>(Status::UnsupportedEndpoint), 0x7f);
+ NL_TEST_ASSERT_EQUALS(inSuite, static_cast<int>(Status::InvalidInState), 0xcb);
+}
+
+void TestClusterStatusCode(nlTestSuite * inSuite, void * inContext)
+{
+ // Basic usage as a Status.
+ {
+ ClusterStatusCode status_code_success{ Status::Success };
+ NL_TEST_ASSERT_EQUALS(inSuite, status_code_success, Status::Success);
+ NL_TEST_ASSERT_EQUALS(inSuite, status_code_success.GetStatus(), Status::Success);
+ NL_TEST_ASSERT(inSuite, !status_code_success.HasClusterSpecificCode());
+ NL_TEST_ASSERT_EQUALS(inSuite, status_code_success.GetClusterSpecificCode(), chip::NullOptional);
+ NL_TEST_ASSERT(inSuite, status_code_success.IsSuccess());
+
+ ClusterStatusCode status_code_failure{ Status::Failure };
+ NL_TEST_ASSERT_EQUALS(inSuite, status_code_failure, Status::Failure);
+ NL_TEST_ASSERT_EQUALS(inSuite, status_code_failure.GetStatus(), Status::Failure);
+ NL_TEST_ASSERT(inSuite, !status_code_failure.HasClusterSpecificCode());
+ NL_TEST_ASSERT(inSuite, !status_code_failure.IsSuccess());
+
+ ClusterStatusCode status_code_unsupported_ep{ Status::UnsupportedEndpoint };
+ NL_TEST_ASSERT_EQUALS(inSuite, status_code_unsupported_ep, Status::UnsupportedEndpoint);
+ NL_TEST_ASSERT_EQUALS(inSuite, status_code_unsupported_ep.GetStatus(), Status::UnsupportedEndpoint);
+ NL_TEST_ASSERT(inSuite, !status_code_unsupported_ep.HasClusterSpecificCode());
+ NL_TEST_ASSERT(inSuite, !status_code_unsupported_ep.IsSuccess());
+
+ ClusterStatusCode status_code_invalid_in_state{ Status::InvalidInState };
+ NL_TEST_ASSERT_EQUALS(inSuite, status_code_invalid_in_state, Status::InvalidInState);
+ NL_TEST_ASSERT_EQUALS(inSuite, status_code_invalid_in_state.GetStatus(), Status::InvalidInState);
+ NL_TEST_ASSERT(inSuite, !status_code_invalid_in_state.HasClusterSpecificCode());
+ NL_TEST_ASSERT(inSuite, !status_code_invalid_in_state.IsSuccess());
+ }
+
+ enum RobotoClusterStatus : uint8_t
+ {
+ kSandwichError = 7,
+ kSauceSuccess = 81,
+ };
+
+ // Cluster-specific usage.
+ {
+ ClusterStatusCode status_code_success = ClusterStatusCode::ClusterSpecificSuccess(RobotoClusterStatus::kSauceSuccess);
+ NL_TEST_ASSERT_EQUALS(inSuite, status_code_success, Status::Success);
+ NL_TEST_ASSERT(inSuite, status_code_success.HasClusterSpecificCode());
+ NL_TEST_ASSERT_EQUALS(inSuite, status_code_success.GetClusterSpecificCode(),
+ static_cast<uint8_t>(RobotoClusterStatus::kSauceSuccess));
+ NL_TEST_ASSERT(inSuite, status_code_success.IsSuccess());
+
+ ClusterStatusCode status_code_failure = ClusterStatusCode::ClusterSpecificFailure(RobotoClusterStatus::kSandwichError);
+ NL_TEST_ASSERT_EQUALS(inSuite, status_code_failure, Status::Failure);
+ NL_TEST_ASSERT(inSuite, status_code_failure.HasClusterSpecificCode());
+ NL_TEST_ASSERT_EQUALS(inSuite, status_code_failure.GetClusterSpecificCode(),
+ static_cast<uint8_t>(RobotoClusterStatus::kSandwichError));
+ NL_TEST_ASSERT(inSuite, !status_code_failure.IsSuccess());
+ }
+
+ // Copy/Assignment
+ {
+ ClusterStatusCode status_code_failure1 = ClusterStatusCode::ClusterSpecificFailure(RobotoClusterStatus::kSandwichError);
+ ClusterStatusCode status_code_failure2(status_code_failure1);
+
+ NL_TEST_ASSERT_EQUALS(inSuite, status_code_failure1, status_code_failure2);
+ NL_TEST_ASSERT(inSuite, status_code_failure1.HasClusterSpecificCode());
+ NL_TEST_ASSERT(inSuite, status_code_failure2.HasClusterSpecificCode());
+
+ NL_TEST_ASSERT_EQUALS(inSuite, status_code_failure1.GetClusterSpecificCode(),
+ static_cast<uint8_t>(RobotoClusterStatus::kSandwichError));
+ NL_TEST_ASSERT_EQUALS(inSuite, status_code_failure2.GetClusterSpecificCode(),
+ static_cast<uint8_t>(RobotoClusterStatus::kSandwichError));
+
+ ClusterStatusCode status_code_failure3{ Status::InvalidCommand };
+ NL_TEST_ASSERT(inSuite, status_code_failure2 != status_code_failure3);
+
+ status_code_failure3 = status_code_failure2;
+ NL_TEST_ASSERT_EQUALS(inSuite, status_code_failure2, status_code_failure3);
+ }
+}
+
+// clang-format off
+const nlTest sTests[] =
+{
+ NL_TEST_DEF("TestStatusBasicValues", TestStatusBasicValues),
+ NL_TEST_DEF("TestClusterStatusCode", TestClusterStatusCode),
+ NL_TEST_SENTINEL()
+};
+// clang-format on
+
+nlTestSuite sSuite = { "Test IM Status Code abstractions", &sTests[0], nullptr, nullptr };
+} // namespace
+
+int TestClusterStatusCode()
+{
+ nlTestRunner(&sSuite, nullptr);
+
+ return (nlTestRunnerStats(&sSuite));
+}
+
+CHIP_REGISTER_TEST_SUITE(TestClusterStatusCode)
