Skip to content

Commit

Permalink
Fix ReadEvent (#12626)
Browse files Browse the repository at this point in the history
  • Loading branch information
@yunhanw-google @pull
yunhanw-google authored and pull[bot] committed Jun 12, 2023
1 parent fdadd54 commit 5322377
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 55 deletions.
17 changes: 0 additions & 17 deletions src/app/ReadClient.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -271,23 +271,6 @@ class ReadClient : public Messaging::ExchangeDelegate
CHIP_ERROR AbortExistingExchangeContext();
const char * GetStateStr() const;

/**
* Validate that the Event ID and Cluster ID in the header match that of the type information present in the object and
* decode the data. The template parameter T is generally expected to be a ClusterName::Events::EventName::Type struct
*
* @param [in] aEventHeader The header of the event being validated.
* @param [in] aEvent The event data.
* @param [in] aReader The tlv reader.
*/
template <typename EventDataT>
CHIP_ERROR DecodeEvent(const EventHeader & aEventHeader, const EventDataT & aEvent, TLV::TLVReader & aReader)
{
VerifyOrReturnError((aEventHeader.mPath.mEventId == aEvent.GetEventId()) &&
(aEventHeader.mPath.mClusterId == aEvent.GetClusterId()),
CHIP_ERROR_INVALID_ARGUMENT);
return DataModel::Decode(aReader, aEvent);
}

/**
* Internal shutdown method that we use when we know what's going on with
* our exchange and don't need to manually close it.
Expand Down
28 changes: 9 additions & 19 deletions src/controller/ReadInteraction.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,17 +88,17 @@ CHIP_ERROR ReadAttribute(Messaging::ExchangeManager * aExchangeMgr, const Sessio
* for a given attribute as well as callbacks for success and failure and either returns a decoded cluster-object representation
* of the requested attribute through the provided success callback or calls the provided failure callback.
*
* The EventTypeInfo is generally expected to be a ClusterName::Events::EventName::TypeInfo struct, but any
* The DecodableEventType is generally expected to be a ClusterName::Events::EventName::DecodableEventType struct, but any
* object that contains type information exposed through a 'DecodableType' type declaration as well as GetClusterId() and
* GetEventId() methods is expected to work.
*/
template <typename DecodableEventTypeInfo>
template <typename DecodableEventType>
CHIP_ERROR ReadEvent(Messaging::ExchangeManager * apExchangeMgr, const SessionHandle sessionHandle, EndpointId endpointId,
typename TypedReadEventCallback<DecodableEventTypeInfo>::OnSuccessCallbackType onSuccessCb,
typename TypedReadEventCallback<DecodableEventTypeInfo>::OnErrorCallbackType onErrorCb)
typename TypedReadEventCallback<DecodableEventType>::OnSuccessCallbackType onSuccessCb,
typename TypedReadEventCallback<DecodableEventType>::OnErrorCallbackType onErrorCb)
{
ClusterId clusterId = DecodableEventTypeInfo::GetClusterId();
EventId eventId = DecodableEventTypeInfo::GetEventId();
ClusterId clusterId = DecodableEventType::GetClusterId();
EventId eventId = DecodableEventType::GetEventId();
app::EventPathParams eventPath(endpointId, clusterId, eventId);
app::ReadPrepareParams readParams(sessionHandle);
app::ReadClient * readClient = nullptr;
Expand All @@ -108,17 +108,15 @@ CHIP_ERROR ReadEvent(Messaging::ExchangeManager * apExchangeMgr, const SessionHa
readParams.mpEventPathParamsList = &eventPath;
readParams.mEventPathParamsListSize = 1;

auto onDone = [](app::ReadClient * apReadClient, TypedReadEventCallback<DecodableEventTypeInfo> * callback) {
auto onDone = [](app::ReadClient * apReadClient, TypedReadEventCallback<DecodableEventType> * callback) {
chip::Platform::Delete(callback);
};

auto callback = chip::Platform::MakeUnique<TypedReadEventCallback<DecodableEventTypeInfo>>(clusterId, eventId, onSuccessCb,
onErrorCb, onDone);
auto callback = chip::Platform::MakeUnique<TypedReadEventCallback<DecodableEventType>>(onSuccessCb, onErrorCb, onDone);

VerifyOrReturnError(callback != nullptr, CHIP_ERROR_NO_MEMORY);

ReturnErrorOnFailure(
engine->NewReadClient(&readClient, app::ReadClient::InteractionType::Read, &callback.get()->GetBufferedCallback()));
ReturnErrorOnFailure(engine->NewReadClient(&readClient, app::ReadClient::InteractionType::Read, callback.get()));

err = readClient->SendReadRequest(readParams);
if (err != CHIP_NO_ERROR)
Expand Down Expand Up @@ -146,13 +144,5 @@ ReadAttribute(Messaging::ExchangeManager * aExchangeMgr, const SessionHandle ses
AttributeTypeInfo::GetClusterId(),
AttributeTypeInfo::GetAttributeId(), onSuccessCb, onErrorCb);
}

template <typename EventTypeInfo>
CHIP_ERROR ReadEvent(Messaging::ExchangeManager * aExchangeMgr, const SessionHandle sessionHandle, EndpointId endpointId,
typename TypedReadEventCallback<typename EventTypeInfo::DecodableType>::OnSuccessCallbackType onSuccessCb,
typename TypedReadEventCallback<typename EventTypeInfo::DecodableType>::OnErrorCallbackType onErrorCb)
{
return ReadEvent<typename EventTypeInfo::DecodableType>(aExchangeMgr, sessionHandle, endpointId, onSuccessCb, onErrorCb);
}
} // namespace Controller
} // namespace chip
18 changes: 8 additions & 10 deletions src/controller/TypedReadCallback.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,31 +107,31 @@ class TypedReadAttributeCallback final : public app::ReadClient::Callback
app::BufferedReadCallback mBufferedReadAdapter;
};

