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

[Group] Update write client for groups #11601

Merged
merged 2 commits into from
Nov 10, 2021
Merged

[Group] Update write client for groups #11601

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
914c376
Update write client for groups
jepenven-silabs Nov 9, 2021
a1b2e89
Apply comments
jepenven-silabs Nov 10, 2021
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
26 changes: 18 additions & 8 deletions src/app/WriteClient.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,12 +244,21 @@ CHIP_ERROR WriteClient::SendWriteRequest(SessionHandle session, System::Clock::T
// Create a new exchange context.
mpExchangeCtx = mpExchangeMgr->NewContext(session, this);
VerifyOrExit(mpExchangeCtx != nullptr, err = CHIP_ERROR_NO_MEMORY);
mpExchangeCtx->SetResponseTimeout(timeout);
if (session.IsGroupSession())
jepenven-silabs marked this conversation as resolved.
Show resolved Hide resolved
{
// Exchange will be closed by WriteClientHandle::SendWriteRequest for group messages
err = mpExchangeCtx->SendMessage(Protocols::InteractionModel::MsgType::WriteRequest, std::move(packet),
Messaging::SendFlags(Messaging::SendMessageFlags::kNoAutoRequestAck));
}
else
{
mpExchangeCtx->SetResponseTimeout(timeout);

err = mpExchangeCtx->SendMessage(Protocols::InteractionModel::MsgType::WriteRequest, std::move(packet),
Messaging::SendFlags(Messaging::SendMessageFlags::kExpectResponse));
SuccessOrExit(err);
MoveToState(State::AwaitingResponse);
err = mpExchangeCtx->SendMessage(Protocols::InteractionModel::MsgType::WriteRequest, std::move(packet),
Messaging::SendFlags(Messaging::SendMessageFlags::kExpectResponse));
SuccessOrExit(err);
MoveToState(State::AwaitingResponse);
}

exit:
if (err != CHIP_NO_ERROR)
Expand Down Expand Up @@ -350,10 +359,11 @@ CHIP_ERROR WriteClientHandle::SendWriteRequest(SessionHandle session, System::Cl
{
CHIP_ERROR err = mpWriteClient->SendWriteRequest(session, timeout);
jepenven-silabs marked this conversation as resolved.
Show resolved Hide resolved

if (err == CHIP_NO_ERROR)
// Transferring ownership of the underlying WriteClient to the IM layer. IM will manage its lifetime.
// For groupcast writes, there is no transfer of ownership since the interaction is done upon transmission of the action
if (err == CHIP_NO_ERROR && !session.IsGroupSession())
jepenven-silabs marked this conversation as resolved.
Show resolved Hide resolved
{
// On success, the InteractionModelEngine will be responible to take care of the lifecycle of the WriteClient, so we release
// the WriteClient without closing it.
// Release the WriteClient without closing it.
mpWriteClient = nullptr;
}
else
Expand Down
29 changes: 29 additions & 0 deletions src/app/tests/TestWriteInteraction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ class TestWriteInteraction
{
public:
static void TestWriteClient(nlTestSuite * apSuite, void * apContext);
static void TestWriteClientGroup(nlTestSuite * apSuite, void * apContext);
static void TestWriteHandler(nlTestSuite * apSuite, void * apContext);
static void TestWriteRoundtrip(nlTestSuite * apSuite, void * apContext);
static void TestWriteRoundtripWithClusterObjects(nlTestSuite * apSuite, void * apContext);
Expand Down Expand Up @@ -233,6 +234,33 @@ void TestWriteInteraction::TestWriteClient(nlTestSuite * apSuite, void * apConte
NL_TEST_ASSERT(apSuite, rm->TestGetCountRetransTable() == 0);
}

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

CHIP_ERROR err = CHIP_NO_ERROR;

app::WriteClient writeClient;
app::WriteClientHandle writeClientHandle;
writeClientHandle.SetWriteClient(&writeClient);

System::PacketBufferHandle buf = System::PacketBufferHandle::New(System::PacketBuffer::kMaxSize);
TestWriteClientCallback callback;
err = writeClient.Init(&ctx.GetExchangeManager(), &callback);
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
AddAttributeDataIB(apSuite, apContext, writeClientHandle);

SessionHandle groupSession = ctx.GetSessionBobToFriends();
NL_TEST_ASSERT(apSuite, groupSession.IsGroupSession());

err = writeClientHandle.SendWriteRequest(groupSession);

// Write will fail until issue #11078 is completed
NL_TEST_ASSERT(apSuite, err == CHIP_ERROR_NOT_CONNECTED);
// The internal WriteClient should be shutdown once we SendWriteRequest for group.
NL_TEST_ASSERT(apSuite, nullptr == writeClientHandle.mpWriteClient);
}

void TestWriteInteraction::TestWriteHandler(nlTestSuite * apSuite, void * apContext)
{
TestContext & ctx = *static_cast<TestContext *>(apContext);
Expand Down Expand Up @@ -384,6 +412,7 @@ namespace {
const nlTest sTests[] =
{
NL_TEST_DEF("CheckWriteClient", chip::app::TestWriteInteraction::TestWriteClient),
NL_TEST_DEF("CheckWriteClientGroup", chip::app::TestWriteInteraction::TestWriteClientGroup),
NL_TEST_DEF("CheckWriteHandler", chip::app::TestWriteInteraction::TestWriteHandler),
NL_TEST_DEF("CheckWriteRoundtrip", chip::app::TestWriteInteraction::TestWriteRoundtrip),
NL_TEST_DEF("TestWriteRoundtripWithClusterObjects", chip::app::TestWriteInteraction::TestWriteRoundtripWithClusterObjects),
Expand Down
5 changes: 5 additions & 0 deletions src/messaging/tests/MessagingContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ SessionHandle MessagingContext::GetSessionAliceToBob()
return SessionHandle(GetBobNodeId(), GetAliceKeyId(), GetBobKeyId(), mDestFabricIndex);
}

SessionHandle MessagingContext::GetSessionBobToFriends()
{
return SessionHandle(GetBobKeyId(), GetFriendsGroupId(), GetFabricIndex());
}

Messaging::ExchangeContext * MessagingContext::NewUnauthenticatedExchangeToAlice(Messaging::ExchangeDelegate * delegate)
{
return mExchangeManager.NewContext(mSessionManager.CreateUnauthenticatedSession(mAliceAddress).Value(), delegate);
Expand Down
11 changes: 7 additions & 4 deletions src/messaging/tests/MessagingContext.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ class MessagingContext

uint16_t GetBobKeyId() const { return mBobKeyId; }
uint16_t GetAliceKeyId() const { return mAliceKeyId; }
GroupId GetFriendsGroupId() const { return mFriendsGroupId; }

void SetBobKeyId(uint16_t id) { mBobKeyId = id; }
void SetAliceKeyId(uint16_t id) { mAliceKeyId = id; }
Expand All @@ -78,6 +79,7 @@ class MessagingContext

SessionHandle GetSessionBobToAlice();
SessionHandle GetSessionAliceToBob();
SessionHandle GetSessionBobToFriends();

Messaging::ExchangeContext * NewUnauthenticatedExchangeToAlice(Messaging::ExchangeDelegate * delegate);
Messaging::ExchangeContext * NewUnauthenticatedExchangeToBob(Messaging::ExchangeDelegate * delegate);
Expand All @@ -94,10 +96,11 @@ class MessagingContext
secure_channel::MessageCounterManager mMessageCounterManager;
IOContext * mIOContext;

NodeId mBobNodeId = 123654;
NodeId mAliceNodeId = 111222333;
uint16_t mBobKeyId = 1;
uint16_t mAliceKeyId = 2;
NodeId mBobNodeId = 123654;
NodeId mAliceNodeId = 111222333;
uint16_t mBobKeyId = 1;
uint16_t mAliceKeyId = 2;
GroupId mFriendsGroupId = 517;
Transport::PeerAddress mAliceAddress;
Transport::PeerAddress mBobAddress;
SecurePairingUsingTestSecret mPairingAliceToBob;
Expand Down