From 5858050f132349e59ab2a6ed51b41e88d0be112e Mon Sep 17 00:00:00 2001 From: lpbeliveau-silabs <112982107+lpbeliveau-silabs@users.noreply.github.com> Date: Thu, 15 Jun 2023 09:33:07 -0400 Subject: [PATCH] [Scenes] Scene Handler Encode and Decode attribute value list (#27089) * Added Encode and Decode helper functions for SceneHandlers to avoid code dupplication * Moved functions from SceneHandler in SceneTableImpl.cpp, applied comments * Removed unused container in SceneHandler encode/decode of attributeValue lists * Apply suggestions from code review Co-authored-by: Boris Zbarsky * Restyled by clang-format --------- Co-authored-by: Boris Zbarsky Co-authored-by: Restyled.io --- .../clusters/scenes-server/SceneTableImpl.cpp | 79 ++++++++++++++ .../clusters/scenes-server/SceneTableImpl.h | 101 +++++------------- src/app/tests/TestSceneTable.cpp | 78 ++------------ 3 files changed, 114 insertions(+), 144 deletions(-) diff --git a/src/app/clusters/scenes-server/SceneTableImpl.cpp b/src/app/clusters/scenes-server/SceneTableImpl.cpp index f4e3de33d94634..99d14e97095aab 100644 --- a/src/app/clusters/scenes-server/SceneTableImpl.cpp +++ b/src/app/clusters/scenes-server/SceneTableImpl.cpp @@ -22,6 +22,85 @@ namespace chip { namespace scenes { +CHIP_ERROR +DefaultSceneHandlerImpl::EncodeAttributeValueList( + const app::DataModel::List & aVlist, + MutableByteSpan & serializedBytes) +{ + TLV::TLVWriter writer; + writer.Init(serializedBytes); + ReturnErrorOnFailure(app::DataModel::Encode(writer, TLV::AnonymousTag(), aVlist)); + serializedBytes.reduce_size(writer.GetLengthWritten()); + + return CHIP_NO_ERROR; +} + +CHIP_ERROR DefaultSceneHandlerImpl::DecodeAttributeValueList( + const ByteSpan & serializedBytes, + app::DataModel::DecodableList & aVlist) +{ + TLV::TLVReader reader; + + reader.Init(serializedBytes); + ReturnErrorOnFailure(reader.Next(TLV::kTLVType_Array, TLV::AnonymousTag())); + ReturnErrorOnFailure(aVlist.Decode(reader)); + + return CHIP_NO_ERROR; +} + +CHIP_ERROR +DefaultSceneHandlerImpl::SerializeAdd(EndpointId endpoint, + const app::Clusters::Scenes::Structs::ExtensionFieldSet::DecodableType & extensionFieldSet, + MutableByteSpan & serializedBytes) +{ + app::Clusters::Scenes::Structs::AttributeValuePair::Type aVPairs[kMaxAvPair]; + + size_t pairTotal = 0; + // Verify size of list + ReturnErrorOnFailure(extensionFieldSet.attributeValueList.ComputeSize(&pairTotal)); + VerifyOrReturnError(pairTotal <= ArraySize(aVPairs), CHIP_ERROR_BUFFER_TOO_SMALL); + + uint8_t pairCount = 0; + auto pair_iterator = extensionFieldSet.attributeValueList.begin(); + while (pair_iterator.Next()) + { + aVPairs[pairCount] = pair_iterator.GetValue(); + pairCount++; + } + ReturnErrorOnFailure(pair_iterator.GetStatus()); + app::DataModel::List attributeValueList(aVPairs, pairCount); + + return EncodeAttributeValueList(attributeValueList, serializedBytes); +} + +CHIP_ERROR DefaultSceneHandlerImpl::Deserialize(EndpointId endpoint, ClusterId cluster, const ByteSpan & serializedBytes, + app::Clusters::Scenes::Structs::ExtensionFieldSet::Type & extensionFieldSet) +{ + app::DataModel::DecodableList attributeValueList; + + ReturnErrorOnFailure(DecodeAttributeValueList(serializedBytes, attributeValueList)); + + // Verify size of list + size_t pairTotal = 0; + ReturnErrorOnFailure(attributeValueList.ComputeSize(&pairTotal)); + VerifyOrReturnError(pairTotal <= ArraySize(mAVPairs), CHIP_ERROR_BUFFER_TOO_SMALL); + + uint8_t pairCount = 0; + auto pair_iterator = attributeValueList.begin(); + while (pair_iterator.Next()) + { + mAVPairs[pairCount] = pair_iterator.GetValue(); + pairCount++; + }; + ReturnErrorOnFailure(pair_iterator.GetStatus()); + + extensionFieldSet.clusterID = cluster; + extensionFieldSet.attributeValueList = mAVPairs; + extensionFieldSet.attributeValueList.reduce_size(pairCount); + + return CHIP_NO_ERROR; +} + /// @brief Tags Used to serialize Scenes so they can be stored in flash memory. /// kSceneCount: Number of scenes in a Fabric /// kStorageIDArray: Array of StorageID struct diff --git a/src/app/clusters/scenes-server/SceneTableImpl.h b/src/app/clusters/scenes-server/SceneTableImpl.h index b70a50ba1e8155..b162f819b4a747 100644 --- a/src/app/clusters/scenes-server/SceneTableImpl.h +++ b/src/app/clusters/scenes-server/SceneTableImpl.h @@ -46,94 +46,41 @@ class DefaultSceneHandlerImpl : public scenes::SceneHandler DefaultSceneHandlerImpl() = default; ~DefaultSceneHandlerImpl() override{}; + /// @brief Encodes an attribute value list into a TLV structure and resizes the buffer to the size of the encoded data + /// @param aVlist[in] Attribute value list to encode + /// @param serializedBytes[out] Buffer to fill from the Attribute value list in a TLV format + /// @return CHIP_ERROR + virtual CHIP_ERROR + EncodeAttributeValueList(const app::DataModel::List & aVlist, + MutableByteSpan & serializedBytes); + + /// @brief Decodes an attribute value list from a TLV structure and ensure it fits the member pair buffer + /// @param serializedBytes [in] Buffer to read from + /// @param aVlist [out] Attribute value list to fill from the TLV structure. Only valid while the buffer backing + /// serializedBytes exists and its contents are not modified. + /// @return CHIP_ERROR + virtual CHIP_ERROR DecodeAttributeValueList( + const ByteSpan & serializedBytes, + app::DataModel::DecodableList & aVlist); + /// @brief From command AddScene, allows handler to filter through clusters in command to serialize only the supported ones. /// @param endpoint[in] Endpoint ID /// @param extensionFieldSet[in] ExtensionFieldSets provided by the AddScene Command, pre initialized - /// @param serialisedBytes[out] Buffer to fill from the ExtensionFieldSet in command - /// @return CHIP_NO_ERROR if successful, CHIP_ERROR_INVALID_ARGUMENT if the cluster is not supported, CHIP_ERROR value otherwise + /// @param serializedBytes[out] Buffer to fill from the ExtensionFieldSet in command + /// @return CHIP_NO_ERROR if successful, CHIP_ERROR_INVALID_ARGUMENT if the cluster is not supported, CHIP_ERROR value + /// otherwise virtual CHIP_ERROR SerializeAdd(EndpointId endpoint, const app::Clusters::Scenes::Structs::ExtensionFieldSet::DecodableType & extensionFieldSet, - MutableByteSpan & serializedBytes) override - { - app::Clusters::Scenes::Structs::AttributeValuePair::DecodableType aVPair; - TLV::TLVWriter writer; - TLV::TLVType outer; - size_t pairTotal = 0; - uint8_t pairCount = 0; - - // Verify size of list - ReturnErrorOnFailure(extensionFieldSet.attributeValueList.ComputeSize(&pairTotal)); - VerifyOrReturnError(pairTotal <= ArraySize(mAVPairs), CHIP_ERROR_BUFFER_TOO_SMALL); - - auto pair_iterator = extensionFieldSet.attributeValueList.begin(); - while (pair_iterator.Next()) - { - aVPair = pair_iterator.GetValue(); - mAVPairs[pairCount].attributeID = aVPair.attributeID; - mAVPairs[pairCount].attributeValue = aVPair.attributeValue; - pairCount++; - } - ReturnErrorOnFailure(pair_iterator.GetStatus()); - - app::DataModel::List attributeValueList; - attributeValueList = mAVPairs; - attributeValueList.reduce_size(pairCount); - - writer.Init(serializedBytes); - ReturnErrorOnFailure(writer.StartContainer(TLV::AnonymousTag(), TLV::kTLVType_Structure, outer)); - ReturnErrorOnFailure(app::DataModel::Encode( - writer, TLV::ContextTag(app::Clusters::Scenes::Structs::ExtensionFieldSet::Fields::kAttributeValueList), - attributeValueList)); - ReturnErrorOnFailure(writer.EndContainer(outer)); - - return CHIP_NO_ERROR; - } + MutableByteSpan & serializedBytes) override; /// @brief Simulates taking data from nvm and loading it in a command object if the cluster is supported by the endpoint /// @param endpoint target endpoint /// @param cluster target cluster - /// @param serialisedBytes data to deserialize into EFS + /// @param serializedBytes data to deserialize into EFS /// @return CHIP_NO_ERROR if Extension Field Set was successfully populated, CHIP_ERROR_INVALID_ARGUMENT if the cluster is not /// supported, specific CHIP_ERROR otherwise - virtual CHIP_ERROR Deserialize(EndpointId endpoint, ClusterId cluster, const ByteSpan & serialisedBytes, - app::Clusters::Scenes::Structs::ExtensionFieldSet::Type & extensionFieldSet) override - { - app::DataModel::DecodableList attributeValueList; - app::Clusters::Scenes::Structs::AttributeValuePair::DecodableType decodePair; - - TLV::TLVReader reader; - TLV::TLVType outer; - size_t pairTotal = 0; - uint8_t pairCount = 0; - - extensionFieldSet.clusterID = cluster; - reader.Init(serialisedBytes); - ReturnErrorOnFailure(reader.Next(TLV::kTLVType_Structure, TLV::AnonymousTag())); - ReturnErrorOnFailure(reader.EnterContainer(outer)); - ReturnErrorOnFailure(reader.Next( - TLV::kTLVType_Array, TLV::ContextTag(app::Clusters::Scenes::Structs::ExtensionFieldSet::Fields::kAttributeValueList))); - attributeValueList.Decode(reader); - - // Verify size of list - ReturnErrorOnFailure(attributeValueList.ComputeSize(&pairTotal)); - VerifyOrReturnError(pairTotal <= ArraySize(mAVPairs), CHIP_ERROR_BUFFER_TOO_SMALL); - - auto pair_iterator = attributeValueList.begin(); - while (pair_iterator.Next()) - { - decodePair = pair_iterator.GetValue(); - mAVPairs[pairCount].attributeID = decodePair.attributeID; - mAVPairs[pairCount].attributeValue = decodePair.attributeValue; - pairCount++; - }; - ReturnErrorOnFailure(pair_iterator.GetStatus()); - - ReturnErrorOnFailure(reader.ExitContainer(outer)); - extensionFieldSet.attributeValueList = mAVPairs; - extensionFieldSet.attributeValueList.reduce_size(pairCount); - - return CHIP_NO_ERROR; - } + virtual CHIP_ERROR Deserialize(EndpointId endpoint, ClusterId cluster, const ByteSpan & serializedBytes, + app::Clusters::Scenes::Structs::ExtensionFieldSet::Type & extensionFieldSet) override; private: app::Clusters::Scenes::Structs::AttributeValuePair::Type mAVPairs[kMaxAvPair]; diff --git a/src/app/tests/TestSceneTable.cpp b/src/app/tests/TestSceneTable.cpp index 8bbbdb350fb900..b45bc3ee67dd28 100644 --- a/src/app/tests/TestSceneTable.cpp +++ b/src/app/tests/TestSceneTable.cpp @@ -473,8 +473,6 @@ void TestHandlerFunctions(nlTestSuite * aSuite, void * aContext) TLV::TLVReader reader; TLV::TLVWriter writer; - TLV::TLVType outer; - TLV::TLVType outerRead; static const uint8_t OO_av_payload = 0x01; static const uint16_t LC_av_payload[2] = { 0x64, 0x01F0 }; @@ -522,36 +520,18 @@ void TestHandlerFunctions(nlTestSuite * aSuite, void * aContext) // Serialize Extension Field sets as if they were recovered from memory writer.Init(OO_buffer); - writer.StartContainer(TLV::AnonymousTag(), TLV::kTLVType_Structure, outer); NL_TEST_ASSERT(aSuite, - CHIP_NO_ERROR == - app::DataModel::Encode(writer, - TLV::ContextTag(to_underlying( - app::Clusters::Scenes::Structs::ExtensionFieldSet::Fields::kAttributeValueList)), - OOextensionFieldSet.attributeValueList)); - writer.EndContainer(outer); + CHIP_NO_ERROR == app::DataModel::Encode(writer, TLV::AnonymousTag(), OOextensionFieldSet.attributeValueList)); OO_buffer_serialized_length = writer.GetLengthWritten(); writer.Init(LC_buffer); - writer.StartContainer(TLV::AnonymousTag(), TLV::kTLVType_Structure, outer); NL_TEST_ASSERT(aSuite, - CHIP_NO_ERROR == - app::DataModel::Encode(writer, - TLV::ContextTag(to_underlying( - app::Clusters::Scenes::Structs::ExtensionFieldSet::Fields::kAttributeValueList)), - LCextensionFieldSet.attributeValueList)); - writer.EndContainer(outer); + CHIP_NO_ERROR == app::DataModel::Encode(writer, TLV::AnonymousTag(), LCextensionFieldSet.attributeValueList)); LC_buffer_serialized_length = writer.GetLengthWritten(); writer.Init(CC_buffer); - writer.StartContainer(TLV::AnonymousTag(), TLV::kTLVType_Structure, outer); NL_TEST_ASSERT(aSuite, - CHIP_NO_ERROR == - app::DataModel::Encode(writer, - TLV::ContextTag(to_underlying( - app::Clusters::Scenes::Structs::ExtensionFieldSet::Fields::kAttributeValueList)), - CCextensionFieldSet.attributeValueList)); - writer.EndContainer(outer); + CHIP_NO_ERROR == app::DataModel::Encode(writer, TLV::AnonymousTag(), CCextensionFieldSet.attributeValueList)); CC_buffer_serialized_length = writer.GetLengthWritten(); // Test Registering SceneHandler @@ -562,10 +542,7 @@ void TestHandlerFunctions(nlTestSuite * aSuite, void * aContext) reader.Init(OO_list); extensionFieldSetIn.clusterID = kOnOffClusterId; NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == reader.Next()); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == reader.EnterContainer(outerRead)); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == reader.Next()); NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == extensionFieldSetIn.attributeValueList.Decode(reader)); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == reader.ExitContainer(outerRead)); NL_TEST_ASSERT(aSuite, sHandler.SupportsCluster(kTestEndpoint1, extensionFieldSetIn.clusterID)); NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == sHandler.SerializeAdd(kTestEndpoint1, extensionFieldSetIn, buff_span)); @@ -573,15 +550,13 @@ void TestHandlerFunctions(nlTestSuite * aSuite, void * aContext) // Verify the handler extracted buffer matches the initial field sets NL_TEST_ASSERT(aSuite, 0 == memcmp(OO_list.data(), buff_span.data(), buff_span.size())); memset(buffer, 0, buff_span.size()); + buff_span = MutableByteSpan(buffer); // Setup the Level Control Extension field set in the expected state from a command reader.Init(LC_list); extensionFieldSetIn.clusterID = kLevelControlClusterId; NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == reader.Next()); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == reader.EnterContainer(outerRead)); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == reader.Next()); NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == extensionFieldSetIn.attributeValueList.Decode(reader)); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == reader.ExitContainer(outerRead)); NL_TEST_ASSERT(aSuite, sHandler.SupportsCluster(kTestEndpoint1, extensionFieldSetIn.clusterID)); NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == sHandler.SerializeAdd(kTestEndpoint1, extensionFieldSetIn, buff_span)); @@ -589,15 +564,13 @@ void TestHandlerFunctions(nlTestSuite * aSuite, void * aContext) // Verify the handler extracted buffer matches the initial field sets NL_TEST_ASSERT(aSuite, 0 == memcmp(LC_list.data(), buff_span.data(), buff_span.size())); memset(buffer, 0, buff_span.size()); + buff_span = MutableByteSpan(buffer); // Setup the Color control Extension field set in the expected state from a command reader.Init(CC_list); extensionFieldSetIn.clusterID = kColorControlClusterId; NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == reader.Next()); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == reader.EnterContainer(outerRead)); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == reader.Next()); NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == extensionFieldSetIn.attributeValueList.Decode(reader)); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == reader.ExitContainer(outerRead)); NL_TEST_ASSERT(aSuite, sHandler.SupportsCluster(kTestEndpoint1, extensionFieldSetIn.clusterID)); NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == sHandler.SerializeAdd(kTestEndpoint1, extensionFieldSetIn, buff_span)); @@ -605,6 +578,7 @@ void TestHandlerFunctions(nlTestSuite * aSuite, void * aContext) // Verify the handler extracted buffer matches the initial field sets NL_TEST_ASSERT(aSuite, 0 == memcmp(CC_list.data(), buff_span.data(), buff_span.size())); memset(buffer, 0, buff_span.size()); + buff_span = MutableByteSpan(buffer); // Verify Deserializing is properly filling out output extension field set for on off NL_TEST_ASSERT(aSuite, sHandler.SupportsCluster(kTestEndpoint1, kOnOffClusterId)); @@ -612,14 +586,8 @@ void TestHandlerFunctions(nlTestSuite * aSuite, void * aContext) // Verify Encoding the Extension field set returns the same data as the one serialized for on off previously writer.Init(buff_span); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == writer.StartContainer(TLV::AnonymousTag(), TLV::kTLVType_Structure, outer)); NL_TEST_ASSERT(aSuite, - CHIP_NO_ERROR == - app::DataModel::Encode(writer, - TLV::ContextTag(to_underlying( - app::Clusters::Scenes::Structs::ExtensionFieldSet::Fields::kAttributeValueList)), - extensionFieldSetOut.attributeValueList)); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == writer.EndContainer(outer)); + CHIP_NO_ERROR == app::DataModel::Encode(writer, TLV::AnonymousTag(), extensionFieldSetOut.attributeValueList)); NL_TEST_ASSERT(aSuite, 0 == memcmp(OO_list.data(), buff_span.data(), buff_span.size())); memset(buffer, 0, buff_span.size()); @@ -630,14 +598,8 @@ void TestHandlerFunctions(nlTestSuite * aSuite, void * aContext) // Verify Encoding the Extension field set returns the same data as the one serialized for level control previously writer.Init(buff_span); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == writer.StartContainer(TLV::AnonymousTag(), TLV::kTLVType_Structure, outer)); NL_TEST_ASSERT(aSuite, - CHIP_NO_ERROR == - app::DataModel::Encode(writer, - TLV::ContextTag(to_underlying( - app::Clusters::Scenes::Structs::ExtensionFieldSet::Fields::kAttributeValueList)), - extensionFieldSetOut.attributeValueList)); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == writer.EndContainer(outer)); + CHIP_NO_ERROR == app::DataModel::Encode(writer, TLV::AnonymousTag(), extensionFieldSetOut.attributeValueList)); NL_TEST_ASSERT(aSuite, 0 == memcmp(LC_list.data(), buff_span.data(), buff_span.size())); memset(buffer, 0, buff_span.size()); @@ -648,14 +610,8 @@ void TestHandlerFunctions(nlTestSuite * aSuite, void * aContext) // Verify Encoding the Extension field set returns the same data as the one serialized for color control previously writer.Init(buff_span); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == writer.StartContainer(TLV::AnonymousTag(), TLV::kTLVType_Structure, outer)); NL_TEST_ASSERT(aSuite, - CHIP_NO_ERROR == - app::DataModel::Encode(writer, - TLV::ContextTag(to_underlying( - app::Clusters::Scenes::Structs::ExtensionFieldSet::Fields::kAttributeValueList)), - extensionFieldSetOut.attributeValueList)); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == writer.EndContainer(outer)); + CHIP_NO_ERROR == app::DataModel::Encode(writer, TLV::AnonymousTag(), extensionFieldSetOut.attributeValueList)); NL_TEST_ASSERT(aSuite, 0 == memcmp(CC_list.data(), buff_span.data(), buff_span.size())); memset(buffer, 0, buff_span.size()); @@ -664,9 +620,6 @@ void TestHandlerFunctions(nlTestSuite * aSuite, void * aContext) app::Clusters::Scenes::Structs::ExtensionFieldSet::DecodableType extensionFieldFailTestIn; app::Clusters::Scenes::Structs::AttributeValuePair::Type TooManyPairs[16]; - TLV::TLVType failWrite; - TLV::TLVType failRead; - uint8_t payloadOk = 0; for (uint8_t i = 0; i < 16; i++) @@ -682,23 +635,14 @@ void TestHandlerFunctions(nlTestSuite * aSuite, void * aContext) // Serialize Extension Field sets as if they were recovered from memory writer.Init(failBuffer); - writer.StartContainer(TLV::AnonymousTag(), TLV::kTLVType_Structure, failWrite); - NL_TEST_ASSERT(aSuite, - CHIP_NO_ERROR == - app::DataModel::Encode(writer, - TLV::ContextTag(to_underlying( - app::Clusters::Scenes::Structs::ExtensionFieldSet::Fields::kAttributeValueList)), - extensionFieldFailTestOut.attributeValueList)); - writer.EndContainer(failWrite); + NL_TEST_ASSERT( + aSuite, CHIP_NO_ERROR == app::DataModel::Encode(writer, TLV::AnonymousTag(), extensionFieldFailTestOut.attributeValueList)); // Setup the On Off Extension field set in the expected state from a command reader.Init(fail_list); extensionFieldFailTestIn.clusterID = kColorControlClusterId; NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == reader.Next()); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == reader.EnterContainer(failRead)); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == reader.Next()); NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == extensionFieldFailTestIn.attributeValueList.Decode(reader)); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == reader.ExitContainer(failRead)); // Verify failure on both serialize and deserialize NL_TEST_ASSERT(aSuite,