diff --git a/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp b/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp index c4ed4eb0c8792e..85fa653f256dac 100644 --- a/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp +++ b/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp @@ -37,7 +37,7 @@ class BridgedActionsAttrAccess : public AttributeAccessInterface // Register for the Bridged Actions cluster on all endpoints. BridgedActionsAttrAccess() : AttributeAccessInterface(Optional::Missing(), BridgedActions::Id) {} - CHIP_ERROR Read(const ConcreteAttributePath & aPath, AttributeValueEncoder & aEncoder) override; + CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override; private: static constexpr uint16_t ClusterRevision = 1; @@ -73,7 +73,7 @@ CHIP_ERROR BridgedActionsAttrAccess::ReadClusterRevision(EndpointId endpoint, At BridgedActionsAttrAccess gAttrAccess; -CHIP_ERROR BridgedActionsAttrAccess::Read(const ConcreteAttributePath & aPath, AttributeValueEncoder & aEncoder) +CHIP_ERROR BridgedActionsAttrAccess::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) { VerifyOrDie(aPath.mClusterId == BridgedActions::Id); diff --git a/examples/tv-app/android/include/cluster-init.cpp b/examples/tv-app/android/include/cluster-init.cpp index 3d7bee4a9cfb48..aba5cafad51f2e 100644 --- a/examples/tv-app/android/include/cluster-init.cpp +++ b/examples/tv-app/android/include/cluster-init.cpp @@ -42,7 +42,7 @@ class TvAttrAccess : public app::AttributeAccessInterface public: TvAttrAccess() : app::AttributeAccessInterface(Optional::Missing(), AttrTypeInfo::GetClusterId()) {} - CHIP_ERROR Read(const app::ConcreteAttributePath & aPath, app::AttributeValueEncoder & aEncoder) override + CHIP_ERROR Read(const app::ConcreteReadAttributePath & aPath, app::AttributeValueEncoder & aEncoder) override { if (aPath.mAttributeId == AttrTypeInfo::GetAttributeId()) { @@ -195,7 +195,7 @@ class ContentLauncherAttrAccess : public app::AttributeAccessInterface ContentLauncherAttrAccess() : app::AttributeAccessInterface(Optional::Missing(), app::Clusters::ContentLauncher::Id) {} - CHIP_ERROR Read(const app::ConcreteAttributePath & aPath, app::AttributeValueEncoder & aEncoder) override + CHIP_ERROR Read(const app::ConcreteReadAttributePath & aPath, app::AttributeValueEncoder & aEncoder) override { if (aPath.mAttributeId == app::Clusters::ContentLauncher::Attributes::AcceptsHeaderList::Id) { diff --git a/examples/tv-app/linux/include/cluster-init.cpp b/examples/tv-app/linux/include/cluster-init.cpp index 3d7bee4a9cfb48..aba5cafad51f2e 100644 --- a/examples/tv-app/linux/include/cluster-init.cpp +++ b/examples/tv-app/linux/include/cluster-init.cpp @@ -42,7 +42,7 @@ class TvAttrAccess : public app::AttributeAccessInterface public: TvAttrAccess() : app::AttributeAccessInterface(Optional::Missing(), AttrTypeInfo::GetClusterId()) {} - CHIP_ERROR Read(const app::ConcreteAttributePath & aPath, app::AttributeValueEncoder & aEncoder) override + CHIP_ERROR Read(const app::ConcreteReadAttributePath & aPath, app::AttributeValueEncoder & aEncoder) override { if (aPath.mAttributeId == AttrTypeInfo::GetAttributeId()) { @@ -195,7 +195,7 @@ class ContentLauncherAttrAccess : public app::AttributeAccessInterface ContentLauncherAttrAccess() : app::AttributeAccessInterface(Optional::Missing(), app::Clusters::ContentLauncher::Id) {} - CHIP_ERROR Read(const app::ConcreteAttributePath & aPath, app::AttributeValueEncoder & aEncoder) override + CHIP_ERROR Read(const app::ConcreteReadAttributePath & aPath, app::AttributeValueEncoder & aEncoder) override { if (aPath.mAttributeId == app::Clusters::ContentLauncher::Attributes::AcceptsHeaderList::Id) { diff --git a/src/app/AttributeAccessInterface.h b/src/app/AttributeAccessInterface.h index 569cfd1c7dbb8a..3459d00afa2cf8 100644 --- a/src/app/AttributeAccessInterface.h +++ b/src/app/AttributeAccessInterface.h @@ -145,7 +145,7 @@ class AttributeAccessInterface * This may involve reading from the attribute store or external * attribute callbacks. */ - virtual CHIP_ERROR Read(const ConcreteAttributePath & aPath, AttributeValueEncoder & aEncoder) = 0; + virtual CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) = 0; /** * Callback for writing attributes. @@ -159,7 +159,7 @@ class AttributeAccessInterface * This may involve writing to the attribute store or external * attribute callbacks. */ - virtual CHIP_ERROR Write(const ConcreteAttributePath & aPath, AttributeValueDecoder & aDecoder) { return CHIP_NO_ERROR; } + virtual CHIP_ERROR Write(const ConcreteDataAttributePath & aPath, AttributeValueDecoder & aDecoder) { return CHIP_NO_ERROR; } /** * Mechanism for keeping track of a chain of AttributeAccessInterfaces. diff --git a/src/app/AttributePathExpandIterator.cpp b/src/app/AttributePathExpandIterator.cpp index f77160df52cdd8..bc03aecb7bc4a5 100644 --- a/src/app/AttributePathExpandIterator.cpp +++ b/src/app/AttributePathExpandIterator.cpp @@ -175,7 +175,7 @@ bool AttributePathExpandIterator::Next() } // Reset to default, invalid value. - mOutputPath = ConcreteAttributePath(); + mOutputPath = ConcreteReadAttributePath(); return false; } } // namespace app diff --git a/src/app/BUILD.gn b/src/app/BUILD.gn index 269df93171cc0b..342f4aa2e02337 100644 --- a/src/app/BUILD.gn +++ b/src/app/BUILD.gn @@ -39,6 +39,7 @@ static_library("app") { "AttributePathExpandIterator.h", "AttributePathParams.cpp", "AttributePathParams.h", + "BufferedReadCallback.cpp", "CASESessionManager.cpp", "CASESessionManager.h", "Command.cpp", diff --git a/src/app/BufferedReadCallback.cpp b/src/app/BufferedReadCallback.cpp new file mode 100644 index 00000000000000..3241a61c7aa988 --- /dev/null +++ b/src/app/BufferedReadCallback.cpp @@ -0,0 +1,266 @@ +/* + * + * Copyright (c) 2021 Project CHIP Authors + * All rights reserved. + * + * 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. + */ + +#include "lib/core/CHIPTLV.h" +#include "lib/core/CHIPTLVTags.h" +#include "lib/core/CHIPTLVTypes.h" +#include "protocols/interaction_model/Constants.h" +#include "system/SystemPacketBuffer.h" +#include "system/TLVPacketBufferBackingStore.h" +#include +#include +#include + +namespace chip { +namespace app { + +void BufferedReadCallback::OnReportBegin(const ReadClient * apReadClient) +{ + mCallback.OnReportBegin(apReadClient); +} + +void BufferedReadCallback::OnReportEnd(const ReadClient * apReadClient) +{ + CHIP_ERROR err = DispatchBufferedData(apReadClient, mBufferedPath, StatusIB(), true); + if (err != CHIP_NO_ERROR) + { + mCallback.OnError(apReadClient, err); + } + + mCallback.OnReportEnd(apReadClient); +} + +CHIP_ERROR BufferedReadCallback::GenerateListTLV(TLV::ScopedBufferTLVReader & aReader) +{ + TLV::TLVType outerType; + Platform::ScopedMemoryBuffer backingBuffer; + + // + // To generate the final reconstituted list, we need to allocate a contiguous + // buffer than can hold the entirety of its contents. To do so, we need to figure out + // how big a buffer to allocate. This requires walking the buffered list items and computing their TLV sizes, + // summing them all up and adding a bit of slop to account for the TLV array the list elements will go into. + // + // The alternative was to use a PacketBufferTLVWriter backed by chained packet buffers to + // write out the list - this would have removed the need for this first pass. However, + // we cannot actually back a TLVReader with a chained buffer since that violates the ability + // for us to create readers off-of readers. Each reader would assume exclusive ownership of the chained + // buffer and mutate the state within TLVPacketBufferBackingStore, preventing shared use. + // + // To avoid that, a single contiguous buffer is the best likely approach for now. + // + uint32_t totalBufSize = 0; + for (size_t i = 0; i < mBufferedList.size(); i++) + { + totalBufSize += mBufferedList[i]->TotalLength(); + } + + // + // Size of the start container and end container are just 1 byte each, but, let's just be safe. + // + totalBufSize += 4; + + backingBuffer.Calloc(totalBufSize); + VerifyOrReturnError(backingBuffer.Get() != nullptr, CHIP_ERROR_NO_MEMORY); + + TLV::ScopedBufferTLVWriter writer(std::move(backingBuffer), totalBufSize); + ReturnErrorOnFailure(writer.StartContainer(TLV::AnonymousTag, TLV::kTLVType_Array, outerType)); + + for (auto & bufHandle : mBufferedList) + { + System::PacketBufferTLVReader reader; + + reader.Init(std::move(bufHandle)); + + ReturnErrorOnFailure(reader.Next()); + ReturnErrorOnFailure(writer.CopyElement(TLV::AnonymousTag, reader)); + } + + ReturnErrorOnFailure(writer.EndContainer(outerType)); + + writer.Finalize(backingBuffer); + + aReader.Init(std::move(backingBuffer), totalBufSize); + + return CHIP_NO_ERROR; +} + +CHIP_ERROR BufferedReadCallback::BufferListItem(TLV::TLVReader & reader) +{ + System::PacketBufferTLVWriter writer; + System::PacketBufferHandle handle; + + // + // We conservatively allocate a packet buffer as big as an IPv6 MTU (since we're buffering + // data received over the wire, which should always fit within that). + // + // We could have snapshotted the reader at its current position, advanced it past the current element + // and computed the delta in its read point to figure out the size of the element before allocating + // our target buffer. However, the reader's current position is already set past the control octet + // and the tag. Consequently, the computed size is always going to omit the sizes of these two parts of the + // TLV element. Since the tag can vary in size, for now, let's just do the safe thing. In the future, if this is a problem, + // we can improve this. + // + handle = System::PacketBufferHandle::New(chip::app::kMaxSecureSduLengthBytes); + + writer.Init(std::move(handle), false); + + ReturnErrorOnFailure(writer.CopyElement(TLV::AnonymousTag, reader)); + ReturnErrorOnFailure(writer.Finalize(&handle)); + + // Compact the buffer down to a more reasonably sized packet buffer + // if we can. + // + handle.RightSize(); + + mBufferedList.push_back(std::move(handle)); + + return CHIP_NO_ERROR; +} + +CHIP_ERROR BufferedReadCallback::BufferData(const ConcreteDataAttributePath & aPath, TLV::TLVReader * apData) +{ + + if (aPath.mListOp == ConcreteDataAttributePath::ListOperation::ReplaceAll) + { + TLV::TLVType outerContainer; + + VerifyOrReturnError(apData->GetType() == TLV::kTLVType_Array, CHIP_ERROR_INVALID_TLV_ELEMENT); + mBufferedList.clear(); + + ReturnErrorOnFailure(apData->EnterContainer(outerContainer)); + + CHIP_ERROR err; + + while ((err = apData->Next()) == CHIP_NO_ERROR) + { + ReturnErrorOnFailure(BufferListItem(*apData)); + } + + if (err == CHIP_END_OF_TLV) + { + err = CHIP_NO_ERROR; + } + + ReturnErrorOnFailure(err); + ReturnErrorOnFailure(apData->ExitContainer(outerContainer)); + } + else if (aPath.mListOp == ConcreteDataAttributePath::ListOperation::AppendItem) + { + ReturnErrorOnFailure(BufferListItem(*apData)); + } + + return CHIP_NO_ERROR; +} + +CHIP_ERROR BufferedReadCallback::DispatchBufferedData(const ReadClient * apReadClient, const ConcreteAttributePath & aPath, + const StatusIB & aStatusIB, bool aEndOfReport) +{ + if (aPath == mBufferedPath) + { + // + // If we encountered the same list again and it's not the last DataIB, then + // we need to continue to buffer up this list's data, so return immediately without dispatching + // the existing buffered up contents. + // + if (!aEndOfReport) + { + return CHIP_NO_ERROR; + } + + // + // If we had previously buffered up data for this list and now we have encountered + // an error for this list, that error takes precedence and the buffered data is now + // rendered invalid. Return immediately without dispatching the existing buffered up contents. + // + if (aStatusIB.mStatus != Protocols::InteractionModel::Status::Success) + { + return CHIP_NO_ERROR; + } + } + + if (!mBufferedPath.IsListOperation()) + { + return CHIP_NO_ERROR; + } + + StatusIB statusIB; + TLV::ScopedBufferTLVReader reader; + + ReturnErrorOnFailure(GenerateListTLV(reader)); + + // + // Update the list operation to now reflect the delivery of the entire list + // i.e a replace all operation. + // + mBufferedPath.mListOp = ConcreteDataAttributePath::ListOperation::ReplaceAll; + + // + // Advance the reader forward to the list itself + // + ReturnErrorOnFailure(reader.Next()); + + mCallback.OnAttributeData(apReadClient, mBufferedPath, &reader, statusIB); + + // + // Clear out our buffered contents to free up allocated buffers, and reset the buffered path. + // + mBufferedList.clear(); + mBufferedPath = ConcreteDataAttributePath(); + + return CHIP_NO_ERROR; +} + +void BufferedReadCallback::OnAttributeData(const ReadClient * apReadClient, const ConcreteDataAttributePath & aPath, + TLV::TLVReader * apData, const StatusIB & aStatus) +{ + CHIP_ERROR err; + + // + // First, let's dispatch to our registered callback any buffered up list data from previous calls. + // + err = DispatchBufferedData(apReadClient, aPath, aStatus); + SuccessOrExit(err); + + // + // We buffer up list data (only if the status was successful) + // + if (aPath.IsListOperation() && aStatus.mStatus == Protocols::InteractionModel::Status::Success) + { + err = BufferData(aPath, apData); + SuccessOrExit(err); + } + else + { + mCallback.OnAttributeData(apReadClient, aPath, apData, aStatus); + } + + // + // Update our latched buffered path. + // + mBufferedPath = aPath; + +exit: + if (err != CHIP_NO_ERROR) + { + mCallback.OnError(apReadClient, err); + } +} + +} // namespace app +} // namespace chip diff --git a/src/app/BufferedReadCallback.h b/src/app/BufferedReadCallback.h new file mode 100644 index 00000000000000..f177d8358b67ba --- /dev/null +++ b/src/app/BufferedReadCallback.h @@ -0,0 +1,101 @@ +/* + * + * Copyright (c) 2021 Project CHIP Authors + * All rights reserved. + * + * 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. + */ + +#pragma once + +#include "lib/core/CHIPTLV.h" +#include "system/SystemPacketBuffer.h" +#include "system/TLVPacketBufferBackingStore.h" +#include +#include +#include + +namespace chip { +namespace app { + +/* + * This is an adapter that intercepts calls that deliver data from the ReadClient, + * selectively buffers up list chunks in TLV and reconstitutes them into a singular, contiguous TLV array + * upon completion of delivery of all chunks. This is then delivered to a compliant ReadClient::Callback + * without any awareness on their part that chunking happened. + * + */ +class BufferedReadCallback : public ReadClient::Callback +{ +public: + BufferedReadCallback(Callback & callback) : mCallback(callback) {} + +private: + /* + * Generates the reconsistuted TLV array from the stored individual list elements + */ + CHIP_ERROR GenerateListTLV(TLV::ScopedBufferTLVReader & reader); + + /* + * Dispatch any buffered list data if we need to. Buffered data will only be dispatched if: + * 1. The path provided in aPath is different from the buffered path being tracked internally AND the type of data + * in the buffer is list data + * + * OR + * + * 2. The path provided in aPath is similar to what is buffered but we've hit the end of the report. + * + */ + CHIP_ERROR DispatchBufferedData(const ReadClient * apReadClient, const ConcreteAttributePath & aPath, const StatusIB & aStatus, + bool aEndOfReport = false); + + /* + * Buffer up list data as they arrive. + */ + CHIP_ERROR BufferData(const ConcreteDataAttributePath & aPath, TLV::TLVReader * apReader); + +private: + // + // ReadClient::Callback + // + void OnReportBegin(const ReadClient * apReadClient) override; + void OnReportEnd(const ReadClient * apReadClient) override; + void OnAttributeData(const ReadClient * apReadClient, const ConcreteDataAttributePath & aPath, TLV::TLVReader * apData, + const StatusIB & aStatus) override; + void OnError(const ReadClient * apReadClient, CHIP_ERROR aError) override { return mCallback.OnError(apReadClient, aError); } + void OnEventData(const ReadClient * apReadClient, const EventHeader & aEventHeader, TLV::TLVReader * apData, + const StatusIB * apStatus) override + { + return mCallback.OnEventData(apReadClient, aEventHeader, apData, apStatus); + } + + void OnDone(ReadClient * apReadClient) override { return mCallback.OnDone(apReadClient); } + void OnSubscriptionEstablished(const ReadClient * apReadClient) override { mCallback.OnSubscriptionEstablished(apReadClient); } + +private: + /* + * Given a reader positioned at a list element, allocate a packet buffer, copy the list item where + * the reader is positioned into that buffer and add it to our buffered list for tracking. + * + * This should be called in list index order starting from the lowest index that needs to be buffered. + * + */ + CHIP_ERROR BufferListItem(TLV::TLVReader & reader); + + ConcreteDataAttributePath mBufferedPath; + std::vector mBufferedList; + Callback & mCallback; +}; + +} // namespace app +} // namespace chip diff --git a/src/app/ConcreteAttributePath.h b/src/app/ConcreteAttributePath.h index 8b05580fa12c8a..4ade1b173fc012 100644 --- a/src/app/ConcreteAttributePath.h +++ b/src/app/ConcreteAttributePath.h @@ -19,16 +19,18 @@ #pragma once #include +#include namespace chip { namespace app { /** - * A representation of a concrete attribute path. + * A representation of a concrete attribute path. This does not convey any list index specifiers. */ struct ConcreteAttributePath { ConcreteAttributePath() {} + ConcreteAttributePath(EndpointId aEndpointId, ClusterId aClusterId, AttributeId aAttributeId) : mEndpointId(aEndpointId), mClusterId(aClusterId), mAttributeId(aAttributeId) {} @@ -42,5 +44,74 @@ struct ConcreteAttributePath ClusterId mClusterId = 0; AttributeId mAttributeId = 0; }; + +/** + * A representation of a concrete path as it appears in a Read or Subscribe + * request after path expansion. This contains support for expressing an + * optional list index. + */ +struct ConcreteReadAttributePath : public ConcreteAttributePath +{ + ConcreteReadAttributePath() {} + + ConcreteReadAttributePath(const ConcreteAttributePath & path) : + ConcreteReadAttributePath(path.mEndpointId, path.mClusterId, path.mAttributeId) + {} + + ConcreteReadAttributePath(EndpointId aEndpointId, ClusterId aClusterId, AttributeId aAttributeId) : + ConcreteAttributePath(aEndpointId, aClusterId, aAttributeId) + {} + + ConcreteReadAttributePath(EndpointId aEndpointId, ClusterId aClusterId, AttributeId aAttributeId, uint16_t aListIndex) : + ConcreteAttributePath(aEndpointId, aClusterId, aAttributeId) + { + mListIndex.SetValue(aListIndex); + } + + Optional mListIndex; +}; + +/** + * A representation of a concrete path as it appears in a Report or Write + * request after path expansion. This contains support for expressing list and list item-specific operations + * like replace, update, delete and append. + */ +struct ConcreteDataAttributePath : public ConcreteAttributePath +{ + enum class ListOperation + { + NotList, // Path points to an attribute that isn't a list. + ReplaceAll, // Path points to an attribute that is a list, indicating that the contents of the list should be replaced in + // its entirety. + ReplaceItem, // Path points to a specific item in a list, indicating that that item should be replaced in its entirety. + DeleteItem, // Path points to a specific item in a list, indicating that that item should be deleted from the list. + AppendItem // Path points to an attribute that is a list, indicating that an item should be appended into the list. + }; + + ConcreteDataAttributePath() {} + + ConcreteDataAttributePath(EndpointId aEndpointId, ClusterId aClusterId, AttributeId aAttributeId) : + ConcreteAttributePath(aEndpointId, aClusterId, aAttributeId) + {} + + ConcreteDataAttributePath(EndpointId aEndpointId, ClusterId aClusterId, AttributeId aAttributeId, ListOperation aListOp, + uint16_t aListIndex) : + ConcreteAttributePath(aEndpointId, aClusterId, aAttributeId) + { + mListOp = aListOp; + mListIndex = aListIndex; + } + + bool IsListOperation() const { return mListOp != ListOperation::NotList; } + bool IsListItemOperation() const { return ((mListOp != ListOperation::NotList) && (mListOp != ListOperation::ReplaceAll)); } + + // + // This index is only valid if `mListOp` is set to a list item operation, i.e + // ReplaceItem, DeleteItem or AppendItem. Otherwise, it is to be ignored. + // + uint16_t mListIndex = 0; + ListOperation mListOp = ListOperation::NotList; +}; + } // namespace app } // namespace chip diff --git a/src/app/DeviceControllerInteractionModelDelegate.h b/src/app/DeviceControllerInteractionModelDelegate.h index 95106c0d0f5855..dd50c5c462cba0 100644 --- a/src/app/DeviceControllerInteractionModelDelegate.h +++ b/src/app/DeviceControllerInteractionModelDelegate.h @@ -2,6 +2,7 @@ #include +#include #include #include #include @@ -22,6 +23,10 @@ class DeviceControllerInteractionModelDelegate : public chip::app::ReadClient::C public chip::app::InteractionModelDelegate { public: + DeviceControllerInteractionModelDelegate() : mBufferedReadAdapter(*this) {} + + app::BufferedReadCallback & GetBufferedCallback() { return mBufferedReadAdapter; } + void OnResponse(app::CommandSender * apCommandSender, const app::ConcreteCommandPath & aPath, const chip::app::StatusIB & aStatus, TLV::TLVReader * aData) override { @@ -72,9 +77,15 @@ class DeviceControllerInteractionModelDelegate : public chip::app::ReadClient::C const app::StatusIB * apStatus) override {} - void OnAttributeData(const app::ReadClient * apReadClient, const app::ConcreteAttributePath & aPath, TLV::TLVReader * apData, - const app::StatusIB & aStatus) override + void OnAttributeData(const app::ReadClient * apReadClient, const app::ConcreteDataAttributePath & aPath, + TLV::TLVReader * apData, const app::StatusIB & aStatus) override { + // + // We shouldn't be getting list item operations in the provided path since that should be handled by the buffered read + // callback. If we do, that's a bug. + // + VerifyOrDie(!aPath.IsListItemOperation()); + IMReadReportAttributesResponseCallback(apReadClient, &aPath, apData, aStatus.mStatus); } @@ -135,6 +146,8 @@ class DeviceControllerInteractionModelDelegate : public chip::app::ReadClient::C uint64_t ApplicationId = UINT64_MAX; app::AttributePathParams Params; }; + + app::BufferedReadCallback mBufferedReadAdapter; AttributePathTransactionMap mAttributePathTransactionMapPool[CHIP_DEVICE_CONTROLLER_SUBSCRIPTION_ATTRIBUTE_PATH_POOL_SIZE]; }; diff --git a/src/app/DeviceProxy.cpp b/src/app/DeviceProxy.cpp index 3c7c6b1f84bfd1..5f3826186ae3a4 100644 --- a/src/app/DeviceProxy.cpp +++ b/src/app/DeviceProxy.cpp @@ -81,7 +81,7 @@ CHIP_ERROR DeviceProxy::SendReadAttributeRequest(app::AttributePathParams aPath, app::ReadClient * readClient = nullptr; ReturnErrorOnFailure(chip::app::InteractionModelEngine::GetInstance()->NewReadClient( - &readClient, app::ReadClient::InteractionType::Read, GetInteractionModelDelegate())); + &readClient, app::ReadClient::InteractionType::Read, &GetInteractionModelDelegate()->GetBufferedCallback())); if (onSuccessCallback != nullptr || onFailureCallback != nullptr) { @@ -118,7 +118,7 @@ CHIP_ERROR DeviceProxy::SendSubscribeAttributeRequest(app::AttributePathParams a app::ReadClient * readClient = nullptr; ReturnErrorOnFailure(chip::app::InteractionModelEngine::GetInstance()->NewReadClient( - &readClient, app::ReadClient::InteractionType::Subscribe, GetInteractionModelDelegate())); + &readClient, app::ReadClient::InteractionType::Subscribe, &GetInteractionModelDelegate()->GetBufferedCallback())); // The application context is used to identify different requests from client application the type of it is intptr_t, here we // use the seqNum. diff --git a/src/app/ReadClient.cpp b/src/app/ReadClient.cpp index 15afb3cc79a72f..f71eb6e4da78cb 100644 --- a/src/app/ReadClient.cpp +++ b/src/app/ReadClient.cpp @@ -22,6 +22,7 @@ * */ +#include "lib/core/CHIPTLVTypes.h" #include #include #include @@ -389,8 +390,13 @@ CHIP_ERROR ReadClient::ProcessReportData(System::PacketBufferHandle && aPayload) { TLV::TLVReader attributeReportIBsReader; attributeReportIBs.GetReader(&attributeReportIBsReader); + + mpCallback->OnReportBegin(this); + err = ProcessAttributeReportIBs(attributeReportIBsReader); SuccessOrExit(err); + + mpCallback->OnReportEnd(this); } if (!suppressResponse) @@ -485,7 +491,7 @@ CHIP_ERROR ReadClient::ProcessAttributeReportIBs(TLV::TLVReader & aAttributeRepo ReturnErrorOnFailure(status.GetErrorStatus(&errorStatus)); ReturnErrorOnFailure(errorStatus.DecodeStatusIB(statusIB)); mpCallback->OnAttributeData( - this, ConcreteAttributePath(clusterInfo.mEndpointId, clusterInfo.mClusterId, clusterInfo.mAttributeId), nullptr, + this, ConcreteDataAttributePath(clusterInfo.mEndpointId, clusterInfo.mClusterId, clusterInfo.mAttributeId), nullptr, statusIB); } else if (CHIP_END_OF_TLV == err) @@ -494,9 +500,15 @@ CHIP_ERROR ReadClient::ProcessAttributeReportIBs(TLV::TLVReader & aAttributeRepo ReturnErrorOnFailure(data.GetPath(&path)); ReturnErrorOnFailure(ProcessAttributePath(path, clusterInfo)); ReturnErrorOnFailure(data.GetData(&dataReader)); - mpCallback->OnAttributeData( - this, ConcreteAttributePath(clusterInfo.mEndpointId, clusterInfo.mClusterId, clusterInfo.mAttributeId), &dataReader, - statusIB); + + ConcreteDataAttributePath attributePath(clusterInfo.mEndpointId, clusterInfo.mClusterId, clusterInfo.mAttributeId); + + if (dataReader.GetType() == TLV::kTLVType_Array) + { + attributePath.mListOp = ConcreteDataAttributePath::ListOperation::ReplaceAll; + } + + mpCallback->OnAttributeData(this, attributePath, &dataReader, statusIB); } } diff --git a/src/app/ReadClient.h b/src/app/ReadClient.h index 1d8a52de120c09..e889e05754cb3c 100644 --- a/src/app/ReadClient.h +++ b/src/app/ReadClient.h @@ -24,6 +24,7 @@ #pragma once #include +#include #include #include #include @@ -61,6 +62,16 @@ class ReadClient : public Messaging::ExchangeDelegate public: virtual ~Callback() = default; + /** + * Used to signal the commencement of processing of the first attribute report received in a given exchange. + */ + virtual void OnReportBegin(const ReadClient * apReadClient) {} + + /** + * Used to signal the completion of processing of the last attribute report in a given exchange. + */ + virtual void OnReportEnd(const ReadClient * apReadClient) {} + /** * The ReadClient object MUST continue to exist after this call is completed. * @@ -93,8 +104,8 @@ class ReadClient : public Messaging::ExchangeDelegate * @param[in] aStatus Attribute-specific status, containing an InteractionModel::Status code as well as an * optional cluster-specific status code. */ - virtual void OnAttributeData(const ReadClient * apReadClient, const ConcreteAttributePath & aPath, TLV::TLVReader * apData, - const StatusIB & aStatus) + virtual void OnAttributeData(const ReadClient * apReadClient, const ConcreteDataAttributePath & aPath, + TLV::TLVReader * apData, const StatusIB & aStatus) {} /** diff --git a/src/app/clusters/descriptor/descriptor.cpp b/src/app/clusters/descriptor/descriptor.cpp index 9a24fdf34624b8..3b61efbc515adb 100644 --- a/src/app/clusters/descriptor/descriptor.cpp +++ b/src/app/clusters/descriptor/descriptor.cpp @@ -42,7 +42,7 @@ class DescriptorAttrAccess : public AttributeAccessInterface // Register for the Descriptor cluster on all endpoints. DescriptorAttrAccess() : AttributeAccessInterface(Optional::Missing(), Descriptor::Id) {} - CHIP_ERROR Read(const ConcreteAttributePath & aPath, AttributeValueEncoder & aEncoder) override; + CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override; private: static constexpr uint16_t ClusterRevision = 1; @@ -121,7 +121,7 @@ CHIP_ERROR DescriptorAttrAccess::ReadClusterRevision(EndpointId endpoint, Attrib DescriptorAttrAccess gAttrAccess; -CHIP_ERROR DescriptorAttrAccess::Read(const ConcreteAttributePath & aPath, AttributeValueEncoder & aEncoder) +CHIP_ERROR DescriptorAttrAccess::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) { VerifyOrDie(aPath.mClusterId == Descriptor::Id); diff --git a/src/app/clusters/ethernet_network_diagnostics_server/ethernet_network_diagnostics_server.cpp b/src/app/clusters/ethernet_network_diagnostics_server/ethernet_network_diagnostics_server.cpp index 245abaa1f1ef58..ee14423d255080 100644 --- a/src/app/clusters/ethernet_network_diagnostics_server/ethernet_network_diagnostics_server.cpp +++ b/src/app/clusters/ethernet_network_diagnostics_server/ethernet_network_diagnostics_server.cpp @@ -42,7 +42,7 @@ class EthernetDiagosticsAttrAccess : public AttributeAccessInterface // Register for the EthernetNetworkDiagnostics cluster on all endpoints. EthernetDiagosticsAttrAccess() : AttributeAccessInterface(Optional::Missing(), EthernetNetworkDiagnostics::Id) {} - CHIP_ERROR Read(const ConcreteAttributePath & aPath, AttributeValueEncoder & aEncoder) override; + CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override; private: template @@ -69,7 +69,7 @@ CHIP_ERROR EthernetDiagosticsAttrAccess::ReadIfSupported(CHIP_ERROR (Connectivit EthernetDiagosticsAttrAccess gAttrAccess; -CHIP_ERROR EthernetDiagosticsAttrAccess::Read(const ConcreteAttributePath & aPath, AttributeValueEncoder & aEncoder) +CHIP_ERROR EthernetDiagosticsAttrAccess::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) { if (aPath.mClusterId != EthernetNetworkDiagnostics::Id) { diff --git a/src/app/clusters/general_diagnostics_server/general_diagnostics_server.cpp b/src/app/clusters/general_diagnostics_server/general_diagnostics_server.cpp index 483c41361e7a61..275750ec4fab31 100644 --- a/src/app/clusters/general_diagnostics_server/general_diagnostics_server.cpp +++ b/src/app/clusters/general_diagnostics_server/general_diagnostics_server.cpp @@ -42,7 +42,7 @@ class GeneralDiagosticsAttrAccess : public AttributeAccessInterface // Register for the GeneralDiagnostics cluster on all endpoints. GeneralDiagosticsAttrAccess() : AttributeAccessInterface(Optional::Missing(), GeneralDiagnostics::Id) {} - CHIP_ERROR Read(const ConcreteAttributePath & aPath, AttributeValueEncoder & aEncoder) override; + CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override; private: template @@ -126,7 +126,7 @@ CHIP_ERROR GeneralDiagosticsAttrAccess::ReadNetworkInterfaces(AttributeValueEnco GeneralDiagosticsAttrAccess gAttrAccess; -CHIP_ERROR GeneralDiagosticsAttrAccess::Read(const ConcreteAttributePath & aPath, AttributeValueEncoder & aEncoder) +CHIP_ERROR GeneralDiagosticsAttrAccess::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) { if (aPath.mClusterId != GeneralDiagnostics::Id) { diff --git a/src/app/clusters/mode-select-server/mode-select-server.cpp b/src/app/clusters/mode-select-server/mode-select-server.cpp index 7d90cf1c030a09..31ca591768f7f5 100644 --- a/src/app/clusters/mode-select-server/mode-select-server.cpp +++ b/src/app/clusters/mode-select-server/mode-select-server.cpp @@ -42,12 +42,12 @@ class ModeSelectAttrAccess : public AttributeAccessInterface public: ModeSelectAttrAccess() : AttributeAccessInterface(Optional::Missing(), ModeSelect::Id) {} - CHIP_ERROR Read(const ConcreteAttributePath & aPath, AttributeValueEncoder & aEncoder) override; + CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override; }; ModeSelectAttrAccess gModeSelectAttrAccess; -CHIP_ERROR ModeSelectAttrAccess::Read(const ConcreteAttributePath & aPath, AttributeValueEncoder & aEncoder) +CHIP_ERROR ModeSelectAttrAccess::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) { VerifyOrDie(aPath.mClusterId == ModeSelect::Id); diff --git a/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp b/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp index c9338fe6a52352..a4818ab0791cf4 100644 --- a/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp +++ b/src/app/clusters/operational-credentials-server/operational-credentials-server.cpp @@ -66,7 +66,7 @@ class OperationalCredentialsAttrAccess : public AttributeAccessInterface AttributeAccessInterface(Optional::Missing(), Clusters::OperationalCredentials::Id) {} - CHIP_ERROR Read(const ConcreteAttributePath & aPath, AttributeValueEncoder & aEncoder) override; + CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override; private: CHIP_ERROR ReadSupportedFabrics(EndpointId endpoint, AttributeValueEncoder & aEncoder); @@ -132,7 +132,7 @@ CHIP_ERROR OperationalCredentialsAttrAccess::ReadRootCertificates(EndpointId end OperationalCredentialsAttrAccess gAttrAccess; -CHIP_ERROR OperationalCredentialsAttrAccess::Read(const ConcreteAttributePath & aPath, AttributeValueEncoder & aEncoder) +CHIP_ERROR OperationalCredentialsAttrAccess::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) { VerifyOrDie(aPath.mClusterId == Clusters::OperationalCredentials::Id); diff --git a/src/app/clusters/software_diagnostics_server/software_diagnostics_server.cpp b/src/app/clusters/software_diagnostics_server/software_diagnostics_server.cpp index d07bbd890aa610..3a29d8eafca146 100644 --- a/src/app/clusters/software_diagnostics_server/software_diagnostics_server.cpp +++ b/src/app/clusters/software_diagnostics_server/software_diagnostics_server.cpp @@ -42,7 +42,7 @@ class SoftwareDiagosticsAttrAccess : public AttributeAccessInterface // Register for the SoftwareDiagnostics cluster on all endpoints. SoftwareDiagosticsAttrAccess() : AttributeAccessInterface(Optional::Missing(), SoftwareDiagnostics::Id) {} - CHIP_ERROR Read(const ConcreteAttributePath & aPath, AttributeValueEncoder & aEncoder) override; + CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override; private: CHIP_ERROR ReadIfSupported(CHIP_ERROR (PlatformManager::*getter)(uint64_t &), AttributeValueEncoder & aEncoder); @@ -51,7 +51,7 @@ class SoftwareDiagosticsAttrAccess : public AttributeAccessInterface SoftwareDiagosticsAttrAccess gAttrAccess; -CHIP_ERROR SoftwareDiagosticsAttrAccess::Read(const ConcreteAttributePath & aPath, AttributeValueEncoder & aEncoder) +CHIP_ERROR SoftwareDiagosticsAttrAccess::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) { if (aPath.mClusterId != SoftwareDiagnostics::Id) { diff --git a/src/app/clusters/test-cluster-server/test-cluster-server.cpp b/src/app/clusters/test-cluster-server/test-cluster-server.cpp index 6f7568b8a25cab..fe34de046e3691 100644 --- a/src/app/clusters/test-cluster-server/test-cluster-server.cpp +++ b/src/app/clusters/test-cluster-server/test-cluster-server.cpp @@ -66,8 +66,8 @@ class TestAttrAccess : public AttributeAccessInterface // Register for the Test Cluster cluster on all endpoints. TestAttrAccess() : AttributeAccessInterface(Optional::Missing(), TestCluster::Id) {} - CHIP_ERROR Read(const ConcreteAttributePath & aPath, AttributeValueEncoder & aEncoder) override; - CHIP_ERROR Write(const ConcreteAttributePath & aPath, AttributeValueDecoder & aDecoder) override; + CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override; + CHIP_ERROR Write(const ConcreteDataAttributePath & aPath, AttributeValueDecoder & aDecoder) override; private: CHIP_ERROR ReadListInt8uAttribute(AttributeValueEncoder & aEncoder); @@ -86,7 +86,7 @@ OctetStringData gListOctetStringData[kAttributeListLength]; OctetStringData gListOperationalCert[kAttributeListLength]; Structs::TestListStructOctet::Type listStructOctetStringData[kAttributeListLength]; -CHIP_ERROR TestAttrAccess::Read(const ConcreteAttributePath & aPath, AttributeValueEncoder & aEncoder) +CHIP_ERROR TestAttrAccess::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) { switch (aPath.mAttributeId) { @@ -110,7 +110,7 @@ CHIP_ERROR TestAttrAccess::Read(const ConcreteAttributePath & aPath, AttributeVa return CHIP_NO_ERROR; } -CHIP_ERROR TestAttrAccess::Write(const ConcreteAttributePath & aPath, AttributeValueDecoder & aDecoder) +CHIP_ERROR TestAttrAccess::Write(const ConcreteDataAttributePath & aPath, AttributeValueDecoder & aDecoder) { switch (aPath.mAttributeId) { diff --git a/src/app/clusters/thread_network_diagnostics_server/thread_network_diagnostics_server.cpp b/src/app/clusters/thread_network_diagnostics_server/thread_network_diagnostics_server.cpp index 52428ddc33c8c7..a7024e656d7b89 100644 --- a/src/app/clusters/thread_network_diagnostics_server/thread_network_diagnostics_server.cpp +++ b/src/app/clusters/thread_network_diagnostics_server/thread_network_diagnostics_server.cpp @@ -46,12 +46,12 @@ class ThreadDiagosticsAttrAccess : public AttributeAccessInterface // Register for the ThreadNetworkDiagnostics cluster on all endpoints. ThreadDiagosticsAttrAccess() : AttributeAccessInterface(Optional::Missing(), ThreadNetworkDiagnostics::Id) {} - CHIP_ERROR Read(const ConcreteAttributePath & aPath, AttributeValueEncoder & aEncoder) override; + CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override; }; ThreadDiagosticsAttrAccess gAttrAccess; -CHIP_ERROR ThreadDiagosticsAttrAccess::Read(const ConcreteAttributePath & aPath, AttributeValueEncoder & aEncoder) +CHIP_ERROR ThreadDiagosticsAttrAccess::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) { if (aPath.mClusterId != ThreadNetworkDiagnostics::Id) { diff --git a/src/app/clusters/wifi_network_diagnostics_server/wifi_network_diagnostics_server.cpp b/src/app/clusters/wifi_network_diagnostics_server/wifi_network_diagnostics_server.cpp index 4372f621992d6d..b83768c332c05d 100644 --- a/src/app/clusters/wifi_network_diagnostics_server/wifi_network_diagnostics_server.cpp +++ b/src/app/clusters/wifi_network_diagnostics_server/wifi_network_diagnostics_server.cpp @@ -43,7 +43,7 @@ class WiFiDiagosticsAttrAccess : public AttributeAccessInterface // Register for the WiFiNetworkDiagnostics cluster on all endpoints. WiFiDiagosticsAttrAccess() : AttributeAccessInterface(Optional::Missing(), WiFiNetworkDiagnostics::Id) {} - CHIP_ERROR Read(const ConcreteAttributePath & aPath, AttributeValueEncoder & aEncoder) override; + CHIP_ERROR Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) override; private: template @@ -90,7 +90,7 @@ CHIP_ERROR WiFiDiagosticsAttrAccess::ReadWiFiBssId(AttributeValueEncoder & aEnco WiFiDiagosticsAttrAccess gAttrAccess; -CHIP_ERROR WiFiDiagosticsAttrAccess::Read(const ConcreteAttributePath & aPath, AttributeValueEncoder & aEncoder) +CHIP_ERROR WiFiDiagosticsAttrAccess::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder) { if (aPath.mClusterId != WiFiNetworkDiagnostics::Id) { diff --git a/src/app/tests/BUILD.gn b/src/app/tests/BUILD.gn index d58ba38ec83e56..2afac555a09b5b 100644 --- a/src/app/tests/BUILD.gn +++ b/src/app/tests/BUILD.gn @@ -59,6 +59,14 @@ chip_test_suite("tests") { "TestWriteInteraction.cpp", ] + # + # On NRF platforms, the allocation of a large number of pbufs in this test + # to exercise chunking causes it to run out of memory. For now, disable it there. + # + if (chip_device_platform != "nrfconnect") { + test_sources += [ "TestBufferedReadCallback.cpp" ] + } + cflags = [ "-Wconversion" ] public_deps = [ diff --git a/src/app/tests/TestBufferedReadCallback.cpp b/src/app/tests/TestBufferedReadCallback.cpp new file mode 100644 index 00000000000000..e10eb49ce76629 --- /dev/null +++ b/src/app/tests/TestBufferedReadCallback.cpp @@ -0,0 +1,628 @@ +/* + * + * Copyright (c) 2021 Project CHIP Authors + * All rights reserved. + * + * 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. + */ + +#include "app-common/zap-generated/ids/Attributes.h" +#include "lib/core/CHIPTLVTags.h" +#include "protocols/interaction_model/Constants.h" +#include "system/SystemPacketBuffer.h" +#include "system/TLVPacketBufferBackingStore.h" +#include +#include +#include +#include +#include +#include +#include +#include + +using TestContext = chip::Test::AppContext; +using namespace chip::app; +using namespace chip; + +namespace { + +nlTestSuite * gSuite = nullptr; + +struct ValidationInstruction +{ + enum ProcessingType + { + kValidChunk, + kDiscardedChunk + }; + + enum ValidationType + { + kSimpleAttributeA, + kSimpleAttributeB, + kListAttributeC_Empty, + kListAttributeC_NotEmpty, + kListAttributeC_NotEmpty_Chunked, + kListAttributeC_Error, + kListAttributeD_Empty, + kListAttributeD_NotEmpty, + kListAttributeD_NotEmpty_Chunked, + kListAttributeD_Error + }; + + ValidationType mValidationType; + ProcessingType mProcessingType = kValidChunk; +}; + +using InstructionListType = std::vector; + +class DataSeriesValidator : public BufferedReadCallback::Callback +{ +public: + DataSeriesValidator(std::vector validationInstructionList) + { + mInstructionList = validationInstructionList; + } + + // + // BufferedReadCallback::Callback + // + + void OnReportBegin(const ReadClient * apReadClient) override; + void OnReportEnd(const ReadClient * apReadClient) override; + void OnAttributeData(const ReadClient * apReadClient, const ConcreteDataAttributePath & aPath, TLV::TLVReader * apData, + const StatusIB & aStatus) override; + void OnDone(ReadClient * apClient) override {} + + std::vector mInstructionList; + uint32_t mCurrentInstruction = 0; +}; + +void DataSeriesValidator::OnReportBegin(const ReadClient * apReadClient) +{ + mCurrentInstruction = 0; +} + +void DataSeriesValidator::OnReportEnd(const ReadClient * apReadClient) {} + +void DataSeriesValidator::OnAttributeData(const ReadClient * apReadClient, const ConcreteDataAttributePath & aPath, + TLV::TLVReader * apData, const StatusIB & aStatus) +{ + uint32_t expectedListLength; + + while ((mCurrentInstruction < mInstructionList.size()) && + (mInstructionList[mCurrentInstruction].mProcessingType == ValidationInstruction::kDiscardedChunk)) + { + mCurrentInstruction++; + } + + if (mCurrentInstruction >= mInstructionList.size()) + { + return; + } + + switch (mInstructionList[mCurrentInstruction].mValidationType) + { + case ValidationInstruction::kSimpleAttributeA: { + ChipLogProgress(DataManagement, "\t\t -- Validating A"); + + Clusters::TestCluster::Attributes::Int8u::TypeInfo::Type value; + NL_TEST_ASSERT(gSuite, + aPath.mEndpointId == 0 && aPath.mClusterId == Clusters::TestCluster::Id && + aPath.mAttributeId == Clusters::TestCluster::Attributes::Int8u::Id && + aPath.mListOp == ConcreteDataAttributePath::ListOperation::NotList); + NL_TEST_ASSERT(gSuite, DataModel::Decode(*apData, value) == CHIP_NO_ERROR); + NL_TEST_ASSERT(gSuite, value == mCurrentInstruction); + break; + } + + case ValidationInstruction::kSimpleAttributeB: { + ChipLogProgress(DataManagement, "\t\t -- Validating B"); + + Clusters::TestCluster::Attributes::Int32u::TypeInfo::Type value; + NL_TEST_ASSERT(gSuite, + aPath.mEndpointId == 0 && aPath.mClusterId == Clusters::TestCluster::Id && + aPath.mAttributeId == Clusters::TestCluster::Attributes::Int32u::Id && + aPath.mListOp == ConcreteDataAttributePath::ListOperation::NotList); + NL_TEST_ASSERT(gSuite, DataModel::Decode(*apData, value) == CHIP_NO_ERROR); + NL_TEST_ASSERT(gSuite, value == mCurrentInstruction); + break; + } + + case ValidationInstruction::kListAttributeC_Empty: { + ChipLogProgress(DataManagement, "\t\t -- Validating C[]"); + + Clusters::TestCluster::Attributes::ListStructOctetString::TypeInfo::DecodableType value; + size_t len; + + NL_TEST_ASSERT(gSuite, + aPath.mEndpointId == 0 && aPath.mClusterId == Clusters::TestCluster::Id && + aPath.mAttributeId == Clusters::TestCluster::Attributes::ListStructOctetString::Id && + aPath.mListOp == ConcreteDataAttributePath::ListOperation::ReplaceAll); + NL_TEST_ASSERT(gSuite, DataModel::Decode(*apData, value) == CHIP_NO_ERROR); + NL_TEST_ASSERT(gSuite, value.ComputeSize(&len) == CHIP_NO_ERROR); + NL_TEST_ASSERT(gSuite, len == 0); + + break; + } + + case ValidationInstruction::kListAttributeC_NotEmpty_Chunked: + case ValidationInstruction::kListAttributeC_NotEmpty: { + if (mInstructionList[mCurrentInstruction].mValidationType == ValidationInstruction::kListAttributeC_NotEmpty_Chunked) + { + expectedListLength = 512; + } + else + { + expectedListLength = 2; + } + + ChipLogProgress(DataManagement, "\t\t -- Validating C[%" PRIu32 "]", expectedListLength); + + Clusters::TestCluster::Attributes::ListStructOctetString::TypeInfo::DecodableType value; + size_t len; + + NL_TEST_ASSERT(gSuite, + aPath.mEndpointId == 0 && aPath.mClusterId == Clusters::TestCluster::Id && + aPath.mAttributeId == Clusters::TestCluster::Attributes::ListStructOctetString::Id && + aPath.mListOp == ConcreteDataAttributePath::ListOperation::ReplaceAll); + NL_TEST_ASSERT(gSuite, DataModel::Decode(*apData, value) == CHIP_NO_ERROR); + NL_TEST_ASSERT(gSuite, value.ComputeSize(&len) == CHIP_NO_ERROR); + NL_TEST_ASSERT(gSuite, len == expectedListLength); + + auto iter = value.begin(); + + uint32_t index = 0; + while (iter.Next() && index < expectedListLength) + { + auto & iterValue = iter.GetValue(); + NL_TEST_ASSERT(gSuite, iterValue.fabricIndex == (index)); + index++; + } + + NL_TEST_ASSERT(gSuite, iter.GetStatus() == CHIP_NO_ERROR); + break; + } + + case ValidationInstruction::kListAttributeD_Empty: { + ChipLogProgress(DataManagement, "\t\t -- Validating D[]"); + + Clusters::TestCluster::Attributes::ListInt8u::TypeInfo::DecodableType value; + size_t len; + + NL_TEST_ASSERT(gSuite, + aPath.mEndpointId == 0 && aPath.mClusterId == Clusters::TestCluster::Id && + aPath.mAttributeId == Clusters::TestCluster::Attributes::ListInt8u::Id && + aPath.mListOp == ConcreteDataAttributePath::ListOperation::ReplaceAll); + NL_TEST_ASSERT(gSuite, DataModel::Decode(*apData, value) == CHIP_NO_ERROR); + NL_TEST_ASSERT(gSuite, value.ComputeSize(&len) == CHIP_NO_ERROR); + NL_TEST_ASSERT(gSuite, len == 0); + + break; + } + + case ValidationInstruction::kListAttributeD_NotEmpty: + case ValidationInstruction::kListAttributeD_NotEmpty_Chunked: { + if (mInstructionList[mCurrentInstruction].mValidationType == ValidationInstruction::kListAttributeD_NotEmpty_Chunked) + { + expectedListLength = 512; + } + else + { + expectedListLength = 2; + } + + ChipLogProgress(DataManagement, "\t\t -- Validating D[%" PRIu32 "]", expectedListLength); + + Clusters::TestCluster::Attributes::ListInt8u::TypeInfo::DecodableType value; + size_t len; + + NL_TEST_ASSERT(gSuite, + aPath.mEndpointId == 0 && aPath.mClusterId == Clusters::TestCluster::Id && + aPath.mAttributeId == Clusters::TestCluster::Attributes::ListInt8u::Id && + aPath.mListOp == ConcreteDataAttributePath::ListOperation::ReplaceAll); + NL_TEST_ASSERT(gSuite, DataModel::Decode(*apData, value) == CHIP_NO_ERROR); + NL_TEST_ASSERT(gSuite, value.ComputeSize(&len) == CHIP_NO_ERROR); + NL_TEST_ASSERT(gSuite, len == expectedListLength); + + auto iter = value.begin(); + + uint8_t index = 0; + while (iter.Next() && index < expectedListLength) + { + auto & iterValue = iter.GetValue(); + NL_TEST_ASSERT(gSuite, iterValue == (index)); + index++; + } + + NL_TEST_ASSERT(gSuite, iter.GetStatus() == CHIP_NO_ERROR); + break; + } + + case ValidationInstruction::kListAttributeC_Error: { + ChipLogProgress(DataManagement, "\t\t -- Validating C|e"); + + NL_TEST_ASSERT(gSuite, + aPath.mEndpointId == 0 && aPath.mClusterId == Clusters::TestCluster::Id && + aPath.mAttributeId == Clusters::TestCluster::Attributes::ListStructOctetString::Id && + aPath.mListOp == ConcreteDataAttributePath::ListOperation::ReplaceAll); + NL_TEST_ASSERT(gSuite, aStatus.mStatus == Protocols::InteractionModel::Status::Failure); + break; + } + + case ValidationInstruction::kListAttributeD_Error: { + ChipLogProgress(DataManagement, "\t\t -- Validating D|e"); + + NL_TEST_ASSERT(gSuite, + aPath.mEndpointId == 0 && aPath.mClusterId == Clusters::TestCluster::Id && + aPath.mAttributeId == Clusters::TestCluster::Attributes::ListInt8u::Id && + aPath.mListOp == ConcreteDataAttributePath::ListOperation::ReplaceAll); + NL_TEST_ASSERT(gSuite, aStatus.mStatus == Protocols::InteractionModel::Status::Failure); + + break; + } + + default: + break; + } + + mCurrentInstruction++; +} + +class DataSeriesGenerator +{ +public: + DataSeriesGenerator(BufferedReadCallback & readCallback, std::vector instructionList) : + mReadCallback(readCallback), mInstructionList(instructionList) + {} + + void Generate(); + +private: + BufferedReadCallback & mReadCallback; + std::vector mInstructionList; +}; + +void DataSeriesGenerator::Generate() +{ + System::PacketBufferHandle handle; + System::PacketBufferTLVWriter writer; + ConcreteDataAttributePath path(0, Clusters::TestCluster::Id, 0); + System::PacketBufferTLVReader reader; + ReadClient::Callback * callback = &mReadCallback; + StatusIB status; + bool hasData; + + callback->OnReportBegin(nullptr); + + uint8_t index = 0; + for (auto & instruction : mInstructionList) + { + handle = System::PacketBufferHandle::New(1000); + writer.Init(std::move(handle), true); + status = StatusIB(); + hasData = true; + + switch (instruction.mValidationType) + { + case ValidationInstruction::kSimpleAttributeA: { + ChipLogProgress(DataManagement, "\t -- Generating A"); + + Clusters::TestCluster::Attributes::Int8u::TypeInfo::Type value = index; + path.mAttributeId = Clusters::TestCluster::Attributes::Int8u::Id; + path.mListOp = ConcreteDataAttributePath::ListOperation::NotList; + NL_TEST_ASSERT(gSuite, DataModel::Encode(writer, TLV::AnonymousTag, value) == CHIP_NO_ERROR); + break; + } + + case ValidationInstruction::kSimpleAttributeB: { + ChipLogProgress(DataManagement, "\t -- Generating B"); + + Clusters::TestCluster::Attributes::Int32u::TypeInfo::Type value = index; + path.mAttributeId = Clusters::TestCluster::Attributes::Int32u::Id; + path.mListOp = ConcreteDataAttributePath::ListOperation::NotList; + NL_TEST_ASSERT(gSuite, DataModel::Encode(writer, TLV::AnonymousTag, value) == CHIP_NO_ERROR); + break; + } + + case ValidationInstruction::kListAttributeC_Empty: { + ChipLogProgress(DataManagement, "\t -- Generating C[]"); + + Clusters::TestCluster::Attributes::ListStructOctetString::TypeInfo::Type value; + path.mAttributeId = Clusters::TestCluster::Attributes::ListStructOctetString::Id; + path.mListOp = ConcreteDataAttributePath::ListOperation::ReplaceAll; + NL_TEST_ASSERT(gSuite, DataModel::Encode(writer, TLV::AnonymousTag, value) == CHIP_NO_ERROR); + break; + } + + case ValidationInstruction::kListAttributeC_NotEmpty: { + ChipLogProgress(DataManagement, "\t -- Generating C[2]"); + + Clusters::TestCluster::Structs::TestListStructOctet::Type listData[2]; + Clusters::TestCluster::Attributes::ListStructOctetString::TypeInfo::Type value(listData); + + uint8_t index2 = 0; + for (auto & item : listData) + { + item.fabricIndex = index2; + index2++; + } + + path.mAttributeId = Clusters::TestCluster::Attributes::ListStructOctetString::Id; + path.mListOp = ConcreteDataAttributePath::ListOperation::ReplaceAll; + + NL_TEST_ASSERT(gSuite, DataModel::Encode(writer, TLV::AnonymousTag, value) == CHIP_NO_ERROR); + break; + } + + case ValidationInstruction::kListAttributeD_Empty: { + ChipLogProgress(DataManagement, "\t -- Generating D[]"); + + Clusters::TestCluster::Attributes::ListInt8u::TypeInfo::Type value; + path.mAttributeId = Clusters::TestCluster::Attributes::ListInt8u::Id; + path.mListOp = ConcreteDataAttributePath::ListOperation::ReplaceAll; + NL_TEST_ASSERT(gSuite, DataModel::Encode(writer, TLV::AnonymousTag, value) == CHIP_NO_ERROR); + break; + } + + case ValidationInstruction::kListAttributeD_NotEmpty: { + ChipLogProgress(DataManagement, "\t -- Generating D[2]"); + + uint8_t listData[2]; + Clusters::TestCluster::Attributes::ListInt8u::TypeInfo::Type value(listData); + + uint8_t index2 = 0; + for (auto & item : listData) + { + item = index2; + index2++; + } + + path.mAttributeId = Clusters::TestCluster::Attributes::ListInt8u::Id; + path.mListOp = ConcreteDataAttributePath::ListOperation::ReplaceAll; + + NL_TEST_ASSERT(gSuite, DataModel::Encode(writer, TLV::AnonymousTag, value) == CHIP_NO_ERROR); + break; + } + + case ValidationInstruction::kListAttributeC_Error: { + ChipLogProgress(DataManagement, "\t -- Generating C|e"); + + path.mAttributeId = Clusters::TestCluster::Attributes::ListStructOctetString::Id; + path.mListOp = ConcreteDataAttributePath::ListOperation::ReplaceAll; + status.mStatus = Protocols::InteractionModel::Status::Failure; + hasData = false; + callback->OnAttributeData(nullptr, path, &reader, status); + break; + } + + case ValidationInstruction::kListAttributeD_Error: { + ChipLogProgress(DataManagement, "\t -- Generating D|e"); + + path.mAttributeId = Clusters::TestCluster::Attributes::ListInt8u::Id; + path.mListOp = ConcreteDataAttributePath::ListOperation::ReplaceAll; + status.mStatus = Protocols::InteractionModel::Status::Failure; + hasData = false; + callback->OnAttributeData(nullptr, path, &reader, status); + break; + } + + case ValidationInstruction::kListAttributeC_NotEmpty_Chunked: { + hasData = false; + Clusters::TestCluster::Attributes::ListStructOctetString::TypeInfo::Type value; + + { + ChipLogProgress(DataManagement, "\t -- Generating C[]"); + + path.mAttributeId = Clusters::TestCluster::Attributes::ListStructOctetString::Id; + path.mListOp = ConcreteDataAttributePath::ListOperation::ReplaceAll; + NL_TEST_ASSERT(gSuite, DataModel::Encode(writer, TLV::AnonymousTag, value) == CHIP_NO_ERROR); + + writer.Finalize(&handle); + reader.Init(std::move(handle)); + NL_TEST_ASSERT(gSuite, reader.Next() == CHIP_NO_ERROR); + callback->OnAttributeData(nullptr, path, &reader, status); + } + + ChipLogProgress(DataManagement, "\t -- Generating C0..C512"); + + for (int i = 0; i < 512; i++) + { + Clusters::TestCluster::Structs::TestListStructOctet::Type listItem; + + handle = System::PacketBufferHandle::New(1000); + writer.Init(std::move(handle), true); + status = StatusIB(); + + path.mAttributeId = Clusters::TestCluster::Attributes::ListStructOctetString::Id; + path.mListOp = ConcreteDataAttributePath::ListOperation::AppendItem; + + listItem.fabricIndex = (uint64_t) i; + + NL_TEST_ASSERT(gSuite, DataModel::Encode(writer, TLV::AnonymousTag, listItem) == CHIP_NO_ERROR); + + writer.Finalize(&handle); + reader.Init(std::move(handle)); + NL_TEST_ASSERT(gSuite, reader.Next() == CHIP_NO_ERROR); + callback->OnAttributeData(nullptr, path, &reader, status); + } + + break; + } + + case ValidationInstruction::kListAttributeD_NotEmpty_Chunked: { + hasData = false; + Clusters::TestCluster::Attributes::ListInt8u::TypeInfo::Type value; + + { + ChipLogProgress(DataManagement, "\t -- Generating D[]"); + + path.mAttributeId = Clusters::TestCluster::Attributes::ListInt8u::Id; + path.mListOp = ConcreteDataAttributePath::ListOperation::ReplaceAll; + NL_TEST_ASSERT(gSuite, DataModel::Encode(writer, TLV::AnonymousTag, value) == CHIP_NO_ERROR); + + writer.Finalize(&handle); + reader.Init(std::move(handle)); + NL_TEST_ASSERT(gSuite, reader.Next() == CHIP_NO_ERROR); + callback->OnAttributeData(nullptr, path, &reader, status); + } + + ChipLogProgress(DataManagement, "\t -- Generating D0..D512"); + + for (int i = 0; i < 512; i++) + { + handle = System::PacketBufferHandle::New(1000); + writer.Init(std::move(handle), true); + status = StatusIB(); + + path.mAttributeId = Clusters::TestCluster::Attributes::ListInt8u::Id; + path.mListOp = ConcreteDataAttributePath::ListOperation::AppendItem; + + NL_TEST_ASSERT(gSuite, DataModel::Encode(writer, TLV::AnonymousTag, (uint8_t)(i)) == CHIP_NO_ERROR); + + writer.Finalize(&handle); + reader.Init(std::move(handle)); + NL_TEST_ASSERT(gSuite, reader.Next() == CHIP_NO_ERROR); + callback->OnAttributeData(nullptr, path, &reader, status); + } + + break; + } + + default: + break; + } + + if (hasData) + { + writer.Finalize(&handle); + reader.Init(std::move(handle)); + NL_TEST_ASSERT(gSuite, reader.Next() == CHIP_NO_ERROR); + callback->OnAttributeData(nullptr, path, &reader, status); + } + + index++; + } + + callback->OnReportEnd(nullptr); +} + +void RunAndValidateSequence(std::vector instructionList) +{ + DataSeriesValidator validator(instructionList); + BufferedReadCallback bufferedCallback(validator); + DataSeriesGenerator generator(bufferedCallback, instructionList); + generator.Generate(); + + NL_TEST_ASSERT(gSuite, validator.mCurrentInstruction == instructionList.size()); +} + +void TestBufferedSequences(nlTestSuite * apSuite, void * apContext) +{ + ChipLogProgress(DataManagement, "Validating various sequences of attribute data IBs..."); + + ChipLogProgress(DataManagement, "A --> A"); + RunAndValidateSequence({ { ValidationInstruction::kSimpleAttributeA } }); + + ChipLogProgress(DataManagement, "A A --> A A"); + RunAndValidateSequence({ { ValidationInstruction::kSimpleAttributeA }, { ValidationInstruction::kSimpleAttributeA } }); + + ChipLogProgress(DataManagement, "A B --> A B"); + RunAndValidateSequence({ { ValidationInstruction::kSimpleAttributeA }, { ValidationInstruction::kSimpleAttributeB } }); + + ChipLogProgress(DataManagement, "A C[] --> A C[]"); + RunAndValidateSequence({ { ValidationInstruction::kSimpleAttributeA }, { ValidationInstruction::kListAttributeC_Empty } }); + + ChipLogProgress(DataManagement, "C[] C[] --> C[]"); + RunAndValidateSequence({ { ValidationInstruction::kListAttributeC_Empty, ValidationInstruction::kDiscardedChunk }, + { ValidationInstruction::kListAttributeC_Empty } }); + + ChipLogProgress(DataManagement, "C[2] C[] --> C[]"); + RunAndValidateSequence({ { ValidationInstruction::kListAttributeC_NotEmpty, ValidationInstruction::kDiscardedChunk }, + { ValidationInstruction::kListAttributeC_Empty } }); + + ChipLogProgress(DataManagement, "C[] C[2] --> C[2]"); + RunAndValidateSequence({ { ValidationInstruction::kListAttributeC_Empty, ValidationInstruction::kDiscardedChunk }, + { ValidationInstruction::kListAttributeC_NotEmpty } }); + + ChipLogProgress(DataManagement, "C[] A C[2] --> C[] A C[2]"); + RunAndValidateSequence({ { ValidationInstruction::kListAttributeC_Empty }, + { ValidationInstruction::kSimpleAttributeA }, + { ValidationInstruction::kListAttributeC_NotEmpty } }); + + ChipLogProgress(DataManagement, "C[] C[2] A --> C[] C[2] A"); + RunAndValidateSequence({ { ValidationInstruction::kListAttributeC_Empty }, + { ValidationInstruction::kSimpleAttributeA }, + { ValidationInstruction::kListAttributeC_NotEmpty } }); + + ChipLogProgress(DataManagement, "C[] D[] --> C[] D[]"); + RunAndValidateSequence({ { ValidationInstruction::kListAttributeC_Empty }, { ValidationInstruction::kListAttributeD_Empty } }); + + ChipLogProgress(DataManagement, "C[2] D[] --> C[2] D[]"); + RunAndValidateSequence( + { { ValidationInstruction::kListAttributeC_NotEmpty }, { ValidationInstruction::kListAttributeD_Empty } }); + + ChipLogProgress(DataManagement, "C[2] C|e --> C|e"); + RunAndValidateSequence({ { ValidationInstruction::kListAttributeC_NotEmpty, ValidationInstruction::kDiscardedChunk }, + { ValidationInstruction::kListAttributeC_Error } }); + + ChipLogProgress(DataManagement, "A C|e --> A C|e"); + RunAndValidateSequence({ { ValidationInstruction::kSimpleAttributeA }, { ValidationInstruction::kListAttributeC_Error } }); + + ChipLogProgress(DataManagement, "C|e C[2] --> C|e C[2]"); + RunAndValidateSequence( + { { ValidationInstruction::kListAttributeC_Error }, { ValidationInstruction::kListAttributeC_NotEmpty } }); + + ChipLogProgress(DataManagement, "C[] C0 C1 --> C[2]"); + RunAndValidateSequence({ { ValidationInstruction::kListAttributeC_NotEmpty_Chunked } }); + + ChipLogProgress(DataManagement, "C[] C0 C1 C[] --> C[]"); + RunAndValidateSequence({ + { ValidationInstruction::kListAttributeC_NotEmpty_Chunked, ValidationInstruction::kDiscardedChunk }, + { ValidationInstruction::kListAttributeC_Empty }, + }); + + ChipLogProgress(DataManagement, "C[] C0 C1 D[] D0 D1 --> C[2] D[2]"); + RunAndValidateSequence({ + { ValidationInstruction::kListAttributeC_NotEmpty_Chunked }, + { ValidationInstruction::kListAttributeD_NotEmpty_Chunked }, + }); +} + +// clang-format off +const nlTest sTests[] = +{ + NL_TEST_DEF("TestBufferedSequences", TestBufferedSequences), + NL_TEST_SENTINEL() +}; + +nlTestSuite theSuite = +{ + "TestBufferedReadCallback", + &sTests[0], + TestContext::Initialize, + TestContext::Finalize +}; + +} +// clang-format on + +int TestBufferedReadCallback() +{ + TestContext gContext; + gSuite = &theSuite; + nlTestRunner(&theSuite, &gContext); + return (nlTestRunnerStats(&theSuite)); +} + +CHIP_REGISTER_TEST_SUITE(TestBufferedReadCallback) diff --git a/src/app/tests/TestReadInteraction.cpp b/src/app/tests/TestReadInteraction.cpp index beb1b979108010..4ee5301381a8e9 100644 --- a/src/app/tests/TestReadInteraction.cpp +++ b/src/app/tests/TestReadInteraction.cpp @@ -176,7 +176,7 @@ class MockInteractionModelApp : public chip::app::ReadClient::Callback, public c mGotEventResponse = true; } - void OnAttributeData(const chip::app::ReadClient * apReadClient, const chip::app::ConcreteAttributePath & aPath, + void OnAttributeData(const chip::app::ReadClient * apReadClient, const chip::app::ConcreteDataAttributePath & aPath, chip::TLV::TLVReader * apData, const chip::app::StatusIB & status) override { if (status.mStatus == chip::Protocols::InteractionModel::Status::Success) diff --git a/src/app/tests/integration/chip_im_initiator.cpp b/src/app/tests/integration/chip_im_initiator.cpp index 24da2e89b28b7a..0d8c70c9f6826a 100644 --- a/src/app/tests/integration/chip_im_initiator.cpp +++ b/src/app/tests/integration/chip_im_initiator.cpp @@ -146,7 +146,7 @@ class MockInteractionModelApp : public chip::app::InteractionModelDelegate, } } } - void OnAttributeData(const chip::app::ReadClient * apReadClient, const chip::app::ConcreteAttributePath & aPath, + void OnAttributeData(const chip::app::ReadClient * apReadClient, const chip::app::ConcreteDataAttributePath & aPath, chip::TLV::TLVReader * aData, const chip::app::StatusIB & status) override {} diff --git a/src/app/util/ember-compatibility-functions.cpp b/src/app/util/ember-compatibility-functions.cpp index 3f4574b1a1b138..b5515fc0ece603 100644 --- a/src/app/util/ember-compatibility-functions.cpp +++ b/src/app/util/ember-compatibility-functions.cpp @@ -234,12 +234,14 @@ CHIP_ERROR ReadSingleClusterData(FabricIndex aAccessingFabricIndex, const Concre AttributeAccessInterface * attrOverride = findAttributeAccessOverride(aPath.mEndpointId, aPath.mClusterId); if (attrOverride != nullptr) { + ConcreteReadAttributePath readPath(aPath); + // TODO: We should probably clone the writer and convert failures here // into status responses, unless our caller already does that. writer = attributeDataIBBuilder.GetWriter(); VerifyOrReturnError(writer != nullptr, CHIP_NO_ERROR); AttributeValueEncoder valueEncoder(writer, aAccessingFabricIndex); - ReturnErrorOnFailure(attrOverride->Read(aPath, valueEncoder)); + ReturnErrorOnFailure(attrOverride->Read(readPath, valueEncoder)); if (valueEncoder.TriedEncode()) { @@ -575,7 +577,7 @@ CHIP_ERROR WriteSingleClusterData(ClusterInfo & aClusterInfo, TLV::TLVReader & a // Named aPath for now to reduce the amount of code change that needs to // happen when the above TODO is resolved. - ConcreteAttributePath aPath(aClusterInfo.mEndpointId, aClusterInfo.mClusterId, aClusterInfo.mAttributeId); + ConcreteDataAttributePath aPath(aClusterInfo.mEndpointId, aClusterInfo.mClusterId, aClusterInfo.mAttributeId); const EmberAfAttributeMetadata * attributeMetadata = emberAfLocateAttributeMetadata(aPath.mEndpointId, aPath.mClusterId, aPath.mAttributeId, CLUSTER_MASK_SERVER, 0); diff --git a/src/controller/ReadInteraction.h b/src/controller/ReadInteraction.h index 686cb45295ff32..1fee2412c3aefa 100644 --- a/src/controller/ReadInteraction.h +++ b/src/controller/ReadInteraction.h @@ -115,7 +115,8 @@ CHIP_ERROR ReadEvent(Messaging::ExchangeManager * apExchangeMgr, const SessionHa VerifyOrReturnError(callback != nullptr, CHIP_ERROR_NO_MEMORY); - ReturnErrorOnFailure(engine->NewReadClient(&readClient, app::ReadClient::InteractionType::Read, callback.get())); + ReturnErrorOnFailure( + engine->NewReadClient(&readClient, app::ReadClient::InteractionType::Read, &callback.get()->GetBufferedCallback())); err = readClient->SendReadRequest(readParams); if (err != CHIP_NO_ERROR) diff --git a/src/controller/TypedReadCallback.h b/src/controller/TypedReadCallback.h index c4015d11335641..c417ddfc991954 100644 --- a/src/controller/TypedReadCallback.h +++ b/src/controller/TypedReadCallback.h @@ -18,6 +18,7 @@ #pragma once +#include #include #include #include @@ -48,16 +49,24 @@ class TypedReadAttributeCallback final : public app::ReadClient::Callback TypedReadAttributeCallback(ClusterId aClusterId, AttributeId aAttributeId, OnSuccessCallbackType aOnSuccess, OnErrorCallbackType aOnError, OnDoneCallbackType aOnDone) : mClusterId(aClusterId), - mAttributeId(aAttributeId), mOnSuccess(aOnSuccess), mOnError(aOnError), mOnDone(aOnDone) + mAttributeId(aAttributeId), mOnSuccess(aOnSuccess), mOnError(aOnError), mOnDone(aOnDone), mBufferedReadAdapter(*this) {} + app::BufferedReadCallback & GetBufferedCallback() { return mBufferedReadAdapter; } + private: - void OnAttributeData(const app::ReadClient * apReadClient, const app::ConcreteAttributePath & aPath, TLV::TLVReader * apData, - const app::StatusIB & aStatus) override + void OnAttributeData(const app::ReadClient * apReadClient, const app::ConcreteDataAttributePath & aPath, + TLV::TLVReader * apData, const app::StatusIB & aStatus) override { CHIP_ERROR err = CHIP_NO_ERROR; DecodableAttributeType value; + // + // We shouldn't be getting list item operations in the provided path since that should be handled by the buffered read + // callback. If we do, that's a bug. + // + VerifyOrDie(!aPath.IsListItemOperation()); + VerifyOrExit(aStatus.mStatus == Protocols::InteractionModel::Status::Success, err = CHIP_ERROR_IM_STATUS_CODE_RECEIVED); VerifyOrExit(aPath.mClusterId == mClusterId && aPath.mAttributeId == mAttributeId, err = CHIP_ERROR_SCHEMA_MISMATCH); VerifyOrExit(apData != nullptr, err = CHIP_ERROR_INVALID_ARGUMENT); @@ -95,6 +104,7 @@ class TypedReadAttributeCallback final : public app::ReadClient::Callback OnSuccessCallbackType mOnSuccess; OnErrorCallbackType mOnError; OnDoneCallbackType mOnDone; + app::BufferedReadCallback mBufferedReadAdapter; }; template diff --git a/src/controller/python/chip/clusters/attribute.cpp b/src/controller/python/chip/clusters/attribute.cpp index e7745cc77f03c6..00267d25038578 100644 --- a/src/controller/python/chip/clusters/attribute.cpp +++ b/src/controller/python/chip/clusters/attribute.cpp @@ -64,9 +64,15 @@ class ReadClientCallback : public ReadClient::Callback public: ReadClientCallback(PyObject * appContext) : mAppContext(appContext) {} - void OnAttributeData(const ReadClient * apReadClient, const ConcreteAttributePath & aPath, TLV::TLVReader * apData, + void OnAttributeData(const ReadClient * apReadClient, const ConcreteDataAttributePath & aPath, TLV::TLVReader * apData, const StatusIB & aStatus) override { + // + // We shouldn't be getting list item operations in the provided path since that should be handled by the buffered read + // callback. If we do, that's a bug. + // + VerifyOrDie(!aPath.IsListItemOperation()); + uint8_t buffer[CHIP_CONFIG_DEFAULT_UDP_MTU_SIZE]; uint32_t size = 0; // When the apData is nullptr, means we did not receive a valid attribute data from server, status will be some error diff --git a/src/controller/python/chip/interaction_model/Delegate.cpp b/src/controller/python/chip/interaction_model/Delegate.cpp index 944fbcf3889522..178ade9bf60234 100644 --- a/src/controller/python/chip/interaction_model/Delegate.cpp +++ b/src/controller/python/chip/interaction_model/Delegate.cpp @@ -15,6 +15,7 @@ * limitations under the License. */ +#include "app/ConcreteAttributePath.h" #include #include @@ -80,9 +81,16 @@ void PythonInteractionModelDelegate::OnError(const app::CommandSender * apComman DeviceControllerInteractionModelDelegate::OnError(apCommandSender, aStatus, aError); } -void PythonInteractionModelDelegate::OnAttributeData(const app::ReadClient * apReadClient, const app::ConcreteAttributePath & aPath, - TLV::TLVReader * apData, const app::StatusIB & status) +void PythonInteractionModelDelegate::OnAttributeData(const app::ReadClient * apReadClient, + const app::ConcreteDataAttributePath & aPath, TLV::TLVReader * apData, + const app::StatusIB & status) { + // + // We shouldn't be getting list item operations in the provided path since that should be handled by the buffered read callback. + // If we do, that's a bug. + // + VerifyOrDie(!aPath.IsListItemOperation()); + if (onReportDataFunct != nullptr) { CHIP_ERROR err = CHIP_NO_ERROR; diff --git a/src/controller/python/chip/interaction_model/Delegate.h b/src/controller/python/chip/interaction_model/Delegate.h index e2cb2f0d817219..c27cf181b14d27 100644 --- a/src/controller/python/chip/interaction_model/Delegate.h +++ b/src/controller/python/chip/interaction_model/Delegate.h @@ -101,8 +101,8 @@ class PythonInteractionModelDelegate : public chip::Controller::DeviceController TLV::TLVReader * aData) override; void OnError(const app::CommandSender * apCommandSender, const app::StatusIB & aStatus, CHIP_ERROR aError) override; - void OnAttributeData(const app::ReadClient * apReadClient, const app::ConcreteAttributePath & aPath, TLV::TLVReader * apData, - const app::StatusIB & status) override; + void OnAttributeData(const app::ReadClient * apReadClient, const app::ConcreteDataAttributePath & aPath, + TLV::TLVReader * apData, const app::StatusIB & status) override; static PythonInteractionModelDelegate & Instance(); diff --git a/src/lib/core/CHIPTLV.h b/src/lib/core/CHIPTLV.h index b2774bdb66c7a6..49080923c7304c 100644 --- a/src/lib/core/CHIPTLV.h +++ b/src/lib/core/CHIPTLV.h @@ -33,6 +33,7 @@ #include #include +#include #include #include @@ -942,6 +943,43 @@ class DLL_EXPORT TLVReader TLVElementType ElementType() const; }; +/* + * A TLVReader that is backed by a scoped memory buffer that is owned by the reader + */ +class ScopedBufferTLVReader : public TLVReader +{ +public: + /* + * Construct and initialize the reader by taking ownership of the provided scoped buffer. + */ + ScopedBufferTLVReader(Platform::ScopedMemoryBuffer && buffer, size_t dataLen) { Init(std::move(buffer), dataLen); } + + ScopedBufferTLVReader() {} + + /* + * Initialize the reader by taking ownership of a passed in scoped buffer. + */ + void Init(Platform::ScopedMemoryBuffer && buffer, size_t dataLen) + { + mBuffer = std::move(buffer); + TLVReader::Init(mBuffer.Get(), dataLen); + } + + /* + * Take back the buffer owned by the reader and transfer its ownership to + * the provided buffer reference. This also re-initializes the reader with + * a null buffer to prevent further use of the reader. + */ + void TakeBuffer(Platform::ScopedMemoryBuffer & buffer) + { + buffer = std::move(mBuffer); + TLVReader::Init(nullptr, 0); + } + +private: + Platform::ScopedMemoryBuffer mBuffer; +}; + /** * A TLVReader that is guaranteed to be backed by a single contiguous buffer. * This allows it to expose some additional methods that allow consumers to @@ -2188,6 +2226,38 @@ class DLL_EXPORT TLVWriter CHIP_ERROR WriteData(const uint8_t * p, uint32_t len); }; +/* + * A TLVWriter that is backed by a scoped memory buffer that is owned by the writer. + */ +class ScopedBufferTLVWriter : public TLVWriter +{ +public: + /* + * Construct and initialize the writer by taking ownership of the provided scoped buffer. + */ + ScopedBufferTLVWriter(Platform::ScopedMemoryBuffer && buffer, size_t dataLen) + { + mBuffer = std::move(buffer); + Init(mBuffer.Get(), dataLen); + } + + /* + * Finalize the writer and take back the buffer owned by the writer. This transfers its + * ownership to the provided buffer reference. This also re-initializes the writer with + * a null buffer to prevent further inadvertent use of the writer. + */ + CHIP_ERROR Finalize(Platform::ScopedMemoryBuffer & buffer) + { + ReturnErrorOnFailure(TLVWriter::Finalize()); + buffer = std::move(mBuffer); + Init(nullptr, 0); + return CHIP_NO_ERROR; + } + +private: + Platform::ScopedMemoryBuffer mBuffer; +}; + /** * Provides a unified Reader/Writer interface for editing/adding/deleting elements in TLV encoding. * diff --git a/src/lib/core/tests/TestCHIPTLV.cpp b/src/lib/core/tests/TestCHIPTLV.cpp index fa7ff80117a21f..8dd23609a4c9f9 100644 --- a/src/lib/core/tests/TestCHIPTLV.cpp +++ b/src/lib/core/tests/TestCHIPTLV.cpp @@ -4294,6 +4294,51 @@ static void CheckGetByteView(nlTestSuite * inSuite, void * inContext) } } +static void CheckCHIPTLVScopedBuffer(nlTestSuite * inSuite, void * inContext) +{ + Platform::ScopedMemoryBuffer buf; + CHIP_ERROR err; + + buf.Calloc(64); + NL_TEST_ASSERT(inSuite, buf.Get() != nullptr); + + { + ScopedBufferTLVWriter writer(std::move(buf), 64); + + NL_TEST_ASSERT(inSuite, buf.Get() == nullptr); + + err = writer.Put(TLV::AnonymousTag, (uint8_t) 33); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + + err = writer.Finalize(buf); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + NL_TEST_ASSERT(inSuite, buf.Get() != nullptr); + + err = writer.Put(TLV::AnonymousTag, (uint8_t) 33); + NL_TEST_ASSERT(inSuite, err != CHIP_NO_ERROR); + } + + { + ScopedBufferTLVReader reader; + uint8_t val; + + reader.Init(std::move(buf), 64); + + err = reader.Next(); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + + err = reader.Get(val); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + NL_TEST_ASSERT(inSuite, val == 33); + + reader.TakeBuffer(buf); + NL_TEST_ASSERT(inSuite, buf.Get() != nullptr); + + err = reader.Get(val); + NL_TEST_ASSERT(inSuite, err != CHIP_NO_ERROR); + } +} + // Test Suite /** @@ -4323,6 +4368,7 @@ static const nlTest sTests[] = NL_TEST_DEF("CHIP TLV Printf, Circular TLV buf", CheckCHIPTLVPutStringFCircular), NL_TEST_DEF("CHIP TLV Skip non-contiguous", CheckCHIPTLVSkipCircular), NL_TEST_DEF("CHIP TLV ByteSpan", CheckCHIPTLVByteSpan), + NL_TEST_DEF("CHIP TLV Scoped Buffer", CheckCHIPTLVScopedBuffer), NL_TEST_DEF("CHIP TLV Check reserve", CheckCloseContainerReserve), NL_TEST_DEF("CHIP TLV Reader Fuzz Test", TLVReaderFuzzTest), NL_TEST_DEF("CHIP TLV GetStringView Test", CheckGetStringView),