diff --git a/examples/energy-management-app/energy-management-common/device-energy-management/src/DEMTestEventTriggers.cpp b/examples/energy-management-app/energy-management-common/device-energy-management/src/DEMTestEventTriggers.cpp index 23ed2afe1b994d..07b12df957c8d1 100644 --- a/examples/energy-management-app/energy-management-common/device-energy-management/src/DEMTestEventTriggers.cpp +++ b/examples/energy-management-app/energy-management-common/device-energy-management/src/DEMTestEventTriggers.cpp @@ -36,7 +36,6 @@ constexpr uint16_t MAX_POWER_ADJUSTMENTS = 5; chip::app::Clusters::DeviceEnergyManagement::Structs::SlotStruct::Type sSlots[MAX_SLOTS]; chip::app::Clusters::DeviceEnergyManagement::Structs::ForecastStruct::Type sForecastStruct; -chip::app::DataModel::Nullable sForecast; chip::app::Clusters::DeviceEnergyManagement::Structs::PowerAdjustStruct::Type sPowerAdjustments[MAX_POWER_ADJUSTMENTS]; chip::app::Clusters::DeviceEnergyManagement::Structs::PowerAdjustCapabilityStruct::Type sPowerAdjustCapabilityStruct; diff --git a/scripts/setup/zap.json b/scripts/setup/zap.json index 8b64288e667f04..b283065f5541aa 100644 --- a/scripts/setup/zap.json +++ b/scripts/setup/zap.json @@ -8,13 +8,13 @@ "mac-amd64", "windows-amd64" ], - "tags": ["version:2@v2024.10.24-nightly.1"] + "tags": ["version:2@v2025.01.10-nightly.1"] }, { "_comment": "Always get the amd64 version on mac until usable arm64 zap build is available", "path": "fuchsia/third_party/zap/mac-amd64", "platforms": ["mac-arm64"], - "tags": ["version:2@v2024.10.24-nightly.1"] + "tags": ["version:2@v2025.01.10-nightly.1"] } ] } diff --git a/scripts/setup/zap.version b/scripts/setup/zap.version index 0696cc2123a3ed..85ec8bf6f8901d 100644 --- a/scripts/setup/zap.version +++ b/scripts/setup/zap.version @@ -1 +1 @@ -v2024.10.24-nightly +v2025.01.10-nightly diff --git a/scripts/tools/zap/zap_execution.py b/scripts/tools/zap/zap_execution.py index 72d68a74ffd4c2..e42d8804dab124 100644 --- a/scripts/tools/zap/zap_execution.py +++ b/scripts/tools/zap/zap_execution.py @@ -23,7 +23,7 @@ # Use scripts/tools/zap/version_update.py to manage ZAP versioning as many # files may need updating for versions # -MIN_ZAP_VERSION = '2024.10.24' +MIN_ZAP_VERSION = '2025.1.10' class ZapTool: diff --git a/src/app/AttributePathExpandIterator.cpp b/src/app/AttributePathExpandIterator.cpp index 419067db9ddf23..23273c7c58fcf3 100644 --- a/src/app/AttributePathExpandIterator.cpp +++ b/src/app/AttributePathExpandIterator.cpp @@ -19,22 +19,86 @@ #include #include +#include + using namespace chip::app::DataModel; namespace chip { namespace app { -AttributePathExpandIterator::AttributePathExpandIterator(DataModel::Provider * provider, - SingleLinkedListNode * attributePath) : - mDataModelProvider(provider), - mpAttributePath(attributePath), mOutputPath(kInvalidEndpointId, kInvalidClusterId, kInvalidAttributeId) +bool AttributePathExpandIterator::AdvanceOutputPath() +{ + /// Output path invariants + /// - kInvalid* constants are used to define "no value available (yet)" and + /// iteration loop will fill the first value when such a value is seen (fixed for non-wildcard + /// or iteration-based in case of wildcards). + /// - Iteration of the output path is done in order: first endpoint, then cluster, then attribute. + /// Processing works like: + /// - Initial state is kInvalidEndpointId/kInvalidClusterId/kInvalidAttributeId + /// - First loop pass fills-in endpointID, followed by clusterID, followed by attributeID + /// - Whenever one level is done iterating (there is no "next") the following + /// "higher path component" is updated: + /// - once a valid path exists, try to advance attributeID + /// - if attributeID fails to advance, try to advance clusterID (and restart attributeID) + /// - if clusterID fails to advance, try to advance endpointID (and restart clusterID) + /// - if endpointID fails to advance, iteration is done + while (true) + { + if (mPosition.mOutputPath.mClusterId != kInvalidClusterId) + { + std::optional nextAttribute = NextAttributeId(); + if (nextAttribute.has_value()) + { + mPosition.mOutputPath.mAttributeId = *nextAttribute; + mPosition.mOutputPath.mExpanded = mPosition.mAttributePath->mValue.IsWildcardPath(); + return true; + } + } + + // no valid attribute, try to advance the cluster, see if a suitable one exists + if (mPosition.mOutputPath.mEndpointId != kInvalidEndpointId) + { + std::optional nextCluster = NextClusterId(); + if (nextCluster.has_value()) + { + // A new cluster ID is to be processed. This sets the cluster ID to the new value and + // ALSO resets the attribute ID to "invalid", to trigger an attribute set/expansion from + // the beginning. + mPosition.mOutputPath.mClusterId = *nextCluster; + mPosition.mOutputPath.mAttributeId = kInvalidAttributeId; + continue; + } + } + + // No valid cluster, try advance the endpoint, see if a suitable one exists. + std::optional nextEndpoint = NextEndpointId(); + if (nextEndpoint.has_value()) + { + // A new endpoint ID is to be processed. This sets the endpoint ID to the new value and + // ALSO resets the cluster ID to "invalid", to trigger a cluster set/expansion from + // the beginning. + mPosition.mOutputPath.mEndpointId = *nextEndpoint; + mPosition.mOutputPath.mClusterId = kInvalidClusterId; + continue; + } + return false; + } +} +bool AttributePathExpandIterator::Next(ConcreteAttributePath & path) { - mOutputPath.mExpanded = true; // this is reset in 'next' if needed + while (mPosition.mAttributePath != nullptr) + { + if (AdvanceOutputPath()) + { + path = mPosition.mOutputPath; + return true; + } + mPosition.mAttributePath = mPosition.mAttributePath->mpNext; + mPosition.mOutputPath = ConcreteReadAttributePath(kInvalidEndpointId, kInvalidClusterId, kInvalidAttributeId); + } - // Make the iterator ready to emit the first valid path in the list. - // TODO: the bool return value here is completely unchecked - Next(); + return false; } bool AttributePathExpandIterator::IsValidAttributeId(AttributeId attributeId) @@ -49,40 +113,43 @@ bool AttributePathExpandIterator::IsValidAttributeId(AttributeId attributeId) break; } - const ConcreteAttributePath attributePath(mOutputPath.mEndpointId, mOutputPath.mClusterId, attributeId); + const ConcreteAttributePath attributePath(mPosition.mOutputPath.mEndpointId, mPosition.mOutputPath.mClusterId, attributeId); return mDataModelProvider->GetAttributeInfo(attributePath).has_value(); } std::optional AttributePathExpandIterator::NextAttributeId() { - if (mOutputPath.mAttributeId == kInvalidAttributeId) + if (mPosition.mOutputPath.mAttributeId == kInvalidAttributeId) { - if (mpAttributePath->mValue.HasWildcardAttributeId()) + if (mPosition.mAttributePath->mValue.HasWildcardAttributeId()) { - AttributeEntry entry = mDataModelProvider->FirstAttribute(mOutputPath); + AttributeEntry entry = mDataModelProvider->FirstAttribute(mPosition.mOutputPath); return entry.IsValid() // ? entry.path.mAttributeId // : Clusters::Globals::Attributes::GeneratedCommandList::Id; // } - // We allow fixed attribute IDs if and only if they are valid: - // - they may be GLOBAL attributes OR - // - they are valid attributes for this cluster - if (IsValidAttributeId(mpAttributePath->mValue.mAttributeId)) + // At this point, the attributeID is NOT a wildcard (i.e. it is fixed). + // + // For wildcard expansion, we validate that this is a valid attribute for the given + // cluster on the given endpoint. If not a wildcard expansion, return it as-is. + if (mPosition.mAttributePath->mValue.IsWildcardPath()) { - return mpAttributePath->mValue.mAttributeId; + if (!IsValidAttributeId(mPosition.mAttributePath->mValue.mAttributeId)) + { + return std::nullopt; + } } - - return std::nullopt; + return mPosition.mAttributePath->mValue.mAttributeId; } - // advance the existing attribute id if it can be advanced - VerifyOrReturnValue(mpAttributePath->mValue.HasWildcardAttributeId(), std::nullopt); + // Advance the existing attribute id if it can be advanced. + VerifyOrReturnValue(mPosition.mAttributePath->mValue.HasWildcardAttributeId(), std::nullopt); // Ensure (including ordering) that GlobalAttributesNotInMetadata is reported as needed for (unsigned i = 0; i < ArraySize(GlobalAttributesNotInMetadata); i++) { - if (GlobalAttributesNotInMetadata[i] != mOutputPath.mAttributeId) + if (GlobalAttributesNotInMetadata[i] != mPosition.mOutputPath.mAttributeId) { continue; } @@ -93,11 +160,12 @@ std::optional AttributePathExpandIterator::NextAttributeId() return GlobalAttributesNotInMetadata[nextAttributeIndex]; } - // reached the end of global attributes + // Reached the end of global attributes. Since global attributes are + // reported last, finishing global attributes means everything completed. return std::nullopt; } - AttributeEntry entry = mDataModelProvider->NextAttribute(mOutputPath); + AttributeEntry entry = mDataModelProvider->NextAttribute(mPosition.mOutputPath); if (entry.IsValid()) { return entry.path.mAttributeId; @@ -111,130 +179,55 @@ std::optional AttributePathExpandIterator::NextAttributeId() std::optional AttributePathExpandIterator::NextClusterId() { - if (mOutputPath.mClusterId == kInvalidClusterId) + if (mPosition.mOutputPath.mClusterId == kInvalidClusterId) { - if (mpAttributePath->mValue.HasWildcardClusterId()) + if (mPosition.mAttributePath->mValue.HasWildcardClusterId()) { - ClusterEntry entry = mDataModelProvider->FirstServerCluster(mOutputPath.mEndpointId); + ClusterEntry entry = mDataModelProvider->FirstServerCluster(mPosition.mOutputPath.mEndpointId); return entry.IsValid() ? std::make_optional(entry.path.mClusterId) : std::nullopt; } - // only return a cluster if it is valid - const ConcreteClusterPath clusterPath(mOutputPath.mEndpointId, mpAttributePath->mValue.mClusterId); - if (!mDataModelProvider->GetServerClusterInfo(clusterPath).has_value()) + // At this point, the clusterID is NOT a wildcard (i.e. is fixed). + // + // For wildcard expansion, we validate that this is a valid cluster for the endpoint. + // If non-wildcard expansion, we return as-is. + if (mPosition.mAttributePath->mValue.IsWildcardPath()) { - return std::nullopt; + const ConcreteClusterPath clusterPath(mPosition.mOutputPath.mEndpointId, mPosition.mAttributePath->mValue.mClusterId); + if (!mDataModelProvider->GetServerClusterInfo(clusterPath).has_value()) + { + return std::nullopt; + } } - return mpAttributePath->mValue.mClusterId; + return mPosition.mAttributePath->mValue.mClusterId; } - VerifyOrReturnValue(mpAttributePath->mValue.HasWildcardClusterId(), std::nullopt); + VerifyOrReturnValue(mPosition.mAttributePath->mValue.HasWildcardClusterId(), std::nullopt); - ClusterEntry entry = mDataModelProvider->NextServerCluster(mOutputPath); + ClusterEntry entry = mDataModelProvider->NextServerCluster(mPosition.mOutputPath); return entry.IsValid() ? std::make_optional(entry.path.mClusterId) : std::nullopt; } std::optional AttributePathExpandIterator::NextEndpointId() { - if (mOutputPath.mEndpointId == kInvalidEndpointId) + if (mPosition.mOutputPath.mEndpointId == kInvalidEndpointId) { - if (mpAttributePath->mValue.HasWildcardEndpointId()) + if (mPosition.mAttributePath->mValue.HasWildcardEndpointId()) { EndpointEntry ep = mDataModelProvider->FirstEndpoint(); return (ep.id != kInvalidEndpointId) ? std::make_optional(ep.id) : std::nullopt; } - return mpAttributePath->mValue.mEndpointId; + return mPosition.mAttributePath->mValue.mEndpointId; } - VerifyOrReturnValue(mpAttributePath->mValue.HasWildcardEndpointId(), std::nullopt); + // Expand endpoints only if it is a wildcard on the endpoint specifically. + VerifyOrReturnValue(mPosition.mAttributePath->mValue.HasWildcardEndpointId(), std::nullopt); - EndpointEntry ep = mDataModelProvider->NextEndpoint(mOutputPath.mEndpointId); + EndpointEntry ep = mDataModelProvider->NextEndpoint(mPosition.mOutputPath.mEndpointId); return (ep.id != kInvalidEndpointId) ? std::make_optional(ep.id) : std::nullopt; } -void AttributePathExpandIterator::ResetCurrentCluster() -{ - // If this is a null iterator, or the attribute id of current cluster info is not a wildcard attribute id, then this function - // will do nothing, since we won't be expanding the wildcard attribute ids under a cluster. - VerifyOrReturn(mpAttributePath != nullptr && mpAttributePath->mValue.HasWildcardAttributeId()); - - // Reset path expansion to ask for the first attribute of the current cluster - mOutputPath.mAttributeId = kInvalidAttributeId; - mOutputPath.mExpanded = true; // we know this is a wildcard attribute - Next(); -} - -bool AttributePathExpandIterator::AdvanceOutputPath() -{ - if (!mpAttributePath->mValue.IsWildcardPath()) - { - if (mOutputPath.mEndpointId != kInvalidEndpointId) - { - return false; // cannot expand non-wildcard path - } - - mOutputPath.mEndpointId = mpAttributePath->mValue.mEndpointId; - mOutputPath.mClusterId = mpAttributePath->mValue.mClusterId; - mOutputPath.mAttributeId = mpAttributePath->mValue.mAttributeId; - mOutputPath.mExpanded = false; - return true; - } - - while (true) - { - if (mOutputPath.mClusterId != kInvalidClusterId) - { - - std::optional nextAttribute = NextAttributeId(); - if (nextAttribute.has_value()) - { - mOutputPath.mAttributeId = *nextAttribute; - return true; - } - } - - // no valid attribute, try to advance the cluster, see if a suitable one exists - if (mOutputPath.mEndpointId != kInvalidEndpointId) - { - std::optional nextCluster = NextClusterId(); - if (nextCluster.has_value()) - { - mOutputPath.mClusterId = *nextCluster; - mOutputPath.mAttributeId = kInvalidAttributeId; // restarts attributes - continue; - } - } - - // no valid cluster, try advance the endpoint, see if a suitable on exists - std::optional nextEndpoint = NextEndpointId(); - if (nextEndpoint.has_value()) - { - mOutputPath.mEndpointId = *nextEndpoint; - mOutputPath.mClusterId = kInvalidClusterId; // restarts clusters - continue; - } - return false; - } -} - -bool AttributePathExpandIterator::Next() -{ - while (mpAttributePath != nullptr) - { - if (AdvanceOutputPath()) - { - return true; - } - mpAttributePath = mpAttributePath->mpNext; - mOutputPath = ConcreteReadAttributePath(kInvalidEndpointId, kInvalidClusterId, kInvalidAttributeId); - mOutputPath.mExpanded = true; // this is reset to false on advancement if needed - } - - mOutputPath = ConcreteReadAttributePath(); - return false; -} - } // namespace app } // namespace chip diff --git a/src/app/AttributePathExpandIterator.h b/src/app/AttributePathExpandIterator.h index 351520f7c49c99..2916a2e3bb7b18 100644 --- a/src/app/AttributePathExpandIterator.h +++ b/src/app/AttributePathExpandIterator.h @@ -20,81 +20,104 @@ #include #include #include +#include #include namespace chip { namespace app { -/** - * AttributePathExpandIterator is used to iterate over a linked list of AttributePathParams-s. - * The AttributePathExpandIterator is copiable, however, the given cluster info must be valid when calling Next(). - * - * AttributePathExpandIterator will expand attribute paths with wildcards, and only emit existing paths for - * AttributePathParams with wildcards. For AttributePathParams with a concrete path (i.e. does not contain wildcards), - * AttributePathExpandIterator will emit them as-is. - * - * The typical use of AttributePathExpandIterator may look like: - * ConcreteAttributePath path; - * for (AttributePathExpandIterator iterator(AttributePathParams); iterator.Get(path); iterator.Next()) {...} - * - * The iterator does not copy the given AttributePathParams. The given AttributePathParams must remain valid when using the - * iterator. If the set of endpoints, clusters, or attributes that are supported changes, AttributePathExpandIterator must be - * reinitialized. - * - * A initialized iterator will return the first valid path, no need to call Next() before calling Get() for the first time. - * - * Note: Next() and Get() are two separate operations by design since a possible call of this iterator might be: - * - Get() - * - Chunk full, return - * - In a new chunk, Get() - * - * TODO: The AttributePathParams may support a group id, the iterator should be able to call group data provider to expand the group - * id. - */ +/// Handles attribute path expansions +/// Usage: +/// +/// - Start iterating by creating an iteration state +/// +/// AttributePathExpandIterator::Position position = AttributePathExpandIterator::Position::StartIterating(path); +/// +/// - Use the iteration state in a for loop: +/// +/// ConcreteAttributePath path; +/// for (AttributePathExpandIterator iterator(position); iterator->Next(path);) { +/// // use `path` here` +/// } +/// +/// OR: +/// +/// ConcreteAttributePath path; +/// AttributePathExpandIterator iterator(position); +/// +/// while (iterator.Next(path)) { +/// // use `path` here` +/// } +/// +/// Usage requirements and assumptions: +/// +/// - An ` AttributePathExpandIterator::Position` can only be used by a single AttributePathExpandIterator at a time. +/// +/// - `position` is automatically updated by the AttributePathExpandIterator, so +/// calling `Next` on the iterator will update the position cursor variable. +/// class AttributePathExpandIterator { public: - AttributePathExpandIterator(DataModel::Provider * provider, SingleLinkedListNode * attributePath); - - /** - * Proceed the iterator to the next attribute path in the given cluster info. - * - * Returns false if AttributePathExpandIteratorDataModeDataModel has exhausted all paths in the given AttributePathParams list. - */ - bool Next(); - - /** - * Fills the aPath with the path the iterator currently points to. - * Returns false if the iterator is not pointing to a valid path (i.e. it has exhausted the cluster info). - */ - bool Get(ConcreteAttributePath & aPath) + class Position { - aPath = mOutputPath; - return (mpAttributePath != nullptr); - } - - /** - * Reset the iterator to the beginning of current cluster if we are in the middle of expanding a wildcard attribute id for some - * cluster. - * - * When attributes are changed in the middle of expanding a wildcard attribute, we need to reset the iterator, to provide the - * client with a consistent state of the cluster. - */ - void ResetCurrentCluster(); - - /** Start iterating over the given `paths` */ - inline void ResetTo(SingleLinkedListNode * paths) - { - *this = AttributePathExpandIterator(mDataModelProvider, paths); - } + public: + // Position is treated as a direct member access by the AttributePathExpandIterator, however it is opaque (except copying) + // for external code. We allow friendship here to not have specific get/set for methods (clearer interface and less + // likelihood of extra code usage). + friend class AttributePathExpandIterator; + + /// External callers can only ever start iterating on a new path from the beginning + static Position StartIterating(SingleLinkedListNode * path) { return Position(path); } + + /// Copies are allowed + Position(const Position &) = default; + Position & operator=(const Position &) = default; + + Position() : mAttributePath(nullptr) {} + + /// Reset the iterator to the beginning of current cluster if we are in the middle of expanding a wildcard attribute id for + /// some cluster. + /// + /// When attributes are changed in the middle of expanding a wildcard attribute, we need to reset the iterator, to provide + /// the client with a consistent state of the cluster. + void IterateFromTheStartOfTheCurrentClusterIfAttributeWildcard() + { + VerifyOrReturn(mAttributePath != nullptr && mAttributePath->mValue.HasWildcardAttributeId()); + mOutputPath.mAttributeId = kInvalidAttributeId; + } + + protected: + Position(SingleLinkedListNode * path) : + mAttributePath(path), mOutputPath(kInvalidEndpointId, kInvalidClusterId, kInvalidAttributeId) + {} + + SingleLinkedListNode * mAttributePath; + ConcreteAttributePath mOutputPath; + }; + + AttributePathExpandIterator(DataModel::Provider * dataModel, Position & position) : + mDataModelProvider(dataModel), mPosition(position) + {} + + // This class may not be copied. A new one should be created when needed and they + // should not overlap. + AttributePathExpandIterator(const AttributePathExpandIterator &) = delete; + AttributePathExpandIterator & operator=(const AttributePathExpandIterator &) = delete; + + /// Get the next path of the expansion (if one exists). + /// + /// On success, true is returned and `path` is filled with the next path in the + /// expansion. + /// On iteration completion, false is returned and the content of path IS NOT DEFINED. + bool Next(ConcreteAttributePath & path); private: DataModel::Provider * mDataModelProvider; - SingleLinkedListNode * mpAttributePath; - ConcreteAttributePath mOutputPath; + Position & mPosition; /// Move to the next endpoint/cluster/attribute triplet that is valid given - /// the current mOutputPath and mpAttributePath + /// the current mOutputPath and mpAttributePath. /// /// returns true if such a next value was found. bool AdvanceOutputPath(); @@ -125,5 +148,52 @@ class AttributePathExpandIterator bool IsValidAttributeId(AttributeId attributeId); }; +/// RollbackAttributePathExpandIterator is an AttributePathExpandIterator wrapper that rolls back the Next() +/// call whenever a new `MarkCompleted()` method is not called. +/// +/// Example use cases: +/// +/// - Iterate over all attributes and process one-by-one, however when the iteration fails, resume at +/// the last failure point: +/// +/// RollbackAttributePathExpandIterator iterator(....); +/// ConcreteAttributePath path; +/// +/// for ( ; iterator.Next(path); iterator.MarkCompleted()) { +/// if (!CanProcess(path)) { +/// // iterator state IS PRESERVED so that Next() will return the SAME path on the next call. +/// return CHIP_ERROR_TRY_AGAIN_LATER; +/// } +/// } +/// +/// - Grab what the next output path would be WITHOUT advancing a state; +/// +/// { +/// RollbackAttributePathExpandIterator iterator(...., state); +/// if (iterator.Next(...)) { ... } +/// } +/// // state here is ROLLED BACK (i.e. initializing a new iterator with it will start at the same place as the previous +/// iteration attempt). +/// +/// +class RollbackAttributePathExpandIterator +{ +public: + RollbackAttributePathExpandIterator(DataModel::Provider * dataModel, AttributePathExpandIterator::Position & position) : + mAttributePathExpandIterator(dataModel, position), mPositionTarget(position), mCompletedPosition(position) + {} + ~RollbackAttributePathExpandIterator() { mPositionTarget = mCompletedPosition; } + + bool Next(ConcreteAttributePath & path) { return mAttributePathExpandIterator.Next(path); } + + /// Marks the current iteration completed (so peek does not actually roll back) + void MarkCompleted() { mCompletedPosition = mPositionTarget; } + +private: + AttributePathExpandIterator mAttributePathExpandIterator; + AttributePathExpandIterator::Position & mPositionTarget; + AttributePathExpandIterator::Position mCompletedPosition; +}; + } // namespace app } // namespace chip diff --git a/src/app/BUILD.gn b/src/app/BUILD.gn index 1ee02cd71d5fc2..3fb0795abcdc43 100644 --- a/src/app/BUILD.gn +++ b/src/app/BUILD.gn @@ -207,6 +207,7 @@ static_library("interaction-model") { ] deps = [ + ":path-expansion", "${chip_root}/src/app:events", "${chip_root}/src/app:global-attributes", ] @@ -406,6 +407,21 @@ source_set("storage-wrapper") { ] } +source_set("path-expansion") { + sources = [ + "AttributePathExpandIterator.cpp", + "AttributePathExpandIterator.h", + ] + + public_deps = [ + ":global-attributes", + ":paths", + "${chip_root}/src/app/data-model-provider", + "${chip_root}/src/lib/core:types", + "${chip_root}/src/lib/support", + ] +} + source_set("attribute-persistence") { sources = [ "DefaultSafeAttributePersistenceProvider.h", @@ -429,9 +445,6 @@ static_library("app") { output_name = "libCHIPDataModel" sources = [ - "AttributePathExpandIterator.cpp", - "AttributePathExpandIterator.h", - "AttributePathExpandIterator.h", "ChunkedWriteCallback.cpp", "ChunkedWriteCallback.h", "CommandResponseHelper.h", @@ -464,6 +477,7 @@ static_library("app") { ":event-reporter", ":global-attributes", ":interaction-model", + ":path-expansion", "${chip_root}/src/app/data-model", "${chip_root}/src/app/data-model-provider", "${chip_root}/src/app/icd/server:icd-server-config", diff --git a/src/app/InteractionModelEngine.cpp b/src/app/InteractionModelEngine.cpp index 2fafe375f07e12..d4afe3347fe271 100644 --- a/src/app/InteractionModelEngine.cpp +++ b/src/app/InteractionModelEngine.cpp @@ -582,12 +582,14 @@ CHIP_ERROR InteractionModelEngine::ParseAttributePaths(const Access::SubjectDesc if (paramsList.mValue.IsWildcardPath()) { - AttributePathExpandIterator pathIterator(GetDataModelProvider(), ¶msList); + + auto state = AttributePathExpandIterator::Position::StartIterating(¶msList); + AttributePathExpandIterator pathIterator(GetDataModelProvider(), state); ConcreteAttributePath readPath; // The definition of "valid path" is "path exists and ACL allows access". The "path exists" part is handled by // AttributePathExpandIterator. So we just need to check the ACL bits. - for (; pathIterator.Get(readPath); pathIterator.Next()) + while (pathIterator.Next(readPath)) { // leave requestPath.entityId optional value unset to indicate wildcard Access::RequestPath requestPath{ .cluster = readPath.mClusterId, @@ -846,8 +848,7 @@ Protocols::InteractionModel::Status InteractionModelEngine::OnReadInitialRequest // We have already reserved enough resources for read requests, and have granted enough resources for current subscriptions, so // we should be able to allocate resources requested by this request. - ReadHandler * handler = - mReadHandlers.CreateObject(*this, apExchangeContext, aInteractionType, mReportScheduler, GetDataModelProvider()); + ReadHandler * handler = mReadHandlers.CreateObject(*this, apExchangeContext, aInteractionType, mReportScheduler); if (handler == nullptr) { ChipLogProgress(InteractionModel, "no resource for %s interaction", diff --git a/src/app/ReadHandler.cpp b/src/app/ReadHandler.cpp index 43f9b9310df848..dbde46bf23ca4e 100644 --- a/src/app/ReadHandler.cpp +++ b/src/app/ReadHandler.cpp @@ -15,14 +15,8 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - -/** - * @file - * This file defines read handler for a CHIP Interaction Data model - * - */ - #include +#include #include #include #include @@ -31,6 +25,7 @@ #include #include #include +#include #include #include @@ -54,9 +49,9 @@ uint16_t ReadHandler::GetPublisherSelectedIntervalLimit() } ReadHandler::ReadHandler(ManagementCallback & apCallback, Messaging::ExchangeContext * apExchangeContext, - InteractionType aInteractionType, Observer * observer, DataModel::Provider * apDataModel) : - mAttributePathExpandIterator(apDataModel, nullptr), - mExchangeCtx(*this), mManagementCallback(apCallback) + InteractionType aInteractionType, Observer * observer) : + mExchangeCtx(*this), + mManagementCallback(apCallback) { VerifyOrDie(apExchangeContext != nullptr); @@ -80,8 +75,8 @@ ReadHandler::ReadHandler(ManagementCallback & apCallback, Messaging::ExchangeCon } #if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS -ReadHandler::ReadHandler(ManagementCallback & apCallback, Observer * observer, DataModel::Provider * apDataModel) : - mAttributePathExpandIterator(apDataModel, nullptr), mExchangeCtx(*this), mManagementCallback(apCallback) +ReadHandler::ReadHandler(ManagementCallback & apCallback, Observer * observer) : + mExchangeCtx(*this), mManagementCallback(apCallback) { mInteractionType = InteractionType::Subscribe; mFlags.ClearAll(); @@ -511,8 +506,8 @@ CHIP_ERROR ReadHandler::ProcessAttributePaths(AttributePathIBs::Parser & aAttrib if (CHIP_END_OF_TLV == err) { mManagementCallback.GetInteractionModelEngine()->RemoveDuplicateConcreteAttributePath(mpAttributePathList); - mAttributePathExpandIterator.ResetTo(mpAttributePathList); - err = CHIP_NO_ERROR; + mAttributePathExpandPosition = AttributePathExpandIterator::Position::StartIterating(mpAttributePathList); + err = CHIP_NO_ERROR; } return err; } @@ -854,16 +849,18 @@ void ReadHandler::PersistSubscription() void ReadHandler::ResetPathIterator() { - mAttributePathExpandIterator.ResetTo(mpAttributePathList); + mAttributePathExpandPosition = AttributePathExpandIterator::Position::StartIterating(mpAttributePathList); mAttributeEncoderState.Reset(); } -void ReadHandler::AttributePathIsDirty(const AttributePathParams & aAttributeChanged) +void ReadHandler::AttributePathIsDirty(DataModel::Provider * apDataModel, const AttributePathParams & aAttributeChanged) { - ConcreteAttributePath path; - mDirtyGeneration = mManagementCallback.GetInteractionModelEngine()->GetReportingEngine().GetDirtySetGeneration(); + // We want to get the value, but not advance the iterator position. + AttributePathExpandIterator::Position tempPosition = mAttributePathExpandPosition; + ConcreteAttributePath path; + // We won't reset the path iterator for every AttributePathIsDirty call to reduce the number of full data reports. // The iterator will be reset after finishing each report session. // @@ -873,7 +870,7 @@ void ReadHandler::AttributePathIsDirty(const AttributePathParams & aAttributeCha // TODO (#16699): Currently we can only guarantee the reports generated from a single path in the request are consistent. The // data might be inconsistent if the user send a request with two paths from the same cluster. We need to clearify the behavior // or make it consistent. - if (mAttributePathExpandIterator.Get(path) && + if (AttributePathExpandIterator(apDataModel, tempPosition).Next(path) && (aAttributeChanged.HasWildcardEndpointId() || aAttributeChanged.mEndpointId == path.mEndpointId) && (aAttributeChanged.HasWildcardClusterId() || aAttributeChanged.mClusterId == path.mClusterId)) { @@ -883,7 +880,8 @@ void ReadHandler::AttributePathIsDirty(const AttributePathParams & aAttributeCha // If we're currently in the middle of generating reports for a given cluster and that in turn is marked dirty, let's reset // our iterator to point back to the beginning of that cluster. This ensures that the receiver will get a coherent view of // the state of the cluster as present on the server - mAttributePathExpandIterator.ResetCurrentCluster(); + mAttributePathExpandPosition.IterateFromTheStartOfTheCurrentClusterIfAttributeWildcard(); + mAttributeEncoderState.Reset(); } diff --git a/src/app/ReadHandler.h b/src/app/ReadHandler.h index 85976ec6bc9e62..d5b7ad2222189d 100644 --- a/src/app/ReadHandler.h +++ b/src/app/ReadHandler.h @@ -212,7 +212,7 @@ class ReadHandler : public Messaging::ExchangeDelegate * */ ReadHandler(ManagementCallback & apCallback, Messaging::ExchangeContext * apExchangeContext, InteractionType aInteractionType, - Observer * observer, DataModel::Provider * apDataModel); + Observer * observer); #if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS /** @@ -222,7 +222,7 @@ class ReadHandler : public Messaging::ExchangeDelegate * The callback passed in has to outlive this handler object. * */ - ReadHandler(ManagementCallback & apCallback, Observer * observer, DataModel::Provider * apDataModel); + ReadHandler(ManagementCallback & apCallback, Observer * observer); #endif const SingleLinkedListNode * GetAttributePathList() const { return mpAttributePathList; } @@ -407,12 +407,12 @@ class ReadHandler : public Messaging::ExchangeDelegate bool IsFabricFiltered() const { return mFlags.Has(ReadHandlerFlags::FabricFiltered); } CHIP_ERROR OnSubscribeRequest(Messaging::ExchangeContext * apExchangeContext, System::PacketBufferHandle && aPayload); void GetSubscriptionId(SubscriptionId & aSubscriptionId) const { aSubscriptionId = mSubscriptionId; } - AttributePathExpandIterator * GetAttributePathExpandIterator() { return &mAttributePathExpandIterator; } + AttributePathExpandIterator::Position & AttributeIterationPosition() { return mAttributePathExpandPosition; } /// @brief Notifies the read handler that a set of attribute paths has been marked dirty. This will schedule a reporting engine /// run if the change to the attribute path makes the ReadHandler reportable. /// @param aAttributeChanged Path to the attribute that was changed. - void AttributePathIsDirty(const AttributePathParams & aAttributeChanged); + void AttributePathIsDirty(DataModel::Provider * apDataModel, const AttributePathParams & aAttributeChanged); bool IsDirty() const { return (mDirtyGeneration > mPreviousReportsBeginGeneration) || mFlags.Has(ReadHandlerFlags::ForceDirty); @@ -519,7 +519,7 @@ class ReadHandler : public Messaging::ExchangeDelegate /// @param aFlag Flag to clear void ClearStateFlag(ReadHandlerFlags aFlag); - AttributePathExpandIterator mAttributePathExpandIterator; + SubscriptionId mSubscriptionId = 0; // The current generation of the reporting engine dirty set the last time we were notified that a path we're interested in was // marked dirty. @@ -561,18 +561,13 @@ class ReadHandler : public Messaging::ExchangeDelegate // engine, the "oldest" subscription is the subscription with the smallest generation. uint64_t mTransactionStartGeneration = 0; - SubscriptionId mSubscriptionId = 0; - uint16_t mMinIntervalFloorSeconds = 0; - uint16_t mMaxInterval = 0; - uint16_t mSubscriberRequestedMaxInterval = 0; - EventNumber mEventMin = 0; // The last schedule event number snapshoted in the beginning when preparing to fill new events to reports EventNumber mLastScheduledEventNumber = 0; - // TODO: We should shutdown the transaction when the session expires. - SessionHolder mSessionHandle; + /// Iterator position state for any ongoing path expansion for handling wildcard reads/subscriptions. + AttributePathExpandIterator::Position mAttributePathExpandPosition; Messaging::ExchangeHolder mExchangeCtx; #if CHIP_CONFIG_UNSAFE_SUBSCRIPTION_EXCHANGE_MANAGER_USE @@ -587,20 +582,26 @@ class ReadHandler : public Messaging::ExchangeDelegate ManagementCallback & mManagementCallback; + // TODO (#27675): Merge all observers into one and that one will dispatch the callbacks to the right place. + Observer * mObserver = nullptr; + uint32_t mLastWrittenEventsBytes = 0; // The detailed encoding state for a single attribute, used by list chunking feature. // The size of AttributeEncoderState is 2 bytes for now. AttributeEncodeState mAttributeEncoderState; + uint16_t mMinIntervalFloorSeconds = 0; + uint16_t mMaxInterval = 0; + uint16_t mSubscriberRequestedMaxInterval = 0; + // Current Handler state HandlerState mState = HandlerState::Idle; PriorityLevel mCurrentPriority = PriorityLevel::Invalid; BitFlags mFlags; InteractionType mInteractionType = InteractionType::Read; - // TODO (#27675): Merge all observers into one and that one will dispatch the callbacks to the right place. - Observer * mObserver = nullptr; + SessionHolder mSessionHandle; }; } // namespace app diff --git a/src/app/SubscriptionResumptionSessionEstablisher.cpp b/src/app/SubscriptionResumptionSessionEstablisher.cpp index f9ea25cb58dc93..6e016684410fc8 100644 --- a/src/app/SubscriptionResumptionSessionEstablisher.cpp +++ b/src/app/SubscriptionResumptionSessionEstablisher.cpp @@ -103,8 +103,7 @@ void SubscriptionResumptionSessionEstablisher::HandleDeviceConnected(void * cont ChipLogProgress(InteractionModel, "no resource for subscription resumption"); return; } - ReadHandler * readHandler = - imEngine->mReadHandlers.CreateObject(*imEngine, imEngine->GetReportScheduler(), imEngine->GetDataModelProvider()); + ReadHandler * readHandler = imEngine->mReadHandlers.CreateObject(*imEngine, imEngine->GetReportScheduler()); if (readHandler == nullptr) { // TODO - Should we keep the subscription here? diff --git a/src/app/clusters/valve-configuration-and-control-server/valve-configuration-and-control-server.cpp b/src/app/clusters/valve-configuration-and-control-server/valve-configuration-and-control-server.cpp index fd89ab135d384c..7ebeebc9de78fb 100644 --- a/src/app/clusters/valve-configuration-and-control-server/valve-configuration-and-control-server.cpp +++ b/src/app/clusters/valve-configuration-and-control-server/valve-configuration-and-control-server.cpp @@ -309,7 +309,6 @@ CHIP_ERROR CloseValve(EndpointId ep) CHIP_ERROR SetValveLevel(EndpointId ep, DataModel::Nullable level, DataModel::Nullable openDuration) { Delegate * delegate = GetDelegate(ep); - Optional status = Optional::Missing(); CHIP_ERROR attribute_error = CHIP_IM_GLOBAL_STATUS(UnsupportedAttribute); if (HasFeature(ep, ValveConfigurationAndControl::Feature::kTimeSync)) diff --git a/src/app/reporting/Engine.cpp b/src/app/reporting/Engine.cpp index a9cef5c27dc333..ed23d44b055434 100644 --- a/src/app/reporting/Engine.cpp +++ b/src/app/reporting/Engine.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -315,8 +316,9 @@ CHIP_ERROR Engine::BuildSingleReportDataAttributeReportIBs(ReportDataMessage::Bu #endif // For each path included in the interested path of the read handler... - for (; apReadHandler->GetAttributePathExpandIterator()->Get(readPath); - apReadHandler->GetAttributePathExpandIterator()->Next()) + for (RollbackAttributePathExpandIterator iterator(mpImEngine->GetDataModelProvider(), + apReadHandler->AttributeIterationPosition()); + iterator.Next(readPath); iterator.MarkCompleted()) { if (!apReadHandler->IsPriming()) { @@ -428,6 +430,7 @@ CHIP_ERROR Engine::BuildSingleReportDataAttributeReportIBs(ReportDataMessage::Bu // Successfully encoded the attribute, clear the internal state. apReadHandler->SetAttributeEncodeState(AttributeEncodeState()); } + // We just visited all paths interested by this read handler and did not abort in the middle of iteration, there are no more // chunks for this report. hasMoreChunks = false; @@ -1042,8 +1045,9 @@ CHIP_ERROR Engine::SetDirty(const AttributePathParams & aAttributePath) { BumpDirtySetGeneration(); - bool intersectsInterestPath = false; - mpImEngine->mReadHandlers.ForEachActiveObject([&aAttributePath, &intersectsInterestPath](ReadHandler * handler) { + bool intersectsInterestPath = false; + DataModel::Provider * dataModel = mpImEngine->GetDataModelProvider(); + mpImEngine->mReadHandlers.ForEachActiveObject([&dataModel, &aAttributePath, &intersectsInterestPath](ReadHandler * handler) { // We call AttributePathIsDirty for both read interactions and subscribe interactions, since we may send inconsistent // attribute data between two chunks. AttributePathIsDirty will not schedule a new run for read handlers which are // waiting for a response to the last message chunk for read interactions. @@ -1053,7 +1057,7 @@ CHIP_ERROR Engine::SetDirty(const AttributePathParams & aAttributePath) { if (object->mValue.Intersects(aAttributePath)) { - handler->AttributePathIsDirty(aAttributePath); + handler->AttributePathIsDirty(dataModel, aAttributePath); intersectsInterestPath = true; break; } diff --git a/src/app/server/DefaultTermsAndConditionsProvider.cpp b/src/app/server/DefaultTermsAndConditionsProvider.cpp index 36431aea456128..4a05a7d3ff8b87 100644 --- a/src/app/server/DefaultTermsAndConditionsProvider.cpp +++ b/src/app/server/DefaultTermsAndConditionsProvider.cpp @@ -211,7 +211,9 @@ CHIP_ERROR chip::app::DefaultTermsAndConditionsProvider::GetAcknowledgementsRequ TermsAndConditions requiredTermsAndConditions = requiredTermsAndConditionsMaybe.Value(); TermsAndConditions acceptedTermsAndConditions = acceptedTermsAndConditionsMaybe.Value(); - outAcknowledgementsRequired = requiredTermsAndConditions.Validate(acceptedTermsAndConditions); + + bool requiredTermsAndConditionsAreAccepted = requiredTermsAndConditions.Validate(acceptedTermsAndConditions); + outAcknowledgementsRequired = !requiredTermsAndConditionsAreAccepted; return CHIP_NO_ERROR; } diff --git a/src/app/tests/TestAttributePathExpandIterator.cpp b/src/app/tests/TestAttributePathExpandIterator.cpp index 913b07e86fb829..4a6088a8196161 100644 --- a/src/app/tests/TestAttributePathExpandIterator.cpp +++ b/src/app/tests/TestAttributePathExpandIterator.cpp @@ -16,6 +16,7 @@ * limitations under the License. */ +#include "pw_unit_test/framework.h" #include #include #include @@ -106,9 +107,18 @@ TEST(TestAttributePathExpandIterator, TestAllWildcard) size_t index = 0; - for (app::AttributePathExpandIterator iter(CodegenDataModelProviderInstance(nullptr /* delegate */), &clusInfo); iter.Get(path); - iter.Next()) + auto position = AttributePathExpandIterator::Position::StartIterating(&clusInfo); + + while (true) { + // re-create the iterator + app::AttributePathExpandIterator iter(CodegenDataModelProviderInstance(nullptr /* delegate */), position); + + if (!iter.Next(path)) + { + break; + } + ChipLogDetail(AppServer, "Visited Attribute: 0x%04X / " ChipLogFormatMEI " / " ChipLogFormatMEI, path.mEndpointId, ChipLogValueMEI(path.mClusterId), ChipLogValueMEI(path.mAttributeId)); EXPECT_LT(index, ArraySize(paths)); @@ -131,9 +141,16 @@ TEST(TestAttributePathExpandIterator, TestWildcardEndpoint) size_t index = 0; - for (app::AttributePathExpandIterator iter(CodegenDataModelProviderInstance(nullptr /* delegate */), &clusInfo); iter.Get(path); - iter.Next()) + auto position = AttributePathExpandIterator::Position::StartIterating(&clusInfo); + while (true) { + // re-create the iterator + app::AttributePathExpandIterator iter(CodegenDataModelProviderInstance(nullptr /* delegate */), position); + + if (!iter.Next(path)) + { + break; + } ChipLogDetail(AppServer, "Visited Attribute: 0x%04X / " ChipLogFormatMEI " / " ChipLogFormatMEI, path.mEndpointId, ChipLogValueMEI(path.mClusterId), ChipLogValueMEI(path.mAttributeId)); EXPECT_LT(index, ArraySize(paths)); @@ -159,9 +176,16 @@ TEST(TestAttributePathExpandIterator, TestWildcardCluster) size_t index = 0; - for (app::AttributePathExpandIterator iter(CodegenDataModelProviderInstance(nullptr /* delegate */), &clusInfo); iter.Get(path); - iter.Next()) + auto position = AttributePathExpandIterator::Position::StartIterating(&clusInfo); + while (true) { + // re-create the iterator + app::AttributePathExpandIterator iter(CodegenDataModelProviderInstance(nullptr /* delegate */), position); + + if (!iter.Next(path)) + { + break; + } ChipLogDetail(AppServer, "Visited Attribute: 0x%04X / " ChipLogFormatMEI " / " ChipLogFormatMEI, path.mEndpointId, ChipLogValueMEI(path.mClusterId), ChipLogValueMEI(path.mAttributeId)); EXPECT_LT(index, ArraySize(paths)); @@ -187,9 +211,17 @@ TEST(TestAttributePathExpandIterator, TestWildcardClusterGlobalAttributeNotInMet size_t index = 0; - for (app::AttributePathExpandIterator iter(CodegenDataModelProviderInstance(nullptr /* delegate */), &clusInfo); iter.Get(path); - iter.Next()) + auto position = AttributePathExpandIterator::Position::StartIterating(&clusInfo); + + while (true) { + // re-create the iterator + app::AttributePathExpandIterator iter(CodegenDataModelProviderInstance(nullptr /* delegate */), position); + + if (!iter.Next(path)) + { + break; + } ChipLogDetail(AppServer, "Visited Attribute: 0x%04X / " ChipLogFormatMEI " / " ChipLogFormatMEI, path.mEndpointId, ChipLogValueMEI(path.mClusterId), ChipLogValueMEI(path.mAttributeId)); EXPECT_LT(index, ArraySize(paths)); @@ -219,9 +251,17 @@ TEST(TestAttributePathExpandIterator, TestWildcardAttribute) size_t index = 0; - for (app::AttributePathExpandIterator iter(CodegenDataModelProviderInstance(nullptr /* delegate */), &clusInfo); iter.Get(path); - iter.Next()) + auto position = AttributePathExpandIterator::Position::StartIterating(&clusInfo); + + while (true) { + // re-create the iterator + app::AttributePathExpandIterator iter(CodegenDataModelProviderInstance(nullptr /* delegate */), position); + + if (!iter.Next(path)) + { + break; + } ChipLogDetail(AppServer, "Visited Attribute: 0x%04X / " ChipLogFormatMEI " / " ChipLogFormatMEI, path.mEndpointId, ChipLogValueMEI(path.mClusterId), ChipLogValueMEI(path.mAttributeId)); EXPECT_LT(index, ArraySize(paths)); @@ -245,9 +285,16 @@ TEST(TestAttributePathExpandIterator, TestNoWildcard) size_t index = 0; - for (app::AttributePathExpandIterator iter(CodegenDataModelProviderInstance(nullptr /* delegate */), &clusInfo); iter.Get(path); - iter.Next()) + auto position = AttributePathExpandIterator::Position::StartIterating(&clusInfo); + while (true) { + // re-create the iterator + app::AttributePathExpandIterator iter(CodegenDataModelProviderInstance(nullptr /* delegate */), position); + + if (!iter.Next(path)) + { + break; + } ChipLogDetail(AppServer, "Visited Attribute: 0x%04X / " ChipLogFormatMEI " / " ChipLogFormatMEI, path.mEndpointId, ChipLogValueMEI(path.mClusterId), ChipLogValueMEI(path.mAttributeId)); EXPECT_LT(index, ArraySize(paths)); @@ -257,6 +304,69 @@ TEST(TestAttributePathExpandIterator, TestNoWildcard) EXPECT_EQ(index, ArraySize(paths)); } +TEST(TestAttributePathExpandIterator, TestFixedPathExpansion) +{ + // expansion logic requires that: + // - paths for wildcard expansion ARE VALIDATED + // - path WITHOUT wildcard expansion ARE NOT VALIDATED + + // invalid attribute across all clusters returns empty + { + SingleLinkedListNode clusInfo; + clusInfo.mValue.mAttributeId = 122333; + + auto position = AttributePathExpandIterator::Position::StartIterating(&clusInfo); + app::AttributePathExpandIterator iter(CodegenDataModelProviderInstance(nullptr /* delegate */), position); + ConcreteAttributePath path; + + EXPECT_FALSE(iter.Next(path)); + } + + // invalid cluster with a valid attribute (featuremap) returns empty + { + SingleLinkedListNode clusInfo; + clusInfo.mValue.mClusterId = 122344; + clusInfo.mValue.mAttributeId = Clusters::Globals::Attributes::FeatureMap::Id; + + auto position = AttributePathExpandIterator::Position::StartIterating(&clusInfo); + app::AttributePathExpandIterator iter(CodegenDataModelProviderInstance(nullptr /* delegate */), position); + ConcreteAttributePath path; + + EXPECT_FALSE(iter.Next(path)); + } + + // invalid cluster with wildcard attribute returns empty + { + SingleLinkedListNode clusInfo; + clusInfo.mValue.mClusterId = 122333; + + auto position = AttributePathExpandIterator::Position::StartIterating(&clusInfo); + app::AttributePathExpandIterator iter(CodegenDataModelProviderInstance(nullptr /* delegate */), position); + ConcreteAttributePath path; + + EXPECT_FALSE(iter.Next(path)); + } + + // even though all above WERE invalid, if we specify a non-wildcard path it is returned as-is + { + SingleLinkedListNode clusInfo; + clusInfo.mValue.mEndpointId = 1; + clusInfo.mValue.mClusterId = 122344; + clusInfo.mValue.mAttributeId = 122333; + + auto position = AttributePathExpandIterator::Position::StartIterating(&clusInfo); + app::AttributePathExpandIterator iter(CodegenDataModelProviderInstance(nullptr /* delegate */), position); + ConcreteAttributePath path; + + EXPECT_TRUE(iter.Next(path)); + EXPECT_EQ(path.mEndpointId, clusInfo.mValue.mEndpointId); + EXPECT_EQ(path.mClusterId, clusInfo.mValue.mClusterId); + EXPECT_EQ(path.mAttributeId, clusInfo.mValue.mAttributeId); + + EXPECT_FALSE(iter.Next(path)); + } +} + TEST(TestAttributePathExpandIterator, TestMultipleClusInfo) { @@ -358,18 +468,47 @@ TEST(TestAttributePathExpandIterator, TestMultipleClusInfo) { kMockEndpoint2, MockClusterId(3), MockAttributeId(3) }, }; - size_t index = 0; + // Test that a one-shot iterate through all works + { + size_t index = 0; + + auto position = AttributePathExpandIterator::Position::StartIterating(&clusInfo1); + app::AttributePathExpandIterator iter(CodegenDataModelProviderInstance(nullptr /* delegate */), position); + + while (iter.Next(path)) + { + ChipLogDetail(AppServer, "Visited Attribute: 0x%04X / " ChipLogFormatMEI " / " ChipLogFormatMEI, path.mEndpointId, + ChipLogValueMEI(path.mClusterId), ChipLogValueMEI(path.mAttributeId)); + EXPECT_LT(index, ArraySize(paths)); + EXPECT_EQ(paths[index], path); + index++; + } + EXPECT_EQ(index, ArraySize(paths)); + } - for (app::AttributePathExpandIterator iter(CodegenDataModelProviderInstance(nullptr /* delegate */), &clusInfo1); - iter.Get(path); iter.Next()) + // identical test, however this checks that position re-use works + // the same as a one-shot iteration. { - ChipLogDetail(AppServer, "Visited Attribute: 0x%04X / " ChipLogFormatMEI " / " ChipLogFormatMEI, path.mEndpointId, - ChipLogValueMEI(path.mClusterId), ChipLogValueMEI(path.mAttributeId)); - EXPECT_LT(index, ArraySize(paths)); - EXPECT_EQ(paths[index], path); - index++; + size_t index = 0; + + auto position = AttributePathExpandIterator::Position::StartIterating(&clusInfo1); + while (true) + { + // re-create the iterator + app::AttributePathExpandIterator iter(CodegenDataModelProviderInstance(nullptr /* delegate */), position); + + if (!iter.Next(path)) + { + break; + } + ChipLogDetail(AppServer, "Visited Attribute: 0x%04X / " ChipLogFormatMEI " / " ChipLogFormatMEI, path.mEndpointId, + ChipLogValueMEI(path.mClusterId), ChipLogValueMEI(path.mAttributeId)); + EXPECT_LT(index, ArraySize(paths)); + EXPECT_EQ(paths[index], path); + index++; + } + EXPECT_EQ(index, ArraySize(paths)); } - EXPECT_EQ(index, ArraySize(paths)); } } // namespace diff --git a/src/app/tests/TestInteractionModelEngine.cpp b/src/app/tests/TestInteractionModelEngine.cpp index 87aa3e3c64f58b..d09f5647d6e275 100644 --- a/src/app/tests/TestInteractionModelEngine.cpp +++ b/src/app/tests/TestInteractionModelEngine.cpp @@ -271,8 +271,7 @@ TEST_F_FROM_FIXTURE(TestInteractionModelEngine, TestSubjectHasActiveSubscription // Create and setup readHandler 1 ReadHandler * readHandler1 = engine->GetReadHandlerPool().CreateObject( - nullCallback, exchangeCtx1, ReadHandler::InteractionType::Subscribe, reporting::GetDefaultReportScheduler(), - CodegenDataModelProviderInstance(nullptr /* delegate */)); + nullCallback, exchangeCtx1, ReadHandler::InteractionType::Subscribe, reporting::GetDefaultReportScheduler()); // Verify that Bob still doesn't have an active subscription EXPECT_FALSE(engine->SubjectHasActiveSubscription(bobFabricIndex, bobNodeId)); @@ -319,16 +318,14 @@ TEST_F_FROM_FIXTURE(TestInteractionModelEngine, TestSubjectHasActiveSubscription // Create readHandler 1 engine->GetReadHandlerPool().CreateObject(nullCallback, exchangeCtx1, ReadHandler::InteractionType::Subscribe, - reporting::GetDefaultReportScheduler(), - CodegenDataModelProviderInstance(nullptr /* delegate */)); + reporting::GetDefaultReportScheduler()); // Verify that Bob still doesn't have an active subscription EXPECT_FALSE(engine->SubjectHasActiveSubscription(bobFabricIndex, bobNodeId)); // Create and setup readHandler 2 ReadHandler * readHandler2 = engine->GetReadHandlerPool().CreateObject( - nullCallback, exchangeCtx2, ReadHandler::InteractionType::Subscribe, reporting::GetDefaultReportScheduler(), - CodegenDataModelProviderInstance(nullptr /* delegate */)); + nullCallback, exchangeCtx2, ReadHandler::InteractionType::Subscribe, reporting::GetDefaultReportScheduler()); // Verify that Bob still doesn't have an active subscription EXPECT_FALSE(engine->SubjectHasActiveSubscription(bobFabricIndex, bobNodeId)); @@ -380,13 +377,11 @@ TEST_F_FROM_FIXTURE(TestInteractionModelEngine, TestSubjectHasActiveSubscription // Create and setup readHandler 1 ReadHandler * readHandler1 = engine->GetReadHandlerPool().CreateObject( - nullCallback, exchangeCtx1, ReadHandler::InteractionType::Subscribe, reporting::GetDefaultReportScheduler(), - CodegenDataModelProviderInstance(nullptr /* delegate */)); + nullCallback, exchangeCtx1, ReadHandler::InteractionType::Subscribe, reporting::GetDefaultReportScheduler()); // Create and setup readHandler 2 ReadHandler * readHandler2 = engine->GetReadHandlerPool().CreateObject( - nullCallback, exchangeCtx2, ReadHandler::InteractionType::Subscribe, reporting::GetDefaultReportScheduler(), - CodegenDataModelProviderInstance(nullptr /* delegate */)); + nullCallback, exchangeCtx2, ReadHandler::InteractionType::Subscribe, reporting::GetDefaultReportScheduler()); // Verify that Bob still doesn't have an active subscription EXPECT_FALSE(engine->SubjectHasActiveSubscription(bobFabricIndex, bobNodeId)); @@ -463,23 +458,19 @@ TEST_F_FROM_FIXTURE(TestInteractionModelEngine, TestSubjectHasActiveSubscription // Create and setup readHandler 1-1 engine->GetReadHandlerPool().CreateObject(nullCallback, exchangeCtx11, ReadHandler::InteractionType::Subscribe, - reporting::GetDefaultReportScheduler(), - CodegenDataModelProviderInstance(nullptr /* delegate */)); + reporting::GetDefaultReportScheduler()); // Create and setup readHandler 1-2 ReadHandler * readHandler12 = engine->GetReadHandlerPool().CreateObject( - nullCallback, exchangeCtx12, ReadHandler::InteractionType::Subscribe, reporting::GetDefaultReportScheduler(), - CodegenDataModelProviderInstance(nullptr /* delegate */)); + nullCallback, exchangeCtx12, ReadHandler::InteractionType::Subscribe, reporting::GetDefaultReportScheduler()); // Create and setup readHandler 2-1 engine->GetReadHandlerPool().CreateObject(nullCallback, exchangeCtx21, ReadHandler::InteractionType::Subscribe, - reporting::GetDefaultReportScheduler(), - CodegenDataModelProviderInstance(nullptr /* delegate */)); + reporting::GetDefaultReportScheduler()); // Create and setup readHandler 2-2 ReadHandler * readHandler22 = engine->GetReadHandlerPool().CreateObject( - nullCallback, exchangeCtx22, ReadHandler::InteractionType::Subscribe, reporting::GetDefaultReportScheduler(), - CodegenDataModelProviderInstance(nullptr /* delegate */)); + nullCallback, exchangeCtx22, ReadHandler::InteractionType::Subscribe, reporting::GetDefaultReportScheduler()); // Verify that both Alice and Bob have no active subscriptions EXPECT_FALSE(engine->SubjectHasActiveSubscription(bobFabricIndex, bobNodeId)); @@ -551,8 +542,7 @@ TEST_F_FROM_FIXTURE(TestInteractionModelEngine, TestSubjectHasActiveSubscription // Create readHandler ReadHandler * readHandler = engine->GetReadHandlerPool().CreateObject( - nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe, reporting::GetDefaultReportScheduler(), - CodegenDataModelProviderInstance(nullptr /* delegate */)); + nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe, reporting::GetDefaultReportScheduler()); // Verify there are not active subscriptions EXPECT_FALSE(engine->SubjectHasActiveSubscription(bobFabricIndex, valideSubjectId)); @@ -749,8 +739,7 @@ TEST_F_FROM_FIXTURE(TestInteractionModelEngine, TestFabricHasAtLeastOneActiveSub // Create and setup readHandler 1 ReadHandler * readHandler1 = engine->GetReadHandlerPool().CreateObject( - nullCallback, exchangeCtx1, ReadHandler::InteractionType::Subscribe, reporting::GetDefaultReportScheduler(), - CodegenDataModelProviderInstance(nullptr /* delegate */)); + nullCallback, exchangeCtx1, ReadHandler::InteractionType::Subscribe, reporting::GetDefaultReportScheduler()); // Verify that fabric 1 still doesn't have an active subscription EXPECT_FALSE(engine->FabricHasAtLeastOneActiveSubscription(fabricIndex1)); @@ -766,8 +755,7 @@ TEST_F_FROM_FIXTURE(TestInteractionModelEngine, TestFabricHasAtLeastOneActiveSub // Create and setup readHandler 2 ReadHandler * readHandler2 = engine->GetReadHandlerPool().CreateObject( - nullCallback, exchangeCtx2, ReadHandler::InteractionType::Subscribe, reporting::GetDefaultReportScheduler(), - CodegenDataModelProviderInstance(nullptr /* delegate */)); + nullCallback, exchangeCtx2, ReadHandler::InteractionType::Subscribe, reporting::GetDefaultReportScheduler()); // Set readHandler2 to active readHandler2->SetStateFlag(ReadHandler::ReadHandlerFlags::ActiveSubscription, true); @@ -805,8 +793,7 @@ TEST_F_FROM_FIXTURE(TestInteractionModelEngine, TestFabricHasAtLeastOneActiveSub // Create and setup readHandler 1 ReadHandler * readHandler1 = engine->GetReadHandlerPool().CreateObject( - nullCallback, exchangeCtx1, ReadHandler::InteractionType::Subscribe, reporting::GetDefaultReportScheduler(), - CodegenDataModelProviderInstance(nullptr /* delegate */)); + nullCallback, exchangeCtx1, ReadHandler::InteractionType::Subscribe, reporting::GetDefaultReportScheduler()); // Verify that the fabric still doesn't have an active subscription EXPECT_FALSE(engine->FabricHasAtLeastOneActiveSubscription(fabricIndex)); @@ -819,8 +806,7 @@ TEST_F_FROM_FIXTURE(TestInteractionModelEngine, TestFabricHasAtLeastOneActiveSub // Create and setup readHandler 2 ReadHandler * readHandler2 = engine->GetReadHandlerPool().CreateObject( - nullCallback, exchangeCtx2, ReadHandler::InteractionType::Subscribe, reporting::GetDefaultReportScheduler(), - CodegenDataModelProviderInstance(nullptr /* delegate */)); + nullCallback, exchangeCtx2, ReadHandler::InteractionType::Subscribe, reporting::GetDefaultReportScheduler()); // Verify that the fabric still has an active subscription EXPECT_TRUE(engine->FabricHasAtLeastOneActiveSubscription(fabricIndex)); diff --git a/src/app/tests/TestReadInteraction.cpp b/src/app/tests/TestReadInteraction.cpp index 21b15d1a808048..b56db3e82dc1b5 100644 --- a/src/app/tests/TestReadInteraction.cpp +++ b/src/app/tests/TestReadInteraction.cpp @@ -565,8 +565,7 @@ void TestReadInteraction::TestReadHandler() { Messaging::ExchangeContext * exchangeCtx = NewExchangeToAlice(nullptr, false); - ReadHandler readHandler(nullCallback, exchangeCtx, chip::app::ReadHandler::InteractionType::Read, gReportScheduler, - CodegenDataModelProviderInstance(nullptr /* delegate */)); + ReadHandler readHandler(nullCallback, exchangeCtx, chip::app::ReadHandler::InteractionType::Read, gReportScheduler); GenerateReportData(reportDatabuf, ReportType::kValid, false /* aSuppressResponse*/); EXPECT_EQ(readHandler.SendReportData(std::move(reportDatabuf), false), CHIP_ERROR_INCORRECT_STATE); @@ -624,8 +623,7 @@ void TestReadInteraction::TestReadHandlerSetMaxReportingInterval() uint16_t maxInterval; // Configure ReadHandler - ReadHandler readHandler(*engine, exchangeCtx, chip::app::ReadHandler::InteractionType::Read, gReportScheduler, - CodegenDataModelProviderInstance(nullptr /* delegate */)); + ReadHandler readHandler(*engine, exchangeCtx, chip::app::ReadHandler::InteractionType::Read, gReportScheduler); writer.Init(std::move(subscribeRequestbuf)); EXPECT_EQ(subscribeRequestBuilder.Init(&writer), CHIP_NO_ERROR); @@ -842,8 +840,7 @@ void TestReadInteraction::TestReadHandlerInvalidAttributePath() { Messaging::ExchangeContext * exchangeCtx = NewExchangeToAlice(nullptr, false); - ReadHandler readHandler(nullCallback, exchangeCtx, chip::app::ReadHandler::InteractionType::Read, gReportScheduler, - CodegenDataModelProviderInstance(nullptr /* delegate */)); + ReadHandler readHandler(nullCallback, exchangeCtx, chip::app::ReadHandler::InteractionType::Read, gReportScheduler); GenerateReportData(reportDatabuf, ReportType::kValid, false /* aSuppressResponse*/); EXPECT_EQ(readHandler.SendReportData(std::move(reportDatabuf), false), CHIP_ERROR_INCORRECT_STATE); @@ -1592,8 +1589,7 @@ void TestReadInteraction::TestProcessSubscribeRequest() Messaging::ExchangeContext * exchangeCtx = NewExchangeToAlice(nullptr, false); { - ReadHandler readHandler(*engine, exchangeCtx, chip::app::ReadHandler::InteractionType::Read, gReportScheduler, - CodegenDataModelProviderInstance(nullptr /* delegate */)); + ReadHandler readHandler(*engine, exchangeCtx, chip::app::ReadHandler::InteractionType::Read, gReportScheduler); writer.Init(std::move(subscribeRequestbuf)); EXPECT_EQ(subscribeRequestBuilder.Init(&writer), CHIP_NO_ERROR); @@ -1654,8 +1650,7 @@ void TestReadInteraction::TestICDProcessSubscribeRequestSupMaxIntervalCeiling() Messaging::ExchangeContext * exchangeCtx = NewExchangeToAlice(nullptr, false); { - ReadHandler readHandler(*engine, exchangeCtx, chip::app::ReadHandler::InteractionType::Read, gReportScheduler, - CodegenDataModelProviderInstance(nullptr /* delegate */)); + ReadHandler readHandler(*engine, exchangeCtx, chip::app::ReadHandler::InteractionType::Read, gReportScheduler); writer.Init(std::move(subscribeRequestbuf)); EXPECT_EQ(subscribeRequestBuilder.Init(&writer), CHIP_NO_ERROR); @@ -1724,8 +1719,7 @@ void TestReadInteraction::TestICDProcessSubscribeRequestInfMaxIntervalCeiling() Messaging::ExchangeContext * exchangeCtx = NewExchangeToAlice(nullptr, false); { - ReadHandler readHandler(*engine, exchangeCtx, chip::app::ReadHandler::InteractionType::Read, gReportScheduler, - CodegenDataModelProviderInstance(nullptr /* delegate */)); + ReadHandler readHandler(*engine, exchangeCtx, chip::app::ReadHandler::InteractionType::Read, gReportScheduler); writer.Init(std::move(subscribeRequestbuf)); EXPECT_EQ(subscribeRequestBuilder.Init(&writer), CHIP_NO_ERROR); @@ -1794,8 +1788,7 @@ void TestReadInteraction::TestICDProcessSubscribeRequestSupMinInterval() Messaging::ExchangeContext * exchangeCtx = NewExchangeToAlice(nullptr, false); { - ReadHandler readHandler(*engine, exchangeCtx, chip::app::ReadHandler::InteractionType::Read, gReportScheduler, - CodegenDataModelProviderInstance(nullptr /* delegate */)); + ReadHandler readHandler(*engine, exchangeCtx, chip::app::ReadHandler::InteractionType::Read, gReportScheduler); writer.Init(std::move(subscribeRequestbuf)); EXPECT_EQ(subscribeRequestBuilder.Init(&writer), CHIP_NO_ERROR); @@ -1864,8 +1857,7 @@ void TestReadInteraction::TestICDProcessSubscribeRequestMaxMinInterval() Messaging::ExchangeContext * exchangeCtx = NewExchangeToAlice(nullptr, false); { - ReadHandler readHandler(*engine, exchangeCtx, chip::app::ReadHandler::InteractionType::Read, gReportScheduler, - CodegenDataModelProviderInstance(nullptr /* delegate */)); + ReadHandler readHandler(*engine, exchangeCtx, chip::app::ReadHandler::InteractionType::Read, gReportScheduler); writer.Init(std::move(subscribeRequestbuf)); EXPECT_EQ(subscribeRequestBuilder.Init(&writer), CHIP_NO_ERROR); @@ -1932,8 +1924,7 @@ void TestReadInteraction::TestICDProcessSubscribeRequestInvalidIdleModeDuration( Messaging::ExchangeContext * exchangeCtx = NewExchangeToAlice(nullptr, false); { - ReadHandler readHandler(*engine, exchangeCtx, chip::app::ReadHandler::InteractionType::Read, gReportScheduler, - CodegenDataModelProviderInstance(nullptr /* delegate */)); + ReadHandler readHandler(*engine, exchangeCtx, chip::app::ReadHandler::InteractionType::Read, gReportScheduler); writer.Init(std::move(subscribeRequestbuf)); EXPECT_EQ(subscribeRequestBuilder.Init(&writer), CHIP_NO_ERROR); diff --git a/src/app/tests/TestReportScheduler.cpp b/src/app/tests/TestReportScheduler.cpp index 03dda963aef6ba..7047dd6191be3f 100644 --- a/src/app/tests/TestReportScheduler.cpp +++ b/src/app/tests/TestReportScheduler.cpp @@ -276,8 +276,7 @@ TEST_F_FROM_FIXTURE(TestReportScheduler, TestReadHandlerList) for (size_t i = 0; i < kNumMaxReadHandlers; i++) { ReadHandler * readHandler = - readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe, &sScheduler, - CodegenDataModelProviderInstance(nullptr /* delegate */)); + readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe, &sScheduler); sScheduler.OnSubscriptionEstablished(readHandler); ASSERT_NE(nullptr, readHandler); ASSERT_NE(nullptr, sScheduler.FindReadHandlerNode(readHandler)); @@ -341,22 +340,19 @@ TEST_F_FROM_FIXTURE(TestReportScheduler, TestReportTiming) // Dirty read handler, will be triggered at min interval // Test OnReadHandler created ReadHandler * readHandler1 = - readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe, &sScheduler, - CodegenDataModelProviderInstance(nullptr /* delegate */)); + readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe, &sScheduler); EXPECT_EQ(CHIP_NO_ERROR, MockReadHandlerSubscriptionTransaction(readHandler1, &sScheduler, 1, 2)); readHandler1->ForceDirtyState(); // Clean read handler, will be triggered at max interval ReadHandler * readHandler2 = - readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe, &sScheduler, - CodegenDataModelProviderInstance(nullptr /* delegate */)); + readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe, &sScheduler); EXPECT_EQ(CHIP_NO_ERROR, MockReadHandlerSubscriptionTransaction(readHandler2, &sScheduler, 0, 3)); // Clean read handler, will be triggered at max interval, but will be cancelled before ReadHandler * readHandler3 = - readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe, &sScheduler, - CodegenDataModelProviderInstance(nullptr /* delegate */)); + readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe, &sScheduler); EXPECT_EQ(CHIP_NO_ERROR, MockReadHandlerSubscriptionTransaction(readHandler3, &sScheduler, 0, 3)); // Confirms that none of the ReadHandlers are currently reportable @@ -409,8 +405,8 @@ TEST_F_FROM_FIXTURE(TestReportScheduler, TestObserverCallbacks) // Initialize mock timestamp sTestTimerDelegate.SetMockSystemTimestamp(Milliseconds64(0)); - ReadHandler * readHandler = readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe, - &sScheduler, CodegenDataModelProviderInstance(nullptr /* delegate */)); + ReadHandler * readHandler = + readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe, &sScheduler); EXPECT_EQ(CHIP_NO_ERROR, MockReadHandlerSubscriptionTransaction(readHandler, &sScheduler, 1, 2)); @@ -486,15 +482,13 @@ TEST_F_FROM_FIXTURE(TestReportScheduler, TestSynchronizedScheduler) sTestTimerSynchronizedDelegate.SetMockSystemTimestamp(System::Clock::Milliseconds64(0)); ReadHandler * readHandler1 = - readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe, &syncScheduler, - CodegenDataModelProviderInstance(nullptr /* delegate */)); + readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe, &syncScheduler); EXPECT_EQ(CHIP_NO_ERROR, MockReadHandlerSubscriptionTransaction(readHandler1, &syncScheduler, 0, 2)); ReadHandlerNode * node1 = syncScheduler.FindReadHandlerNode(readHandler1); ReadHandler * readHandler2 = - readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe, &syncScheduler, - CodegenDataModelProviderInstance(nullptr /* delegate */)); + readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe, &syncScheduler); EXPECT_EQ(CHIP_NO_ERROR, MockReadHandlerSubscriptionTransaction(readHandler2, &syncScheduler, 1, 3)); ReadHandlerNode * node2 = syncScheduler.FindReadHandlerNode(readHandler2); @@ -622,8 +616,7 @@ TEST_F_FROM_FIXTURE(TestReportScheduler, TestSynchronizedScheduler) sTestTimerSynchronizedDelegate.IncrementMockTimestamp(System::Clock::Milliseconds64(1000)); ReadHandler * readHandler3 = - readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe, &syncScheduler, - CodegenDataModelProviderInstance(nullptr /* delegate */)); + readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe, &syncScheduler); EXPECT_EQ(CHIP_NO_ERROR, MockReadHandlerSubscriptionTransaction(readHandler3, &syncScheduler, 2, 3)); ReadHandlerNode * node3 = syncScheduler.FindReadHandlerNode(readHandler3); @@ -677,8 +670,7 @@ TEST_F_FROM_FIXTURE(TestReportScheduler, TestSynchronizedScheduler) // Now simulate a new readHandler being added with a max forcing a conflict ReadHandler * readHandler4 = - readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe, &syncScheduler, - CodegenDataModelProviderInstance(nullptr /* delegate */)); + readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe, &syncScheduler); EXPECT_EQ(CHIP_NO_ERROR, MockReadHandlerSubscriptionTransaction(readHandler4, &syncScheduler, 0, 1)); ReadHandlerNode * node4 = syncScheduler.FindReadHandlerNode(readHandler4); diff --git a/src/app/tests/TestReportingEngine.cpp b/src/app/tests/TestReportingEngine.cpp index 60852e2c033c91..bfeb96141abf78 100644 --- a/src/app/tests/TestReportingEngine.cpp +++ b/src/app/tests/TestReportingEngine.cpp @@ -209,8 +209,7 @@ TEST_F_FROM_FIXTURE(TestReportingEngine, TestBuildAndSendSingleReportData) EXPECT_EQ(readRequestBuilder.GetError(), CHIP_NO_ERROR); EXPECT_EQ(writer.Finalize(&readRequestbuf), CHIP_NO_ERROR); app::ReadHandler readHandler(dummy, exchangeCtx, chip::app::ReadHandler::InteractionType::Read, - app::reporting::GetDefaultReportScheduler(), - CodegenDataModelProviderInstance(nullptr /* delegate */)); + app::reporting::GetDefaultReportScheduler()); readHandler.OnInitialRequest(std::move(readRequestbuf)); EXPECT_EQ(InteractionModelEngine::GetInstance()->GetReportingEngine().BuildAndSendSingleReportData(&readHandler), diff --git a/src/lib/core/Optional.h b/src/lib/core/Optional.h index a8f544ce1d9819..eb661609f31908 100644 --- a/src/lib/core/Optional.h +++ b/src/lib/core/Optional.h @@ -48,41 +48,39 @@ template class Optional { public: - constexpr Optional() : mHasValue(false) {} - constexpr Optional(NullOptionalType) : mHasValue(false) {} + constexpr Optional() {} + constexpr Optional(NullOptionalType) {} - ~Optional() + explicit Optional(const T & value) { - // NOLINTNEXTLINE(clang-analyzer-core.uninitialized.Branch): mData is set when mHasValue - if (mHasValue) - { - mValue.mData.~T(); - } + mValueHolder.mHasValue = true; + new (&mValueHolder.mValue.mData) T(value); } - explicit Optional(const T & value) : mHasValue(true) { new (&mValue.mData) T(value); } - template - constexpr explicit Optional(InPlaceType, Args &&... args) : mHasValue(true) + constexpr explicit Optional(InPlaceType, Args &&... args) { - new (&mValue.mData) T(std::forward(args)...); + mValueHolder.mHasValue = true; + new (&mValueHolder.mValue.mData) T(std::forward(args)...); } - constexpr Optional(const Optional & other) : mHasValue(other.mHasValue) + constexpr Optional(const Optional & other) { - if (mHasValue) + mValueHolder.mHasValue = other.mValueHolder.mHasValue; + if (mValueHolder.mHasValue) { - new (&mValue.mData) T(other.mValue.mData); + new (&mValueHolder.mValue.mData) T(other.mValueHolder.mValue.mData); } } // Converts an Optional of an implicitly convertible type template && std::is_convertible_v, bool> = true> - constexpr Optional(const Optional & other) : mHasValue(other.HasValue()) + constexpr Optional(const Optional & other) { - if (mHasValue) + mValueHolder.mHasValue = other.HasValue(); + if (mValueHolder.mHasValue) { - new (&mValue.mData) T(other.Value()); + new (&mValueHolder.mValue.mData) T(other.Value()); } } @@ -90,50 +88,52 @@ class Optional template && !std::is_convertible_v && std::is_constructible_v, bool> = true> - constexpr explicit Optional(const Optional & other) : mHasValue(other.HasValue()) + constexpr explicit Optional(const Optional & other) { - if (mHasValue) + mValueHolder.mHasValue = other.HasValue(); + if (mValueHolder.mHasValue) { - new (&mValue.mData) T(other.Value()); + new (&mValueHolder.mValue.mData) T(other.Value()); } } - constexpr Optional(Optional && other) : mHasValue(other.mHasValue) + constexpr Optional(Optional && other) { - if (mHasValue) + mValueHolder.mHasValue = other.mValueHolder.mHasValue; + if (mValueHolder.mHasValue) { - new (&mValue.mData) T(std::move(other.mValue.mData)); - other.mValue.mData.~T(); - other.mHasValue = false; + new (&mValueHolder.mValue.mData) T(std::move(other.mValueHolder.mValue.mData)); + other.mValueHolder.mValue.mData.~T(); + other.mValueHolder.mHasValue = false; } } constexpr Optional & operator=(const Optional & other) { - if (mHasValue) + if (mValueHolder.mHasValue) { - mValue.mData.~T(); + mValueHolder.mValue.mData.~T(); } - mHasValue = other.mHasValue; - if (mHasValue) + mValueHolder.mHasValue = other.mValueHolder.mHasValue; + if (mValueHolder.mHasValue) { - new (&mValue.mData) T(other.mValue.mData); + new (&mValueHolder.mValue.mData) T(other.mValueHolder.mValue.mData); } return *this; } constexpr Optional & operator=(Optional && other) { - if (mHasValue) + if (mValueHolder.mHasValue) { - mValue.mData.~T(); + mValueHolder.mValue.mData.~T(); } - mHasValue = other.mHasValue; - if (mHasValue) + mValueHolder.mHasValue = other.mValueHolder.mHasValue; + if (mValueHolder.mHasValue) { - new (&mValue.mData) T(std::move(other.mValue.mData)); - other.mValue.mData.~T(); - other.mHasValue = false; + new (&mValueHolder.mValue.mData) T(std::move(other.mValueHolder.mValue.mData)); + other.mValueHolder.mValue.mData.~T(); + other.mValueHolder.mHasValue = false; } return *this; } @@ -142,24 +142,24 @@ class Optional template constexpr T & Emplace(Args &&... args) { - if (mHasValue) + if (mValueHolder.mHasValue) { - mValue.mData.~T(); + mValueHolder.mValue.mData.~T(); } - mHasValue = true; - new (&mValue.mData) T(std::forward(args)...); - return mValue.mData; + mValueHolder.mHasValue = true; + new (&mValueHolder.mValue.mData) T(std::forward(args)...); + return mValueHolder.mValue.mData; } /** Make the optional contain a specific value */ constexpr void SetValue(const T & value) { - if (mHasValue) + if (mValueHolder.mHasValue) { - mValue.mData.~T(); + mValueHolder.mValue.mData.~T(); } - mHasValue = true; - new (&mValue.mData) T(value); + mValueHolder.mHasValue = true; + new (&mValueHolder.mValue.mData) T(value); } constexpr void SetValue(std::optional & value) @@ -177,36 +177,36 @@ class Optional /** Make the optional contain a specific value */ constexpr void SetValue(T && value) { - if (mHasValue) + if (mValueHolder.mHasValue) { - mValue.mData.~T(); + mValueHolder.mValue.mData.~T(); } - mHasValue = true; - new (&mValue.mData) T(std::move(value)); + mValueHolder.mHasValue = true; + new (&mValueHolder.mValue.mData) T(std::move(value)); } /** Invalidate the value inside the optional. Optional now has no value */ constexpr void ClearValue() { - if (mHasValue) + if (mValueHolder.mHasValue) { - mValue.mData.~T(); + mValueHolder.mValue.mData.~T(); } - mHasValue = false; + mValueHolder.mHasValue = false; } /** Gets the current value of the optional. Valid IFF `HasValue`. */ T & Value() & { VerifyOrDie(HasValue()); - return mValue.mData; + return mValueHolder.mValue.mData; } /** Gets the current value of the optional. Valid IFF `HasValue`. */ const T & Value() const & { VerifyOrDie(HasValue()); - return mValue.mData; + return mValueHolder.mValue.mData; } /** Gets the current value of the optional if the optional has a value; @@ -214,11 +214,12 @@ class Optional const T & ValueOr(const T & defaultValue) const { return HasValue() ? Value() : defaultValue; } /** Checks if the optional contains a value or not */ - constexpr bool HasValue() const { return mHasValue; } + constexpr bool HasValue() const { return mValueHolder.mHasValue; } bool operator==(const Optional & other) const { - return (mHasValue == other.mHasValue) && (!other.mHasValue || (mValue.mData == other.mValue.mData)); + return (mValueHolder.mHasValue == other.mValueHolder.mHasValue) && + (!other.mValueHolder.mHasValue || (mValueHolder.mValue.mData == other.mValueHolder.mValue.mData)); } bool operator!=(const Optional & other) const { return !(*this == other); } bool operator==(const T & other) const { return HasValue() && Value() == other; } @@ -241,13 +242,47 @@ class Optional } private: - bool mHasValue; - union Value + // A container of bool + value (without constructor/destructor) when the underlying + // type has a trivial destructor + class TrivialDestructor + { + public: + bool mHasValue = false; + union Value + { + Value() {} + T mData; + } mValue; + }; + + // A container of bool + value that destroys the underlying type when mHasValue is true. + // To be used for non-trivial destructor classes. + class NonTrivialDestructor + { + public: + ~NonTrivialDestructor() + { + // NOLINTNEXTLINE(clang-analyzer-core.uninitialized.Branch): mData is set when mHasValue + if (mHasValue) + { + mValue.mData.~T(); + } + } + + bool mHasValue = false; + union Value + { + Value() {} + ~Value() {} + T mData; + } mValue; + }; + + class ValueHolder : public std::conditional_t, TrivialDestructor, NonTrivialDestructor> { - Value() {} - ~Value() {} - T mData; - } mValue; + }; + + ValueHolder mValueHolder; }; template diff --git a/src/lib/core/tests/TestOptional.cpp b/src/lib/core/tests/TestOptional.cpp index e57657464369bd..2e2a543ac10b60 100644 --- a/src/lib/core/tests/TestOptional.cpp +++ b/src/lib/core/tests/TestOptional.cpp @@ -17,22 +17,18 @@ * limitations under the License. */ -/** - * @file - * This file implements a test for CHIP core library reference counted object. - * - */ - #include #include #include #include +#include #include #include #include #include +#include using namespace chip; @@ -76,6 +72,10 @@ struct CountMovable : public Count int Count::created; int Count::destroyed; +// Optional is trivially destructible if the underlying type is trivially destructible +static_assert(std::is_trivially_destructible_v>); +static_assert(!std::is_trivially_destructible_v>); + TEST(TestOptional, TestBasic) { // Set up our test Count objects, which will mess with counts, before we reset the