Encode the user label list in place so that we can free the label lis… (

project-chip#16510) * Encode the user label list in place so that we can free the label list after usage * Update src/app/clusters/user-label-server/user-label-server.cpp Co-authored-by: Boris Zbarsky <[email protected]> * Refactor userlabellist API to DeviceInfoProvider * Add API documentation * Update src/include/platform/DeviceInfoProvider.h Co-authored-by: Boris Zbarsky <[email protected]> * Update src/include/platform/DeviceInfoProvider.h Co-authored-by: Boris Zbarsky <[email protected]> * Rename SetUserLabelCount/GetUserLabelCount to SetUserLabelLength/GetUserLabelLength Co-authored-by: Boris Zbarsky <[email protected]>
chencheung · Apr 6, 2022 · b2141df · b2141df
1 parent 2d3cd5e
commit b2141df
Show file tree

Hide file tree

Showing 12 changed files with 210 additions and 119 deletions.
diff --git a/src/app/clusters/user-label-server/user-label-server.cpp b/src/app/clusters/user-label-server/user-label-server.cpp
@@ -58,18 +58,32 @@ UserLabelAttrAccess gAttrAccess;
 CHIP_ERROR UserLabelAttrAccess::ReadLabelList(EndpointId endpoint, AttributeValueEncoder & aEncoder)
 {
     CHIP_ERROR err = CHIP_NO_ERROR;
-    DeviceLayer::AttributeList<app::Clusters::UserLabel::Structs::LabelStruct::Type, DeviceLayer::kMaxUserLabels> labelList;
 
-    if (DeviceLayer::PlatformMgr().GetUserLabelList(endpoint, labelList) == CHIP_NO_ERROR)
+    DeviceLayer::DeviceInfoProvider * provider = DeviceLayer::GetDeviceInfoProvider();
+
+    if (provider)
     {
-        err = aEncoder.EncodeList([&labelList](const auto & encoder) -> CHIP_ERROR {
-            for (auto label : labelList)
-            {
-                ReturnErrorOnFailure(encoder.Encode(label));
-            }
-
-            return CHIP_NO_ERROR;
-        });
+        DeviceLayer::DeviceInfoProvider::UserLabelIterator * it = provider->IterateUserLabel(endpoint);
+
+        if (it)
+        {
+            err = aEncoder.EncodeList([&it](const auto & encoder) -> CHIP_ERROR {
+                UserLabel::Structs::LabelStruct::Type userlabel;
+
+                while (it->Next(userlabel))
+                {
+                    ReturnErrorOnFailure(encoder.Encode(userlabel));
+                }
+
+                return CHIP_NO_ERROR;
+            });
+
+            it->Release();
+        }
+        else
+        {
+            err = aEncoder.EncodeEmptyList();
+        }
     }
     else
     {
@@ -84,7 +98,7 @@ CHIP_ERROR UserLabelAttrAccess::WriteLabelList(const ConcreteDataAttributePath &
     EndpointId endpoint = aPath.mEndpointId;
     if (!aPath.IsListItemOperation())
     {
-        DeviceLayer::AttributeList<Structs::LabelStruct::Type, DeviceLayer::kMaxUserLabels> labelList;
+        DeviceLayer::AttributeList<Structs::LabelStruct::Type, DeviceLayer::kMaxUserLabelListLength> labelList;
         LabelList::TypeInfo::DecodableType decodablelist;
 
         ReturnErrorOnFailure(aDecoder.Decode(decodablelist));
@@ -97,16 +111,13 @@ CHIP_ERROR UserLabelAttrAccess::WriteLabelList(const ConcreteDataAttributePath &
         }
         ReturnErrorOnFailure(iter.GetStatus());
 
-        return DeviceLayer::PlatformMgr().SetUserLabelList(endpoint, labelList);
+        return DeviceLayer::GetDeviceInfoProvider()->SetUserLabelList(endpoint, labelList);
     }
     else if (aPath.mListOp == ConcreteDataAttributePath::ListOperation::AppendItem)
     {
         Structs::LabelStruct::DecodableType entry;
-        DeviceLayer::AttributeList<Structs::LabelStruct::Type, DeviceLayer::kMaxUserLabels> labelList;
-        ReturnErrorOnFailure(DeviceLayer::PlatformMgr().GetUserLabelList(endpoint, labelList));
         ReturnErrorOnFailure(aDecoder.Decode(entry));
-        ReturnErrorOnFailure(labelList.add(entry));
-        return DeviceLayer::PlatformMgr().SetUserLabelList(endpoint, labelList);
+        return DeviceLayer::GetDeviceInfoProvider()->AppendUserLabel(endpoint, entry);
     }
     else
     {

diff --git a/src/include/platform/DeviceInfoProvider.h b/src/include/platform/DeviceInfoProvider.h
@@ -28,8 +28,9 @@
 namespace chip {
 namespace DeviceLayer {
 
-static constexpr size_t kMaxLabelNameLength  = 16;
-static constexpr size_t kMaxLabelValueLength = 16;
+static constexpr size_t kMaxUserLabelListLength = 10;
+static constexpr size_t kMaxLabelNameLength     = 16;
+static constexpr size_t kMaxLabelValueLength    = 16;
 
 class DeviceInfoProvider
 {
@@ -63,8 +64,10 @@ class DeviceInfoProvider
     };
 
     using FixedLabelType = app::Clusters::FixedLabel::Structs::LabelStruct::Type;
+    using UserLabelType  = app::Clusters::UserLabel::Structs::LabelStruct::Type;
 
     using FixedLabelIterator = Iterator<FixedLabelType>;
+    using UserLabelIterator  = Iterator<UserLabelType>;
 
     DeviceInfoProvider() = default;
 
@@ -74,15 +77,49 @@ class DeviceInfoProvider
     DeviceInfoProvider(const DeviceInfoProvider &) = delete;
     DeviceInfoProvider & operator=(const DeviceInfoProvider &) = delete;
 
+    CHIP_ERROR SetUserLabelList(EndpointId endpoint, const AttributeList<UserLabelType, kMaxUserLabelListLength> & labelList);
+    CHIP_ERROR AppendUserLabel(EndpointId endpoint, const UserLabelType & label);
+
     // Iterators
     /**
-     *  Creates an iterator that may be used to obtain the list of user labels associated with the given endpoint.
+     *  Creates an iterator that may be used to obtain the list of labels associated with the given endpoint.
      *  In order to release the allocated memory, the Release() method must be called after the iteration is finished.
-     *  Modifying the user label during the iteration is currently not supported, and may yield unexpected behaviour.
+     *  Modifying the label during the iteration is currently not supported, and may yield unexpected behaviour.
      *  @retval An instance of EndpointIterator on success
      *  @retval nullptr if no iterator instances are available.
      */
     virtual FixedLabelIterator * IterateFixedLabel(EndpointId endpoint) = 0;
+    virtual UserLabelIterator * IterateUserLabel(EndpointId endpoint)   = 0;
+
+protected:
+    /**
+     * @brief Set the UserLabel at the specified index of the UserLabelList on a given endpoint
+     *
+     * @param endpoint - id to UserLabelList on which to set the UserLabel.
+     * @param index - index within the UserLabelList for which to set the UserLabel.
+     * @param userLabel - user label to set.
+     * @return CHIP_NO_ERROR on success, CHIP_ERROR_INVALID_KEY_ID if index exceed the range (Total length - 1),
+     *         or other CHIP_ERROR values from implementation on other errors.
+     */
+    virtual CHIP_ERROR SetUserLabelAt(EndpointId endpoint, size_t index, const UserLabelType & userLabel) = 0;
+
+    /**
+     * @brief Set the total length of the UserLabelList on a given endpoint
+     *
+     * @param endpoint - id of the UserLabelList.
+     * @param val - total count of the UserLabelList.
+     * @return CHIP_NO_ERROR on success, other CHIP_ERROR values from implementation on other errors.
+     */
+    virtual CHIP_ERROR SetUserLabelLength(EndpointId endpoint, size_t val) = 0;
+
+    /**
+     * @brief Get the total length of the UserLabelList on a given endpoint
+     *
+     * @param endpoint - id of the UserLabelList.
+     * @param val - output of the total count of the UserLabelList.
+     * @return CHIP_NO_ERROR on success, other CHIP_ERROR values from implementation on other errors.
+     */
+    virtual CHIP_ERROR GetUserLabelLength(EndpointId endpoint, size_t & val) = 0;
 };
 
 /**

diff --git a/src/include/platform/PlatformManager.h b/src/include/platform/PlatformManager.h
@@ -37,7 +37,6 @@ class DiscoveryImplPlatform;
 
 namespace DeviceLayer {
 
-static constexpr size_t kMaxUserLabels    = 10;
 static constexpr size_t kMaxLanguageTags  = 254; // Maximum number of entry type 'ARRAY' supports
 static constexpr size_t kMaxCalendarTypes = 12;
 
@@ -194,10 +193,6 @@ class PlatformManager
     bool IsChipStackLockedByCurrentThread() const;
 #endif
 
-    CHIP_ERROR SetUserLabelList(EndpointId endpoint,
-                                AttributeList<app::Clusters::UserLabel::Structs::LabelStruct::Type, kMaxUserLabels> & labelList);
-    CHIP_ERROR GetUserLabelList(EndpointId endpoint,
-                                AttributeList<app::Clusters::UserLabel::Structs::LabelStruct::Type, kMaxUserLabels> & labelList);
     CHIP_ERROR GetSupportedLocales(AttributeList<chip::CharSpan, kMaxLanguageTags> & supportedLocales);
     CHIP_ERROR GetSupportedCalendarTypes(
         AttributeList<app::Clusters::TimeFormatLocalization::CalendarType, kMaxCalendarTypes> & supportedCalendarTypes);
@@ -453,19 +448,6 @@ inline CHIP_ERROR PlatformManager::StartChipTimer(System::Clock::Timeout duratio
 {
     return static_cast<ImplClass *>(this)->_StartChipTimer(duration);
 }
-inline CHIP_ERROR
-PlatformManager::SetUserLabelList(EndpointId endpoint,
-                                  AttributeList<app::Clusters::UserLabel::Structs::LabelStruct::Type, kMaxUserLabels> & labelList)
-{
-    return static_cast<ImplClass *>(this)->_SetUserLabelList(endpoint, labelList);
-}
-
-inline CHIP_ERROR
-PlatformManager::GetUserLabelList(EndpointId endpoint,
-                                  AttributeList<app::Clusters::UserLabel::Structs::LabelStruct::Type, kMaxUserLabels> & labelList)
-{
-    return static_cast<ImplClass *>(this)->_GetUserLabelList(endpoint, labelList);
-}
 
 inline CHIP_ERROR PlatformManager::GetSupportedLocales(AttributeList<chip::CharSpan, kMaxLanguageTags> & supportedLocales)
 {

diff --git a/src/include/platform/internal/GenericPlatformManagerImpl.h b/src/include/platform/internal/GenericPlatformManagerImpl.h
@@ -60,10 +60,6 @@ class GenericPlatformManagerImpl
     void _ScheduleWork(AsyncWorkFunct workFunct, intptr_t arg);
     void _DispatchEvent(const ChipDeviceEvent * event);
 
-    CHIP_ERROR _SetUserLabelList(EndpointId endpoint,
-                                 AttributeList<app::Clusters::UserLabel::Structs::LabelStruct::Type, kMaxUserLabels> & labelList);
-    CHIP_ERROR _GetUserLabelList(EndpointId endpoint,
-                                 AttributeList<app::Clusters::UserLabel::Structs::LabelStruct::Type, kMaxUserLabels> & labelList);
     CHIP_ERROR _GetSupportedLocales(AttributeList<chip::CharSpan, kMaxLanguageTags> & supportedLocales);
     CHIP_ERROR _GetSupportedCalendarTypes(
         AttributeList<app::Clusters::TimeFormatLocalization::CalendarType, kMaxCalendarTypes> & supportedCalendarTypes);
@@ -83,20 +79,6 @@ class GenericPlatformManagerImpl
 // Instruct the compiler to instantiate the template only when explicitly told to do so.
 extern template class GenericPlatformManagerImpl<PlatformManagerImpl>;
 
-template <class ImplClass>
-inline CHIP_ERROR GenericPlatformManagerImpl<ImplClass>::_SetUserLabelList(
-    EndpointId endpoint, AttributeList<app::Clusters::UserLabel::Structs::LabelStruct::Type, kMaxUserLabels> & labelList)
-{
-    return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE;
-}
-
-template <class ImplClass>
-inline CHIP_ERROR GenericPlatformManagerImpl<ImplClass>::_GetUserLabelList(
-    EndpointId endpoint, AttributeList<app::Clusters::UserLabel::Structs::LabelStruct::Type, kMaxUserLabels> & labelList)
-{
-    return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE;
-}
-
 template <class ImplClass>
 inline CHIP_ERROR
 GenericPlatformManagerImpl<ImplClass>::_GetSupportedLocales(AttributeList<chip::CharSpan, kMaxLanguageTags> & supportedLocales)

diff --git a/src/platform/Darwin/PlatformManagerImpl.cpp b/src/platform/Darwin/PlatformManagerImpl.cpp
@@ -136,13 +136,6 @@ CHIP_ERROR PlatformManagerImpl::_PostEvent(const ChipDeviceEvent * event)
     return CHIP_NO_ERROR;
 }
 
-CHIP_ERROR
-PlatformManagerImpl::_SetUserLabelList(
-    EndpointId endpoint, AttributeList<app::Clusters::UserLabel::Structs::LabelStruct::Type, kMaxUserLabels> & labelList)
-{
-    return CHIP_NO_ERROR;
-}
-
 CHIP_ERROR
 PlatformManagerImpl::_GetSupportedLocales(AttributeList<chip::CharSpan, kMaxLanguageTags> & supportedLocales)
 {

diff --git a/src/platform/Darwin/PlatformManagerImpl.h b/src/platform/Darwin/PlatformManagerImpl.h
@@ -64,8 +64,6 @@ class PlatformManagerImpl final : public PlatformManager, public Internal::Gener
     CHIP_ERROR _StartEventLoopTask();
     CHIP_ERROR _StopEventLoopTask();
 
-    CHIP_ERROR _SetUserLabelList(EndpointId endpoint,
-                                 AttributeList<app::Clusters::UserLabel::Structs::LabelStruct::Type, kMaxUserLabels> & labelList);
     CHIP_ERROR _GetSupportedLocales(AttributeList<chip::CharSpan, kMaxLanguageTags> & supportedLocales);
     CHIP_ERROR _GetSupportedCalendarTypes(
         AttributeList<app::Clusters::TimeFormatLocalization::CalendarType, kMaxCalendarTypes> & supportedCalendarTypes);

diff --git a/src/platform/DeviceInfoProvider.cpp b/src/platform/DeviceInfoProvider.cpp
@@ -35,6 +35,35 @@ DeviceInfoProvider * gDeviceInfoProvider = nullptr;
 
 } // namespace
 
+CHIP_ERROR DeviceInfoProvider::SetUserLabelList(EndpointId endpoint,
+                                                const AttributeList<UserLabelType, kMaxUserLabelListLength> & labelList)
+{
+    size_t index = 0;
+
+    ReturnErrorOnFailure(SetUserLabelLength(endpoint, labelList.size()));
+
+    for (const UserLabelType & label : labelList)
+    {
+        ReturnErrorOnFailure(SetUserLabelAt(endpoint, index++, label));
+    }
+
+    return CHIP_NO_ERROR;
+}
+
+CHIP_ERROR DeviceInfoProvider::AppendUserLabel(EndpointId endpoint, const UserLabelType & label)
+{
+    size_t length;
+
+    // Increase the size of UserLabelList by 1
+    ReturnErrorOnFailure(GetUserLabelLength(endpoint, length));
+    ReturnErrorOnFailure(SetUserLabelLength(endpoint, length + 1));
+
+    // Append the user label at the end of UserLabelList
+    ReturnErrorOnFailure(SetUserLabelAt(endpoint, length, label));
+
+    return CHIP_NO_ERROR;
+}
+
 /**
  * Instance getter for the global DeviceInfoProvider.
  *

diff --git a/src/platform/Linux/DeviceInfoProviderImpl.cpp b/src/platform/Linux/DeviceInfoProviderImpl.cpp
@@ -105,5 +105,96 @@ 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;
+}
+
+CHIP_ERROR DeviceInfoProviderImpl::GetUserLabelLength(EndpointId endpoint, size_t & val)
+{
+    // TODO:: read the user label count. temporarily return the size of hardcoded labelList.
+    val = 4;
+
+    return CHIP_NO_ERROR;
+}
+
+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;
+}
+
+DeviceInfoProvider::UserLabelIterator * DeviceInfoProviderImpl::IterateUserLabel(EndpointId endpoint)
+{
+    return new UserLabelIteratorImpl(*this, endpoint);
+}
+
+DeviceInfoProviderImpl::UserLabelIteratorImpl::UserLabelIteratorImpl(DeviceInfoProviderImpl & provider, EndpointId endpoint) :
+    mProvider(provider), mEndpoint(endpoint)
+{
+    size_t total = 0;
+
+    ReturnOnFailure(mProvider.GetUserLabelLength(mEndpoint, total));
+    mTotal = total;
+    mIndex = 0;
+}
+
+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;
+    }
+
+    if (err == CHIP_NO_ERROR)
+    {
+        VerifyOrReturnError(std::strlen(labelPtr) <= kMaxLabelNameLength, false);
+        VerifyOrReturnError(std::strlen(valuePtr) <= kMaxLabelValueLength, false);
+
+        Platform::CopyString(mUserLabelNameBuf, kMaxLabelNameLength + 1, labelPtr);
+        Platform::CopyString(mUserLabelValueBuf, kMaxLabelValueLength + 1, valuePtr);
+
+        output.label = CharSpan::fromCharString(mUserLabelNameBuf);
+        output.value = CharSpan::fromCharString(mUserLabelValueBuf);
+
+        mIndex++;
+
+        return true;
+    }
+    else
+    {
+        return false;
+    }
+}
+
 } // namespace DeviceLayer
 } // namespace chip