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.

Sign up for GitHub

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

fix subscription id #21609

Merged
merged 2 commits into from
Aug 4, 2022
Merged

fix subscription id #21609

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
92093d7
fix subscription id
yunhanw-google Aug 3, 2022
268ceee
Update src/app/ReadClient.cpp
yunhanw-google Aug 3, 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
17 changes: 9 additions & 8 deletions src/app/ReadClient.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,12 @@ ReadClient::ReadClient(InteractionModelEngine * apImEngine, Messaging::ExchangeM

void ReadClient::ClearActiveSubscriptionState()
{
mIsReporting = false;
mIsPrimingReports = true;
mPendingMoreChunks = false;
mMinIntervalFloorSeconds = 0;
mMaxInterval = 0;
mSubscriptionId = 0;
mIsReporting = false;
mWaitingForFirstPrimingReport = true;
mPendingMoreChunks = false;
mMinIntervalFloorSeconds = 0;
mMaxInterval = 0;
mSubscriptionId = 0;
MoveToState(ClientState::Idle);
}

Expand Down Expand Up @@ -504,7 +504,8 @@ CHIP_ERROR ReadClient::ProcessReportData(System::PacketBufferHandle && aPayload)
err = report.GetSubscriptionId(&subscriptionId);
if (CHIP_NO_ERROR == err)
{
if (mIsPrimingReports)
VerifyOrExit(IsSubscriptionType(), err = CHIP_ERROR_INVALID_ARGUMENT);
if (mWaitingForFirstPrimingReport)
{
mSubscriptionId = subscriptionId;
}
Expand Down Expand Up @@ -592,7 +593,7 @@ CHIP_ERROR ReadClient::ProcessReportData(System::PacketBufferHandle && aPayload)
err = StatusResponse::Send(Status::Success, mExchange.Get(), !noResponseExpected);
}

mIsPrimingReports = false;
mWaitingForFirstPrimingReport = false;
return err;
}

Expand Down
17 changes: 9 additions & 8 deletions src/app/ReadClient.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -496,14 +496,15 @@ class ReadClient : public Messaging::ExchangeDelegate
Messaging::ExchangeManager * mpExchangeMgr = nullptr;
Messaging::ExchangeHolder mExchange;
Callback & mpCallback;
ClientState mState = ClientState::Idle;
bool mIsReporting = false;
bool mIsInitialReport = true;
bool mIsPrimingReports = true;
bool mPendingMoreChunks = false;
uint16_t mMinIntervalFloorSeconds = 0;
uint16_t mMaxInterval = 0;
SubscriptionId mSubscriptionId = 0;
ClientState mState = ClientState::Idle;
bool mIsReporting = false;
bool mIsInitialReport = true;
// boolean to check if client is waiting for the first priming report
bool mWaitingForFirstPrimingReport = true;
bool mPendingMoreChunks = false;
uint16_t mMinIntervalFloorSeconds = 0;
uint16_t mMaxInterval = 0;
SubscriptionId mSubscriptionId = 0;
ScopedNodeId mPeer;
InteractionType mInteractionType = InteractionType::Read;
Timestamp mEventTimestamp;
Expand Down
38 changes: 36 additions & 2 deletions src/app/tests/TestReadInteraction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,7 @@ class TestReadInteraction
{
public:
static void TestReadClient(nlTestSuite * apSuite, void * apContext);
static void TestReadUnexpectedSubscriptionId(nlTestSuite * apSuite, void * apContext);
static void TestReadHandler(nlTestSuite * apSuite, void * apContext);
static void TestReadClientGenerateAttributePathList(nlTestSuite * apSuite, void * apContext);
static void TestReadClientGenerateInvalidAttributePathList(nlTestSuite * apSuite, void * apContext);
Expand Down Expand Up @@ -331,11 +332,11 @@ class TestReadInteraction

private:
static void GenerateReportData(nlTestSuite * apSuite, void * apContext, System::PacketBufferHandle & aPayload,
bool aNeedInvalidReport, bool aSuppressResponse);
bool aNeedInvalidReport, bool aSuppressResponse, bool aHasSubscriptionId);
};

