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

Cleanup IM read #9305

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 2 additions & 7 deletions src/app/InteractionModelEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -317,17 +317,12 @@ void InteractionModelEngine::OnResponseTimeout(Messaging::ExchangeContext * ec)
ChipLogProgress(InteractionModel, "Time out! failed to receive echo response from Exchange: %d", ec->GetExchangeId());
}

CHIP_ERROR InteractionModelEngine::SendReadRequest(NodeId aNodeId, FabricIndex aFabricIndex, SessionHandle * apSecureSession,
EventPathParams * apEventPathParamsList, size_t aEventPathParamsListSize,
AttributePathParams * apAttributePathParamsList,
size_t aAttributePathParamsListSize, EventNumber aEventNumber,
uint64_t aAppIdentifier)
CHIP_ERROR InteractionModelEngine::SendReadRequest(ReadPrepareParams & aReadPrepareParams, uint64_t aAppIdentifier)
{
ReadClient * client = nullptr;
CHIP_ERROR err = CHIP_NO_ERROR;
ReturnErrorOnFailure(NewReadClient(&client, aAppIdentifier));
err = client->SendReadRequest(aNodeId, aFabricIndex, apSecureSession, apEventPathParamsList, aEventPathParamsListSize,
apAttributePathParamsList, aAttributePathParamsListSize, aEventNumber);
err = client->SendReadRequest(aReadPrepareParams);
if (err != CHIP_NO_ERROR)
{
client->Shutdown();
Expand Down
5 changes: 1 addition & 4 deletions src/app/InteractionModelEngine.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,7 @@ class InteractionModelEngine : public Messaging::ExchangeDelegate
* @retval #CHIP_ERROR_NO_MEMORY If there is no ReadClient available
* @retval #CHIP_NO_ERROR On success.
*/
CHIP_ERROR SendReadRequest(NodeId aNodeId, FabricIndex aFabricIndex, SessionHandle * apSecureSession,
EventPathParams * apEventPathParamsList, size_t aEventPathParamsListSize,
AttributePathParams * apAttributePathParamsList, size_t aAttributePathParamsListSize,
EventNumber aEventNumber, uint64_t aAppIdentifier = 0);
CHIP_ERROR SendReadRequest(ReadPrepareParams & aReadPrepareParams, uint64_t aAppIdentifier = 0);

/**
* Retrieve a WriteClient that the SDK consumer can use to send a write. If the call succeeds,
Expand Down
30 changes: 11 additions & 19 deletions src/app/ReadClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,7 @@ void ReadClient::MoveToState(const ClientState aTargetState)
GetStateStr());
}

CHIP_ERROR ReadClient::SendReadRequest(NodeId aNodeId, FabricIndex aFabricIndex, SessionHandle * apSecureSession,
EventPathParams * apEventPathParamsList, size_t aEventPathParamsListSize,
AttributePathParams * apAttributePathParamsList, size_t aAttributePathParamsListSize,
EventNumber aEventNumber, uint32_t timeout)
CHIP_ERROR ReadClient::SendReadRequest(ReadPrepareParams & aReadPrepareParams)
{
// TODO: SendRequest parameter is too long, need to have the structure to represent it
CHIP_ERROR err = CHIP_NO_ERROR;
Expand All @@ -120,24 +117,26 @@ CHIP_ERROR ReadClient::SendReadRequest(NodeId aNodeId, FabricIndex aFabricIndex,
err = request.Init(&writer);
SuccessOrExit(err);

if (aEventPathParamsListSize != 0 && apEventPathParamsList != nullptr)
if (aReadPrepareParams.mEventPathParamsListSize != 0 && aReadPrepareParams.mpEventPathParamsList != nullptr)
{
EventPathList::Builder & eventPathListBuilder = request.CreateEventPathListBuilder();
SuccessOrExit(err = eventPathListBuilder.GetError());
err = GenerateEventPathList(eventPathListBuilder, apEventPathParamsList, aEventPathParamsListSize);
err = GenerateEventPathList(eventPathListBuilder, aReadPrepareParams.mpEventPathParamsList,
aReadPrepareParams.mEventPathParamsListSize);
SuccessOrExit(err);
if (aEventNumber != 0)
if (aReadPrepareParams.mEventNumber != 0)
{
// EventNumber is optional
request.EventNumber(aEventNumber);
request.EventNumber(aReadPrepareParams.mEventNumber);
}
}

if (aAttributePathParamsListSize != 0 && apAttributePathParamsList != nullptr)
if (aReadPrepareParams.mAttributePathParamsListSize != 0 && aReadPrepareParams.mpAttributePathParamsList != nullptr)
{
AttributePathList::Builder attributePathListBuilder = request.CreateAttributePathListBuilder();
SuccessOrExit(err = attributePathListBuilder.GetError());
err = GenerateAttributePathList(attributePathListBuilder, apAttributePathParamsList, aAttributePathParamsListSize);
err = GenerateAttributePathList(attributePathListBuilder, aReadPrepareParams.mpAttributePathParamsList,
aReadPrepareParams.mAttributePathParamsListSize);
SuccessOrExit(err);
}

Expand All @@ -148,16 +147,9 @@ CHIP_ERROR ReadClient::SendReadRequest(NodeId aNodeId, FabricIndex aFabricIndex,
SuccessOrExit(err);
}

if (apSecureSession != nullptr)
{
mpExchangeCtx = mpExchangeMgr->NewContext(*apSecureSession, this);
}
else
{
mpExchangeCtx = mpExchangeMgr->NewContext(SessionHandle(aNodeId, 0, 0, aFabricIndex), this);
}
mpExchangeCtx = mpExchangeMgr->NewContext(aReadPrepareParams.mSessionHandle, this);
VerifyOrExit(mpExchangeCtx != nullptr, err = CHIP_ERROR_NO_MEMORY);
mpExchangeCtx->SetResponseTimeout(timeout);
mpExchangeCtx->SetResponseTimeout(aReadPrepareParams.mTimeout);

err = mpExchangeCtx->SendMessage(Protocols::InteractionModel::MsgType::ReadRequest, std::move(msgBuf),
Messaging::SendFlags(Messaging::SendMessageFlags::kExpectResponse));
Expand Down
6 changes: 2 additions & 4 deletions src/app/ReadClient.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include <app/EventPathParams.h>
#include <app/InteractionModelDelegate.h>
#include <app/MessageDef/ReadRequest.h>
#include <app/ReadPrepareParams.h>
#include <core/CHIPCore.h>
#include <core/CHIPTLVDebug.hpp>
#include <messaging/ExchangeContext.h>
Expand Down Expand Up @@ -75,10 +76,7 @@ class ReadClient : public Messaging::ExchangeDelegate
* @retval #others fail to send read request
* @retval #CHIP_NO_ERROR On success.
*/
CHIP_ERROR SendReadRequest(NodeId aNodeId, FabricIndex aFabricIndex, SessionHandle * aSecureSession,
EventPathParams * apEventPathParamsList, size_t aEventPathParamsListSize,
AttributePathParams * apAttributePathParamsList, size_t aAttributePathParamsListSize,
EventNumber aEventNumber, uint32_t timeout = kImMessageTimeoutMsec);
CHIP_ERROR SendReadRequest(ReadPrepareParams & aReadPrepareParams);

uint64_t GetAppIdentifier() const { return mAppIdentifier; }
Messaging::ExchangeContext * GetExchangeContext() const { return mpExchangeCtx; }
Expand Down
83 changes: 83 additions & 0 deletions src/app/ReadPrepareParams.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/*
*
* Copyright (c) 2021 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/AttributePathParams.h>
#include <app/EventPathParams.h>
#include <app/util/basic-types.h>
#include <core/CHIPCore.h>
#include <core/CHIPTLV.h>
#include <messaging/ExchangeMgr.h>

namespace chip {
namespace app {
struct ReadPrepareParams
{
SessionHandle mSessionHandle;
EventPathParams * mpEventPathParamsList = nullptr;
size_t mEventPathParamsListSize = 0;
AttributePathParams * mpAttributePathParamsList = nullptr;
size_t mAttributePathParamsListSize = 0;
EventNumber mEventNumber = 0;
uint32_t mTimeout = kImMessageTimeoutMsec;
uint16_t mMinIntervalSeconds = 0;
uint16_t mMaxIntervalSeconds = 0;

ReadPrepareParams() {}
ReadPrepareParams(ReadPrepareParams && other)
{
mSessionHandle = other.mSessionHandle;
mpEventPathParamsList = other.mpEventPathParamsList;
mEventPathParamsListSize = other.mEventPathParamsListSize;
mpAttributePathParamsList = other.mpAttributePathParamsList;
mAttributePathParamsListSize = other.mAttributePathParamsListSize;
mEventNumber = other.mEventNumber;
mMinIntervalSeconds = other.mMinIntervalSeconds;
mMaxIntervalSeconds = other.mMaxIntervalSeconds;
mTimeout = other.mTimeout;
other.mpEventPathParamsList = nullptr;
other.mEventPathParamsListSize = 0;
other.mpAttributePathParamsList = nullptr;
other.mAttributePathParamsListSize = 0;
}

ReadPrepareParams & operator=(ReadPrepareParams && other)
{
if (&other == this)
return *this;

mSessionHandle = other.mSessionHandle;
mpEventPathParamsList = other.mpEventPathParamsList;
mEventPathParamsListSize = other.mEventPathParamsListSize;
mpAttributePathParamsList = other.mpAttributePathParamsList;
mAttributePathParamsListSize = other.mAttributePathParamsListSize;
mEventNumber = other.mEventNumber;
mMinIntervalSeconds = other.mMinIntervalSeconds;
mMaxIntervalSeconds = other.mMaxIntervalSeconds;
mTimeout = other.mTimeout;
other.mpEventPathParamsList = nullptr;
other.mEventPathParamsListSize = 0;
other.mpAttributePathParamsList = nullptr;
other.mAttributePathParamsListSize = 0;

return *this;
}
};
} // namespace app
} // namespace chip
26 changes: 12 additions & 14 deletions src/app/tests/TestReadInteraction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -265,16 +265,13 @@ void TestReadInteraction::TestReadClient(nlTestSuite * apSuite, void * apContext
CHIP_ERROR err = CHIP_NO_ERROR;
TestContext & ctx = *static_cast<TestContext *>(apContext);
app::ReadClient readClient;
EventNumber eventNumber = 0;

chip::app::InteractionModelDelegate delegate;
System::PacketBufferHandle buf = System::PacketBufferHandle::New(System::PacketBuffer::kMaxSize);
err = readClient.Init(&ctx.GetExchangeManager(), &delegate, 0 /* application identifier */);
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
SessionHandle session = ctx.GetSessionLocalToPeer();
err = readClient.SendReadRequest(ctx.GetDestinationNodeId(), ctx.GetFabricIndex(), &session, nullptr /*apEventPathParamsList*/,
0 /*aEventPathParamsListSize*/, nullptr /*apAttributePathParamsList*/,
0 /*aAttributePathParamsListSize*/, eventNumber /*aEventNumber*/);
ReadPrepareParams readPrepareParams;
readPrepareParams.mSessionHandle = ctx.GetSessionLocalToPeer();
err = readClient.SendReadRequest(readPrepareParams);
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);

