Skip to content

Commit

Permalink
Add an EncodeEmptyList helper on AttributeValueEncoder. (#13135)
Browse files Browse the repository at this point in the history
Fixes #12850
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Jul 20, 2023
1 parent 6268069 commit 3921150
Show file tree
Hide file tree
Showing 13 changed files with 66 additions and 44 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,13 @@ constexpr uint16_t BridgedActionsAttrAccess::ClusterRevision;
CHIP_ERROR BridgedActionsAttrAccess::ReadActionListAttribute(EndpointId endpoint, AttributeValueEncoder & aEncoder)
{
// Just return an empty list
return aEncoder.Encode(DataModel::List<BridgedActions::Structs::ActionStruct::Type>());
return aEncoder.EncodeEmptyList();
}

CHIP_ERROR BridgedActionsAttrAccess::ReadEndpointListAttribute(EndpointId endpoint, AttributeValueEncoder & aEncoder)
{
// Just return an empty list
return aEncoder.Encode(DataModel::List<BridgedActions::Structs::EndpointListStruct::Type>());
return aEncoder.EncodeEmptyList();
}

CHIP_ERROR BridgedActionsAttrAccess::ReadSetupUrlAttribute(EndpointId endpoint, AttributeValueEncoder & aEncoder)
Expand Down
2 changes: 1 addition & 1 deletion src/app/AttributeAccessInterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ CHIP_ERROR AttributeReportBuilder::FinishAttribute(AttributeReportIBs::Builder &
return aAttributeReportIBsBuilder.GetAttributeReport().EndOfAttributeReportIB().GetError();
}

CHIP_ERROR AttributeValueEncoder::EncodeEmptyList()
CHIP_ERROR AttributeValueEncoder::EnsureListStarted()
{
if (mCurrentEncodingListIndex == kInvalidListIndex)
{
Expand Down
18 changes: 14 additions & 4 deletions src/app/AttributeAccessInterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,15 @@ class AttributeValueEncoder
return Encode(DataModel::Nullable<uint8_t>());
}

/**
* Encode an explicit empty list.
*/
CHIP_ERROR EncodeEmptyList()
{
// Doesn't matter what type List we use here.
return Encode(DataModel::List<uint8_t>());
}

/**
* aCallback is expected to take a const auto & argument and Encode() on it as many times as needed to encode all the list
* elements one by one. If any of those Encode() calls returns failure, aCallback must stop encoding and return failure. When
Expand All @@ -200,7 +209,7 @@ class AttributeValueEncoder
// EmptyList acts as the beginning of the whole array type attribute report.
// An empty list is encoded iff both mCurrentEncodingListIndex and mEncodeState.mCurrentEncodingListIndex are invalid
// values. After encoding the empty list, mEncodeState.mCurrentEncodingListIndex and mCurrentEncodingListIndex are set to 0.
ReturnErrorOnFailure(EncodeEmptyList());
ReturnErrorOnFailure(EnsureListStarted());
ReturnErrorOnFailure(aCallback(ListEncodeHelper(*this)));
// The Encode procedure finished without any error, clear the state.
mEncodeState = AttributeEncodeState();
Expand All @@ -226,7 +235,7 @@ class AttributeValueEncoder
template <typename... Ts>
CHIP_ERROR EncodeListItem(Ts &&... aArgs)
{
// EncodeListItem must be called after EncodeEmptyList(), thus mCurrentEncodingListIndex and
// EncodeListItem must be called after EnsureListStarted(), thus mCurrentEncodingListIndex and
// mEncodeState.mCurrentEncodingListIndex are not invalid values.
if (mCurrentEncodingListIndex < mEncodeState.mCurrentEncodingListIndex)
{
Expand Down Expand Up @@ -273,7 +282,8 @@ class AttributeValueEncoder
}

/**
* EncodeEmptyList encodes the first item of one report with lists (an empty list).
* EnsureListStarted encodes the first item of one report with lists (an
* empty list), as needed.
*
* If internal state indicates we have already encoded the empty list, this function will encode nothing, set
* mCurrentEncodingListIndex to 0 and return CHIP_NO_ERROR.
Expand All @@ -282,7 +292,7 @@ class AttributeValueEncoder
* after it returns, because at that point we will be encoding the list
* items.
*/
CHIP_ERROR EncodeEmptyList();
CHIP_ERROR EnsureListStarted();

bool mTriedEncode = false;
AttributeReportIBs::Builder & mAttributeReportIBsBuilder;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ CHIP_ERROR AccessControlAttribute::ReadAcl(AttributeValueEncoder & aEncoder)

CHIP_ERROR AccessControlAttribute::ReadExtension(AttributeValueEncoder & aEncoder)
{
return aEncoder.EncodeList([&](const auto & encoder) -> CHIP_ERROR { return CHIP_NO_ERROR; });
return aEncoder.EncodeEmptyList();
}

CHIP_ERROR AccessControlAttribute::Write(const ConcreteDataAttributePath & aPath, AttributeValueDecoder & aDecoder)
Expand Down
2 changes: 1 addition & 1 deletion src/app/clusters/descriptor/descriptor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ CHIP_ERROR DescriptorAttrAccess::ReadPartsAttribute(EndpointId endpoint, Attribu
}
else
{
err = aEncoder.Encode(DataModel::List<EndpointId>());
err = aEncoder.EncodeEmptyList();
}

return err;
Expand Down
2 changes: 1 addition & 1 deletion src/app/clusters/fixed-label-server/fixed-label-server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ CHIP_ERROR FixedLabelAttrAccess::ReadLabelList(EndpointId endpoint, AttributeVal
}
else
{
err = aEncoder.Encode(DataModel::List<Structs::LabelStruct::Type>());
err = aEncoder.EncodeEmptyList();
}

return err;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ CHIP_ERROR GeneralDiagosticsAttrAccess::ReadListIfSupported(CHIP_ERROR (Diagnost
}
else
{
err = aEncoder.Encode(DataModel::List<uint8_t>());
err = aEncoder.EncodeEmptyList();
}

return err;
Expand All @@ -127,7 +127,7 @@ CHIP_ERROR GeneralDiagosticsAttrAccess::ReadNetworkInterfaces(AttributeValueEnco
}
else
{
err = aEncoder.Encode(DataModel::List<EndpointId>());
err = aEncoder.EncodeEmptyList();
}

return err;
Expand Down
2 changes: 1 addition & 1 deletion src/app/clusters/mode-select-server/mode-select-server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ CHIP_ERROR ModeSelectAttrAccess::Read(const ConcreteReadAttributePath & aPath, A
gSupportedModeManager->getModeOptionsProvider(aPath.mEndpointId);
if (modeOptionsProvider.begin() == nullptr)
{
aEncoder.Encode(DataModel::List<ModeSelect::Structs::ModeOptionStruct::Type>());
aEncoder.EncodeEmptyList();
return CHIP_NO_ERROR;
}
CHIP_ERROR err;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ CHIP_ERROR SoftwareDiagosticsAttrAccess::ReadThreadMetrics(AttributeValueEncoder
}
else
{
err = aEncoder.Encode(DataModel::List<EndpointId>());
err = aEncoder.EncodeEmptyList();
}

return err;
Expand Down
2 changes: 1 addition & 1 deletion src/app/clusters/user-label-server/user-label-server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ CHIP_ERROR UserLabelAttrAccess::ReadLabelList(EndpointId endpoint, AttributeValu
}
else
{
err = aEncoder.Encode(DataModel::List<Structs::LabelStruct::Type>());
err = aEncoder.EncodeEmptyList();
}

return err;
Expand Down
66 changes: 39 additions & 27 deletions src/app/tests/TestAttributeValueEncoder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -216,30 +216,39 @@ void TestEncodeListOfBools2(nlTestSuite * aSuite, void * aContext)
VERIFY_BUFFER_STATE(aSuite, test, expected);
}

void TestEncodeEmptyList(nlTestSuite * aSuite, void * aContext)
constexpr uint8_t emptyListExpected[] = {
// clang-format off
0x15, 0x36, 0x01, // Test overhead, Start Anonymous struct + Start 1 byte Tag Array + Tag (01)
0x15, // Start anonymous struct
0x35, 0x01, // Start 1 byte tag struct + Tag (01)
0x24, 0x00, 0x99, // Tag (00) Value (1 byte uint) 0x99 (Attribute Version)
0x37, 0x01, // Start 1 byte tag list + Tag (01) (Attribute Path)
0x24, 0x02, 0x55, // Tag (02) Value (1 byte uint) 0x55
0x24, 0x03, 0xaa, // Tag (03) Value (1 byte uint) 0xaa
0x24, 0x04, 0xcc, // Tag (04) Value (1 byte uint) 0xcc
0x18, // End of container
// Intended empty array
0x36, 0x02, // Start 1 byte tag array + Tag (02) (Attribute Value)
0x18, // End of container
0x18, // End of container
0x18, // End of container
// clang-format on
};

void TestEncodeEmptyList1(nlTestSuite * aSuite, void * aContext)
{
TestSetup test(aSuite);
CHIP_ERROR err = test.encoder.EncodeList([](const auto & encoder) -> CHIP_ERROR { return CHIP_NO_ERROR; });
NL_TEST_ASSERT(aSuite, err == CHIP_NO_ERROR);
const uint8_t expected[] = {
// clang-format off
0x15, 0x36, 0x01, // Test overhead, Start Anonymous struct + Start 1 byte Tag Array + Tag (01)
0x15, // Start anonymous struct
0x35, 0x01, // Start 1 byte tag struct + Tag (01)
0x24, 0x00, 0x99, // Tag (00) Value (1 byte uint) 0x99 (Attribute Version)
0x37, 0x01, // Start 1 byte tag list + Tag (01) (Attribute Path)
0x24, 0x02, 0x55, // Tag (02) Value (1 byte uint) 0x55
0x24, 0x03, 0xaa, // Tag (03) Value (1 byte uint) 0xaa
0x24, 0x04, 0xcc, // Tag (04) Value (1 byte uint) 0xcc
0x18, // End of container
// Intended empty array
0x36, 0x02, // Start 1 byte tag array + Tag (02) (Attribute Value)
0x18, // End of container
0x18, // End of container
0x18, // End of container
// clang-format on
};
VERIFY_BUFFER_STATE(aSuite, test, expected);
VERIFY_BUFFER_STATE(aSuite, test, emptyListExpected);
}

void TestEncodeEmptyList2(nlTestSuite * aSuite, void * aContext)
{
TestSetup test(aSuite);
CHIP_ERROR err = test.encoder.EncodeEmptyList();
NL_TEST_ASSERT(aSuite, err == CHIP_NO_ERROR);
VERIFY_BUFFER_STATE(aSuite, test, emptyListExpected);
}

void TestEncodeFabricScoped(nlTestSuite * aSuite, void * aContext)
Expand Down Expand Up @@ -375,17 +384,20 @@ void TestEncodeListChunking(nlTestSuite * aSuite, void * aContext)
}
}

#undef VERIFY_STATE
#undef VERIFY_BUFFER_STATE

} // anonymous namespace