void TestReadInteraction::GenerateReportData(nlTestSuite * apSuite, void * apContext, System::PacketBufferHandle & aPayload,
bool aNeedInvalidReport, bool aSuppressResponse)
bool aNeedInvalidReport, bool aSuppressResponse, bool aHasSubscriptionId = false)
{
CHIP_ERROR err = CHIP_NO_ERROR;
System::PacketBufferTLVWriter writer;
Expand All @@ -346,6 +347,12 @@ void TestReadInteraction::GenerateReportData(nlTestSuite * apSuite, void * apCon
err = reportDataMessageBuilder.Init(&writer);
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);

if (aHasSubscriptionId)
{
reportDataMessageBuilder.SubscriptionId(1);
NL_TEST_ASSERT(apSuite, reportDataMessageBuilder.GetError() == CHIP_NO_ERROR);
}

AttributeReportIBs::Builder & attributeReportIBsBuilder = reportDataMessageBuilder.CreateAttributeReportIBs();
NL_TEST_ASSERT(apSuite, reportDataMessageBuilder.GetError() == CHIP_NO_ERROR);

Expand Down Expand Up @@ -435,6 +442,32 @@ void TestReadInteraction::TestReadClient(nlTestSuite * apSuite, void * apContext
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
}

void TestReadInteraction::TestReadUnexpectedSubscriptionId(nlTestSuite * apSuite, void * apContext)
{
CHIP_ERROR err = CHIP_NO_ERROR;
TestContext & ctx = *static_cast<TestContext *>(apContext);
MockInteractionModelApp delegate;
app::ReadClient readClient(chip::app::InteractionModelEngine::GetInstance(), &ctx.GetExchangeManager(), delegate,
chip::app::ReadClient::InteractionType::Read);
System::PacketBufferHandle buf = System::PacketBufferHandle::New(System::PacketBuffer::kMaxSize);

ReadPrepareParams readPrepareParams(ctx.GetSessionBobToAlice());
err = readClient.SendRequest(readPrepareParams);
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);

// We don't actually want to deliver that message, because we want to
// synthesize the read response. But we don't want it hanging around
// forever either.
ctx.GetLoopback().mNumMessagesToDrop = 1;
ctx.DrainAndServiceIO();

// For read, we don't expect there is subscription id in report data.
GenerateReportData(apSuite, apContext, buf, false /*aNeedInvalidReport*/, true /* aSuppressResponse*/,
true /*aHasSubscriptionId*/);
err = readClient.ProcessReportData(std::move(buf));
NL_TEST_ASSERT(apSuite, err == CHIP_ERROR_INVALID_ARGUMENT);
}

void TestReadInteraction::TestReadHandler(nlTestSuite * apSuite, void * apContext)
{
CHIP_ERROR err = CHIP_NO_ERROR;
Expand Down Expand Up @@ -3350,6 +3383,7 @@ const nlTest sTests[] =
NL_TEST_DEF("TestReadChunking", chip::app::TestReadInteraction::TestReadChunking),
NL_TEST_DEF("TestSetDirtyBetweenChunks", chip::app::TestReadInteraction::TestSetDirtyBetweenChunks),
NL_TEST_DEF("CheckReadClient", chip::app::TestReadInteraction::TestReadClient),
NL_TEST_DEF("TestReadUnexpectedSubscriptionId", chip::app::TestReadInteraction::TestReadUnexpectedSubscriptionId),
NL_TEST_DEF("CheckReadHandler", chip::app::TestReadInteraction::TestReadHandler),
NL_TEST_DEF("TestReadClientGenerateAttributePathList", chip::app::TestReadInteraction::TestReadClientGenerateAttributePathList),
NL_TEST_DEF("TestReadClientGenerateInvalidAttributePathList", chip::app::TestReadInteraction::TestReadClientGenerateInvalidAttributePathList),
Expand Down