Skip to content

Commit

Permalink
Cleanup IM read (#9305)
Browse files Browse the repository at this point in the history
Summary of Changes:
-- SendReadRequest takes too many parameters, we need to add one
structure to pack those paramters.
  • Loading branch information
yunhanw-google authored Aug 30, 2021
1 parent a5569db commit ef73ba2
Show file tree
Hide file tree
Showing 8 changed files with 125 additions and 55 deletions.
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

0 comments on commit ef73ba2

Please sign in to comment.