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

[IM] Write Chunking Support #13641

Merged
merged 40 commits into from
Feb 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
1e24b61
[IM] Implement write chunking with set+append for large lists
erjiaqing Jan 12, 2022
d8b75c2
Python API update
erjiaqing Jan 18, 2022
3848190
Update LongListAttribute
erjiaqing Jan 18, 2022
8cc6f98
Update TestSuite
erjiaqing Jan 18, 2022
f8236e7
Add more tests
erjiaqing Jan 18, 2022
cde8200
Fix WriteClient construct call
erjiaqing Jan 20, 2022
62e7ea3
Run Codegen
erjiaqing Jan 18, 2022
3d9d9e0
Address comments
erjiaqing Jan 24, 2022
df1ace7
Merge branch 'master' into im/write-chunking
erjiaqing Jan 24, 2022
56fa166
Restyle
erjiaqing Jan 24, 2022
0e4113d
Fix test
erjiaqing Jan 24, 2022
3e539b3
Merge branch 'master' into im/write-chunking
erjiaqing Jan 27, 2022
798e4f2
Run Codegen
erjiaqing Jan 27, 2022
038f28d
Fix dirty merge
erjiaqing Jan 27, 2022
4cffef3
Address comments
erjiaqing Jan 28, 2022
8d263c5
Update access-control-server
erjiaqing Jan 28, 2022
984a234
Merge branch 'master' into im/write-chunking
erjiaqing Jan 28, 2022
d577b75
Fix merge
erjiaqing Jan 28, 2022
429f8b0
Merge branch 'master' into im/write-chunking
mrjerryjohns Jan 31, 2022
a6b3b05
Updated code-gen
mrjerryjohns Jan 31, 2022
27b9a23
Address comments
erjiaqing Feb 7, 2022
655b6da
Merge branch 'master' into im/write-chunking
erjiaqing Feb 7, 2022
2eb5388
Fix unclean merge
erjiaqing Feb 7, 2022
1d92df0
Fix GroupWrite Encoding
erjiaqing Feb 7, 2022
e8d8bfd
Fix merge and make access-control-server diff readable
erjiaqing Feb 7, 2022
7dc6a04
Merge branch 'master' into im/write-chunking
erjiaqing Feb 8, 2022
0807720
Fix dirty merge
erjiaqing Feb 8, 2022
2aa4096
Update group-key-mgmt-server
erjiaqing Feb 8, 2022
165d175
Fix GroupWrite attribute path on the server side
erjiaqing Feb 8, 2022
963d272
Merge branch 'master' into im/write-chunking
erjiaqing Feb 8, 2022
31f5f44
Adapt WriteHandler for #14821
erjiaqing Feb 8, 2022
ab16d0f
Add ACL check cache
erjiaqing Feb 7, 2022
5256016
Merge branch 'master' into im/write-chunking
erjiaqing Feb 9, 2022
371fb1c
Resolve comments
erjiaqing Feb 9, 2022
42a8f5c
Merge branch 'master' into im/write-chunking
erjiaqing Feb 9, 2022
4c57cc3
Merge branch 'master' into im/write-chunking
erjiaqing Feb 9, 2022
ee8a686
Fix typo and fix merge
erjiaqing Feb 9, 2022
94696c6
Fix
erjiaqing Feb 9, 2022
6f986c2
Comment out #if CONFIG_IM_BUILD_FOR_UNIT_TEST in WriteClient.h
erjiaqing Feb 9, 2022
2eeae3d
Comment out #if CONFIG_IM_BUILD_FOR_UNIT_TEST in WriteClient.h
erjiaqing Feb 9, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2957,7 +2957,7 @@ server cluster TestCluster = 1295 {
attribute int8s rangeRestrictedInt8s = 39;
attribute int16u rangeRestrictedInt16u = 40;
attribute int16s rangeRestrictedInt16s = 41;
readonly attribute LONG_OCTET_STRING listLongOctetString[] = 42;
attribute LONG_OCTET_STRING listLongOctetString[] = 42;
readonly attribute TestFabricScoped listFabricScoped[] = 43;
attribute boolean timedWriteBoolean = 48;
attribute boolean generalErrorBoolean = 49;
Expand Down
4 changes: 2 additions & 2 deletions examples/chip-tool/commands/clusters/WriteAttributeCommand.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class WriteAttribute : public ModelCommand, public chip::app::WriteClient::Callb
}