namespace {
const nlTest sTests[] = {
NL_TEST_DEF("TestEncodeNothing", TestEncodeNothing), NL_TEST_DEF("TestEncodeBool", TestEncodeBool),
NL_TEST_DEF("TestEncodeEmptyList", TestEncodeEmptyList), NL_TEST_DEF("TestEncodeListOfBools1", TestEncodeListOfBools1),
NL_TEST_DEF("TestEncodeListOfBools2", TestEncodeListOfBools2), NL_TEST_DEF("TestEncodeListChunking", TestEncodeListChunking),
NL_TEST_DEF("TestEncodeFabricScoped", TestEncodeFabricScoped), NL_TEST_SENTINEL()
};
const nlTest sTests[] = { NL_TEST_DEF("TestEncodeNothing", TestEncodeNothing),
NL_TEST_DEF("TestEncodeBool", TestEncodeBool),
NL_TEST_DEF("TestEncodeEmptyList1", TestEncodeEmptyList1),
NL_TEST_DEF("TestEncodeEmptyList2", TestEncodeEmptyList2),
NL_TEST_DEF("TestEncodeListOfBools1", TestEncodeListOfBools1),
NL_TEST_DEF("TestEncodeListOfBools2", TestEncodeListOfBools2),
NL_TEST_DEF("TestEncodeListChunking", TestEncodeListChunking),
NL_TEST_DEF("TestEncodeFabricScoped", TestEncodeFabricScoped),
NL_TEST_SENTINEL() };
}

