Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@yufengwangca
yufengwangca committed Nov 11, 2021
1 parent 889f33e commit e070547
Show file tree
Hide file tree
Showing 5 changed files with 694 additions and 733 deletions.
49 changes: 33 additions & 16 deletions src/app/AttributeAccessInterface.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

#include <app/ConcreteAttributePath.h>
#include <app/MessageDef/AttributeDataIB.h>
#include <app/data-model/Decode.h>
#include <app/data-model/Encode.h>
#include <app/data-model/List.h> // So we can encode lists
#include <app/data-model/TagBoundEncoder.h>
Expand Down Expand Up @@ -101,6 +102,29 @@ class AttributeValueEncoder : protected TagBoundEncoder
const FabricIndex mAccessingFabricIndex;
};

class AttributeValueDecoder
{
public:
AttributeValueDecoder(TLV::TLVReader * aReader) : mReader(aReader) {}

template <typename T>
CHIP_ERROR Decode(T & aArg)
{
mTriedDecode = true;
if (mReader == nullptr)
{
return CHIP_NO_ERROR;
}
return DataModel::Decode(*mReader, aArg);
}

bool TriedDecode() const { return mTriedDecode; }

private:
TLV::TLVReader * mReader;
bool mTriedDecode = false;
};

class AttributeAccessInterface
{
public:
Expand Down Expand Up @@ -130,23 +154,16 @@ class AttributeAccessInterface
/**
* Callback for writing attributes.
*
* @param [in] aPath indicates which exact data is being write.
* @param [in] aTLVReader A pointer to a TLVReader, which should point to the beginning
* of this AttributeDataElement to write.
* @param [out] aDataWrite whether we actually tried to write data. If
* this function returns success and aDataWrite is
* false, the AttributeAccessInterface did not try
* to write any data. In this case, normal attribute
* access will happen for the write. This may involve
* writing to the attribute store or external attribute
* callbacks.
* @param [in] aPath indicates which exact data is being written.
* @param [in] aDecoder the AttributeValueDecoder to use for decoding the
* data. If this function returns scucess and no attempt is
* made to decode data using aDecoder, the
* AttributeAccessInterface did not try to write any data. In
* this case, normal attribute access will happen for the write.
* This may involve writing to the attribute store or external
* attribute callbacks.
*/
virtual CHIP_ERROR Write(const ConcreteAttributePath & aPath, TLV::TLVReader & aReader, bool * aDataWrite)
{
*aDataWrite = false;

return CHIP_NO_ERROR;
}
virtual CHIP_ERROR Write(const ConcreteAttributePath & aPath, AttributeValueDecoder & aDecoder) { return CHIP_NO_ERROR; }

/**
* Mechanism for keeping track of a chain of AttributeAccessInterfaces.
Expand Down
49 changes: 22 additions & 27 deletions src/app/clusters/test-cluster-server/test-cluster-server.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,10 @@ using namespace chip::app::Clusters::TestCluster;
using namespace chip::app::Clusters::TestCluster::Commands;
using namespace chip::app::Clusters::TestCluster::Attributes;

constexpr uint8_t kAttributeListLength = 4;
// The number of elements in the test attribute list
constexpr uint8_t kAttributeListLength = 4;

// The maximum length of the attribute list element in byte
constexpr uint8_t kAttributeEntryLength = 6;

namespace {
Expand All @@ -52,17 +55,17 @@ class TestAttrAccess : public AttributeAccessInterface
TestAttrAccess() : AttributeAccessInterface(Optional<EndpointId>::Missing(), TestCluster::Id) {}

CHIP_ERROR Read(const ConcreteAttributePath & aPath, AttributeValueEncoder & aEncoder) override;
CHIP_ERROR Write(const ConcreteAttributePath & aPath, TLV::TLVReader & aReader, bool * aDataWrite) override;
CHIP_ERROR Write(const ConcreteAttributePath & aPath, AttributeValueDecoder & aDecoder) override;

private:
CHIP_ERROR ReadListInt8uAttribute(AttributeValueEncoder & aEncoder);
CHIP_ERROR WriteListInt8uAttribute(TLV::TLVReader & aReader);
CHIP_ERROR WriteListInt8uAttribute(AttributeValueDecoder & aDecoder);
CHIP_ERROR ReadListOctetStringAttribute(AttributeValueEncoder & aEncoder);
CHIP_ERROR WriteListOctetStringAttribute(TLV::TLVReader & aReader);
CHIP_ERROR WriteListOctetStringAttribute(AttributeValueDecoder & aDecoder);
CHIP_ERROR ReadListStructOctetStringAttribute(AttributeValueEncoder & aEncoder);
CHIP_ERROR WriteListStructOctetStringAttribute(TLV::TLVReader & aReader);
CHIP_ERROR WriteListStructOctetStringAttribute(AttributeValueDecoder & aDecoder);
CHIP_ERROR ReadListNullablesAndOptionalsStructAttribute(AttributeValueEncoder & aEncoder);
CHIP_ERROR WriteListNullablesAndOptionalsStructAttribute(TLV::TLVReader & aReader);
CHIP_ERROR WriteListNullablesAndOptionalsStructAttribute(AttributeValueDecoder & aDecoder);
};

TestAttrAccess gAttrAccess;
Expand Down Expand Up @@ -95,26 +98,23 @@ CHIP_ERROR TestAttrAccess::Read(const ConcreteAttributePath & aPath, AttributeVa
return CHIP_NO_ERROR;
}

CHIP_ERROR TestAttrAccess::Write(const ConcreteAttributePath & aPath, TLV::TLVReader & aReader, bool * aDataWrite)
CHIP_ERROR TestAttrAccess::Write(const ConcreteAttributePath & aPath, AttributeValueDecoder & aDecoder)
{
*aDataWrite = true;

switch (aPath.mAttributeId)
{
case ListInt8u::Id: {
return WriteListInt8uAttribute(aReader);
return WriteListInt8uAttribute(aDecoder);
}
case ListOctetString::Id: {
return WriteListOctetStringAttribute(aReader);
return WriteListOctetStringAttribute(aDecoder);
}
case ListStructOctetString::Id: {
return WriteListStructOctetStringAttribute(aReader);
return WriteListStructOctetStringAttribute(aDecoder);
}
case ListNullablesAndOptionalsStruct::Id: {
return WriteListNullablesAndOptionalsStructAttribute(aReader);
return WriteListNullablesAndOptionalsStructAttribute(aDecoder);
}
default: {
*aDataWrite = false;
break;
}
}
Expand All @@ -133,11 +133,11 @@ CHIP_ERROR TestAttrAccess::ReadListInt8uAttribute(AttributeValueEncoder & aEncod
});
}

CHIP_ERROR TestAttrAccess::WriteListInt8uAttribute(TLV::TLVReader & aReader)
CHIP_ERROR TestAttrAccess::WriteListInt8uAttribute(AttributeValueDecoder & aDecoder)
{
DataModel::DecodableList<uint8_t> list;

ReturnErrorOnFailure(DataModel::Decode(aReader, list));
ReturnErrorOnFailure(aDecoder.Decode(list));

uint8_t index = 0;
auto iter = list.begin();
Expand All @@ -147,12 +147,7 @@ CHIP_ERROR TestAttrAccess::WriteListInt8uAttribute(TLV::TLVReader & aReader)
gListUint8Data[index++] = entry;
}

if (iter.GetStatus() != CHIP_NO_ERROR)
{
return CHIP_ERROR_INVALID_DATA_LIST;
}

return CHIP_NO_ERROR;
return iter.GetStatus();
}

CHIP_ERROR TestAttrAccess::ReadListOctetStringAttribute(AttributeValueEncoder & aEncoder)
Expand All @@ -167,11 +162,11 @@ CHIP_ERROR TestAttrAccess::ReadListOctetStringAttribute(AttributeValueEncoder &
});
}

CHIP_ERROR TestAttrAccess::WriteListOctetStringAttribute(TLV::TLVReader & aReader)
CHIP_ERROR TestAttrAccess::WriteListOctetStringAttribute(AttributeValueDecoder & aDecoder)
{
DataModel::DecodableList<ByteSpan> list;

ReturnErrorOnFailure(DataModel::Decode(aReader, list));
ReturnErrorOnFailure(aDecoder.Decode(list));

uint8_t index = 0;
auto iter = list.begin();
Expand Down Expand Up @@ -202,11 +197,11 @@ CHIP_ERROR TestAttrAccess::ReadListStructOctetStringAttribute(AttributeValueEnco
});
}

CHIP_ERROR TestAttrAccess::WriteListStructOctetStringAttribute(TLV::TLVReader & aReader)
CHIP_ERROR TestAttrAccess::WriteListStructOctetStringAttribute(AttributeValueDecoder & aDecoder)
{
DataModel::DecodableList<Structs::TestListStructOctet::Type> list;

ReturnErrorOnFailure(DataModel::Decode(aReader, list));
ReturnErrorOnFailure(aDecoder.Decode(list));

uint8_t index = 0;
auto iter = list.begin();
Expand Down Expand Up @@ -241,7 +236,7 @@ CHIP_ERROR TestAttrAccess::ReadListNullablesAndOptionalsStructAttribute(Attribut
});
}

CHIP_ERROR TestAttrAccess::WriteListNullablesAndOptionalsStructAttribute(TLV::TLVReader & aReader)
CHIP_ERROR TestAttrAccess::WriteListNullablesAndOptionalsStructAttribute(AttributeValueDecoder & aDecoder)
{
// TODO Add yaml test case for NullablesAndOptionalsStruct list
return CHIP_NO_ERROR;
Expand Down
7 changes: 3 additions & 4 deletions src/app/util/ember-compatibility-functions.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -508,12 +508,11 @@ CHIP_ERROR WriteSingleClusterData(ClusterInfo & aClusterInfo, TLV::TLVReader & a
AttributeAccessInterface * attrOverride = findAttributeAccessOverride(aClusterInfo.mEndpointId, aClusterInfo.mClusterId);
if (attrOverride != nullptr)
{
bool dataWrite;
ConcreteAttributePath path(aClusterInfo.mEndpointId, aClusterInfo.mClusterId, aClusterInfo.mAttributeId);
AttributeValueDecoder valueDecoder(&aReader);
ReturnErrorOnFailure(attrOverride->Write(path, valueDecoder));

ReturnErrorOnFailure(attrOverride->Write(path, aReader, &dataWrite));

if (dataWrite)
if (valueDecoder.TriedDecode())
{
return apWriteHandler->AddStatus(attributePathParams, Protocols::InteractionModel::Status::Success);
}
Expand Down
Loading

0 comments on commit e070547

Please sign in to comment.