From 191960132e888f7a2bf23c002621d21614a361b7 Mon Sep 17 00:00:00 2001 From: yunhanw-google Date: Wed, 24 Nov 2021 20:58:19 -0800 Subject: [PATCH] Add initial event wildcard support (#12145) --- src/app/AttributePathExpandIterator.cpp | 2 +- src/app/AttributePathParams.cpp | 52 ------------- src/app/AttributePathParams.h | 5 +- src/app/BUILD.gn | 2 +- src/app/ClusterInfo.h | 17 ++++- src/app/ConcreteEventPath.h | 5 -- src/app/EventManagement.cpp | 10 +-- src/app/EventPathParams.h | 28 ++++--- src/app/MessageDef/AttributePathIB.cpp | 31 +++++++- src/app/MessageDef/AttributePathIB.h | 5 +- src/app/MessageDef/EventDataIB.cpp | 1 - src/app/MessageDef/EventPathIB.cpp | 29 +++++-- src/app/MessageDef/EventPathIB.h | 8 +- src/app/ReadClient.cpp | 21 ++---- src/app/ReadHandler.cpp | 43 +++++++---- src/app/WriteClient.cpp | 22 ++---- src/app/WriteClient.h | 1 - src/app/WriteHandler.cpp | 11 +-- src/app/WriteHandler.h | 2 - src/app/tests/TestClusterInfo.cpp | 75 ++++++++++++++++++- src/app/tests/TestEventLogging.cpp | 17 ++--- src/app/tests/TestEventPathParams.cpp | 24 ++---- src/app/tests/TestReadInteraction.cpp | 16 +--- .../tests/integration/chip_im_initiator.cpp | 5 -- src/app/util/mock/Constants.h | 4 + 25 files changed, 237 insertions(+), 199 deletions(-) delete mode 100644 src/app/AttributePathParams.cpp diff --git a/src/app/AttributePathExpandIterator.cpp b/src/app/AttributePathExpandIterator.cpp index bc03aecb7bc4a5..505997e57d7133 100644 --- a/src/app/AttributePathExpandIterator.cpp +++ b/src/app/AttributePathExpandIterator.cpp @@ -120,7 +120,7 @@ bool AttributePathExpandIterator::Next() if (mEndpointIndex == UINT16_MAX) { // Special case: If this is a concrete path, we just return its value as-is. - if (!mpClusterInfo->HasWildcard()) + if (!mpClusterInfo->HasAttributeWildcard()) { mOutputPath.mEndpointId = mpClusterInfo->mEndpointId; mOutputPath.mClusterId = mpClusterInfo->mClusterId; diff --git a/src/app/AttributePathParams.cpp b/src/app/AttributePathParams.cpp deleted file mode 100644 index 01a4311b9544c1..00000000000000 --- a/src/app/AttributePathParams.cpp +++ /dev/null @@ -1,52 +0,0 @@ -/* - * - * 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. - */ - -#include -#include - -namespace chip { -namespace app { - -CHIP_ERROR AttributePathParams::BuildAttributePath(AttributePathIB::Builder & aBuilder) const -{ - if (!HasWildcardEndpointId()) - { - aBuilder.Endpoint(mEndpointId); - } - - if (!HasWildcardClusterId()) - { - aBuilder.Cluster(mClusterId); - } - - if (!HasWildcardAttributeId()) - { - aBuilder.Attribute(mAttributeId); - } - - if (!HasWildcardListIndex()) - { - aBuilder.ListIndex(mListIndex); - } - - aBuilder.EndOfAttributePathIB(); - - return aBuilder.GetError(); -} -} // namespace app -} // namespace chip diff --git a/src/app/AttributePathParams.h b/src/app/AttributePathParams.h index a583d6c04051a5..173f9e80b6bb65 100644 --- a/src/app/AttributePathParams.h +++ b/src/app/AttributePathParams.h @@ -21,7 +21,6 @@ #include #include -#include namespace chip { namespace app { @@ -51,9 +50,7 @@ struct AttributePathParams AttributePathParams() {} - CHIP_ERROR BuildAttributePath(AttributePathIB::Builder & aBuilder) const; - - bool HasWildcard() const { return HasWildcardEndpointId() || HasWildcardClusterId() || HasWildcardAttributeId(); } + bool HasAttributeWildcard() const { return HasWildcardEndpointId() || HasWildcardClusterId() || HasWildcardAttributeId(); } /** * SPEC 8.9.2.2 diff --git a/src/app/BUILD.gn b/src/app/BUILD.gn index 342f4aa2e02337..64d255211680f4 100644 --- a/src/app/BUILD.gn +++ b/src/app/BUILD.gn @@ -37,7 +37,6 @@ static_library("app") { sources = [ "AttributePathExpandIterator.cpp", "AttributePathExpandIterator.h", - "AttributePathParams.cpp", "AttributePathParams.h", "BufferedReadCallback.cpp", "CASESessionManager.cpp", @@ -49,6 +48,7 @@ static_library("app") { "DeviceProxy.cpp", "DeviceProxy.h", "EventManagement.cpp", + "EventPathParams.h", "InteractionModelEngine.cpp", "MessageDef/ArrayBuilder.cpp", "MessageDef/ArrayParser.cpp", diff --git a/src/app/ClusterInfo.h b/src/app/ClusterInfo.h index 9a282dd1e621b9..e6f8e4031eb1dd 100644 --- a/src/app/ClusterInfo.h +++ b/src/app/ClusterInfo.h @@ -19,6 +19,7 @@ #pragma once #include +#include #include #include #include @@ -38,7 +39,7 @@ struct ClusterInfo private: // Allow AttributePathParams access these constants. friend struct AttributePathParams; - friend struct ConcreteEventPath; + friend struct EventPathParams; // The ClusterId, AttributeId and EventId are MEIs, // 0xFFFF is not a valid manufacturer code, thus 0xFFFF'FFFF is not a valid MEI @@ -68,8 +69,17 @@ struct ClusterInfo return true; } - bool HasWildcard() const { return HasWildcardEndpointId() || HasWildcardClusterId() || HasWildcardAttributeId(); } + bool IsEventPathSupersetOf(const ConcreteEventPath & other) const + { + VerifyOrReturnError(HasWildcardEndpointId() || mEndpointId == other.mEndpointId, false); + VerifyOrReturnError(HasWildcardClusterId() || mClusterId == other.mClusterId, false); + VerifyOrReturnError(HasWildcardEventId() || mEventId == other.mEventId, false); + return true; + } + + bool HasAttributeWildcard() const { return HasWildcardEndpointId() || HasWildcardClusterId() || HasWildcardAttributeId(); } + bool HasEventWildcard() const { return HasWildcardEndpointId() || HasWildcardClusterId() || HasWildcardEventId(); } /** * Check that the path meets some basic constraints of an attribute path: If list index is not wildcard, then field id must not * be wildcard. This does not verify that the attribute being targeted is actually of list type when the list index is not @@ -77,6 +87,9 @@ struct ClusterInfo */ bool IsValidAttributePath() const { return HasWildcardListIndex() || !HasWildcardAttributeId(); } + // For event, an event id can only be interpreted if the cluster id is known. + bool IsValidEventPath() const { return !(HasWildcardClusterId() && !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 525aaef2e82450..819f4928927222 100644 --- a/src/app/ConcreteEventPath.h +++ b/src/app/ConcreteEventPath.h @@ -18,7 +18,6 @@ #pragma once -#include #include namespace chip { @@ -51,10 +50,6 @@ 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/EventManagement.cpp b/src/app/EventManagement.cpp index 1657b839068e48..e1424dc58606b5 100644 --- a/src/app/EventManagement.cpp +++ b/src/app/EventManagement.cpp @@ -610,20 +610,18 @@ CHIP_ERROR EventManagement::CopyEvent(const TLVReader & aReader, TLVWriter & aWr static bool IsInterestedEventPaths(EventLoadOutContext * eventLoadOutContext, const EventEnvelopeContext & event) { - ClusterInfo * interestedEventPaths = eventLoadOutContext->mpInterestedEventPaths; if (eventLoadOutContext->mCurrentEventNumber < eventLoadOutContext->mStartingEventNumber) { return false; } - while (interestedEventPaths != nullptr) + ConcreteEventPath path(event.mEndpointId, event.mClusterId, event.mEventId); + for (auto * interestedPath = eventLoadOutContext->mpInterestedEventPaths; interestedPath != nullptr; + interestedPath = interestedPath->mpNext) { - // TODO: Support wildcard event path - if (interestedEventPaths->mNodeId == event.mNodeId && interestedEventPaths->mEndpointId == event.mEndpointId && - interestedEventPaths->mClusterId == event.mClusterId && interestedEventPaths->mEventId == event.mEventId) + if (interestedPath->IsEventPathSupersetOf(path)) { return true; } - interestedEventPaths = interestedEventPaths->mpNext; } return false; } diff --git a/src/app/EventPathParams.h b/src/app/EventPathParams.h index 3efeca59efd57c..32420970b40e30 100644 --- a/src/app/EventPathParams.h +++ b/src/app/EventPathParams.h @@ -20,27 +20,33 @@ #include +#include + namespace chip { namespace app { struct EventPathParams { - EventPathParams(NodeId aNodeId, EndpointId aEndpointId, ClusterId aClusterId, EventId aEventId, bool aIsUrgent) : - mNodeId(aNodeId), mEndpointId(aEndpointId), mClusterId(aClusterId), mEventId(aEventId), mIsUrgent(aIsUrgent) - {} EventPathParams(EndpointId aEndpointId, ClusterId aClusterId, EventId aEventId) : - EventPathParams(0, aEndpointId, aClusterId, aEventId, false) + mEndpointId(aEndpointId), mClusterId(aClusterId), mEventId(aEventId) {} EventPathParams() {} bool IsSamePath(const EventPathParams & other) const { - return other.mNodeId == mNodeId && other.mEndpointId == mEndpointId && other.mClusterId == mClusterId && - other.mEventId == mEventId; + return other.mEndpointId == mEndpointId && other.mClusterId == mClusterId && other.mEventId == mEventId; } - NodeId mNodeId = 0; - EndpointId mEndpointId = 0; - ClusterId mClusterId = 0; - EventId mEventId = 0; - bool mIsUrgent = false; + + bool HasEventWildcard() const { return HasWildcardEndpointId() || HasWildcardClusterId() || HasWildcardEventId(); } + + // For event, an event id can only be interpreted if the cluster id is known. + bool IsValidEventPath() const { return !(HasWildcardClusterId() && !HasWildcardEventId()); } + + inline bool HasWildcardEndpointId() const { return mEndpointId == kInvalidEndpointId; } + inline bool HasWildcardClusterId() const { return mClusterId == ClusterInfo::kInvalidClusterId; } + inline bool HasWildcardEventId() const { return mEventId == ClusterInfo::kInvalidEventId; } + + EndpointId mEndpointId = kInvalidEndpointId; + ClusterId mClusterId = ClusterInfo::kInvalidClusterId; + EventId mEventId = ClusterInfo::kInvalidEventId; }; } // namespace app } // namespace chip diff --git a/src/app/MessageDef/AttributePathIB.cpp b/src/app/MessageDef/AttributePathIB.cpp index aa49fd468282e0..f296866f672203 100644 --- a/src/app/MessageDef/AttributePathIB.cpp +++ b/src/app/MessageDef/AttributePathIB.cpp @@ -250,5 +250,32 @@ AttributePathIB::Builder & AttributePathIB::Builder::EndOfAttributePathIB() EndOfContainer(); return *this; } -}; // namespace app -}; // namespace chip + +CHIP_ERROR AttributePathIB::Builder::Encode(const AttributePathParams & aAttributePathParams) +{ + if (!(aAttributePathParams.HasWildcardEndpointId())) + { + Endpoint(aAttributePathParams.mEndpointId); + } + + if (!(aAttributePathParams.HasWildcardClusterId())) + { + Cluster(aAttributePathParams.mClusterId); + } + + if (!(aAttributePathParams.HasWildcardAttributeId())) + { + Attribute(aAttributePathParams.mAttributeId); + } + + if (!(aAttributePathParams.HasWildcardListIndex())) + { + ListIndex(aAttributePathParams.mListIndex); + } + + EndOfAttributePathIB(); + return GetError(); +} + +} // namespace app +} // namespace chip diff --git a/src/app/MessageDef/AttributePathIB.h b/src/app/MessageDef/AttributePathIB.h index c07b1026126f6b..27db4cd2353550 100644 --- a/src/app/MessageDef/AttributePathIB.h +++ b/src/app/MessageDef/AttributePathIB.h @@ -22,6 +22,7 @@ #include "ListParser.h" #include +#include #include #include #include @@ -192,7 +193,9 @@ class Builder : public ListBuilder * @return A reference to *this */ AttributePathIB::Builder & EndOfAttributePathIB(); + + CHIP_ERROR Encode(const AttributePathParams & aAttributePathParams); }; -}; // namespace AttributePathIB +} // namespace AttributePathIB } // namespace app } // namespace chip diff --git a/src/app/MessageDef/EventDataIB.cpp b/src/app/MessageDef/EventDataIB.cpp index 9d7931450865d9..11020576149ae2 100644 --- a/src/app/MessageDef/EventDataIB.cpp +++ b/src/app/MessageDef/EventDataIB.cpp @@ -392,7 +392,6 @@ CHIP_ERROR EventDataIB::Parser::ProcessEventPath(EventPathIB::Parser & aEventPat 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; } diff --git a/src/app/MessageDef/EventPathIB.cpp b/src/app/MessageDef/EventPathIB.cpp index 12d35064206d2e..74668e320efedb 100644 --- a/src/app/MessageDef/EventPathIB.cpp +++ b/src/app/MessageDef/EventPathIB.cpp @@ -15,11 +15,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -/** - * @file - * This file defines EventPath parser and builder in CHIP interaction model - * - */ #include "EventPathIB.h" @@ -222,5 +217,25 @@ EventPathIB::Builder & EventPathIB::Builder::EndOfEventPathIB() return *this; } -}; // namespace app -}; // namespace chip +CHIP_ERROR EventPathIB::Builder::Encode(const EventPathParams & aEventPathParams) +{ + if (!(aEventPathParams.HasWildcardEndpointId())) + { + Endpoint(aEventPathParams.mEndpointId); + } + + if (!(aEventPathParams.HasWildcardClusterId())) + { + Cluster(aEventPathParams.mClusterId); + } + + if (!(aEventPathParams.HasWildcardEventId())) + { + Event(aEventPathParams.mEventId); + } + + EndOfEventPathIB(); + return GetError(); +} +} // namespace app +} // namespace chip diff --git a/src/app/MessageDef/EventPathIB.h b/src/app/MessageDef/EventPathIB.h index 25c6893c58c190..ee914f162e78c4 100644 --- a/src/app/MessageDef/EventPathIB.h +++ b/src/app/MessageDef/EventPathIB.h @@ -15,11 +15,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -/** - * @file - * This file defines EventPath parser and builder in CHIP interaction model - * - */ #pragma once @@ -27,6 +22,7 @@ #include "ListParser.h" #include +#include #include #include #include @@ -175,6 +171,8 @@ class Builder : public ListBuilder * @return A reference to *this */ EventPathIB::Builder & EndOfEventPathIB(); + + CHIP_ERROR Encode(const EventPathParams & aEventPathParams); }; } // namespace EventPathIB } // namespace app diff --git a/src/app/ReadClient.cpp b/src/app/ReadClient.cpp index 4073e5721e6529..fdc783b058d2a2 100644 --- a/src/app/ReadClient.cpp +++ b/src/app/ReadClient.cpp @@ -200,25 +200,14 @@ CHIP_ERROR ReadClient::SendReadRequest(ReadPrepareParams & aReadPrepareParams) CHIP_ERROR ReadClient::GenerateEventPaths(EventPaths::Builder & aEventPathsBuilder, EventPathParams * apEventPathParamsList, size_t aEventPathParamsListSize) { - CHIP_ERROR err = CHIP_NO_ERROR; - - for (size_t eventIndex = 0; eventIndex < aEventPathParamsListSize; ++eventIndex) + for (size_t index = 0; index < aEventPathParamsListSize; ++index) { - EventPathIB::Builder eventPathBuilder = aEventPathsBuilder.CreatePath(); - EventPathParams eventPath = apEventPathParamsList[eventIndex]; - eventPathBuilder.Node(eventPath.mNodeId) - .Event(eventPath.mEventId) - .Endpoint(eventPath.mEndpointId) - .Cluster(eventPath.mClusterId) - .EndOfEventPathIB(); - SuccessOrExit(err = eventPathBuilder.GetError()); + VerifyOrReturnError(apEventPathParamsList[index].IsValidEventPath(), CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH); + ReturnErrorOnFailure(aEventPathsBuilder.CreatePath().Encode(apEventPathParamsList[index])); } aEventPathsBuilder.EndOfEventPaths(); - SuccessOrExit(err = aEventPathsBuilder.GetError()); - -exit: - return err; + return aEventPathsBuilder.GetError(); } CHIP_ERROR ReadClient::GenerateAttributePathList(AttributePathIBs::Builder & aAttributePathIBsBuilder, @@ -228,7 +217,7 @@ CHIP_ERROR ReadClient::GenerateAttributePathList(AttributePathIBs::Builder & aAt for (size_t index = 0; index < aAttributePathParamsListSize; index++) { VerifyOrReturnError(apAttributePathParamsList[index].IsValidAttributePath(), CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH); - ReturnErrorOnFailure(apAttributePathParamsList[index].BuildAttributePath(aAttributePathIBsBuilder.CreateAttributePath())); + ReturnErrorOnFailure(aAttributePathIBsBuilder.CreateAttributePath().Encode(apAttributePathParamsList[index])); } aAttributePathIBsBuilder.EndOfAttributePathIBs(); return aAttributePathIBsBuilder.GetError(); diff --git a/src/app/ReadHandler.cpp b/src/app/ReadHandler.cpp index 5e3ce4ae9f2f78..380cdc9ed582bb 100644 --- a/src/app/ReadHandler.cpp +++ b/src/app/ReadHandler.cpp @@ -417,26 +417,45 @@ CHIP_ERROR ReadHandler::ProcessEventPaths(EventPaths::Parser & aEventPathsParser while (CHIP_NO_ERROR == (err = reader.Next())) { - VerifyOrExit(TLV::AnonymousTag == reader.GetTag(), err = CHIP_ERROR_INVALID_TLV_TAG); - VerifyOrExit(TLV::kTLVType_List == reader.GetType(), err = CHIP_ERROR_WRONG_TLV_TYPE); + VerifyOrReturnError(TLV::AnonymousTag == reader.GetTag(), CHIP_ERROR_INVALID_TLV_TAG); ClusterInfo clusterInfo; EventPathIB::Parser path; - err = path.Init(reader); - SuccessOrExit(err); - err = path.GetNode(&(clusterInfo.mNodeId)); - SuccessOrExit(err); + ReturnErrorOnFailure(path.Init(reader)); + err = path.GetEndpoint(&(clusterInfo.mEndpointId)); - SuccessOrExit(err); + if (err == CHIP_NO_ERROR) + { + VerifyOrReturnError(!clusterInfo.HasWildcardEndpointId(), err = CHIP_ERROR_IM_MALFORMED_EVENT_PATH); + } + else if (err == CHIP_END_OF_TLV) + { + err = CHIP_NO_ERROR; + } + ReturnErrorOnFailure(err); + err = path.GetCluster(&(clusterInfo.mClusterId)); - SuccessOrExit(err); + if (err == CHIP_NO_ERROR) + { + VerifyOrReturnError(!clusterInfo.HasWildcardClusterId(), err = CHIP_ERROR_IM_MALFORMED_EVENT_PATH); + } + else if (err == CHIP_END_OF_TLV) + { + err = CHIP_NO_ERROR; + } + ReturnErrorOnFailure(err); + err = path.GetEvent(&(clusterInfo.mEventId)); if (CHIP_END_OF_TLV == err) { err = CHIP_NO_ERROR; } - SuccessOrExit(err); - err = InteractionModelEngine::GetInstance()->PushFront(mpEventClusterInfoList, clusterInfo); - SuccessOrExit(err); + else if (err == CHIP_NO_ERROR) + { + VerifyOrReturnError(!clusterInfo.HasWildcardEventId(), err = CHIP_ERROR_IM_MALFORMED_EVENT_PATH); + } + ReturnErrorOnFailure(err); + + ReturnErrorOnFailure(InteractionModelEngine::GetInstance()->PushFront(mpEventClusterInfoList, clusterInfo)); } // if we have exhausted this container @@ -444,8 +463,6 @@ CHIP_ERROR ReadHandler::ProcessEventPaths(EventPaths::Parser & aEventPathsParser { err = CHIP_NO_ERROR; } - -exit: return err; } diff --git a/src/app/WriteClient.cpp b/src/app/WriteClient.cpp index 13d46d2665990c..2419a9cc0b8cf1 100644 --- a/src/app/WriteClient.cpp +++ b/src/app/WriteClient.cpp @@ -136,14 +136,11 @@ CHIP_ERROR WriteClient::ProcessWriteResponseMessage(System::PacketBufferHandle & CHIP_ERROR WriteClient::PrepareAttribute(const AttributePathParams & attributePathParams) { - CHIP_ERROR err = CHIP_NO_ERROR; - - AttributeDataIB::Builder AttributeDataIB = mWriteRequestBuilder.GetWriteRequests().CreateAttributeDataIBBuilder(); - SuccessOrExit(AttributeDataIB.GetError()); - err = ConstructAttributePath(attributePathParams, AttributeDataIB); - -exit: - return err; + VerifyOrReturnError(attributePathParams.IsValidAttributePath(), CHIP_ERROR_INVALID_PATH_LIST); + AttributeDataIB::Builder attributeDataIB = mWriteRequestBuilder.GetWriteRequests().CreateAttributeDataIBBuilder(); + ReturnErrorOnFailure(attributeDataIB.GetError()); + ReturnErrorOnFailure(attributeDataIB.CreatePath().Encode(attributePathParams)); + return CHIP_NO_ERROR; } CHIP_ERROR WriteClient::FinishAttribute() @@ -167,13 +164,6 @@ TLV::TLVWriter * WriteClient::GetAttributeDataIBTLVWriter() return mWriteRequestBuilder.GetWriteRequests().GetAttributeDataIBBuilder().GetWriter(); } -CHIP_ERROR WriteClient::ConstructAttributePath(const AttributePathParams & aAttributePathParams, - AttributeDataIB::Builder aAttributeDataIB) -{ - VerifyOrReturnError(aAttributePathParams.IsValidAttributePath(), CHIP_ERROR_INVALID_PATH_LIST); - return aAttributePathParams.BuildAttributePath(aAttributeDataIB.CreatePath()); -} - CHIP_ERROR WriteClient::FinalizeMessage(System::PacketBufferHandle & aPacket) { CHIP_ERROR err = CHIP_NO_ERROR; @@ -344,7 +334,7 @@ CHIP_ERROR WriteClient::ProcessAttributeStatusIB(AttributeStatusIB::Parser & aAt } // TODO: (#11423) Attribute paths has a pattern of invalid paths, should add a function for checking invalid paths here. // NOTE: We don't support wildcard write for now, reject all wildcard paths. - VerifyOrExit(!attributePathParams.HasWildcard() && attributePathParams.IsValidAttributePath(), + VerifyOrExit(!attributePathParams.HasAttributeWildcard() && attributePathParams.IsValidAttributePath(), err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH); err = aAttributeStatusIB.GetErrorStatus(&(StatusIBParser)); diff --git a/src/app/WriteClient.h b/src/app/WriteClient.h index cb7b1562873608..f470b7d7dbdac5 100644 --- a/src/app/WriteClient.h +++ b/src/app/WriteClient.h @@ -174,7 +174,6 @@ class WriteClient : public Messaging::ExchangeDelegate void MoveToState(const State aTargetState); CHIP_ERROR ProcessWriteResponseMessage(System::PacketBufferHandle && payload); CHIP_ERROR ProcessAttributeStatusIB(AttributeStatusIB::Parser & aAttributeStatusIB); - CHIP_ERROR ConstructAttributePath(const AttributePathParams & aAttributePathParams, AttributeDataIB::Builder aAttributeDataIB); void ClearExistingExchangeContext(); const char * GetStateStr() const; void ClearState(); diff --git a/src/app/WriteHandler.cpp b/src/app/WriteHandler.cpp index 7f55a8e16149b7..2cd1bcd927deea 100644 --- a/src/app/WriteHandler.cpp +++ b/src/app/WriteHandler.cpp @@ -165,7 +165,7 @@ CHIP_ERROR WriteHandler::ProcessAttributeDataIBs(TLV::TLVReader & aAttributeData } // We do not support Wildcard writes for now, reject all wildcard write requests. - VerifyOrExit(clusterInfo.IsValidAttributePath() && !clusterInfo.HasWildcard(), + VerifyOrExit(clusterInfo.IsValidAttributePath() && !clusterInfo.HasAttributeWildcard(), err = CHIP_ERROR_IM_MALFORMED_ATTRIBUTE_PATH); err = element.GetData(&dataReader); @@ -235,13 +235,6 @@ CHIP_ERROR WriteHandler::ProcessWriteRequest(System::PacketBufferHandle && aPayl return err; } -CHIP_ERROR WriteHandler::ConstructAttributePath(const AttributePathParams & aAttributePathParams, - AttributeStatusIB::Builder aAttributeStatusIB) -{ - AttributePathIB::Builder attributePath = aAttributeStatusIB.CreatePath(); - return aAttributePathParams.BuildAttributePath(attributePath); -} - CHIP_ERROR WriteHandler::AddStatus(const AttributePathParams & aAttributePathParams, const Protocols::InteractionModel::Status aStatus) { @@ -252,7 +245,7 @@ CHIP_ERROR WriteHandler::AddStatus(const AttributePathParams & aAttributePathPar err = attributeStatusIB.GetError(); SuccessOrExit(err); - err = ConstructAttributePath(aAttributePathParams, attributeStatusIB); + err = attributeStatusIB.CreatePath().Encode(aAttributePathParams); SuccessOrExit(err); statusIB.mStatus = aStatus; diff --git a/src/app/WriteHandler.h b/src/app/WriteHandler.h index 1b01393f41d82f..6251a876651921 100644 --- a/src/app/WriteHandler.h +++ b/src/app/WriteHandler.h @@ -97,8 +97,6 @@ class WriteHandler CHIP_ERROR ProcessWriteRequest(System::PacketBufferHandle && aPayload); CHIP_ERROR FinalizeMessage(System::PacketBufferHandle & packet); CHIP_ERROR SendWriteResponse(); - CHIP_ERROR ConstructAttributePath(const AttributePathParams & aAttributePathParams, - AttributeStatusIB::Builder aAttributeStatusIB); void MoveToState(const State aTargetState); void ClearState(); diff --git a/src/app/tests/TestClusterInfo.cpp b/src/app/tests/TestClusterInfo.cpp index f1651e7355d97b..951ae8da191f5d 100644 --- a/src/app/tests/TestClusterInfo.cpp +++ b/src/app/tests/TestClusterInfo.cpp @@ -23,9 +23,12 @@ */ #include +#include #include #include +using namespace chip::Test; + namespace chip { namespace app { namespace TestClusterInfo { @@ -94,6 +97,72 @@ void TestAttributePathIncludedDifferentClusterId(nlTestSuite * apSuite, void * a clusterInfo2.mClusterId = 2; NL_TEST_ASSERT(apSuite, !clusterInfo1.IsAttributePathSupersetOf(clusterInfo2)); } + +/* +{ClusterInfo::kInvalidEndpointId, ClusterInfo::kInvalidClusterId, ClusterInfo::kInvalidEventId}, +{ClusterInfo::kInvalidEndpointId, MockClusterId(1), ClusterInfo::kInvalidEventId}, +{ClusterInfo::kInvalidEndpointId, MockClusterId(1), MockEventId(1)}, +{kMockEndpoint1, ClusterInfo::kInvalidClusterId, ClusterInfo::kInvalidEventId}, +{kMockEndpoint1, MockClusterId(1), ClusterInfo::kInvalidEventId}, +{kMockEndpoint1, MockClusterId(1), MockEventId(1)}, +*/ +chip::app::ClusterInfo validEventpaths[6]; +void InitEventPaths() +{ + validEventpaths[1].mClusterId = MockClusterId(1); + validEventpaths[2].mClusterId = MockClusterId(1); + validEventpaths[2].mEventId = MockEventId(1); + validEventpaths[3].mEndpointId = kMockEndpoint1; + validEventpaths[4].mEndpointId = kMockEndpoint1; + validEventpaths[4].mClusterId = MockClusterId(1); + validEventpaths[5].mEndpointId = kMockEndpoint1; + validEventpaths[5].mClusterId = MockClusterId(1); + validEventpaths[5].mEventId = MockEventId(1); +} + +void TestEventPathSameEventId(nlTestSuite * apSuite, void * apContext) +{ + ConcreteEventPath testPath(kMockEndpoint1, MockClusterId(1), MockEventId(1)); + for (auto & path : validEventpaths) + { + NL_TEST_ASSERT(apSuite, path.IsValidEventPath()); + NL_TEST_ASSERT(apSuite, path.IsEventPathSupersetOf(testPath)); + } +} + +void TestEventPathDifferentEventId(nlTestSuite * apSuite, void * apContext) +{ + ConcreteEventPath testPath(kMockEndpoint1, MockClusterId(1), MockEventId(2)); + NL_TEST_ASSERT(apSuite, validEventpaths[0].IsEventPathSupersetOf(testPath)); + NL_TEST_ASSERT(apSuite, validEventpaths[1].IsEventPathSupersetOf(testPath)); + NL_TEST_ASSERT(apSuite, !validEventpaths[2].IsEventPathSupersetOf(testPath)); + NL_TEST_ASSERT(apSuite, validEventpaths[3].IsEventPathSupersetOf(testPath)); + NL_TEST_ASSERT(apSuite, validEventpaths[4].IsEventPathSupersetOf(testPath)); + NL_TEST_ASSERT(apSuite, !validEventpaths[5].IsEventPathSupersetOf(testPath)); +} + +void TestEventPathDifferentClusterId(nlTestSuite * apSuite, void * apContext) +{ + ConcreteEventPath testPath(kMockEndpoint1, MockClusterId(2), MockEventId(1)); + NL_TEST_ASSERT(apSuite, validEventpaths[0].IsEventPathSupersetOf(testPath)); + NL_TEST_ASSERT(apSuite, !validEventpaths[1].IsEventPathSupersetOf(testPath)); + NL_TEST_ASSERT(apSuite, !validEventpaths[2].IsEventPathSupersetOf(testPath)); + NL_TEST_ASSERT(apSuite, validEventpaths[3].IsEventPathSupersetOf(testPath)); + NL_TEST_ASSERT(apSuite, !validEventpaths[4].IsEventPathSupersetOf(testPath)); + NL_TEST_ASSERT(apSuite, !validEventpaths[5].IsEventPathSupersetOf(testPath)); +} + +void TestEventPathDifferentEndpointId(nlTestSuite * apSuite, void * apContext) +{ + ConcreteEventPath testPath(kMockEndpoint2, MockClusterId(1), MockEventId(1)); + NL_TEST_ASSERT(apSuite, validEventpaths[0].IsEventPathSupersetOf(testPath)); + NL_TEST_ASSERT(apSuite, validEventpaths[1].IsEventPathSupersetOf(testPath)); + NL_TEST_ASSERT(apSuite, validEventpaths[2].IsEventPathSupersetOf(testPath)); + NL_TEST_ASSERT(apSuite, !validEventpaths[3].IsEventPathSupersetOf(testPath)); + NL_TEST_ASSERT(apSuite, !validEventpaths[4].IsEventPathSupersetOf(testPath)); + NL_TEST_ASSERT(apSuite, !validEventpaths[5].IsEventPathSupersetOf(testPath)); +} + } // namespace TestClusterInfo } // namespace app } // namespace chip @@ -106,6 +175,10 @@ const nlTest sTests[] = { chip::app::TestClusterInfo::TestAttributePathIncludedDifferentEndpointId), NL_TEST_DEF("TestAttributePathIncludedDifferentClusterId", chip::app::TestClusterInfo::TestAttributePathIncludedDifferentClusterId), + NL_TEST_DEF("TestEventPathSameEventId", chip::app::TestClusterInfo::TestEventPathSameEventId), + NL_TEST_DEF("TestEventPathDifferentEventId", chip::app::TestClusterInfo::TestEventPathDifferentEventId), + NL_TEST_DEF("TestEventPathDifferentClusterId", chip::app::TestClusterInfo::TestEventPathDifferentClusterId), + NL_TEST_DEF("TestEventPathDifferentEndpointId", chip::app::TestClusterInfo::TestEventPathDifferentEndpointId), NL_TEST_SENTINEL() }; } @@ -113,7 +186,7 @@ const nlTest sTests[] = { int TestClusterInfo() { nlTestSuite theSuite = { "ClusterInfo", &sTests[0], nullptr, nullptr }; - + chip::app::TestClusterInfo::InitEventPaths(); nlTestRunner(&theSuite, nullptr); return (nlTestRunnerStats(&theSuite)); diff --git a/src/app/tests/TestEventLogging.cpp b/src/app/tests/TestEventLogging.cpp index 0511a183f56d1b..34efc81e1113fb 100644 --- a/src/app/tests/TestEventLogging.cpp +++ b/src/app/tests/TestEventLogging.cpp @@ -44,10 +44,10 @@ namespace { static const chip::NodeId kTestDeviceNodeId1 = 0x18B4300000000001ULL; -static const chip::NodeId kTestDeviceNodeId2 = 0x18B4300000000002ULL; static const chip::ClusterId kLivenessClusterId = 0x00000022; static const uint32_t kLivenessChangeEvent = 1; -static const chip::EndpointId kTestEndpointId = 2; +static const chip::EndpointId kTestEndpointId1 = 2; +static const chip::EndpointId kTestEndpointId2 = 3; static const chip::TLV::Tag kLivenessDeviceStatus = chip::TLV::ContextTag(1); static uint8_t gDebugEventBuffer[128]; @@ -171,9 +171,9 @@ static void CheckLogEventWithEvictToNextBuffer(nlTestSuite * apSuite, void * apC { CHIP_ERROR err = CHIP_NO_ERROR; chip::EventNumber eid1, eid2, eid3, eid4, eid5, eid6; - chip::app::EventSchema schema1 = { kTestDeviceNodeId1, kTestEndpointId, kLivenessClusterId, kLivenessChangeEvent, + chip::app::EventSchema schema1 = { kTestDeviceNodeId1, kTestEndpointId1, kLivenessClusterId, kLivenessChangeEvent, chip::app::PriorityLevel::Info }; - chip::app::EventSchema schema2 = { kTestDeviceNodeId2, kTestEndpointId, kLivenessClusterId, kLivenessChangeEvent, + chip::app::EventSchema schema2 = { kTestDeviceNodeId1, kTestEndpointId2, kLivenessClusterId, kLivenessChangeEvent, chip::app::PriorityLevel::Info }; chip::app::EventOptions options1; chip::app::EventOptions options2; @@ -220,12 +220,11 @@ static void CheckLogEventWithEvictToNextBuffer(nlTestSuite * apSuite, void * apC chip::app::ClusterInfo testClusterInfo1; testClusterInfo1.mNodeId = kTestDeviceNodeId1; - testClusterInfo1.mEndpointId = kTestEndpointId; + testClusterInfo1.mEndpointId = kTestEndpointId1; testClusterInfo1.mClusterId = kLivenessClusterId; - testClusterInfo1.mEventId = kLivenessChangeEvent; chip::app::ClusterInfo testClusterInfo2; - testClusterInfo2.mNodeId = kTestDeviceNodeId2; - testClusterInfo2.mEndpointId = kTestEndpointId; + testClusterInfo2.mNodeId = kTestDeviceNodeId1; + testClusterInfo2.mEndpointId = kTestEndpointId2; testClusterInfo2.mClusterId = kLivenessClusterId; testClusterInfo2.mEventId = kLivenessChangeEvent; @@ -241,7 +240,7 @@ static void CheckLogEventWithDiscardLowEvent(nlTestSuite * apSuite, void * apCon { CHIP_ERROR err = CHIP_NO_ERROR; chip::EventNumber eid1, eid2, eid3, eid4, eid5, eid6; - chip::app::EventSchema schema = { kTestDeviceNodeId1, kTestEndpointId, kLivenessClusterId, kLivenessChangeEvent, + chip::app::EventSchema schema = { kTestDeviceNodeId1, kTestEndpointId1, kLivenessClusterId, kLivenessChangeEvent, chip::app::PriorityLevel::Debug }; chip::app::EventOptions options; TestEventGenerator testEventGenerator; diff --git a/src/app/tests/TestEventPathParams.cpp b/src/app/tests/TestEventPathParams.cpp index 502f687b60632f..da0b3315290815 100644 --- a/src/app/tests/TestEventPathParams.cpp +++ b/src/app/tests/TestEventPathParams.cpp @@ -31,36 +31,29 @@ namespace app { namespace TestEventPathParams { void TestSamePath(nlTestSuite * apSuite, void * apContext) { - EventPathParams eventPathParams1(1, 2, 3, 4, false); - EventPathParams eventPathParams2(1, 2, 3, 4, false); + EventPathParams eventPathParams1(2, 3, 4); + EventPathParams eventPathParams2(2, 3, 4); NL_TEST_ASSERT(apSuite, eventPathParams1.IsSamePath(eventPathParams2)); } -void TestDifferentNodeId(nlTestSuite * apSuite, void * apContext) -{ - EventPathParams eventPathParams1(1, 2, 3, 4, false); - EventPathParams eventPathParams2(6, 2, 3, 4, false); - NL_TEST_ASSERT(apSuite, !eventPathParams1.IsSamePath(eventPathParams2)); -} - void TestDifferentEndpointId(nlTestSuite * apSuite, void * apContext) { - EventPathParams eventPathParams1(1, 2, 3, 4, false); - EventPathParams eventPathParams2(1, 6, 3, 4, false); + EventPathParams eventPathParams1(2, 3, 4); + EventPathParams eventPathParams2(6, 3, 4); NL_TEST_ASSERT(apSuite, !eventPathParams1.IsSamePath(eventPathParams2)); } void TestDifferentClusterId(nlTestSuite * apSuite, void * apContext) { - EventPathParams eventPathParams1(1, 2, 3, 4, false); - EventPathParams eventPathParams2(1, 2, 6, 4, false); + EventPathParams eventPathParams1(2, 3, 4); + EventPathParams eventPathParams2(2, 6, 4); NL_TEST_ASSERT(apSuite, !eventPathParams1.IsSamePath(eventPathParams2)); } void TestDifferentEventId(nlTestSuite * apSuite, void * apContext) { - EventPathParams eventPathParams1(1, 2, 3, 4, false); - EventPathParams eventPathParams2(1, 2, 3, 6, false); + EventPathParams eventPathParams1(2, 3, 4); + EventPathParams eventPathParams2(2, 3, 6); NL_TEST_ASSERT(apSuite, !eventPathParams1.IsSamePath(eventPathParams2)); } } // namespace TestEventPathParams @@ -69,7 +62,6 @@ void TestDifferentEventId(nlTestSuite * apSuite, void * apContext) namespace { const nlTest sTests[] = { NL_TEST_DEF("TestSamePath", chip::app::TestEventPathParams::TestSamePath), - NL_TEST_DEF("TestDifferentNodeId", chip::app::TestEventPathParams::TestDifferentNodeId), NL_TEST_DEF("TestDifferentEndpointId", chip::app::TestEventPathParams::TestDifferentEndpointId), NL_TEST_DEF("TestDifferentClusterId", chip::app::TestEventPathParams::TestDifferentClusterId), NL_TEST_DEF("TestDifferentEventId", chip::app::TestEventPathParams::TestDifferentEventId), diff --git a/src/app/tests/TestReadInteraction.cpp b/src/app/tests/TestReadInteraction.cpp index 53e8601594ad50..6da78d25a274c5 100644 --- a/src/app/tests/TestReadInteraction.cpp +++ b/src/app/tests/TestReadInteraction.cpp @@ -575,8 +575,7 @@ void TestReadInteraction::TestReadClientGenerateOneEventPaths(nlTestSuite * apSu err = readClient.Init(&ctx.GetExchangeManager(), &delegate, chip::app::ReadClient::InteractionType::Read); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); - chip::app::EventPathParams eventPathParams[2]; - eventPathParams[0].mNodeId = 1; + chip::app::EventPathParams eventPathParams[1]; eventPathParams[0].mEndpointId = 2; eventPathParams[0].mClusterId = 3; eventPathParams[0].mEventId = 4; @@ -625,12 +624,10 @@ void TestReadInteraction::TestReadClientGenerateTwoEventPaths(nlTestSuite * apSu NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); chip::app::EventPathParams eventPathParams[2]; - eventPathParams[0].mNodeId = 1; eventPathParams[0].mEndpointId = 2; eventPathParams[0].mClusterId = 3; eventPathParams[0].mEventId = 4; - eventPathParams[1].mNodeId = 1; eventPathParams[1].mEndpointId = 2; eventPathParams[1].mClusterId = 3; eventPathParams[1].mEventId = 5; @@ -677,16 +674,9 @@ void TestReadInteraction::TestReadRoundtrip(nlTestSuite * apSuite, void * apCont NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); NL_TEST_ASSERT(apSuite, !delegate.mGotEventResponse); - chip::app::EventPathParams eventPathParams[2]; - eventPathParams[0].mNodeId = kTestNodeId; + chip::app::EventPathParams eventPathParams[1]; eventPathParams[0].mEndpointId = kTestEndpointId; eventPathParams[0].mClusterId = kTestClusterId; - eventPathParams[0].mEventId = kTestEventIdDebug; - - eventPathParams[1].mNodeId = kTestNodeId; - eventPathParams[1].mEndpointId = kTestEndpointId; - eventPathParams[1].mClusterId = kTestClusterId; - eventPathParams[1].mEventId = kTestEventIdCritical; chip::app::AttributePathParams attributePathParams[2]; attributePathParams[0].mEndpointId = kTestEndpointId; @@ -700,7 +690,7 @@ void TestReadInteraction::TestReadRoundtrip(nlTestSuite * apSuite, void * apCont ReadPrepareParams readPrepareParams(ctx.GetSessionBobToAlice()); readPrepareParams.mpEventPathParamsList = eventPathParams; - readPrepareParams.mEventPathParamsListSize = 2; + readPrepareParams.mEventPathParamsListSize = 1; readPrepareParams.mpAttributePathParamsList = attributePathParams; readPrepareParams.mAttributePathParamsListSize = 2; err = chip::app::InteractionModelEngine::GetInstance()->SendReadRequest(readPrepareParams, &delegate); diff --git a/src/app/tests/integration/chip_im_initiator.cpp b/src/app/tests/integration/chip_im_initiator.cpp index ef5818b4d27e84..359ffb46953d37 100644 --- a/src/app/tests/integration/chip_im_initiator.cpp +++ b/src/app/tests/integration/chip_im_initiator.cpp @@ -300,12 +300,9 @@ CHIP_ERROR SendReadRequest() { CHIP_ERROR err = CHIP_NO_ERROR; chip::app::EventPathParams eventPathParams[2]; - eventPathParams[0].mNodeId = kTestNodeId; eventPathParams[0].mEndpointId = kTestEndpointId; eventPathParams[0].mClusterId = kTestClusterId; - eventPathParams[0].mEventId = kTestChangeEvent1; - eventPathParams[1].mNodeId = kTestNodeId; eventPathParams[1].mEndpointId = kTestEndpointId; eventPathParams[1].mClusterId = kTestClusterId; eventPathParams[1].mEventId = kTestChangeEvent2; @@ -377,12 +374,10 @@ CHIP_ERROR SendSubscribeRequest() chip::app::EventPathParams eventPathParams[2]; chip::app::AttributePathParams attributePathParams[1]; readPrepareParams.mpEventPathParamsList = eventPathParams; - readPrepareParams.mpEventPathParamsList[0].mNodeId = kTestNodeId; readPrepareParams.mpEventPathParamsList[0].mEndpointId = kTestEndpointId; readPrepareParams.mpEventPathParamsList[0].mClusterId = kTestClusterId; readPrepareParams.mpEventPathParamsList[0].mEventId = kTestChangeEvent1; - readPrepareParams.mpEventPathParamsList[1].mNodeId = kTestNodeId; readPrepareParams.mpEventPathParamsList[1].mEndpointId = kTestEndpointId; readPrepareParams.mpEventPathParamsList[1].mClusterId = kTestClusterId; readPrepareParams.mpEventPathParamsList[1].mEventId = kTestChangeEvent2; diff --git a/src/app/util/mock/Constants.h b/src/app/util/mock/Constants.h index 2b3e71d795222a..73e965a8028cd5 100644 --- a/src/app/util/mock/Constants.h +++ b/src/app/util/mock/Constants.h @@ -45,5 +45,9 @@ constexpr AttributeId MockClusterId(const uint16_t & id) return (0xFFF1'0000 | id); } +constexpr EventId MockEventId(const uint16_t & id) +{ + return (0xFFF1'0000 | id); +} } // namespace Test } // namespace chip