Skip to content

Remove AttributeList from Device Layer #41546

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
+112 −350
Open

Remove AttributeList from Device Layer #41546

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f8b6393
Remove AttributeList from Device Layer
ksperling-apple Oct 21, 2025
77b277c
Add missing const to Span type
ksperling-apple Oct 29, 2025
e10370f
Move max size constants into the header and add more tests
ksperling-apple Nov 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/BUILD.gn
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ if (chip_build_tests) {
"${chip_root}/src/crypto/tests",
"${chip_root}/src/data-model-providers/codedriven/endpoint/tests",
"${chip_root}/src/inet/tests",
"${chip_root}/src/include/platform/tests",
"${chip_root}/src/lib/address_resolve/tests",
"${chip_root}/src/app/server-cluster/tests",
"${chip_root}/src/lib/asn1/tests",
Expand Down
92 changes: 90 additions & 2 deletions src/app/clusters/user-label-server/tests/TestUserLabelCluster.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include <clusters/UserLabel/Enums.h>
#include <clusters/UserLabel/Metadata.h>
#include <clusters/UserLabel/Structs.h>
#include <platform/DeviceInfoProvider.h>

namespace {

Expand All @@ -35,21 +36,55 @@ using namespace chip::app::Clusters::UserLabel;
using namespace chip::app::Clusters::UserLabel::Attributes;
using namespace chip::Test;

// Mock DeviceInfoProvider for testing
class MockDeviceInfoProvider : public DeviceLayer::DeviceInfoProvider
{
public:
MockDeviceInfoProvider() = default;
~MockDeviceInfoProvider() override = default;

FixedLabelIterator * IterateFixedLabel(EndpointId endpoint) override { return nullptr; }
UserLabelIterator * IterateUserLabel(EndpointId endpoint) override { return nullptr; }
SupportedCalendarTypesIterator * IterateSupportedCalendarTypes() override { return nullptr; }
SupportedLocalesIterator * IterateSupportedLocales() override { return nullptr; }

protected:
// Simple no-op implementations - we only need these to return success
// so that the cluster's validation logic can be tested
CHIP_ERROR SetUserLabelLength(EndpointId endpoint, size_t val) override { return CHIP_NO_ERROR; }
CHIP_ERROR GetUserLabelLength(EndpointId endpoint, size_t & val) override
{
val = 0;
return CHIP_NO_ERROR;
}
CHIP_ERROR SetUserLabelAt(EndpointId endpoint, size_t index, const UserLabelType & userLabel) override { return CHIP_NO_ERROR; }
CHIP_ERROR DeleteUserLabelAt(EndpointId endpoint, size_t index) override { return CHIP_NO_ERROR; }
};

struct TestUserLabelCluster : public ::testing::Test
{
static void SetUpTestSuite() { ASSERT_EQ(chip::Platform::MemoryInit(), CHIP_NO_ERROR); }

static void TearDownTestSuite() { chip::Platform::MemoryShutdown(); }

void SetUp() override { ASSERT_EQ(userLabel.Startup(context), CHIP_NO_ERROR); }
void SetUp() override
{
DeviceLayer::SetDeviceInfoProvider(&mDeviceInfoProvider);
ASSERT_EQ(userLabel.Startup(context), CHIP_NO_ERROR);
}

void TearDown() override { userLabel.Shutdown(); }
void TearDown() override
{
userLabel.Shutdown();
DeviceLayer::SetDeviceInfoProvider(nullptr);
}

TestUserLabelCluster() : context(testContext.Create()), userLabel(kRootEndpointId) {}

chip::Test::TestServerClusterContext testContext;
ServerClusterContext context;
UserLabelCluster userLabel;
MockDeviceInfoProvider mDeviceInfoProvider;
};

} // namespace
Expand Down Expand Up @@ -83,3 +118,56 @@ TEST_F(TestUserLabelCluster, ReadAttributeTest)
ASSERT_GT(it.GetValue().label.size(), 0u);
}
}