template <typename DecodableEventTypeInfo>
template <typename DecodableEventType>
class TypedReadEventCallback final : public app::ReadClient::Callback
{
public:
using OnSuccessCallbackType = std::function<void(const app::EventHeader & aEventHeader, const DecodableEventTypeInfo & aData)>;
using OnSuccessCallbackType = std::function<void(const app::EventHeader & aEventHeader, const DecodableEventType & aData)>;
using OnErrorCallbackType = std::function<void(const app::EventHeader * apEventHeader,
Protocols::InteractionModel::Status aIMStatus, CHIP_ERROR aError)>;
using OnDoneCallbackType = std::function<void(app::ReadClient * client, TypedReadEventCallback * callback)>;

TypedReadEventCallback(ClusterId aClusterId, EventId aEventId, OnSuccessCallbackType aOnSuccess, OnErrorCallbackType aOnError,
OnDoneCallbackType aOnDone) :
mClusterId(aClusterId),
mEventId(aEventId), mOnSuccess(aOnSuccess), mOnError(aOnError), mOnDone(aOnDone)
TypedReadEventCallback(OnSuccessCallbackType aOnSuccess, OnErrorCallbackType aOnError, OnDoneCallbackType aOnDone) :
mOnSuccess(aOnSuccess), mOnError(aOnError), mOnDone(aOnDone)
{}

private:
void OnEventData(const app::ReadClient * apReadClient, const app::EventHeader & aEventHeader, TLV::TLVReader * apData,
const app::StatusIB * apStatus) override
{
CHIP_ERROR err = CHIP_NO_ERROR;
DecodableEventTypeInfo value;
DecodableEventType value;

VerifyOrExit(apData != nullptr, err = CHIP_ERROR_INVALID_ARGUMENT);

err = apReadClient->DecodeEvent(aEventHeader, value, *apData);
VerifyOrExit((aEventHeader.mPath.mEventId == value.GetEventId()) && (aEventHeader.mPath.mClusterId == value.GetClusterId()),
CHIP_ERROR_SCHEMA_MISMATCH);
err = app::DataModel::Decode(*apData, value);
SuccessOrExit(err);

mOnSuccess(aEventHeader, value);
Expand All @@ -150,8 +150,6 @@ class TypedReadEventCallback final : public app::ReadClient::Callback

void OnDone(app::ReadClient * apReadClient) override { mOnDone(apReadClient, this); }

ClusterId mClusterId;
EventId mEventId;
OnSuccessCallbackType mOnSuccess;
OnErrorCallbackType mOnError;
OnDoneCallbackType mOnDone;
Expand Down
52 changes: 43 additions & 9 deletions src/controller/tests/data_model/TestRead.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,14 +122,15 @@ class TestReadInteraction
public:
TestReadInteraction() {}

static void TestDataResponse(nlTestSuite * apSuite, void * apContext);
static void TestAttributeError(nlTestSuite * apSuite, void * apContext);
static void TestReadTimeout(nlTestSuite * apSuite, void * apContext);
static void TestReadAttributeResponse(nlTestSuite * apSuite, void * apContext);
static void TestReadAttributeError(nlTestSuite * apSuite, void * apContext);
static void TestReadAttributeTimeout(nlTestSuite * apSuite, void * apContext);
static void TestReadEventResponse(nlTestSuite * apSuite, void * apContext);

private:
};

void TestReadInteraction::TestDataResponse(nlTestSuite * apSuite, void * apContext)
void TestReadInteraction::TestReadAttributeResponse(nlTestSuite * apSuite, void * apContext)
{
TestContext & ctx = *static_cast<TestContext *>(apContext);
auto sessionHandle = ctx.GetSessionBobToAlice();
Expand Down Expand Up @@ -174,7 +175,39 @@ void TestReadInteraction::TestDataResponse(nlTestSuite * apSuite, void * apConte
NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0);
}

void TestReadInteraction::TestAttributeError(nlTestSuite * apSuite, void * apContext)
void TestReadInteraction::TestReadEventResponse(nlTestSuite * apSuite, void * apContext)
{
TestContext & ctx = *static_cast<TestContext *>(apContext);
auto sessionHandle = ctx.GetSessionBobToAlice();
bool onSuccessCbInvoked = false, onFailureCbInvoked = false;

// Passing of stack variables by reference is only safe because of synchronous completion of the interaction. Otherwise, it's
// not safe to do so.
auto onSuccessCb = [apSuite, &onSuccessCbInvoked](const app::EventHeader & eventHeader, const auto & EventResponse) {
// TODO: Need to add check when IM event server integration completes
IgnoreUnusedVariable(apSuite);
onSuccessCbInvoked = true;
};

// Passing of stack variables by reference is only safe because of synchronous completion of the interaction. Otherwise, it's
// not safe to do so.
auto onFailureCb = [&onFailureCbInvoked](const app::EventHeader * eventHeader, Protocols::InteractionModel::Status aIMStatus,
CHIP_ERROR aError) { onFailureCbInvoked = true; };

chip::Controller::ReadEvent<TestCluster::Events::TestEvent::DecodableType>(&ctx.GetExchangeManager(), sessionHandle,
kTestEndpointId, onSuccessCb, onFailureCb);

ctx.DrainAndServiceIO();
chip::app::InteractionModelEngine::GetInstance()->GetReportingEngine().Run();
ctx.DrainAndServiceIO();

NL_TEST_ASSERT(apSuite, !onFailureCbInvoked);
NL_TEST_ASSERT(apSuite, chip::app::InteractionModelEngine::GetInstance()->GetNumActiveReadClients() == 0);
NL_TEST_ASSERT(apSuite, chip::app::InteractionModelEngine::GetInstance()->GetNumActiveReadHandlers() == 0);
NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0);
}