/////////// WriteClient Callback Interface /////////
void OnResponse(const chip::app::WriteClient * client, const chip::app::ConcreteAttributePath & path,
void OnResponse(const chip::app::WriteClient * client, const chip::app::ConcreteDataAttributePath & path,
chip::app::StatusIB status) override
{
CHIP_ERROR error = status.ToChipError();
Expand Down Expand Up @@ -98,7 +98,7 @@ class WriteAttribute : public ModelCommand, public chip::app::WriteClient::Callb

mWriteClient = std::make_unique<chip::app::WriteClient>(device->GetExchangeManager(), this, mTimedInteractionTimeoutMs);

ReturnErrorOnFailure(mWriteClient->EncodeAttributeWritePayload(attributePathParams, value));
ReturnErrorOnFailure(mWriteClient->EncodeAttribute(attributePathParams, value));
return mWriteClient->SendWriteRequest(device->GetSecureSession().Value());
}

Expand Down
45 changes: 45 additions & 0 deletions src/app/AttributeAccessToken.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/*
*
* Copyright (c) 2022 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 <access/Privilege.h>
#include <app/ConcreteAttributePath.h>

namespace chip {
namespace app {

/**
* AttributeAccessToken records the privilege granted for accessing the specified attribute. This struct is used in chunked write
* to avoid losing privilege when updating ACL items.
*/
struct AttributeAccessToken
{
ConcreteAttributePath mPath;
Access::Privilege mPrivilege;

bool operator==(const AttributeAccessToken & other) const { return mPath == other.mPath && mPrivilege == other.mPrivilege; }

bool Matches(const ConcreteAttributePath & aPath, const Access::Privilege & aPrivilege) const
{
return mPath == aPath && mPrivilege == aPrivilege;
}
};

} // namespace app
} // namespace chip
2 changes: 2 additions & 0 deletions src/app/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ static_library("app") {
"CASEClientPool.h",
"CASESessionManager.cpp",
"CASESessionManager.h",
"ChunkedWriteCallback.cpp",
"ChunkedWriteCallback.h",
"CommandHandler.cpp",
"CommandResponseHelper.h",
"CommandSender.cpp",
Expand Down
91 changes: 91 additions & 0 deletions src/app/ChunkedWriteCallback.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
/*
*
* Copyright (c) 2022 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/ChunkedWriteCallback.h>

namespace chip {
namespace app {

void ChunkedWriteCallback::OnResponse(const WriteClient * apWriteClient, const ConcreteDataAttributePath & aPath, StatusIB aStatus)
{
// We may send a chunked list. To make the behavior consistent whether a list is being chunked or not,
// we merge the write responses for a chunked list here and provide our consumer with a single status response.
if (mLastAttributePath.HasValue())
{
// This is not the first write response.
if (IsAppendingToLastItem(aPath))
{
// This is a response on the same path as what we already have stored. Report the first
// failure status we encountered, and ignore subsequent ones.
if (mAttributeStatus.IsSuccess())
{
mAttributeStatus = aStatus;
}
return;
}
else
{
// This is a response to another attribute write. Report the final result of last attribute write.
callback->OnResponse(apWriteClient, mLastAttributePath.Value(), mAttributeStatus);
}
}

// This is the first report for a new attribute. We assume it will never be a list item operation.
if (aPath.IsListItemOperation())
{
aStatus = StatusIB(CHIP_ERROR_INCORRECT_STATE);
}

mLastAttributePath.SetValue(aPath);
mAttributeStatus = aStatus;
// For the last status in the response, we will call the application callback in OnDone()
}

void ChunkedWriteCallback::OnError(const WriteClient * apWriteClient, CHIP_ERROR aError)
{
callback->OnError(apWriteClient, aError);
}

void ChunkedWriteCallback::OnDone(WriteClient * apWriteClient)
{
if (mLastAttributePath.HasValue())
{
// We have a cached status that has yet to be reported to the application so report it now.
// If we failed to receive the response, or we received a malformed response, OnResponse won't be called,
// mLastAttributePath will be Missing() in this case.
callback->OnResponse(apWriteClient, mLastAttributePath.Value(), mAttributeStatus);
}

callback->OnDone(apWriteClient);
}

bool ChunkedWriteCallback::IsAppendingToLastItem(const ConcreteDataAttributePath & aPath)
{
if (!aPath.IsListItemOperation())
{
return false;
}
if (!mLastAttributePath.HasValue() || !(mLastAttributePath.Value() == aPath))
{
return false;
}
return aPath.mListOp == ConcreteDataAttributePath::ListOperation::AppendItem;
}

} // namespace app
} // namespace chip
55 changes: 55 additions & 0 deletions src/app/ChunkedWriteCallback.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*
*
* Copyright (c) 2022 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 <app/WriteClient.h>

namespace chip {
namespace app {

/*
* This is an adapter that intercepts calls that deliver status codes from the WriteClient and
* selectively "merge"s the status codes for a chunked list write as follows:
* - If the whole list was successfully written, callback->OnResponse will be called with success.
* - If any element in the list was not successfully written, callback->OnResponse will be called with the first error received.
* - callback->OnResponse will always have NotList as mListOp since we have merged the chunked responses.
* The merge logic assumes all list operations are part of list chunking.
*/
class ChunkedWriteCallback : public WriteClient::Callback
{
public:
ChunkedWriteCallback(WriteClient::Callback * apCallback) : callback(apCallback) {}
bzbarsky-apple marked this conversation as resolved.
Show resolved Hide resolved

void OnResponse(const WriteClient * apWriteClient, const ConcreteDataAttributePath & aPath, StatusIB status) override;
void OnError(const WriteClient * apWriteClient, CHIP_ERROR aError) override;
void OnDone(WriteClient * apWriteClient) override;

private:
bool IsAppendingToLastItem(const ConcreteDataAttributePath & aPath);

// We are using the casts between ConcreteAttributePath and ConcreteDataAttributePath, then all paths passed to upper
// applications will always have NotList as mListOp.
Optional<ConcreteAttributePath> mLastAttributePath;
StatusIB mAttributeStatus;

WriteClient::Callback * callback;
};

} // namespace app
} // namespace chip
5 changes: 3 additions & 2 deletions src/app/InteractionModelEngine.h
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,8 @@ CHIP_ERROR ReadSingleClusterData(const Access::SubjectDescriptor & aSubjectDescr
/**
* TODO: Document.
*/
CHIP_ERROR WriteSingleClusterData(const Access::SubjectDescriptor & aSubjectDescriptor, ClusterInfo & aClusterInfo,
TLV::TLVReader & aReader, WriteHandler * apWriteHandler);
CHIP_ERROR WriteSingleClusterData(const Access::SubjectDescriptor & aSubjectDescriptor,
const ConcreteDataAttributePath & aAttributePath, TLV::TLVReader & aReader,
WriteHandler * apWriteHandler);
} // namespace app
} // namespace chip
51 changes: 51 additions & 0 deletions src/app/MessageDef/AttributePathIB.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,33 @@ CHIP_ERROR AttributePathIB::Parser::GetListIndex(DataModel::Nullable<ListIndex>
return GetNullableUnsignedInteger(to_underlying(Tag::kListIndex), apListIndex);
}

