Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Buffered List Chunk Reader #11705

Merged
merged 13 commits into from
Nov 20, 2021
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class BridgedActionsAttrAccess : public AttributeAccessInterface
// Register for the Bridged Actions cluster on all endpoints.
BridgedActionsAttrAccess() : AttributeAccessInterface(Optional<EndpointId>::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;
Expand Down Expand Up @@ -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);

Expand Down
4 changes: 2 additions & 2 deletions examples/tv-app/android/include/cluster-init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class TvAttrAccess : public app::AttributeAccessInterface
public:
TvAttrAccess() : app::AttributeAccessInterface(Optional<EndpointId>::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())
{
Expand Down Expand Up @@ -195,7 +195,7 @@ class ContentLauncherAttrAccess : public app::AttributeAccessInterface
ContentLauncherAttrAccess() : app::AttributeAccessInterface(Optional<EndpointId>::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)
{
Expand Down
4 changes: 2 additions & 2 deletions examples/tv-app/linux/include/cluster-init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class TvAttrAccess : public app::AttributeAccessInterface
public:
TvAttrAccess() : app::AttributeAccessInterface(Optional<EndpointId>::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())
{
Expand Down Expand Up @@ -195,7 +195,7 @@ class ContentLauncherAttrAccess : public app::AttributeAccessInterface
ContentLauncherAttrAccess() : app::AttributeAccessInterface(Optional<EndpointId>::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)
{
Expand Down
4 changes: 2 additions & 2 deletions src/app/AttributeAccessInterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion src/app/AttributePathExpandIterator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ bool AttributePathExpandIterator::Next()
}

// Reset to default, invalid value.
mOutputPath = ConcreteAttributePath();
mOutputPath = ConcreteReadAttributePath();
return false;
}
} // namespace app
Expand Down
1 change: 1 addition & 0 deletions src/app/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ static_library("app") {
"AttributePathExpandIterator.h",
"AttributePathParams.cpp",
"AttributePathParams.h",
"BufferedReadCallback.cpp",
"CASESessionManager.cpp",
"CASESessionManager.h",
"Command.cpp",
Expand Down
266 changes: 266 additions & 0 deletions src/app/BufferedReadCallback.cpp
Original file line number Diff line number Diff line change
@@ -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 <app/BufferedReadCallback.h>
#include <app/InteractionModelEngine.h>
#include <lib/support/ScopedBuffer.h>

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<uint8_t> 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);
mrjerryjohns marked this conversation as resolved.
Show resolved Hide resolved

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;
mrjerryjohns marked this conversation as resolved.
Show resolved Hide resolved
}

//
// 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();
mrjerryjohns marked this conversation as resolved.
Show resolved Hide resolved
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
Loading