[IC-Device] Initial implementation of Client Monitoring Registration table (#24096)

mkardous-silabs · bzbarsky-apple · Damian-Nordic · pull[bot] · commit 1069664b4970 · 2024-02-05T13:53:07.000Z
* Client Monitoring table implementation

* CM table tests

* Add sub-struct to add a validation function
Add unit test for invalid values

* Address PR comments

* Apply suggestions from code review

Co-authored-by: Boris Zbarsky &lt;bzbarsky@apple.com&gt;

* Increase max unit test limit

* Apply suggestions from code review

Co-authored-by: Damian Królik &lt;66667989+Damian-Nordic@users.noreply.github.com&gt;

* Move comments to headeR

* update test assert

* Add provisional comment

* restyle

Co-authored-by: Boris Zbarsky &lt;bzbarsky@apple.com&gt;
Co-authored-by: Damian Królik &lt;66667989+Damian-Nordic@users.noreply.github.com&gt;
diff --git a/src/app/tests/BUILD.gn b/src/app/tests/BUILD.gn
@@ -54,6 +54,18 @@ source_set("binding-test-srcs") {
   ]
 }
 
+source_set("client-monitoring-test-srcs") {
+  sources = [
+    "${chip_root}/src/app/util/ClientMonitoringRegistrationTable.cpp",
+    "${chip_root}/src/app/util/ClientMonitoringRegistrationTable.h",
+  ]
+
+  public_deps = [
+    "${chip_root}/src/app/common:cluster-objects",
+    "${chip_root}/src/lib/core",
+  ]
+}
+
 source_set("ota-requestor-test-srcs") {
   sources = [
     "${chip_root}/src/app/clusters/ota-requestor/DefaultOTARequestorStorage.cpp",
@@ -77,6 +89,7 @@ chip_test_suite("tests") {
     "TestAttributeValueEncoder.cpp",
     "TestBindingTable.cpp",
     "TestBuilderParser.cpp",
+    "TestClientMonitoringRegistrationTable.cpp",
     "TestClusterInfo.cpp",
     "TestCommandInteraction.cpp",
     "TestCommandPathParams.cpp",
@@ -117,6 +130,7 @@ chip_test_suite("tests") {
 
   public_deps = [
     ":binding-test-srcs",
+    ":client-monitoring-test-srcs",
     ":ota-requestor-test-srcs",
     "${chip_root}/src/app",
     "${chip_root}/src/app/common:cluster-objects",
diff --git a/src/app/tests/TestClientMonitoringRegistrationTable.cpp b/src/app/tests/TestClientMonitoringRegistrationTable.cpp
@@ -0,0 +1,141 @@
+/*
+ *
+ *    Copyright (c) 2022 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.
+ */
+
+#include <app/util/ClientMonitoringRegistrationTable.h>
+#include <lib/core/CHIPError.h>
+#include <lib/support/DefaultStorageKeyAllocator.h>
+#include <lib/support/TestPersistentStorageDelegate.h>
+#include <lib/support/UnitTestRegistration.h>
+#include <nlunit-test.h>
+
+using chip::ClientMonitoringRegistrationTable;
+using chip::NullOptional;
+
+namespace {
+
+constexpr chip::FabricIndex kTestFabricIndex = 1;
+constexpr uint64_t kTestICid                 = 10;
+constexpr chip::NodeId kTestClientNodeId     = 11;
+
+constexpr chip::FabricIndex kTestFabricIndex_2 = 2;
+constexpr uint64_t kTestICid_2                 = 20;
+constexpr chip::NodeId kTestClientNodeId_2     = 21;
+
+void TestDefaultClientValues(nlTestSuite * aSuite, void * aContext)
+{
+    chip::TestPersistentStorageDelegate testStorage;
+    ClientMonitoringRegistrationTable registration(testStorage);
+
+    NL_TEST_ASSERT(aSuite, !registration.GetClientRegistrationEntry().IsValid());
+    NL_TEST_ASSERT(aSuite, registration.GetClientRegistrationEntry().clientNodeId == chip::kUndefinedFabricIndex);
+    NL_TEST_ASSERT(aSuite, registration.GetClientRegistrationEntry().ICid == chip::kUndefinedNodeId);
+    NL_TEST_ASSERT(aSuite, registration.GetClientRegistrationEntry().fabricIndex == chip::kInvalidIcId);
+}
+
+void TestLoadFromStorageEmptyValue(nlTestSuite * aSuite, void * aContext)
+{
+    chip::TestPersistentStorageDelegate testStorage;
+    ClientMonitoringRegistrationTable registration(testStorage);
+
+    CHIP_ERROR err = registration.LoadFromStorage(kTestFabricIndex);
+    NL_TEST_ASSERT(aSuite, err == CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND);
+}
+
+void TestSaveAndLoadRegistrationValue(nlTestSuite * aSuite, void * aContext)
+{
+    chip::TestPersistentStorageDelegate testStorage;
+    ClientMonitoringRegistrationTable savedRegistration(testStorage);
+    ClientMonitoringRegistrationTable loadedRegistration(testStorage);
+
+    savedRegistration.GetClientRegistrationEntry().clientNodeId = kTestClientNodeId;
+    savedRegistration.GetClientRegistrationEntry().ICid         = kTestICid;
+    savedRegistration.GetClientRegistrationEntry().fabricIndex  = kTestFabricIndex;
+
+    CHIP_ERROR err = savedRegistration.SaveToStorage();
+    NL_TEST_ASSERT(aSuite, err == CHIP_NO_ERROR);
+
+    err = loadedRegistration.LoadFromStorage(kTestFabricIndex);
+    NL_TEST_ASSERT(aSuite, err == CHIP_NO_ERROR);
+
+    NL_TEST_ASSERT(aSuite, loadedRegistration.GetClientRegistrationEntry().clientNodeId == kTestClientNodeId);
+    NL_TEST_ASSERT(aSuite, loadedRegistration.GetClientRegistrationEntry().ICid == kTestICid);
+    NL_TEST_ASSERT(aSuite, loadedRegistration.GetClientRegistrationEntry().fabricIndex == kTestFabricIndex);
+}
+
+void TestSaveLoadRegistrationValueForMultipleFabrics(nlTestSuite * aSuite, void * aContexT)
+{
+    chip::TestPersistentStorageDelegate testStorage;
+    ClientMonitoringRegistrationTable savedRegistration(testStorage);
+    ClientMonitoringRegistrationTable loadedRegistration(testStorage);
+
+    savedRegistration.GetClientRegistrationEntry().clientNodeId = kTestClientNodeId;
+    savedRegistration.GetClientRegistrationEntry().ICid         = kTestICid;
+    savedRegistration.GetClientRegistrationEntry().fabricIndex  = kTestFabricIndex;
+
+    CHIP_ERROR err = savedRegistration.SaveToStorage();
+    NL_TEST_ASSERT(aSuite, err == CHIP_NO_ERROR);
+
+    savedRegistration.GetClientRegistrationEntry().clientNodeId = kTestClientNodeId_2;
+    savedRegistration.GetClientRegistrationEntry().ICid         = kTestICid_2;
+    savedRegistration.GetClientRegistrationEntry().fabricIndex  = kTestFabricIndex_2;
+
+    err = savedRegistration.SaveToStorage();
+    NL_TEST_ASSERT(aSuite, err == CHIP_NO_ERROR);
+
+    err = loadedRegistration.LoadFromStorage(kTestFabricIndex);
+    NL_TEST_ASSERT(aSuite, err == CHIP_NO_ERROR);
+
+    NL_TEST_ASSERT(aSuite, loadedRegistration.GetClientRegistrationEntry().clientNodeId == kTestClientNodeId);
+    NL_TEST_ASSERT(aSuite, loadedRegistration.GetClientRegistrationEntry().ICid == kTestICid);
+    NL_TEST_ASSERT(aSuite, loadedRegistration.GetClientRegistrationEntry().fabricIndex == kTestFabricIndex);
+
+    err = loadedRegistration.LoadFromStorage(kTestFabricIndex_2);
+    NL_TEST_ASSERT(aSuite, err == CHIP_NO_ERROR);
+
+    NL_TEST_ASSERT(aSuite, loadedRegistration.GetClientRegistrationEntry().clientNodeId == kTestClientNodeId_2);
+    NL_TEST_ASSERT(aSuite, loadedRegistration.GetClientRegistrationEntry().ICid == kTestICid_2);
+    NL_TEST_ASSERT(aSuite, loadedRegistration.GetClientRegistrationEntry().fabricIndex == kTestFabricIndex_2);
+}
+
+void TestSaveAllInvalidRegistrationValues(nlTestSuite * aSuite, void * context)
+{
+    chip::TestPersistentStorageDelegate testStorage;
+    ClientMonitoringRegistrationTable registration(testStorage);
+
+    CHIP_ERROR err = registration.SaveToStorage();
+    NL_TEST_ASSERT(aSuite, err == CHIP_ERROR_INCORRECT_STATE);
+}
+
+} // namespace
+
+int TestClientMonitoringRegistrationTable()
+{
+    static nlTest sTests[] = { NL_TEST_DEF("TestDefaultClientValues", TestDefaultClientValues),
+                               NL_TEST_DEF("TestLoadFromStorageEmptyValue", TestLoadFromStorageEmptyValue),
+                               NL_TEST_DEF("TestSaveAndLoadRegistrationValue", TestSaveAndLoadRegistrationValue),
+                               NL_TEST_DEF("TestSaveAllInvalidRegistrationValues", TestSaveAllInvalidRegistrationValues),
+                               NL_TEST_DEF("TestSaveLoadRegistrationValueForMultipleFabrics",
+                                           TestSaveLoadRegistrationValueForMultipleFabrics),
+                               NL_TEST_SENTINEL() };
+
+    nlTestSuite cmSuite = { "TestClientMonitoringRegistrationTable", &sTests[0], nullptr, nullptr };
+
+    nlTestRunner(&cmSuite, nullptr);
+    return (nlTestRunnerStats(&cmSuite));
+}
+
+CHIP_REGISTER_TEST_SUITE(TestClientMonitoringRegistrationTable)
diff --git a/src/app/util/ClientMonitoringRegistrationTable.cpp b/src/app/util/ClientMonitoringRegistrationTable.cpp
@@ -17,59 +17,53 @@
 
 #include "ClientMonitoringRegistrationTable.h"
 
-namespace chip {
+#include <lib/support/DefaultStorageKeyAllocator.h>
 
-/**********************************************************
- * Attributes Definition
- *********************************************************/
+namespace chip {
 
 /**********************************************************
  * ClientMonitoringRegistrationTable Implementation
  *********************************************************/
 
-ClientMonitoringRegistrationTable::ClientMonitoringRegistrationTable(FabricIndex fabricIndex)
-{
-    this->LoadFromStorage(fabricIndex);
-}
+ClientMonitoringRegistrationTable::ClientMonitoringRegistrationTable(PersistentStorageDelegate & storage) : mStorage(storage) {}
 
-void ClientMonitoringRegistrationTable::LoadFromStorage(FabricIndex fabricIndex)
+CHIP_ERROR ClientMonitoringRegistrationTable::LoadFromStorage(FabricIndex fabricIndex)
 {
-    // TODO: Implement load from NVM logic
-}
+    uint8_t buffer[kRegStorageSize] = { 0 };
+    uint16_t size                   = sizeof(buffer);
 
-void ClientMonitoringRegistrationTable::SaveToStorage()
-{
-    // Store to NVM based of class attributes
-}
+    ReturnErrorOnFailure(
+        mStorage.SyncGetKeyValue(DefaultStorageKeyAllocator::ClientMonitoringTableEntry(fabricIndex).KeyName(), buffer, size));
 
-NodeId ClientMonitoringRegistrationTable::getClientNodeId()
-{
-    return mRegisteredClient.clientNodeId;
-}
+    TLV::TLVReader reader;
+    reader.Init(buffer, size);
+    ReturnErrorOnFailure(reader.Next(TLV::kTLVType_Structure, TLV::AnonymousTag()));
 
-uint64_t ClientMonitoringRegistrationTable::getICid()
-{
-    return mRegisteredClient.ICid;
-}
+    ReturnErrorOnFailure(mRegisteredClient.Decode(reader));
 
-FabricIndex ClientMonitoringRegistrationTable::getFaricIndex()
-{
-    return mRegisteredClient.fabricIndex;
-}
+    mRegisteredClient.fabricIndex = fabricIndex;
 
-void ClientMonitoringRegistrationTable::setClientNodeId(NodeId clientNodeId)
-{
-    mRegisteredClient.clientNodeId = clientNodeId;
+    return CHIP_NO_ERROR;
 }
 
-void ClientMonitoringRegistrationTable::setICid(uint64_t ICid)
+CHIP_ERROR ClientMonitoringRegistrationTable::SaveToStorage()
 {
-    mRegisteredClient.ICid = ICid;
+    VerifyOrReturnError(mRegisteredClient.IsValid(), CHIP_ERROR_INCORRECT_STATE);
+
+    uint8_t buffer[kRegStorageSize] = { 0 };
+    TLV::TLVWriter writer;
+
+    writer.Init(buffer);
+    ReturnErrorOnFailure(mRegisteredClient.EncodeForWrite(writer, TLV::AnonymousTag()));
+    ReturnErrorOnFailure(writer.Finalize());
+
+    return mStorage.SyncSetKeyValue(DefaultStorageKeyAllocator::ClientMonitoringTableEntry(mRegisteredClient.fabricIndex).KeyName(),
+                                    buffer, static_cast<uint16_t>(writer.GetLengthWritten()));
 }
 
-void ClientMonitoringRegistrationTable::setFabricIndex(FabricIndex fabric)
+ClientMonitoringRegistrationTable::ClientRegistrationEntry & ClientMonitoringRegistrationTable::GetClientRegistrationEntry()
 {
-    mRegisteredClient.fabricIndex = fabric;
+    return mRegisteredClient;
 }
 
 } // namespace chip
diff --git a/src/app/util/ClientMonitoringRegistrationTable.h b/src/app/util/ClientMonitoringRegistrationTable.h
@@ -19,33 +19,62 @@
 
 #include <app-common/zap-generated/cluster-objects.h>
 #include <lib/core/CHIPConfig.h>
+#include <lib/core/CHIPPersistentStorageDelegate.h>
+#include <lib/core/DataModelTypes.h>
 #include <lib/support/CodeUtils.h>
-#include <platform/CHIPDeviceConfig.h>
 
 namespace chip {
+
+/**
+ * @brief ClientMonitoringRegistrationTable exists to manage the persistence of entries in the ClientMonitoring Cluster.
+ *        To access persisted data with the ClientMonitoringRegistrationTable class, instantiate an instance of this class
+ *        and call the LoadFromStorage function.
+ *
+ *        This class can only manage one fabric at a time. The flow is load a fabric, execute necessary operations,
+ *        save it if there are any changes and load another fabric.
+ */
 class ClientMonitoringRegistrationTable
 {
 public:
     using MonitoringRegistrationStruct = chip::app::Clusters::ClientMonitoring::Structs::MonitoringRegistration::Type;
 
-    ClientMonitoringRegistrationTable(FabricIndex fabricIndex);
+    struct ClientRegistrationEntry : MonitoringRegistrationStruct
+    {
+        bool IsValid() { return clientNodeId != kUndefinedNodeId && ICid != kInvalidIcId && fabricIndex != kUndefinedFabricIndex; }
+    };
+
+    ClientMonitoringRegistrationTable(PersistentStorageDelegate & storage);
     ~ClientMonitoringRegistrationTable(){};
 
-    void SaveToStorage();
+    /**
+     * @brief Function saves the mRegisteredClient attribute to persitant storage
+     *        To correctly persit an entry, the values must be stored in the structures attributes
+     *
+     * @return CHIP_ERROR
+     */
+    CHIP_ERROR SaveToStorage();
 
-    // Getter
-    NodeId getClientNodeId();
-    uint64_t getICid();
-    FabricIndex getFaricIndex();
+    /**
+     * @brief Function loads a client registration entry from persistent storage for a single fabric
+     *
+     * @param[in] fabricIndex fabric index to load from storage
+     * @return CHIP_ERROR
+     */
+    CHIP_ERROR LoadFromStorage(FabricIndex fabricIndex);
 
-    // Setter
-    void setClientNodeId(NodeId clientNodeId);
-    void setICid(uint64_t ICid);
-    void setFabricIndex(FabricIndex fabric);
+    /**
+     * @brief Accessor function that returns the client registration entry that was loaded for a fabric from persistant storage.
+     * @see LoadFromStorage
+     *
+     * @return ClientMonitoringRegistrationTable::ClientRegistrationEntry&
+     */
+    ClientRegistrationEntry & GetClientRegistrationEntry();
 
 private:
-    void LoadFromStorage(FabricIndex fabricIndex);
-    MonitoringRegistrationStruct mRegisteredClient;
+    static constexpr uint8_t kRegStorageSize = TLV::EstimateStructOverhead(sizeof(NodeId), sizeof(uint64_t));
+
+    ClientRegistrationEntry mRegisteredClient;
+    PersistentStorageDelegate & mStorage;
 };
 
 } // namespace chip
diff --git a/src/lib/core/DataModelTypes.h b/src/lib/core/DataModelTypes.h
@@ -58,6 +58,10 @@ constexpr EndpointId kRootEndpointId    = 0;
 constexpr ListIndex kInvalidListIndex   = 0xFFFF; // List index is a uint16 thus 0xFFFF is a invalid list index.
 constexpr KeysetId kInvalidKeysetId     = 0xFFFF;
 
+// Invalid IC identifier is provisional. Value will most likely change when identifying token is defined
+// https://github.com/project-chip/connectedhomeip/issues/24251
+constexpr uint64_t kInvalidIcId = 0;
+
 // These are MEIs, 0xFFFF is not a valid manufacturer code,
 // thus 0xFFFF'FFFF is not a valid MEI.
 static constexpr ClusterId kInvalidClusterId     = 0xFFFF'FFFF;
diff --git a/src/lib/support/DefaultStorageKeyAllocator.h b/src/lib/support/DefaultStorageKeyAllocator.h
@@ -175,6 +175,13 @@ class DefaultStorageKeyAllocator
     static StorageKeyName BindingTable() { return StorageKeyName::FromConst("g/bt"); }
     static StorageKeyName BindingTableEntry(uint8_t index) { return StorageKeyName::Formatted("g/bt/%x", index); }
 
+    // Client Monitoring
+
+    static StorageKeyName ClientMonitoringTableEntry(chip::FabricIndex fabric)
+    {
+        return StorageKeyName::Formatted("f/%x/cm", fabric);
+    }
+
     static StorageKeyName OTADefaultProviders() { return StorageKeyName::FromConst("g/o/dp"); }
     static StorageKeyName OTACurrentProvider() { return StorageKeyName::FromConst("g/o/cp"); }
     static StorageKeyName OTAUpdateToken() { return StorageKeyName::FromConst("g/o/ut"); }
diff --git a/src/lib/support/UnitTestRegistration.cpp b/src/lib/support/UnitTestRegistration.cpp
@@ -22,7 +22,7 @@
 
 namespace chip {
 
-const size_t kTestSuitesMax = 128;
+const size_t kTestSuitesMax = 256;
 
 typedef struct
 {

Original file line number	Diff line number	Diff line change
`@@ -22,7 +22,7 @@`
`22`	`22`
`23`	`23`	`namespace chip {`
`24`	`24`
`25`		`-const size_t kTestSuitesMax = 128;`
	`25`	`+const size_t kTestSuitesMax = 256;`
`26`	`26`
`27`	`27`	`typedef struct`
`28`	`28`	`{`