GenerateReportData(apSuite, apContext, buf);
Expand Down Expand Up @@ -395,16 +392,14 @@ void TestReadInteraction::TestReadClientInvalidReport(nlTestSuite * apSuite, voi
TestContext & ctx = *static_cast<TestContext *>(apContext);
app::ReadClient readClient;
chip::app::InteractionModelDelegate delegate;
EventNumber eventNumber = 0;

System::PacketBufferHandle buf = System::PacketBufferHandle::New(System::PacketBuffer::kMaxSize);
err = readClient.Init(&ctx.GetExchangeManager(), &delegate, 0 /* application identifier */);
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);

SessionHandle session = ctx.GetSessionLocalToPeer();
err = readClient.SendReadRequest(ctx.GetDestinationNodeId(), ctx.GetFabricIndex(), &session, nullptr /*apEventPathParamsList*/,
0 /*aEventPathParamsListSize*/, nullptr /*apAttributePathParamsList*/,
0 /*aAttributePathParamsListSize*/, eventNumber /*aEventNumber*/);
ReadPrepareParams readPrepareParams;
readPrepareParams.mSessionHandle = ctx.GetSessionLocalToPeer();
err = readClient.SendReadRequest(readPrepareParams);
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);

GenerateReportData(apSuite, apContext, buf, true /*aNeedInvalidReport*/);
Expand Down Expand Up @@ -594,9 +589,12 @@ void TestReadInteraction::TestReadEventRoundtrip(nlTestSuite * apSuite, void * a
eventPathParams[1].mClusterId = kTestClusterId;
eventPathParams[1].mEventId = kTestEventIdCritical;

SessionHandle session = ctx.GetSessionLocalToPeer();
err = chip::app::InteractionModelEngine::GetInstance()->SendReadRequest(ctx.GetDestinationNodeId(), ctx.GetFabricIndex(),
&session, eventPathParams, 2, nullptr, 1, 0);
ReadPrepareParams readPrepareParams;
readPrepareParams.mSessionHandle = ctx.GetSessionLocalToPeer();
readPrepareParams.mpEventPathParamsList = eventPathParams;
readPrepareParams.mEventPathParamsListSize = 2;

err = chip::app::InteractionModelEngine::GetInstance()->SendReadRequest(readPrepareParams);
NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);