void TestReadInteraction::TestReadAttributeError(nlTestSuite * apSuite, void * apContext)
{
TestContext & ctx = *static_cast<TestContext *>(apContext);
auto sessionHandle = ctx.GetSessionBobToAlice();
Expand Down Expand Up @@ -209,7 +242,7 @@ void TestReadInteraction::TestAttributeError(nlTestSuite * apSuite, void * apCon
NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0);
}

void TestReadInteraction::TestReadTimeout(nlTestSuite * apSuite, void * apContext)
void TestReadInteraction::TestReadAttributeTimeout(nlTestSuite * apSuite, void * apContext)
{
TestContext & ctx = *static_cast<TestContext *>(apContext);
auto sessionHandle = ctx.GetSessionBobToAlice();
Expand Down Expand Up @@ -268,9 +301,10 @@ void TestReadInteraction::TestReadTimeout(nlTestSuite * apSuite, void * apContex
// clang-format off
const nlTest sTests[] =
{
NL_TEST_DEF("TestDataResponse", TestReadInteraction::TestDataResponse),
NL_TEST_DEF("TestAttributeError", TestReadInteraction::TestAttributeError),
NL_TEST_DEF("TestReadTimeout", TestReadInteraction::TestReadTimeout),
NL_TEST_DEF("TestReadAttributeResponse", TestReadInteraction::TestReadAttributeResponse),
NL_TEST_DEF("TestReadEventResponse", TestReadInteraction::TestReadEventResponse),
NL_TEST_DEF("TestReadAttributeError", TestReadInteraction::TestReadAttributeError),
NL_TEST_DEF("TestReadAttributeTimeout", TestReadInteraction::TestReadAttributeTimeout),
NL_TEST_SENTINEL()
};
// clang-format on
Expand Down

0 comments on commit 5322377

Please sign in to comment.