Fix ReadEvent (#12626)

src/app/ReadClient.h

@@ -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.

src/controller/ReadInteraction.h

@@ -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;
@@ -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)
@@ -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

src/controller/TypedReadCallback.h

@@ -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);
@@ -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;

src/controller/tests/data_model/TestRead.cpp

@@ -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();
@@ -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();
@@ -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();
@@ -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