TEST_F(TestUserLabelCluster, WriteValidLabelListTest)
{
ClusterTester tester(userLabel);
Structs::LabelStruct::Type labels[] = {
{ .label = "room"_span, .value = "bedroom 2"_span },
{ .label = "orientation"_span, .value = "North"_span },
};
ASSERT_EQ(tester.WriteAttribute(LabelList::Id, DataModel::List(labels)), CHIP_NO_ERROR);
}

TEST_F(TestUserLabelCluster, WriteLabelWithLabelTooLongTest)
{
ClusterTester tester(userLabel);
constexpr auto tooLongLabel = "this_label_is_way_too_long"_span;
static_assert(tooLongLabel.size() > UserLabelCluster::kMaxLabelSize);
Structs::LabelStruct::Type labels[] = {
{ .label = tooLongLabel, .value = "value"_span },
};
ASSERT_EQ(tester.WriteAttribute(LabelList::Id, DataModel::List(labels)), CHIP_IM_GLOBAL_STATUS(ConstraintError));
}

TEST_F(TestUserLabelCluster, WriteLabelWithValueTooLongTest)
{
ClusterTester tester(userLabel);
constexpr auto tooLongValue = "this_value_is_way_too_long"_span;
static_assert(tooLongValue.size() > UserLabelCluster::kMaxValueSize);
Structs::LabelStruct::Type labels[] = {
{ .label = "room"_span, .value = tooLongValue },
};
ASSERT_EQ(tester.WriteAttribute(LabelList::Id, DataModel::List(labels)), CHIP_IM_GLOBAL_STATUS(ConstraintError));
}

TEST_F(TestUserLabelCluster, WriteEmptyLabelsTest)
{
ClusterTester tester(userLabel);
Structs::LabelStruct::Type labels[] = {
{ .label = ""_span, .value = ""_span }, // empty label and value are allowed per spec
};
ASSERT_EQ(tester.WriteAttribute(LabelList::Id, DataModel::List(labels)), CHIP_NO_ERROR);
}

TEST_F(TestUserLabelCluster, WriteMaxSizeLabelListTest)
{
ClusterTester tester(userLabel);
std::array<Structs::LabelStruct::Type, chip::DeviceLayer::kMaxUserLabelListLength> labels;
for (size_t i = 0; i < labels.size(); i++)
{
labels[i].label = "label"_span;
labels[i].value = "value"_span;
}
ASSERT_EQ(tester.WriteAttribute(LabelList::Id, DataModel::List(labels.data(), labels.size())), CHIP_NO_ERROR);
}
32 changes: 10 additions & 22 deletions src/app/clusters/user-label-server/user-label-cluster.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
#include <app/server/Server.h>
#include <clusters/UserLabel/Metadata.h>

#include <array>