CHIP_ERROR AttributePathIB::Parser::GetListIndex(ConcreteDataAttributePath & aAttributePath) const
{
CHIP_ERROR err = CHIP_NO_ERROR;
DataModel::Nullable<ListIndex> listIndex;
err = GetListIndex(&(listIndex));
if (err == CHIP_NO_ERROR)
{
if (listIndex.IsNull())
{
aAttributePath.mListOp = ConcreteDataAttributePath::ListOperation::AppendItem;
}
else
{
// TODO: Add ListOperation::ReplaceItem support. (Attribute path with valid list index)
err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH;
}
}
else if (CHIP_END_OF_TLV == err)
{
// We do not have the context for the actual data type here. We always set the list operation to not list and the users
// should interpret it as ReplaceAll when the attribute type is a list.
aAttributePath.mListOp = ConcreteDataAttributePath::ListOperation::NotList;
err = CHIP_NO_ERROR;
}
return err;
}

AttributePathIB::Builder & AttributePathIB::Builder::EnableTagCompression(const bool aEnableTagCompression)
{
// skip if error has already been set
Expand Down Expand Up @@ -300,5 +327,29 @@ CHIP_ERROR AttributePathIB::Builder::Encode(const AttributePathParams & aAttribu
return GetError();
}

CHIP_ERROR AttributePathIB::Builder::Encode(const ConcreteDataAttributePath & aAttributePath)
{
Endpoint(aAttributePath.mEndpointId);
Cluster(aAttributePath.mClusterId);
Attribute(aAttributePath.mAttributeId);

if (!aAttributePath.IsListOperation() || aAttributePath.mListOp == ConcreteDataAttributePath::ListOperation::ReplaceAll)
{
/* noop */
}
else if (aAttributePath.mListOp == ConcreteDataAttributePath::ListOperation::AppendItem)
{
ListIndex(DataModel::NullNullable);
}
else
{
// TODO: Add ListOperation::ReplaceItem support. (Attribute path with valid list index)
return CHIP_ERROR_INVALID_ARGUMENT;
}

EndOfAttributePathIB();
return GetError();
}

} // namespace app
} // namespace chip
13 changes: 13 additions & 0 deletions src/app/MessageDef/AttributePathIB.h
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,18 @@ class Parser : public ListParser
* #CHIP_END_OF_TLV if there is no such element
*/
CHIP_ERROR GetListIndex(DataModel::Nullable<ListIndex> * const apListIndex) const;