InteractionModelEngine::GetInstance()->GetReportingEngine().Run();
Expand Down
11 changes: 7 additions & 4 deletions src/app/tests/integration/chip_im_initiator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,8 @@ CHIP_ERROR SendBadCommandRequest(chip::app::CommandSender * commandSender)

CHIP_ERROR SendReadRequest()
{
CHIP_ERROR err = CHIP_NO_ERROR;
chip::EventNumber number = 0;
CHIP_ERROR err = CHIP_NO_ERROR;
chip::app::ReadPrepareParams readPrepareParams;
chip::app::EventPathParams eventPathParams[2];
eventPathParams[0].mNodeId = kTestNodeId;
eventPathParams[0].mEndpointId = kTestEndpointId;
Expand All @@ -198,8 +198,11 @@ CHIP_ERROR SendReadRequest()

printf("\nSend read request message to Node: %" PRIu64 "\n", chip::kTestDeviceNodeId);

err = chip::app::InteractionModelEngine::GetInstance()->SendReadRequest(
chip::kTestDeviceNodeId, gFabricIndex, nullptr, eventPathParams, 2, &attributePathParams, 1, number, gMessageTimeoutMsec);
readPrepareParams.mSessionHandle = chip::SessionHandle(chip::kTestDeviceNodeId, 0, 0, gFabricIndex);
readPrepareParams.mTimeout = gMessageTimeoutMsec;
readPrepareParams.mpEventPathParamsList = eventPathParams;
readPrepareParams.mEventPathParamsListSize = 2;
err = chip::app::InteractionModelEngine::GetInstance()->SendReadRequest(readPrepareParams, gMessageTimeoutMsec);
SuccessOrExit(err);

exit:
Expand Down
10 changes: 7 additions & 3 deletions src/controller/CHIPDevice.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
#endif // CHIP_SYSTEM_CONFIG_USE_LWIP

#include <app/CommandSender.h>
#include <app/ReadPrepareParams.h>
#include <app/util/DataModelHandler.h>
#include <core/CHIPCore.h>
#include <core/CHIPEncoding.h>
Expand Down Expand Up @@ -678,6 +679,7 @@ void Device::AddReportHandler(EndpointId endpoint, ClusterId cluster, AttributeI
CHIP_ERROR Device::SendReadAttributeRequest(app::AttributePathParams aPath, Callback::Cancelable * onSuccessCallback,
Callback::Cancelable * onFailureCallback, app::TLVDataFilter aTlvDataFilter)
{
chip::app::ReadPrepareParams readPrepareParams;
bool loadedSecureSession = false;
uint8_t seqNum = GetNextSequenceNumber();
aPath.mNodeId = GetDeviceId();
Expand All @@ -690,9 +692,11 @@ CHIP_ERROR Device::SendReadAttributeRequest(app::AttributePathParams aPath, Call
}
// The application context is used to identify different requests from client applicaiton the type of it is intptr_t, here we
// use the seqNum.
CHIP_ERROR err = chip::app::InteractionModelEngine::GetInstance()->SendReadRequest(
GetDeviceId(), 0, &mSecureSession, nullptr /*event path params list*/, 0, &aPath, 1, 0 /* event number */,
seqNum /* application context */);
readPrepareParams.mSessionHandle = mSecureSession;
readPrepareParams.mpAttributePathParamsList = &aPath;
readPrepareParams.mAttributePathParamsListSize = 1;
CHIP_ERROR err =
chip::app::InteractionModelEngine::GetInstance()->SendReadRequest(readPrepareParams, seqNum /* application context */);
if (err != CHIP_NO_ERROR)
{
CancelResponseHandler(seqNum);
Expand Down