namespace chip::app::Clusters {

using namespace UserLabel;
Expand Down Expand Up @@ -66,24 +68,8 @@ CHIP_ERROR ReadLabelList(EndpointId endpoint, AttributeValueEncoder & encoder)
/// Matches constraints on a LabelStruct.
bool IsValidLabelEntry(const Structs::LabelStruct::Type & entry)
{
constexpr size_t kMaxLabelSize = 16;
constexpr size_t kMaxValueSize = 16;

// NOTE: spec default for label and value is empty, so empty is accepted here
return (entry.label.size() <= kMaxLabelSize) && (entry.value.size() <= kMaxValueSize);
}

bool IsValidLabelEntryList(const LabelList::TypeInfo::DecodableType & list)
{
auto iter = list.begin();
while (iter.Next())
{
if (!IsValidLabelEntry(iter.GetValue()))
{
return false;
}
}
return true;
return (entry.label.size() <= UserLabelCluster::kMaxLabelSize) && (entry.value.size() <= UserLabelCluster::kMaxValueSize);
}

CHIP_ERROR WriteLabelList(const ConcreteDataAttributePath & path, AttributeValueDecoder & decoder)
Expand All @@ -96,20 +82,22 @@ CHIP_ERROR WriteLabelList(const ConcreteDataAttributePath & path, AttributeValue

if (!path.IsListItemOperation())
{
DeviceLayer::AttributeList<Structs::LabelStruct::Type, DeviceLayer::kMaxUserLabelListLength> labelList;
size_t numLabels = 0;
std::array<Structs::LabelStruct::Type, DeviceLayer::kMaxUserLabelListLength> labels;
LabelList::TypeInfo::DecodableType decodablelist;

ReturnErrorOnFailure(decoder.Decode(decodablelist));
VerifyOrReturnError(IsValidLabelEntryList(decodablelist), CHIP_IM_GLOBAL_STATUS(ConstraintError));

auto iter = decodablelist.begin();
while (iter.Next())
{
ReturnErrorOnFailure(labelList.add(iter.GetValue()));
auto & label = iter.GetValue();
VerifyOrReturnError(IsValidLabelEntry(label), CHIP_IM_GLOBAL_STATUS(ConstraintError));
Copy link
Contributor

@andy31415 andy31415 Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

User label cluster is a code driven clusters, so it has testing infrastructure for unit tests.

Could we test this? Codecov says 58.33333% line coverage.

Copy link
Contributor

@andy31415 andy31415 Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least these lines need test coverage:

image

Copy link
Contributor Author

@ksperling-apple ksperling-apple Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why it's showing those two lines at the top... that helper method is being removed entirely.
The second 4 lines should now be covered, but I'm not sure how/when codecov refreshes or which CI build it's looking at.

VerifyOrReturnError(numLabels < labels.size(), CHIP_ERROR_NO_MEMORY);
labels[numLabels++] = label;
}
ReturnErrorOnFailure(iter.GetStatus());

return provider->SetUserLabelList(endpoint, labelList);
return provider->SetUserLabelList(endpoint, Span(labels.data(), numLabels));
}

if (path.mListOp == ConcreteDataAttributePath::ListOperation::AppendItem)
Expand Down
4 changes: 4 additions & 0 deletions src/app/clusters/user-label-server/user-label-cluster.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ class UserLabelCluster : public DefaultServerCluster, public chip::FabricTable::
DataModel::ActionReturnStatus WriteAttribute(const DataModel::WriteAttributeRequest & request,
AttributeValueDecoder & decoder) override;
CHIP_ERROR Attributes(const ConcreteClusterPath & path, ReadOnlyBufferBuilder<DataModel::AttributeEntry> & builder) override;

// Constraints for label entries
static constexpr size_t kMaxLabelSize = 16;
static constexpr size_t kMaxValueSize = 16;
};

} // namespace chip::app::Clusters
142 changes: 0 additions & 142 deletions src/include/platform/AttributeList.h
View file
Open in desktop

This file was deleted.

4 changes: 2 additions & 2 deletions src/include/platform/DeviceInfoProvider.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
#include <app/util/basic-types.h>
#include <lib/core/CHIPError.h>
#include <lib/core/CHIPPersistentStorageDelegate.h>
#include <platform/AttributeList.h>
#include <lib/support/Span.h>

namespace chip {
namespace DeviceLayer {
Expand Down Expand Up @@ -101,7 +101,7 @@ class DeviceInfoProvider
* @return CHIP_NO_ERROR on success.
* @return CHIP_ERROR if an error occurs.
*/
CHIP_ERROR SetUserLabelList(EndpointId endpoint, const AttributeList<UserLabelType, kMaxUserLabelListLength> & labelList);
CHIP_ERROR SetUserLabelList(EndpointId endpoint, Span<const UserLabelType> labelList);

/**
* @brief Clears the user label list for a specified endpoint.
Expand Down
1 change: 0 additions & 1 deletion src/include/platform/PlatformManager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@

#pragma once

#include <platform/AttributeList.h>
#include <platform/CHIPDeviceConfig.h>
#include <platform/CHIPDeviceEvent.h>
#include <system/PlatformEventSupport.h>
Expand Down
Loading
Loading