/**
* @brief Get the ListIndex, and set the mListIndex and mListOp fields in the ConcreteDataAttributePath accordingly. It will set
* ListOp to NotList when the list index is missing, users should interpret it as ReplaceAll according to the context.
*
* @param [in] aAttributePath The attribute path object for setting list index and list op.
*
* @return #CHIP_NO_ERROR on success
*/
CHIP_ERROR GetListIndex(ConcreteDataAttributePath & aAttributePath) const;

// TODO(#14934) Add a function to get ConcreteDataAttributePath from AttributePathIB::Parser directly.
};

class Builder : public ListBuilder
Expand Down Expand Up @@ -209,6 +221,7 @@ class Builder : public ListBuilder
AttributePathIB::Builder & EndOfAttributePathIB();

CHIP_ERROR Encode(const AttributePathParams & aAttributePathParams);
CHIP_ERROR Encode(const ConcreteDataAttributePath & aAttributePathParams);
};
} // namespace AttributePathIB
} // namespace app
Expand Down
3 changes: 1 addition & 2 deletions src/app/MessageDef/WriteRequestMessage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,7 @@ CHIP_ERROR WriteRequestMessage::Parser::CheckSchemaValidity() const

if (CHIP_END_OF_TLV == err)
{
const int RequiredFields = (1 << to_underlying(Tag::kIsFabricFiltered)) | (1 << to_underlying(Tag::kTimedRequest)) |
(1 << to_underlying(Tag::kWriteRequests));
const int RequiredFields = ((1 << to_underlying(Tag::kTimedRequest)) | (1 << to_underlying(Tag::kWriteRequests)));

if ((tagPresenceMask & RequiredFields) == RequiredFields)
{
Expand Down
17 changes: 1 addition & 16 deletions src/app/ReadClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -533,22 +533,7 @@ CHIP_ERROR ReadClient::ProcessAttributePath(AttributePathIB::Parser & aAttribute
VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH);
err = aAttributePathParser.GetAttribute(&(aAttributePath.mAttributeId));
VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH);

DataModel::Nullable<ListIndex> listIndex;
err = aAttributePathParser.GetListIndex(&(listIndex));
if (CHIP_END_OF_TLV == err)
{
err = CHIP_NO_ERROR;
}
else if (listIndex.IsNull())
{
aAttributePath.mListOp = ConcreteDataAttributePath::ListOperation::AppendItem;
}
else
{
// TODO: Add ListOperation::ReplaceItem support. (Attribute path with valid list index)
err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH;
}
err = aAttributePathParser.GetListIndex(aAttributePath);
bzbarsky-apple marked this conversation as resolved.
Show resolved Hide resolved
bzbarsky-apple marked this conversation as resolved.
Show resolved Hide resolved
VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH);
return CHIP_NO_ERROR;
}
Expand Down
Loading