int TestAttributeValueEncoder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ inline CHIP_ERROR GenericConnectivityManagerImpl_NoThread<ImplClass>::_WriteThre
case app::Clusters::ThreadNetworkDiagnostics::Attributes::SecurityPolicy::Id:
case app::Clusters::ThreadNetworkDiagnostics::Attributes::OperationalDatasetComponents::Id:
case app::Clusters::ThreadNetworkDiagnostics::Attributes::ActiveNetworkFaultsList::Id: {
err = encoder.Encode(app::DataModel::List<EndpointId>());
err = encoder.EncodeEmptyList();
break;
}
default: {
Expand Down
2 changes: 1 addition & 1 deletion src/platform/Linux/ThreadStackManagerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ CHIP_ERROR ThreadStackManagerImpl::_WriteThreadNetworkDiagnosticAttributeToTlv(A
case ThreadNetworkDiagnostics::Attributes::SecurityPolicy::Id:
case ThreadNetworkDiagnostics::Attributes::OperationalDatasetComponents::Id:
case ThreadNetworkDiagnostics::Attributes::ActiveNetworkFaultsList::Id: {
err = encoder.Encode(DataModel::List<EndpointId>());
err = encoder.EncodeEmptyList();
break;
}
default: {
Expand Down

0 comments on commit 3921150

Please sign in to comment.