[linux] Implement reading/writing UserLabelList from/to persistent storage (#16975)

yufengwangca · bzbarsky-apple · pull[bot] · commit 77d0df961d5f · 2023-12-19T18:23:00.000Z
* Implment reading/writing user labelList from/to persistent storage

* Add userLabel yaml test

* Address review comments

* Update src/platform/Linux/DeviceInfoProviderImpl.cpp

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

* Update src/platform/Linux/DeviceInfoProviderImpl.cpp

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

* Update src/platform/Linux/DeviceInfoProviderImpl.cpp

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

* Update test case

Co-authored-by: Boris Zbarsky &lt;bzbarsky@apple.com&gt;
diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml
@@ -187,7 +187,7 @@ jobs:
                   ./scripts/run_in_build_env.sh \
                   "./scripts/tests/run_test_suite.py \
                      --chip-tool ./out/darwin-x64-chip-tool${CHIP_TOOL_VARIANT}-${BUILD_VARIANT}/chip-tool \
-                     --target-skip-glob '{TestGroupMessaging}' \
+                     --target-skip-glob '{TestGroupMessaging,TestUserLabelCluster}' \
                      run \
                      --iterations 1 \
                      --all-clusters-app ./out/darwin-x64-all-clusters-${BUILD_VARIANT}/chip-all-clusters-app \
diff --git a/examples/chip-tool/templates/tests/tests.js b/examples/chip-tool/templates/tests/tests.js
@@ -547,6 +547,7 @@ function getTests()
     'TestModeSelectCluster',
     'TestSystemCommands',
     'TestBinding',
+    'TestUserLabelCluster',
   ];
 
   const MultiAdmin = [
diff --git a/src/app/tests/suites/TestUserLabelCluster.yaml b/src/app/tests/suites/TestUserLabelCluster.yaml
@@ -0,0 +1,84 @@
+# 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.
+
+name: User Label Cluster Tests
+
+config:
+    nodeId: 0x12344321
+    cluster: "User Label"
+    endpoint: 0
+    discriminator:
+        type: INT16U
+        defaultValue: 3840
+
+tests:
+    - label: "Wait for the commissioned device to be retrieved"
+      cluster: "DelayCommands"
+      command: "WaitForCommissionee"
+      arguments:
+          values:
+              - name: "nodeId"
+                value: nodeId
+
+    - label: "Clear User Label List"
+      command: "writeAttribute"
+      attribute: "label list"
+      arguments:
+          value: []
+
+    - label: "Read User Label List"
+      command: "readAttribute"
+      attribute: "label list"
+      response:
+          value: []
+
+    - label: "Write User Label List"
+      command: "writeAttribute"
+      attribute: "label list"
+      arguments:
+          value:
+              [
+                  { label: "room", value: "bedroom 2" },
+                  { label: "orientation", value: "North" },
+                  { label: "floor", value: "5" },
+                  { label: "direction", value: "up" },
+              ]
+
+    - label: "Reboot target device"
+      cluster: "SystemCommands"
+      command: "Reboot"
+      arguments:
+          values:
+              - name: "discriminator"
+                value: discriminator
+
+    - label: "Wait for the commissioned device to be retrieved"
+      cluster: "DelayCommands"
+      command: "WaitForCommissionee"
+      arguments:
+          values:
+              - name: "nodeId"
+                value: nodeId
+
+    - label: "Verify"
+      command: "readAttribute"
+      attribute: "label list"
+      response:
+          value:
+              [
+                  { label: "room", value: "bedroom 2" },
+                  { label: "orientation", value: "North" },
+                  { label: "floor", value: "5" },
+                  { label: "direction", value: "up" },
+              ]
diff --git a/src/lib/support/DefaultStorageKeyAllocator.h b/src/lib/support/DefaultStorageKeyAllocator.h
@@ -76,6 +76,10 @@ class DefaultStorageKeyAllocator
     const char * GroupDataCounter() { return Format("g/gdc"); }
     const char * GroupControlCounter() { return Format("g/gcc"); }
 
+    // Device Information Provider
+    const char * UserLabelLengthKey(EndpointId endpoint) { return Format("g/userlbl/%x", endpoint); }
+    const char * UserLabelIndexKey(EndpointId endpoint, uint32_t index) { return Format("g/userlbl/%x/%" PRIx32, endpoint, index); }
+
     // Group Data Provider
 
     // List of fabric indices that have endpoint-to-group associations defined.
diff --git a/src/platform/Linux/CHIPLinuxStorage.h b/src/platform/Linux/CHIPLinuxStorage.h
@@ -48,6 +48,10 @@
 #define LOCALSTATEDIR "/tmp"
 #endif
 
+#ifndef DEVICEINFODIR
+#define DEVICEINFODIR "/tmp"
+#endif
+
 #define CHIP_DEFAULT_FACTORY_PATH                                                                                                  \
     FATCONFDIR "/"                                                                                                                 \
                "chip_factory.ini"
@@ -57,6 +61,9 @@
 #define CHIP_DEFAULT_DATA_PATH                                                                                                     \
     LOCALSTATEDIR "/"                                                                                                              \
                   "chip_counters.ini"
+#define CHIP_DEVICE_INFO_PATH                                                                                                      \
+    DEVICEINFODIR "/"                                                                                                              \
+                  "chip_device_info.ini"
 
 namespace chip {
 namespace DeviceLayer {
diff --git a/src/platform/Linux/DeviceInfoProviderImpl.cpp b/src/platform/Linux/DeviceInfoProviderImpl.cpp
@@ -18,6 +18,7 @@
 #include <lib/core/CHIPTLV.h>
 #include <lib/support/CHIPMemString.h>
 #include <lib/support/CodeUtils.h>
+#include <lib/support/DefaultStorageKeyAllocator.h>
 #include <platform/Linux/DeviceInfoProviderImpl.h>
 #include <platform/internal/CHIPDeviceLayerInternal.h>
 
@@ -27,6 +28,16 @@
 namespace chip {
 namespace DeviceLayer {
 
+namespace {
+constexpr TLV::Tag kLabelNameTag  = TLV::ContextTag(0);
+constexpr TLV::Tag kLabelValueTag = TLV::ContextTag(1);
+} // anonymous namespace
+
+CHIP_ERROR DeviceInfoProviderImpl::Init()
+{
+    return mStorage.Init(CHIP_DEVICE_INFO_PATH);
+}
+
 DeviceInfoProviderImpl & DeviceInfoProviderImpl::GetDefaultInstance()
 {
     static DeviceInfoProviderImpl sInstance;
@@ -107,23 +118,33 @@ bool DeviceInfoProviderImpl::FixedLabelIteratorImpl::Next(FixedLabelType & outpu
 
 CHIP_ERROR DeviceInfoProviderImpl::SetUserLabelLength(EndpointId endpoint, size_t val)
 {
-    // TODO:: store the user label count.
-    return CHIP_ERROR_NOT_IMPLEMENTED;
+    DefaultStorageKeyAllocator keyAlloc;
+
+    return mStorage.WriteValue(keyAlloc.UserLabelLengthKey(endpoint), val);
 }
 
 CHIP_ERROR DeviceInfoProviderImpl::GetUserLabelLength(EndpointId endpoint, size_t & val)
 {
-    // TODO:: read the user label count. temporarily return the size of hardcoded labelList.
-    val = 4;
+    DefaultStorageKeyAllocator keyAlloc;
 
-    return CHIP_NO_ERROR;
+    return mStorage.ReadValue(keyAlloc.UserLabelLengthKey(endpoint), val);
 }
 
 CHIP_ERROR DeviceInfoProviderImpl::SetUserLabelAt(EndpointId endpoint, size_t index, const UserLabelType & userLabel)
 {
-    // TODO:: store the user labelList, and read back stored user labelList if it has been set.
-    // Add yaml test to verify this.
-    return CHIP_ERROR_NOT_IMPLEMENTED;
+    DefaultStorageKeyAllocator keyAlloc;
+    uint8_t buf[UserLabelTLVMaxSize()];
+    TLV::TLVWriter writer;
+    writer.Init(buf);
+
+    TLV::TLVType outerType;
+    ReturnErrorOnFailure(writer.StartContainer(TLV::AnonymousTag(), TLV::kTLVType_Structure, outerType));
+    ReturnErrorOnFailure(writer.PutString(kLabelNameTag, userLabel.label));
+    ReturnErrorOnFailure(writer.PutString(kLabelValueTag, userLabel.value));
+    ReturnErrorOnFailure(writer.EndContainer(outerType));
+    ReturnErrorOnFailure(mStorage.WriteValueBin(keyAlloc.UserLabelIndexKey(endpoint, index), buf, writer.GetLengthWritten()));
+
+    return mStorage.Commit();
 }
 
 DeviceInfoProvider::UserLabelIterator * DeviceInfoProviderImpl::IterateUserLabel(EndpointId endpoint)
@@ -143,57 +164,45 @@ DeviceInfoProviderImpl::UserLabelIteratorImpl::UserLabelIteratorImpl(DeviceInfoP
 
 bool DeviceInfoProviderImpl::UserLabelIteratorImpl::Next(UserLabelType & output)
 {
-    // TODO:: get the user labelList from persistent storage, temporarily, use the following
-    // hardcoded labelList on all endpoints.
     CHIP_ERROR err = CHIP_NO_ERROR;
 
-    const char * labelPtr = nullptr;
-    const char * valuePtr = nullptr;
-
     VerifyOrReturnError(mIndex < mTotal, false);
 
-    switch (mIndex)
-    {
-    case 0:
-        labelPtr = "room";
-        valuePtr = "bedroom 2";
-        break;
-    case 1:
-        labelPtr = "orientation";
-        valuePtr = "North";
-        break;
-    case 2:
-        labelPtr = "floor";
-        valuePtr = "2";
-        break;
-    case 3:
-        labelPtr = "direction";
-        valuePtr = "up";
-        break;
-    default:
-        err = CHIP_ERROR_PERSISTED_STORAGE_VALUE_NOT_FOUND;
-        break;
-    }
+    DefaultStorageKeyAllocator keyAlloc;
+    uint8_t buf[UserLabelTLVMaxSize()];
+    size_t outLen;
+    err = mProvider.mStorage.ReadValueBin(keyAlloc.UserLabelIndexKey(mEndpoint, mIndex), buf, sizeof(buf), outLen);
+    VerifyOrReturnError(err == CHIP_NO_ERROR, false);
 
-    if (err == CHIP_NO_ERROR)
-    {
-        VerifyOrReturnError(std::strlen(labelPtr) <= kMaxLabelNameLength, false);
-        VerifyOrReturnError(std::strlen(valuePtr) <= kMaxLabelValueLength, false);
+    TLV::ContiguousBufferTLVReader reader;
+    reader.Init(buf);
+    err = reader.Next(TLV::kTLVType_Structure, TLV::AnonymousTag());
+    VerifyOrReturnError(err == CHIP_NO_ERROR, false);
 
-        Platform::CopyString(mUserLabelNameBuf, kMaxLabelNameLength + 1, labelPtr);
-        Platform::CopyString(mUserLabelValueBuf, kMaxLabelValueLength + 1, valuePtr);
+    TLV::TLVType containerType;
+    VerifyOrReturnError(reader.EnterContainer(containerType) == CHIP_NO_ERROR, false);
 
-        output.label = CharSpan::fromCharString(mUserLabelNameBuf);
-        output.value = CharSpan::fromCharString(mUserLabelValueBuf);
+    chip::CharSpan label;
+    chip::CharSpan value;
 
-        mIndex++;
+    VerifyOrReturnError(reader.Next(kLabelNameTag) == CHIP_NO_ERROR, false);
+    VerifyOrReturnError(reader.Get(label) == CHIP_NO_ERROR, false);
 
-        return true;
-    }
-    else
-    {
-        return false;
-    }
+    VerifyOrReturnError(reader.Next(kLabelValueTag) == CHIP_NO_ERROR, false);
+    VerifyOrReturnError(reader.Get(value) == CHIP_NO_ERROR, false);
+
+    VerifyOrReturnError(reader.VerifyEndOfContainer() == CHIP_NO_ERROR, false);
+    VerifyOrReturnError(reader.ExitContainer(containerType) == CHIP_NO_ERROR, false);
+
+    Platform::CopyString(mUserLabelNameBuf, label);
+    Platform::CopyString(mUserLabelValueBuf, value);
+
+    output.label = CharSpan::fromCharString(mUserLabelNameBuf);
+    output.value = CharSpan::fromCharString(mUserLabelValueBuf);
+
+    mIndex++;
+
+    return true;
 }
 
 DeviceInfoProvider::SupportedLocalesIterator * DeviceInfoProviderImpl::IterateSupportedLocales()
diff --git a/src/platform/Linux/DeviceInfoProviderImpl.h b/src/platform/Linux/DeviceInfoProviderImpl.h
@@ -16,7 +16,9 @@
  */
 #pragma once
 
+#include <lib/support/EnforceFormat.h>
 #include <platform/DeviceInfoProvider.h>
+#include <platform/Linux/CHIPLinuxStorage.h>
 
 namespace chip {
 namespace DeviceLayer {
@@ -27,6 +29,15 @@ class DeviceInfoProviderImpl : public DeviceInfoProvider
     DeviceInfoProviderImpl() = default;
     ~DeviceInfoProviderImpl() override {}
 
+    /**
+     *  Initialize the DeviceInfoProvider, including possibly any persistent
+     *  data store initialization done by the implementation. Must be called once
+     *  before any other API succeeds.
+     *
+     *  @retval #CHIP_NO_ERROR on success
+     */
+    CHIP_ERROR Init();
+
     // Iterators
     FixedLabelIterator * IterateFixedLabel(EndpointId endpoint) override;
     UserLabelIterator * IterateUserLabel(EndpointId endpoint) override;
@@ -96,6 +107,11 @@ class DeviceInfoProviderImpl : public DeviceInfoProvider
     CHIP_ERROR SetUserLabelLength(EndpointId endpoint, size_t val) override;
     CHIP_ERROR GetUserLabelLength(EndpointId endpoint, size_t & val) override;
     CHIP_ERROR SetUserLabelAt(EndpointId endpoint, size_t index, const UserLabelType & userLabel) override;
+
+private:
+    DeviceLayer::Internal::ChipLinuxStorage mStorage;
+
+    static constexpr size_t UserLabelTLVMaxSize() { return TLV::EstimateStructOverhead(kMaxLabelNameLength, kMaxLabelValueLength); }
 };
 
 } // namespace DeviceLayer
diff --git a/src/platform/Linux/PlatformManagerImpl.cpp b/src/platform/Linux/PlatformManagerImpl.cpp
@@ -174,7 +174,6 @@ void PlatformManagerImpl::WiFIIPChangeListener()
 
 CHIP_ERROR PlatformManagerImpl::_InitChipStack()
 {
-    CHIP_ERROR err;
     struct sigaction action;
 
     memset(&action, 0, sizeof(action));
@@ -200,21 +199,19 @@ CHIP_ERROR PlatformManagerImpl::_InitChipStack()
 #endif
 
     // Initialize the configuration system.
-    err = Internal::PosixConfig::Init();
-    SuccessOrExit(err);
+    ReturnErrorOnFailure(Internal::PosixConfig::Init());
     SetConfigurationMgr(&ConfigurationManagerImpl::GetDefaultInstance());
     SetDiagnosticDataProvider(&DiagnosticDataProviderImpl::GetDefaultInstance());
     SetDeviceInfoProvider(&DeviceInfoProviderImpl::GetDefaultInstance());
+    ReturnErrorOnFailure(DeviceInfoProviderImpl::GetDefaultInstance().Init());
 
     // Call _InitChipStack() on the generic implementation base class
     // to finish the initialization process.
-    err = Internal::GenericPlatformManagerImpl_POSIX<PlatformManagerImpl>::_InitChipStack();
-    SuccessOrExit(err);
+    ReturnErrorOnFailure(Internal::GenericPlatformManagerImpl_POSIX<PlatformManagerImpl>::_InitChipStack());
 
     mStartTime = System::SystemClock().GetMonotonicTimestamp();
 
-exit:
-    return err;
+    return CHIP_NO_ERROR;
 }
 
 CHIP_ERROR PlatformManagerImpl::_Shutdown()
diff --git a/zzz_generated/chip-tool/zap-generated/test/Commands.h b/zzz_generated/chip-tool/zap-generated/test/Commands.h
