Skip to content

Commit

Permalink
Fix missing data version parameter pass in EncodeAttribute (#17801)
Browse files Browse the repository at this point in the history 
* Fix missing data version parameter pass in EncodeAttribute

* address comments
  • Loading branch information
@yunhanw-google
yunhanw-google authored Apr 27, 2022
1 parent 408db8a commit 4b985dd
Show file tree
Hide file tree
Showing 2 changed files with 117 additions and 5 deletions.
2 changes: 1 addition & 1 deletion src/app/WriteClient.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ class WriteClient : public Messaging::ExchangeDelegate
value);
}

return EncodeAttribute(attributePath, value.Value());
return EncodeAttribute(attributePath, value.Value(), aDataVersion);
}

/**
Expand Down
120 changes: 116 additions & 4 deletions src/app/tests/TestWriteInteraction.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,11 @@ using TestContext = chip::Test::AppContext;
namespace {

uint8_t attributeDataTLV[CHIP_CONFIG_DEFAULT_UDP_MTU_SIZE];
size_t attributeDataTLVLen = 0;

constexpr uint16_t kMaxGroupsPerFabric = 5;
constexpr uint16_t kMaxGroupKeysPerFabric = 8;
size_t attributeDataTLVLen = 0;
constexpr chip::DataVersion kRejectedDataVersion = 1;
constexpr chip::DataVersion kAcceptedDataVersion = 5;
constexpr uint16_t kMaxGroupsPerFabric = 5;
constexpr uint16_t kMaxGroupKeysPerFabric = 8;

chip::TestPersistentStorageDelegate gTestStorage;
chip::Credentials::GroupDataProviderImpl gGroupsProvider(kMaxGroupsPerFabric, kMaxGroupKeysPerFabric);
Expand All @@ -57,6 +58,8 @@ class TestWriteInteraction
static void TestWriteHandler(nlTestSuite * apSuite, void * apContext);
static void TestWriteRoundtrip(nlTestSuite * apSuite, void * apContext);
static void TestWriteRoundtripWithClusterObjects(nlTestSuite * apSuite, void * apContext);
static void TestWriteRoundtripWithClusterObjectsVersionMatch(nlTestSuite * apSuite, void * apContext);
static void TestWriteRoundtripWithClusterObjectsVersionMismatch(nlTestSuite * apSuite, void * apContext);

private:
static void AddAttributeDataIB(nlTestSuite * apSuite, void * apContext, WriteClient & aWriteClient);
Expand All @@ -83,6 +86,7 @@ class TestWriteClientCallback : public chip::app::WriteClient::Callback
void ResetCounter() { mOnSuccessCalled = mOnErrorCalled = mOnDoneCalled = 0; }
void OnResponse(const WriteClient * apWriteClient, const chip::app::ConcreteDataAttributePath & path, StatusIB status) override
{
mStatus = status;
mOnSuccessCalled++;
}
void OnError(const WriteClient * apWriteClient, CHIP_ERROR chipError) override { mOnErrorCalled++; }
Expand All @@ -91,6 +95,7 @@ class TestWriteClientCallback : public chip::app::WriteClient::Callback
int mOnSuccessCalled = 0;
int mOnErrorCalled = 0;
int mOnDoneCalled = 0;
StatusIB mStatus;
};

void TestWriteInteraction::AddAttributeDataIB(nlTestSuite * apSuite, void * apContext, WriteClient & aWriteClient)
Expand Down Expand Up @@ -330,6 +335,11 @@ const EmberAfAttributeMetadata * GetAttributeMetadata(const ConcreteAttributePat
CHIP_ERROR WriteSingleClusterData(const Access::SubjectDescriptor & aSubjectDescriptor, const ConcreteDataAttributePath & aPath,
TLV::TLVReader & aReader, WriteHandler * aWriteHandler)
{
if (aPath.mDataVersion.HasValue() && aPath.mDataVersion.Value() == kRejectedDataVersion)
{
return aWriteHandler->AddStatus(aPath, Protocols::InteractionModel::Status::DataVersionMismatch);
}

TLV::TLVWriter writer;
writer.Init(attributeDataTLV);
writer.CopyElement(TLV::AnonymousTag(), aReader);
Expand Down Expand Up @@ -405,6 +415,106 @@ void TestWriteInteraction::TestWriteRoundtripWithClusterObjects(nlTestSuite * ap
engine->Shutdown();
}

void TestWriteInteraction::TestWriteRoundtripWithClusterObjectsVersionMatch(nlTestSuite * apSuite, void * apContext)
{
TestContext & ctx = *static_cast<TestContext *>(apContext);

CHIP_ERROR err = CHIP_NO_ERROR;

Messaging::ReliableMessageMgr * rm = ctx.GetExchangeManager().GetReliableMessageMgr();
// Shouldn't have anything in the retransmit table when starting the test.
NL_TEST_ASSERT(apSuite, rm->TestGetCountRetransTable() == 0);

TestWriteClientCallback callback;
auto * engine = chip::app::InteractionModelEngine::GetInstance();
err = engine->Init(&ctx.GetExchangeManager(), &ctx.GetFabricTable());
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);

app::WriteClient writeClient(engine->GetExchangeManager(), &callback, Optional<uint16_t>::Missing());

System::PacketBufferHandle buf = System::PacketBufferHandle::New(System::PacketBuffer::kMaxSize);

AttributePathParams attributePathParams;
attributePathParams.mEndpointId = 2;
attributePathParams.mClusterId = 3;
attributePathParams.mAttributeId = 4;

DataModel::Nullable<app::Clusters::TestCluster::Structs::SimpleStruct::Type> dataTx;

Optional<DataVersion> version(kAcceptedDataVersion);

writeClient.EncodeAttribute(attributePathParams, dataTx, version);
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);

NL_TEST_ASSERT(apSuite, callback.mOnSuccessCalled == 0);

err = writeClient.SendWriteRequest(ctx.GetSessionBobToAlice());
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);

ctx.DrainAndServiceIO();

NL_TEST_ASSERT(apSuite,
callback.mOnSuccessCalled == 1 && callback.mOnErrorCalled == 0 && callback.mOnDoneCalled == 1 &&
callback.mStatus.mStatus == Protocols::InteractionModel::Status::Success);

// By now we should have closed all exchanges and sent all pending acks, so
// there should be no queued-up things in the retransmit table.
NL_TEST_ASSERT(apSuite, rm->TestGetCountRetransTable() == 0);

engine->Shutdown();
}

void TestWriteInteraction::TestWriteRoundtripWithClusterObjectsVersionMismatch(nlTestSuite * apSuite, void * apContext)
{
TestContext & ctx = *static_cast<TestContext *>(apContext);

CHIP_ERROR err = CHIP_NO_ERROR;

Messaging::ReliableMessageMgr * rm = ctx.GetExchangeManager().GetReliableMessageMgr();
// Shouldn't have anything in the retransmit table when starting the test.
NL_TEST_ASSERT(apSuite, rm->TestGetCountRetransTable() == 0);

TestWriteClientCallback callback;
auto * engine = chip::app::InteractionModelEngine::GetInstance();
err = engine->Init(&ctx.GetExchangeManager(), &ctx.GetFabricTable());
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);

app::WriteClient writeClient(engine->GetExchangeManager(), &callback, Optional<uint16_t>::Missing());

System::PacketBufferHandle buf = System::PacketBufferHandle::New(System::PacketBuffer::kMaxSize);

AttributePathParams attributePathParams;
attributePathParams.mEndpointId = 2;
attributePathParams.mClusterId = 3;
attributePathParams.mAttributeId = 4;

app::Clusters::TestCluster::Structs::SimpleStruct::Type dataTxValue;
dataTxValue.a = 12;
dataTxValue.b = true;
DataModel::Nullable<app::Clusters::TestCluster::Structs::SimpleStruct::Type> dataTx;
dataTx.SetNonNull(dataTxValue);
Optional<DataVersion> version(kRejectedDataVersion);
writeClient.EncodeAttribute(attributePathParams, dataTx, version);
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);

NL_TEST_ASSERT(apSuite, callback.mOnSuccessCalled == 0);

err = writeClient.SendWriteRequest(ctx.GetSessionBobToAlice());
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);

ctx.DrainAndServiceIO();

NL_TEST_ASSERT(apSuite,
callback.mOnSuccessCalled == 1 && callback.mOnErrorCalled == 0 && callback.mOnDoneCalled == 1 &&
callback.mStatus.mStatus == Protocols::InteractionModel::Status::DataVersionMismatch);

// By now we should have closed all exchanges and sent all pending acks, so
// there should be no queued-up things in the retransmit table.
NL_TEST_ASSERT(apSuite, rm->TestGetCountRetransTable() == 0);

engine->Shutdown();
}

void TestWriteInteraction::TestWriteRoundtrip(nlTestSuite * apSuite, void * apContext)
{
TestContext & ctx = *static_cast<TestContext *>(apContext);
Expand Down Expand Up @@ -458,6 +568,8 @@ const nlTest sTests[] =
NL_TEST_DEF("CheckWriteHandler", chip::app::TestWriteInteraction::TestWriteHandler),
NL_TEST_DEF("CheckWriteRoundtrip", chip::app::TestWriteInteraction::TestWriteRoundtrip),
NL_TEST_DEF("TestWriteRoundtripWithClusterObjects", chip::app::TestWriteInteraction::TestWriteRoundtripWithClusterObjects),
NL_TEST_DEF("TestWriteRoundtripWithClusterObjectsVersionMatch", chip::app::TestWriteInteraction::TestWriteRoundtripWithClusterObjectsVersionMatch),
NL_TEST_DEF("TestWriteRoundtripWithClusterObjectsVersionMismatch", chip::app::TestWriteInteraction::TestWriteRoundtripWithClusterObjectsVersionMismatch),
NL_TEST_SENTINEL()
};
// clang-format on
Expand Down

0 comments on commit 4b985dd

Please sign in to comment.