From 25b76b68b91bf9fbe06823694ad7fbf0fd5421f7 Mon Sep 17 00:00:00 2001 From: yunhanw Date: Fri, 12 Nov 2021 15:53:46 -0800 Subject: [PATCH] address comments --- src/app/ClusterInfo.h | 3 +- src/app/ConcreteEventPath.h | 5 + ...DeviceControllerInteractionModelDelegate.h | 2 +- src/app/EventHeader.h | 1 - src/app/EventLoggingTypes.h | 4 + src/app/EventManagement.cpp | 46 +------ src/app/MessageDef/EventDataIB.cpp | 84 +++++++++++++ src/app/MessageDef/EventDataIB.h | 6 + src/app/MessageDef/EventReportIB.cpp | 16 ++- src/app/ReadClient.cpp | 116 +----------------- src/app/ReadClient.h | 32 +++-- src/app/tests/TestEventLogging.cpp | 6 +- src/app/tests/TestMessageDef.cpp | 7 +- src/app/tests/TestReadInteraction.cpp | 2 +- .../tests/integration/chip_im_initiator.cpp | 2 +- src/controller/TypedReadCallback.h | 15 +-- src/lib/core/CHIPError.cpp | 3 + src/lib/core/CHIPError.h | 9 ++ 18 files changed, 164 insertions(+), 195 deletions(-) diff --git a/src/app/ClusterInfo.h b/src/app/ClusterInfo.h index 564d9bb81a3206..f32c6dad3b27e5 100644 --- a/src/app/ClusterInfo.h +++ b/src/app/ClusterInfo.h @@ -37,6 +37,7 @@ struct ClusterInfo private: // Allow AttributePathParams access these constants. friend struct AttributePathParams; + friend struct ConcreteEventPath; // The ClusterId, AttributeId and EventId are MEIs, // 0xFFFF is not a valid manufacturer code, thus 0xFFFF'FFFF is not a valid MEI @@ -66,8 +67,6 @@ struct ClusterInfo */ bool IsValidAttributePath() const { return HasWildcardListIndex() || !HasWildcardAttributeId(); } - bool IsValidEventPath() const { return !HasWildcardEventId(); } - inline bool HasWildcardNodeId() const { return mNodeId == kUndefinedNodeId; } inline bool HasWildcardEndpointId() const { return mEndpointId == kInvalidEndpointId; } inline bool HasWildcardClusterId() const { return mClusterId == kInvalidClusterId; } diff --git a/src/app/ConcreteEventPath.h b/src/app/ConcreteEventPath.h index 819f4928927222..525aaef2e82450 100644 --- a/src/app/ConcreteEventPath.h +++ b/src/app/ConcreteEventPath.h @@ -18,6 +18,7 @@ #pragma once +#include #include namespace chip { @@ -50,6 +51,10 @@ struct ConcreteEventPath return mEndpointId == other.mEndpointId && mClusterId == other.mClusterId && mEventId == other.mEventId; } + bool IsValidEventPath() const { return !HasWildcardEventId(); } + + inline bool HasWildcardEventId() const { return mEventId == ClusterInfo::kInvalidEventId; } + EndpointId mEndpointId = 0; ClusterId mClusterId = 0; EventId mEventId = 0; diff --git a/src/app/DeviceControllerInteractionModelDelegate.h b/src/app/DeviceControllerInteractionModelDelegate.h index c8ad90d5afe45b..95106c0d0f5855 100644 --- a/src/app/DeviceControllerInteractionModelDelegate.h +++ b/src/app/DeviceControllerInteractionModelDelegate.h @@ -69,7 +69,7 @@ class DeviceControllerInteractionModelDelegate : public chip::app::ReadClient::C void OnDone(app::WriteClient * apWriteClient) override {} void OnEventData(const app::ReadClient * apReadClient, const app::EventHeader & aEventHeader, TLV::TLVReader * apData, - const app::StatusIB & aStatus) override + const app::StatusIB * apStatus) override {} void OnAttributeData(const app::ReadClient * apReadClient, const app::ConcreteAttributePath & aPath, TLV::TLVReader * apData, diff --git a/src/app/EventHeader.h b/src/app/EventHeader.h index a51610c0175cc0..c3ba210bfbfa5d 100644 --- a/src/app/EventHeader.h +++ b/src/app/EventHeader.h @@ -30,7 +30,6 @@ struct EventHeader EventNumber mEventNumber = 0; PriorityLevel mPriorityLevel = PriorityLevel::Invalid; Timestamp mTimestamp; - Timestamp mDeltaTimestamp; }; } // namespace app } // namespace chip diff --git a/src/app/EventLoggingTypes.h b/src/app/EventLoggingTypes.h index b4ef727e06d678..3871fb70402f5d 100644 --- a/src/app/EventLoggingTypes.h +++ b/src/app/EventLoggingTypes.h @@ -129,6 +129,10 @@ struct Timestamp return timestamp; } + bool IsValid() { return mType != Type::kInvalid; } + bool IsSystem() { return mType == Type::kSystem; } + bool IsEpoch() { return mType == Type::kEpoch; } + Type mType = Type::kInvalid; uint64_t mValue = 0; }; diff --git a/src/app/EventManagement.cpp b/src/app/EventManagement.cpp index 67d82329c0eb02..6ca331deefd2a1 100644 --- a/src/app/EventManagement.cpp +++ b/src/app/EventManagement.cpp @@ -304,9 +304,6 @@ CHIP_ERROR EventManagement::ConstructEvent(EventLoadOutContext * apContext, Even EventReportIB::Builder eventReportBuilder; EventDataIB::Builder eventDataIBBuilder; EventPathIB::Builder eventPathBuilder; - EventStatusIB::Builder eventStatusIBBuilder; - StatusIB::Builder statusIBBuilder; - StatusIB status; uint64_t deltatime = 0; VerifyOrExit(apContext->mCurrentEventNumber >= apContext->mStartingEventNumber, @@ -318,26 +315,6 @@ CHIP_ERROR EventManagement::ConstructEvent(EventLoadOutContext * apContext, Even eventReportBuilder.Init(&(apContext->mWriter)); // TODO: Update IsUrgent, issue 11386 // TODO: Update statusIB, issue 11388 - eventStatusIBBuilder = eventReportBuilder.CreateEventStatus(); - eventPathBuilder = eventStatusIBBuilder.CreatePath(); - err = eventStatusIBBuilder.GetError(); - SuccessOrExit(err); - eventPathBuilder.Node(apOptions->mpEventSchema->mNodeId) - .Endpoint(apOptions->mpEventSchema->mEndpointId) - .Cluster(apOptions->mpEventSchema->mClusterId) - .Event(apOptions->mpEventSchema->mEventId) - .IsUrgent(false) - .EndOfEventPathIB(); - err = eventPathBuilder.GetError(); - SuccessOrExit(err); - statusIBBuilder = eventStatusIBBuilder.CreateErrorStatus(); - err = eventStatusIBBuilder.GetError(); - SuccessOrExit(err); - statusIBBuilder.EncodeStatusIB(status); - eventStatusIBBuilder.EndOfEventStatusIB(); - err = statusIBBuilder.GetError(); - SuccessOrExit(err); - eventDataIBBuilder = eventReportBuilder.CreateEventData(); eventPathBuilder = eventDataIBBuilder.CreatePath(); err = eventDataIBBuilder.GetError(); @@ -621,23 +598,6 @@ CHIP_ERROR EventManagement::CopyEvent(const TLVReader & aReader, TLVWriter & aWr ReturnErrorOnFailure(reader.EnterContainer(containerType)); ReturnErrorOnFailure(aWriter.StartContainer(AnonymousTag, kTLVType_Structure, containerType)); - ReturnErrorOnFailure(reader.Next()); - ReturnErrorOnFailure(reader.EnterContainer(containerType1)); - ReturnErrorOnFailure(aWriter.StartContainer(TLV::ContextTag(to_underlying(EventReportIB::Tag::kEventStatus)), - kTLVType_Structure, containerType1)); - ReturnErrorOnFailure(reader.Next()); - do - { - ReturnErrorOnFailure(aWriter.CopyElement(reader)); - } while (CHIP_NO_ERROR == (err = reader.Next())); - if (err == CHIP_END_OF_TLV) - { - err = CHIP_NO_ERROR; - } - ReturnErrorOnFailure(err); - ReturnErrorOnFailure(reader.ExitContainer(containerType1)); - ReturnErrorOnFailure(aWriter.EndContainer(containerType1)); - ReturnErrorOnFailure(reader.Next()); ReturnErrorOnFailure(reader.EnterContainer(containerType1)); ReturnErrorOnFailure( @@ -685,8 +645,7 @@ CHIP_ERROR EventManagement::EventIterator(const TLVReader & aReader, size_t aDep innerReader.Init(aReader); ReturnErrorOnFailure(innerReader.EnterContainer(tlvType)); ReturnErrorOnFailure(innerReader.Next()); - // Skip EventStatus Element - ReturnErrorOnFailure(innerReader.Next()); + ReturnErrorOnFailure(innerReader.EnterContainer(tlvType1)); err = TLV::Utilities::Iterate(innerReader, FetchEventParameters, &event, false /*recurse*/); if (event.mFieldsToRead != kRequiredEventField) @@ -836,8 +795,7 @@ CHIP_ERROR EventManagement::EvictEvent(CHIPCircularTLVBuffer & apBuffer, void * TLVType containerType1; ReturnErrorOnFailure(aReader.EnterContainer(containerType)); ReturnErrorOnFailure(aReader.Next()); - // Skip EventStatus - ReturnErrorOnFailure(aReader.Next()); + ReturnErrorOnFailure(aReader.EnterContainer(containerType1)); EventEnvelopeContext context; constexpr bool recurse = false; diff --git a/src/app/MessageDef/EventDataIB.cpp b/src/app/MessageDef/EventDataIB.cpp index 108bf25bbbb46a..e142dd60771126 100644 --- a/src/app/MessageDef/EventDataIB.cpp +++ b/src/app/MessageDef/EventDataIB.cpp @@ -380,6 +380,90 @@ CHIP_ERROR EventDataIB::Parser::GetData(TLV::TLVReader * const apReader) const return mReader.FindElementWithTag(TLV::ContextTag(to_underlying(Tag::kData)), *apReader); } +CHIP_ERROR EventDataIB::Parser::ProcessEventPath(EventPathIB::Parser & aEventPath, ConcreteEventPath & aConcreteEventPath) +{ + // The ReportData must contain a concrete event path + CHIP_ERROR err = aEventPath.GetEndpoint(&(aConcreteEventPath.mEndpointId)); + VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_ERROR_IM_MALFORMED_EVENT_PATH); + + err = aEventPath.GetCluster(&(aConcreteEventPath.mClusterId)); + VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_ERROR_IM_MALFORMED_EVENT_PATH); + + err = aEventPath.GetEvent(&(aConcreteEventPath.mEventId)); + VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_ERROR_IM_MALFORMED_EVENT_PATH); + + VerifyOrReturnError(aConcreteEventPath.IsValidEventPath(), CHIP_ERROR_IM_MALFORMED_EVENT_PATH); + return CHIP_NO_ERROR; +} + +CHIP_ERROR EventDataIB::Parser::ProcessEventTimestamp(EventHeader & aEventHeader) +{ + CHIP_ERROR err = CHIP_NO_ERROR; + uint64_t timeStampVal = 0; + err = GetDeltaSystemTimestamp(&timeStampVal); + if (err == CHIP_END_OF_TLV) + { + err = CHIP_NO_ERROR; + } + else if (err == CHIP_NO_ERROR) + { + VerifyOrReturnError(aEventHeader.mTimestamp.IsSystem(), CHIP_ERROR_IM_MALFORMED_EVENT_DATA_ELEMENT); + aEventHeader.mTimestamp.mValue += timeStampVal; + } + ReturnErrorOnFailure(err); + + err = GetDeltaEpochTimestamp(&timeStampVal); + if (err == CHIP_END_OF_TLV) + { + err = CHIP_NO_ERROR; + } + else if (err == CHIP_NO_ERROR) + { + VerifyOrReturnError(aEventHeader.mTimestamp.IsEpoch(), CHIP_ERROR_IM_MALFORMED_EVENT_DATA_ELEMENT); + aEventHeader.mTimestamp.mValue += timeStampVal; + } + ReturnErrorOnFailure(err); + + err = GetSystemTimestamp(&timeStampVal); + if (err == CHIP_END_OF_TLV) + { + err = CHIP_NO_ERROR; + } + else if (err == CHIP_NO_ERROR) + { + VerifyOrReturnError(!aEventHeader.mTimestamp.IsValid(), CHIP_ERROR_IM_MALFORMED_EVENT_DATA_ELEMENT); + aEventHeader.mTimestamp.mType = Timestamp::Type::kSystem; + aEventHeader.mTimestamp.mValue = timeStampVal; + } + ReturnErrorOnFailure(err); + + err = GetEpochTimestamp(&timeStampVal); + if (err == CHIP_END_OF_TLV) + { + err = CHIP_NO_ERROR; + } + else if (err == CHIP_NO_ERROR) + { + VerifyOrReturnError(!aEventHeader.mTimestamp.IsValid(), CHIP_ERROR_IM_MALFORMED_EVENT_DATA_ELEMENT); + aEventHeader.mTimestamp.mType = Timestamp::Type::kEpoch; + aEventHeader.mTimestamp.mValue = timeStampVal; + } + return err; +} + +CHIP_ERROR EventDataIB::Parser::DecodeEventHeader(EventHeader & aEventHeader) +{ + uint8_t priorityLevel = 0; + EventPathIB::Parser path; + ReturnErrorOnFailure(GetPath(&path)); + ReturnErrorOnFailure(ProcessEventPath(path, aEventHeader.mPath)); + ReturnErrorOnFailure(GetEventNumber(&(aEventHeader.mEventNumber))); + ReturnErrorOnFailure(GetPriority(&priorityLevel)); + aEventHeader.mPriorityLevel = static_cast(priorityLevel); + ReturnErrorOnFailure(ProcessEventTimestamp(aEventHeader)); + return CHIP_NO_ERROR; +} + EventPathIB::Builder & EventDataIB::Builder::CreatePath() { // skip if error has already been set diff --git a/src/app/MessageDef/EventDataIB.h b/src/app/MessageDef/EventDataIB.h index 700e0da810ead1..0e0f662becbeab 100644 --- a/src/app/MessageDef/EventDataIB.h +++ b/src/app/MessageDef/EventDataIB.h @@ -27,6 +27,7 @@ #include "StructBuilder.h" #include "StructParser.h" #include +#include #include #include #include @@ -156,9 +157,14 @@ class Parser : public StructParser */ CHIP_ERROR GetData(TLV::TLVReader * const apReader) const; + CHIP_ERROR DecodeEventHeader(EventHeader & aEventHeader); + protected: // A recursively callable function to parse a data element and pretty-print it. CHIP_ERROR ParseData(TLV::TLVReader & aReader, int aDepth) const; + + CHIP_ERROR ProcessEventPath(EventPathIB::Parser & aEventPath, ConcreteEventPath & aConcreteEventPath); + CHIP_ERROR ProcessEventTimestamp(EventHeader & aEventHeader); }; class Builder : public StructBuilder diff --git a/src/app/MessageDef/EventReportIB.cpp b/src/app/MessageDef/EventReportIB.cpp index a64d3e1fbbf916..8dac9c789db944 100644 --- a/src/app/MessageDef/EventReportIB.cpp +++ b/src/app/MessageDef/EventReportIB.cpp @@ -84,9 +84,21 @@ CHIP_ERROR EventReportIB::Parser::CheckSchemaValidity() const if (CHIP_END_OF_TLV == err) { - const int RequiredFields = (1 << to_underlying(Tag::kEventData)) | (1 << to_underlying(Tag::kEventStatus)); + // check for at most field: + const int CheckDataField = 1 << to_underlying(Tag::kEventData); + const int CheckStatusField = (1 << to_underlying(Tag::kEventStatus)); - if ((TagPresenceMask & RequiredFields) == RequiredFields) + if ((TagPresenceMask & CheckDataField) == CheckDataField && (TagPresenceMask & CheckStatusField) == CheckStatusField) + { + // kEventData and kEventStatus both exist + err = CHIP_ERROR_IM_MALFORMED_EVENT_REPORT_IB; + } + else if ((TagPresenceMask & CheckDataField) != CheckDataField && (TagPresenceMask & CheckStatusField) != CheckStatusField) + { + // kEventData and kErrorStatus not exist + err = CHIP_ERROR_IM_MALFORMED_EVENT_REPORT_IB; + } + else { err = CHIP_NO_ERROR; } diff --git a/src/app/ReadClient.cpp b/src/app/ReadClient.cpp index 815c69fdedf529..aa4a99e598814e 100644 --- a/src/app/ReadClient.cpp +++ b/src/app/ReadClient.cpp @@ -471,29 +471,6 @@ CHIP_ERROR ReadClient::ProcessAttributePath(AttributePathIB::Parser & aAttribute return CHIP_NO_ERROR; } -CHIP_ERROR ReadClient::ProcessEventPath(EventPathIB::Parser & aEventPath, ClusterInfo & aClusterInfo) -{ - CHIP_ERROR err = aEventPath.GetNode(&(aClusterInfo.mNodeId)); - if (err == CHIP_END_OF_TLV) - { - err = CHIP_NO_ERROR; - } - VerifyOrReturnError(err == CHIP_NO_ERROR, err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH); - - // The ReportData must contain a concrete event path - err = aEventPath.GetEndpoint(&(aClusterInfo.mEndpointId)); - VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_ERROR_IM_MALFORMED_EVENT_PATH); - - err = aEventPath.GetCluster(&(aClusterInfo.mClusterId)); - VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_ERROR_IM_MALFORMED_EVENT_PATH); - - err = aEventPath.GetEvent(&(aClusterInfo.mEventId)); - VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_ERROR_IM_MALFORMED_EVENT_PATH); - - VerifyOrReturnError(aClusterInfo.IsValidEventPath(), CHIP_ERROR_IM_MALFORMED_EVENT_PATH); - return CHIP_NO_ERROR; -} - CHIP_ERROR ReadClient::ProcessAttributeReportIBs(TLV::TLVReader & aAttributeReportIBsReader) { CHIP_ERROR err = CHIP_NO_ERROR; @@ -541,72 +518,6 @@ CHIP_ERROR ReadClient::ProcessAttributeReportIBs(TLV::TLVReader & aAttributeRepo return err; } -CHIP_ERROR ReadClient::ProcessEventTimestamp(EventDataIB::Parser & aEventData, EventHeader & aEventHeader) -{ - CHIP_ERROR err = CHIP_NO_ERROR; - uint64_t timeStampVal = 0; - bool hasSystemTimestamp = false; - bool hasEpochTimestamp = false; - bool hasDeltaSystemTimestamp = false; - bool hasDeltaEpochTimestamp = false; - err = aEventData.GetDeltaSystemTimestamp(&timeStampVal); - if (err == CHIP_END_OF_TLV) - { - err = CHIP_NO_ERROR; - } - else if (err == CHIP_NO_ERROR) - { - aEventHeader.mDeltaTimestamp.mType = Timestamp::Type::kSystem; - aEventHeader.mDeltaTimestamp.mValue = timeStampVal; - hasDeltaSystemTimestamp = true; - } - - err = aEventData.GetDeltaEpochTimestamp(&timeStampVal); - if (err == CHIP_END_OF_TLV) - { - err = CHIP_NO_ERROR; - } - else if (err == CHIP_NO_ERROR) - { - aEventHeader.mDeltaTimestamp.mType = Timestamp::Type::kEpoch; - aEventHeader.mDeltaTimestamp.mValue = timeStampVal; - hasDeltaEpochTimestamp = true; - } - - err = aEventData.GetSystemTimestamp(&timeStampVal); - if (err == CHIP_END_OF_TLV) - { - err = CHIP_NO_ERROR; - } - else if (err == CHIP_NO_ERROR) - { - aEventHeader.mTimestamp.mType = Timestamp::Type::kSystem; - aEventHeader.mTimestamp.mValue = timeStampVal; - hasSystemTimestamp = true; - } - - err = aEventData.GetEpochTimestamp(&timeStampVal); - if (err == CHIP_END_OF_TLV) - { - err = CHIP_NO_ERROR; - } - else if (err == CHIP_NO_ERROR) - { - aEventHeader.mDeltaTimestamp.mType = Timestamp::Type::kEpoch; - aEventHeader.mDeltaTimestamp.mValue = timeStampVal; - hasEpochTimestamp = true; - } - - if ((hasSystemTimestamp && !hasEpochTimestamp && !hasDeltaSystemTimestamp && !hasDeltaEpochTimestamp) || - (!hasSystemTimestamp && hasEpochTimestamp && !hasDeltaSystemTimestamp && !hasDeltaEpochTimestamp) || - (!hasSystemTimestamp && !hasEpochTimestamp && hasDeltaSystemTimestamp && !hasDeltaEpochTimestamp) || - (!hasSystemTimestamp && !hasEpochTimestamp && !hasDeltaSystemTimestamp && hasDeltaEpochTimestamp)) - { - return CHIP_NO_ERROR; - } - return CHIP_ERROR_IM_MALFORMED_EVENT_DATA_ELEMENT; -} - CHIP_ERROR ReadClient::ProcessEventReportIBs(TLV::TLVReader & aEventReportIBsReader) { CHIP_ERROR err = CHIP_NO_ERROR; @@ -615,35 +526,20 @@ CHIP_ERROR ReadClient::ProcessEventReportIBs(TLV::TLVReader & aEventReportIBsRea TLV::TLVReader dataReader; EventReportIB::Parser report; EventDataIB::Parser data; - EventStatusIB::Parser status; - EventPathIB::Parser path; - ClusterInfo clusterInfo; - StatusIB statusIB; EventHeader header; - uint8_t priorityLevel = 0; TLV::TLVReader reader = aEventReportIBsReader; ReturnErrorOnFailure(report.Init(reader)); - // EventStatus and EventData would coexist - err = report.GetEventStatus(&status); - ReturnErrorOnFailure(err); - StatusIB::Parser errorStatus; - ReturnErrorOnFailure(status.GetPath(&path)); - ReturnErrorOnFailure(ProcessEventPath(path, clusterInfo)); - ReturnErrorOnFailure(status.GetErrorStatus(&errorStatus)); - ReturnErrorOnFailure(errorStatus.DecodeStatusIB(statusIB)); ReturnErrorOnFailure(report.GetEventData(&data)); - ReturnErrorOnFailure(data.GetPath(&path)); - ReturnErrorOnFailure(ProcessEventPath(path, clusterInfo)); - header.mPath = ConcreteEventPath(clusterInfo.mEndpointId, clusterInfo.mClusterId, clusterInfo.mEventId); - ReturnErrorOnFailure(data.GetEventNumber(&(header.mEventNumber))); - ReturnErrorOnFailure(data.GetPriority(&priorityLevel)); - header.mPriorityLevel = static_cast(priorityLevel); - ReturnErrorOnFailure(ProcessEventTimestamp(data, header)); + + header.mTimestamp = mEventTimestamp; + ReturnErrorOnFailure(data.DecodeEventHeader(header)); + mEventTimestamp = header.mTimestamp; + ReturnErrorOnFailure(data.GetData(&dataReader)); - mpCallback->OnEventData(this, header, &dataReader, statusIB); + mpCallback->OnEventData(this, header, &dataReader, nullptr); } if (CHIP_END_OF_TLV == err) diff --git a/src/app/ReadClient.h b/src/app/ReadClient.h index 2fb5d8380d9d4e..8bafbb48892848 100644 --- a/src/app/ReadClient.h +++ b/src/app/ReadClient.h @@ -23,7 +23,6 @@ */ #pragma once - #include #include #include @@ -33,6 +32,7 @@ #include #include #include +#include #include #include #include @@ -62,23 +62,18 @@ class ReadClient : public Messaging::ExchangeDelegate virtual ~Callback() = default; /** - * OnResponse will be called when a report data response has been received and processed for the given path. - * * The ReadClient object MUST continue to exist after this call is completed. * - * This callback will be called when: - * - Receiving event data as response of Read interactions - * - Receiving event data as reports of subscriptions - * - Receiving event data as initial reports of subscriptions + * This callback will be called when receiving event data received in the Read and Subscribe interactions * * @param[in] apReadClient: The read client object that initiated the read or subscribe transaction. * @param[in] aEventHeader: The event header in report response. * @param[in] apData: The event data of the given path, will be a nullptr if status is not Success. - * @param[in] aStatus: Event-specific status, containing an InteractionModel::Status code as well as an optional + * @param[in] apStatus: Event-specific status, containing an InteractionModel::Status code as well as an optional * cluster-specific status code. */ virtual void OnEventData(const ReadClient * apReadClient, const EventHeader & aEventHeader, TLV::TLVReader * apData, - const StatusIB & aStatus) + const StatusIB * apStatus) {} /** @@ -262,12 +257,26 @@ class ReadClient : public Messaging::ExchangeDelegate void CancelLivenessCheckTimer(); void MoveToState(const ClientState aTargetState); CHIP_ERROR ProcessAttributePath(AttributePathIB::Parser & aAttributePath, ClusterInfo & aClusterInfo); - CHIP_ERROR ProcessEventPath(EventPathIB::Parser & aEventPath, ClusterInfo & aClusterInfo); - CHIP_ERROR ProcessEventTimestamp(EventDataIB::Parser & aEventData, EventHeader & aEventHeader); CHIP_ERROR ProcessReportData(System::PacketBufferHandle && aPayload); 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 before + * decoding 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. + */ + template + CHIP_ERROR CheckEventHeaderWithClusterObject(const EventHeader & aEventHeader, const EventDataT & aEvent) + { + VerifyOrReturnError((aEventHeader.mPath.mEventId == aEvent.GetEventId()) && + (aEventHeader.mPath.mClusterId == aEvent.GetClusterId()), + CHIP_ERROR_INVALID_ARGUMENT); + return CHIP_NO_ERROR; + } + /** * Internal shutdown method that we use when we know what's going on with * our exchange and don't need to manually close it. @@ -285,6 +294,7 @@ class ReadClient : public Messaging::ExchangeDelegate NodeId mPeerNodeId = kUndefinedNodeId; FabricIndex mFabricIndex = kUndefinedFabricIndex; InteractionType mInteractionType = InteractionType::Read; + Timestamp mEventTimestamp; }; }; // namespace app diff --git a/src/app/tests/TestEventLogging.cpp b/src/app/tests/TestEventLogging.cpp index 68bc9956c3a5fd..0511a183f56d1b 100644 --- a/src/app/tests/TestEventLogging.cpp +++ b/src/app/tests/TestEventLogging.cpp @@ -50,9 +50,9 @@ static const uint32_t kLivenessChangeEvent = 1; static const chip::EndpointId kTestEndpointId = 2; static const chip::TLV::Tag kLivenessDeviceStatus = chip::TLV::ContextTag(1); -static uint8_t gDebugEventBuffer[256]; -static uint8_t gInfoEventBuffer[256]; -static uint8_t gCritEventBuffer[256]; +static uint8_t gDebugEventBuffer[128]; +static uint8_t gInfoEventBuffer[128]; +static uint8_t gCritEventBuffer[128]; static chip::app::CircularEventBuffer gCircularEventBuffer[3]; class TestContext : public chip::Test::AppContext diff --git a/src/app/tests/TestMessageDef.cpp b/src/app/tests/TestMessageDef.cpp index 7c4d94e9c6b4b6..f5abebbfe6a193 100644 --- a/src/app/tests/TestMessageDef.cpp +++ b/src/app/tests/TestMessageDef.cpp @@ -432,10 +432,6 @@ void ParseEventStatusIB(nlTestSuite * apSuite, EventStatusIB::Parser & aEventSta void BuildEventReportIB(nlTestSuite * apSuite, EventReportIB::Builder & aEventReportIBBuilder) { - EventStatusIB::Builder eventStatusIBBuilder = aEventReportIBBuilder.CreateEventStatus(); - NL_TEST_ASSERT(apSuite, aEventReportIBBuilder.GetError() == CHIP_NO_ERROR); - BuildEventStatusIB(apSuite, eventStatusIBBuilder); - EventDataIB::Builder eventDataIBBuilder = aEventReportIBBuilder.CreateEventData(); NL_TEST_ASSERT(apSuite, aEventReportIBBuilder.GetError() == CHIP_NO_ERROR); BuildEventDataIB(apSuite, eventDataIBBuilder); @@ -454,8 +450,7 @@ void ParseEventReportIB(nlTestSuite * apSuite, EventReportIB::Parser & aEventRep err = aEventReportIBParser.CheckSchemaValidity(); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); #endif - err = aEventReportIBParser.GetEventStatus(&eventStatusParser); - NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + err = aEventReportIBParser.GetEventData(&eventDataParser); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); } diff --git a/src/app/tests/TestReadInteraction.cpp b/src/app/tests/TestReadInteraction.cpp index 1f6cbe00be3fd1..50b7a01fe25fb0 100644 --- a/src/app/tests/TestReadInteraction.cpp +++ b/src/app/tests/TestReadInteraction.cpp @@ -157,7 +157,7 @@ class MockInteractionModelApp : public chip::app::ReadClient::Callback, public c { public: void OnEventData(const chip::app::ReadClient * apReadClient, const chip::app::EventHeader & aEventHeader, - chip::TLV::TLVReader * apData, const chip::app::StatusIB & aStatus) override + chip::TLV::TLVReader * apData, const chip::app::StatusIB * apStatus) override { static int numDataElementIndex = 0; diff --git a/src/app/tests/integration/chip_im_initiator.cpp b/src/app/tests/integration/chip_im_initiator.cpp index 43f8f7ec021db6..24da2e89b28b7a 100644 --- a/src/app/tests/integration/chip_im_initiator.cpp +++ b/src/app/tests/integration/chip_im_initiator.cpp @@ -133,7 +133,7 @@ class MockInteractionModelApp : public chip::app::InteractionModelDelegate, { public: void OnEventData(const chip::app::ReadClient * apReadClient, const chip::app::EventHeader & aEventHeader, - chip::TLV::TLVReader * apData, const chip::app::StatusIB & aStatus) override + chip::TLV::TLVReader * apData, const chip::app::StatusIB * apStatus) override {} void OnSubscriptionEstablished(const chip::app::ReadClient * apReadClient) override { diff --git a/src/controller/TypedReadCallback.h b/src/controller/TypedReadCallback.h index 8cb758879be137..c4015d11335641 100644 --- a/src/controller/TypedReadCallback.h +++ b/src/controller/TypedReadCallback.h @@ -114,12 +114,10 @@ class TypedReadEventCallback final : public app::ReadClient::Callback private: void OnEventData(const app::ReadClient * apReadClient, const app::EventHeader & aEventHeader, TLV::TLVReader * apData, - const app::StatusIB & aStatus) override + const app::StatusIB * apStatus) override { CHIP_ERROR err = CHIP_NO_ERROR; DecodableEventTypeInfo value; - - VerifyOrExit(aStatus.mStatus == Protocols::InteractionModel::Status::Success, err = CHIP_ERROR_IM_STATUS_CODE_RECEIVED); VerifyOrExit(aEventHeader.mPath.mClusterId == DecodableEventTypeInfo::GetClusterId() && aEventHeader.mPath.mEventId == DecodableEventTypeInfo::GetEventId(), CHIP_ERROR_SCHEMA_MISMATCH); @@ -133,16 +131,7 @@ class TypedReadEventCallback final : public app::ReadClient::Callback exit: if (err != CHIP_NO_ERROR) { - // - // Override status to indicate an error if something bad happened above. - // - Protocols::InteractionModel::Status imStatus = aStatus.mStatus; - if (aStatus.mStatus == Protocols::InteractionModel::Status::Success) - { - imStatus = Protocols::InteractionModel::Status::Failure; - } - - mOnError(&aEventHeader, imStatus, err); + mOnError(&aEventHeader, Protocols::InteractionModel::Status::Failure, err); } } diff --git a/src/lib/core/CHIPError.cpp b/src/lib/core/CHIPError.cpp index edec0415c29220..31e62df963d625 100644 --- a/src/lib/core/CHIPError.cpp +++ b/src/lib/core/CHIPError.cpp @@ -668,6 +668,9 @@ bool FormatCHIPError(char * buf, uint16_t bufSize, CHIP_ERROR err) case CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_REPORT_MESSAGE.AsInteger(): desc = "Malformed Interaction Model Attribute Report MESSAGE"; break; + case CHIP_ERROR_IM_MALFORMED_EVENT_REPORT_IB.AsInteger(): + desc = "Malformed Interaction Model Event Report IB"; + break; } #endif // !CHIP_CONFIG_SHORT_ERROR_STR diff --git a/src/lib/core/CHIPError.h b/src/lib/core/CHIPError.h index f101c2478b68bb..51c5fcb1df1df0 100644 --- a/src/lib/core/CHIPError.h +++ b/src/lib/core/CHIPError.h @@ -2261,6 +2261,15 @@ using CHIP_ERROR = ::chip::ChipError; */ #define CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_REPORT_MESSAGE CHIP_CORE_ERROR(0xcf) +/** + * @def CHIP_ERROR_IM_MALFORMED_EVENT_REPORT_IB + * + * @brief + * The EventReport is malformed: it either does not contain + * the required elements + */ +#define CHIP_ERROR_IM_MALFORMED_EVENT_REPORT_IB CHIP_CORE_ERROR(0xd0) + /** * @} */