diff --git a/src/app/AttributeValueDecoder.h b/src/app/AttributeValueDecoder.h index 5f5e2255b2fa3e..79a749bbb812b4 100644 --- a/src/app/AttributeValueDecoder.h +++ b/src/app/AttributeValueDecoder.h @@ -65,6 +65,8 @@ class AttributeValueDecoder const Access::SubjectDescriptor & GetSubjectDescriptor() const { return mSubjectDescriptor; } private: + friend class TestOnlyAttributeValueDecoderAccessor; + TLV::TLVReader & mReader; bool mTriedDecode = false; const Access::SubjectDescriptor mSubjectDescriptor; diff --git a/src/app/EventManagement.cpp b/src/app/EventManagement.cpp index 7c0df844872b47..271f72ec107f3b 100644 --- a/src/app/EventManagement.cpp +++ b/src/app/EventManagement.cpp @@ -855,6 +855,12 @@ void EventManagement::SetScheduledEventInfo(EventNumber & aEventNumber, uint32_t aInitialWrittenEventBytes = mBytesWritten; } +CHIP_ERROR EventManagement::GenerateEvent(EventLoggingDelegate * eventPayloadWriter, const EventOptions & options, + EventNumber & generatedEventNumber) +{ + return LogEvent(eventPayloadWriter, options, generatedEventNumber); +} + void CircularEventBuffer::Init(uint8_t * apBuffer, uint32_t aBufferLength, CircularEventBuffer * apPrev, CircularEventBuffer * apNext, PriorityLevel aPriorityLevel) { @@ -914,5 +920,6 @@ CHIP_ERROR CircularEventBufferWrapper::GetNextBuffer(TLVReader & aReader, const exit: return err; } + } // namespace app } // namespace chip diff --git a/src/app/EventManagement.h b/src/app/EventManagement.h index ce5a34039ea0fe..76220350fc2507 100644 --- a/src/app/EventManagement.h +++ b/src/app/EventManagement.h @@ -31,6 +31,7 @@ #include #include #include +#include #include #include #include @@ -196,7 +197,7 @@ struct LogStorageResources * more space for new events. */ -class EventManagement +class EventManagement : public DataModel::EventsGenerator { public: /** @@ -387,6 +388,10 @@ class EventManagement */ void SetScheduledEventInfo(EventNumber & aEventNumber, uint32_t & aInitialWrittenEventBytes) const; + /* EventsGenerator implementation */ + CHIP_ERROR GenerateEvent(EventLoggingDelegate * eventPayloadWriter, const EventOptions & options, + EventNumber & generatedEventNumber) override; + private: /** * @brief @@ -559,5 +564,6 @@ class EventManagement System::Clock::Milliseconds64 mMonotonicStartupTime; }; + } // namespace app } // namespace chip diff --git a/src/app/InteractionModelEngine.cpp b/src/app/InteractionModelEngine.cpp index 88c1777b6c8224..dcf73341a2a1eb 100644 --- a/src/app/InteractionModelEngine.cpp +++ b/src/app/InteractionModelEngine.cpp @@ -891,7 +891,7 @@ Protocols::InteractionModel::Status InteractionModelEngine::OnWriteRequest(Messa { if (writeHandler.IsFree()) { - VerifyOrReturnError(writeHandler.Init(this) == CHIP_NO_ERROR, Status::Busy); + VerifyOrReturnError(writeHandler.Init(GetDataModelProvider(), this) == CHIP_NO_ERROR, Status::Busy); return writeHandler.OnWriteRequest(apExchangeContext, std::move(aPayload), aIsTimedWrite); } } @@ -992,6 +992,9 @@ CHIP_ERROR InteractionModelEngine::OnMessageReceived(Messaging::ExchangeContext Protocols::InteractionModel::Status status = Status::Failure; + // Ensure that DataModel::Provider has access to the exchange the message was received on. + CurrentExchangeValueScope scopedExchangeContext(*this, apExchangeContext); + // Group Message can only be an InvokeCommandRequest or WriteRequest if (apExchangeContext->IsGroupExchangeContext() && !aPayloadHeader.HasMessageType(Protocols::InteractionModel::MsgType::InvokeCommandRequest) && @@ -1729,16 +1732,44 @@ DataModel::Provider * InteractionModelEngine::SetDataModelProvider(DataModel::Pr // Alternting data model should not be done while IM is actively handling requests. VerifyOrDie(mReadHandlers.begin() == mReadHandlers.end()); - DataModel::Provider * oldModel = GetDataModelProvider(); - mDataModelProvider = model; + DataModel::Provider * oldModel = mDataModelProvider; + if (oldModel != nullptr) + { + CHIP_ERROR err = oldModel->Shutdown(); + if (err != CHIP_NO_ERROR) + { + ChipLogError(InteractionModel, "Failure on interaction model shutdown: %" CHIP_ERROR_FORMAT, err.Format()); + } + } + + mDataModelProvider = model; + if (mDataModelProvider != nullptr) + { + DataModel::InteractionModelContext context; + + context.eventsGenerator = &EventManagement::GetInstance(); + context.dataModelChangeListener = &mReportingEngine; + context.actionContext = this; + + CHIP_ERROR err = mDataModelProvider->Startup(context); + if (err != CHIP_NO_ERROR) + { + ChipLogError(InteractionModel, "Failure on interaction model startup: %" CHIP_ERROR_FORMAT, err.Format()); + } + } + return oldModel; } -DataModel::Provider * InteractionModelEngine::GetDataModelProvider() const +DataModel::Provider * InteractionModelEngine::GetDataModelProvider() { #if CHIP_CONFIG_USE_DATA_MODEL_INTERFACE - // TODO: this should be temporary, we should fully inject the data model - VerifyOrReturnValue(mDataModelProvider != nullptr, CodegenDataModelProviderInstance()); + if (mDataModelProvider == nullptr) + { + // These should be called within the CHIP processing loop. + assertChipStackLockedByCurrentThread(); + SetDataModelProvider(CodegenDataModelProviderInstance()); + } #endif return mDataModelProvider; } diff --git a/src/app/InteractionModelEngine.h b/src/app/InteractionModelEngine.h index c371bad23b2d2c..8b7bd0743782cd 100644 --- a/src/app/InteractionModelEngine.h +++ b/src/app/InteractionModelEngine.h @@ -86,6 +86,7 @@ namespace app { */ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler, public Messaging::ExchangeDelegate, + private DataModel::ActionContext, public CommandResponseSender::Callback, public CommandHandlerImpl::Callback, public ReadHandler::ManagementCallback, @@ -402,7 +403,10 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler, } #endif - DataModel::Provider * GetDataModelProvider() const; + // Temporarily NOT const because the data model provider will be auto-set + // to codegen on first usage. This behaviour will be changed once each + // application must explicitly set the data model provider. + DataModel::Provider * GetDataModelProvider(); // MUST NOT be used while the interaction model engine is running as interaction // model functionality (e.g. active reads/writes/subscriptions) rely on data model @@ -412,6 +416,9 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler, DataModel::Provider * SetDataModelProvider(DataModel::Provider * model); private: + /* DataModel::ActionContext implementation */ + Messaging::ExchangeContext * CurrentExchange() override { return mCurrentExchange; } + friend class reporting::Engine; friend class TestCommandInteraction; friend class TestInteractionModelEngine; @@ -698,7 +705,23 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler, SubscriptionResumptionStorage * mpSubscriptionResumptionStorage = nullptr; - DataModel::Provider * mDataModelProvider = nullptr; + DataModel::Provider * mDataModelProvider = nullptr; + Messaging::ExchangeContext * mCurrentExchange = nullptr; + + // Changes the current exchange context of a InteractionModelEngine to a given context + class CurrentExchangeValueScope + { + public: + CurrentExchangeValueScope(InteractionModelEngine & engine, Messaging::ExchangeContext * context) : mEngine(engine) + { + mEngine.mCurrentExchange = context; + } + + ~CurrentExchangeValueScope() { mEngine.mCurrentExchange = nullptr; } + + private: + InteractionModelEngine & mEngine; + }; }; } // namespace app diff --git a/src/app/WriteHandler.cpp b/src/app/WriteHandler.cpp index 15776eb545fc21..5050cbbee2fcb5 100644 --- a/src/app/WriteHandler.cpp +++ b/src/app/WriteHandler.cpp @@ -18,19 +18,25 @@ #include #include +#include #include #include #include #include #include +#include #include #include #include #include +#include #include #include +#include #include +#include + namespace chip { namespace app { @@ -38,10 +44,14 @@ using namespace Protocols::InteractionModel; using Status = Protocols::InteractionModel::Status; constexpr uint8_t kListAttributeType = 0x48; -CHIP_ERROR WriteHandler::Init(WriteHandlerDelegate * apWriteHandlerDelegate) +CHIP_ERROR WriteHandler::Init(DataModel::Provider * apProvider, WriteHandlerDelegate * apWriteHandlerDelegate) { VerifyOrReturnError(!mExchangeCtx, CHIP_ERROR_INCORRECT_STATE); VerifyOrReturnError(apWriteHandlerDelegate, CHIP_ERROR_INVALID_ARGUMENT); +#if CHIP_CONFIG_USE_DATA_MODEL_INTERFACE + VerifyOrReturnError(apProvider, CHIP_ERROR_INVALID_ARGUMENT); + mDataModelProvider = apProvider; +#endif // CHIP_CONFIG_USE_DATA_MODEL_INTERFACE mDelegate = apWriteHandlerDelegate; MoveToState(State::Initialized); @@ -63,6 +73,9 @@ void WriteHandler::Close() DeliverFinalListWriteEnd(false /* wasSuccessful */); mExchangeCtx.Release(); mStateFlags.Clear(StateBits::kSuppressResponse); +#if CHIP_CONFIG_USE_DATA_MODEL_INTERFACE + mDataModelProvider = nullptr; +#endif // CHIP_CONFIG_USE_DATA_MODEL_INTERFACE MoveToState(State::Uninitialized); } @@ -354,7 +367,7 @@ CHIP_ERROR WriteHandler::ProcessAttributeDataIBs(TLV::TLVReader & aAttributeData err = CHIP_NO_ERROR; } SuccessOrExit(err); - err = WriteSingleClusterData(subjectDescriptor, dataAttributePath, dataReader, this); + err = WriteClusterData(subjectDescriptor, dataAttributePath, dataReader); if (err != CHIP_NO_ERROR) { mWriteResponseBuilder.GetWriteResponses().Rollback(backup); @@ -501,7 +514,7 @@ CHIP_ERROR WriteHandler::ProcessGroupAttributeDataIBs(TLV::TLVReader & aAttribut DataModelCallbacks::GetInstance()->AttributeOperation(DataModelCallbacks::OperationType::Write, DataModelCallbacks::OperationOrder::Pre, dataAttributePath); - err = WriteSingleClusterData(subjectDescriptor, dataAttributePath, tmpDataReader, this); + err = WriteClusterData(subjectDescriptor, dataAttributePath, tmpDataReader); if (err != CHIP_NO_ERROR) { ChipLogError(DataManagement, @@ -552,6 +565,10 @@ Status WriteHandler::ProcessWriteRequest(System::PacketBufferHandle && aPayload, // our callees hand out Status as well. Status status = Status::InvalidAction; +#if CHIP_CONFIG_USE_DATA_MODEL_INTERFACE + mLastSuccessfullyWrittenPath = std::nullopt; +#endif // CHIP_CONFIG_USE_DATA_MODEL_INTERFACE + reader.Init(std::move(aPayload)); err = writeRequestParser.Init(reader); @@ -559,7 +576,7 @@ Status WriteHandler::ProcessWriteRequest(System::PacketBufferHandle && aPayload, #if CHIP_CONFIG_IM_PRETTY_PRINT writeRequestParser.PrettyPrint(); -#endif +#endif // CHIP_CONFIG_IM_PRETTY_PRINT bool boolValue; boolValue = mStateFlags.Has(StateBits::kSuppressResponse); @@ -703,5 +720,32 @@ void WriteHandler::MoveToState(const State aTargetState) ChipLogDetail(DataManagement, "IM WH moving to [%s]", GetStateStr()); } +CHIP_ERROR WriteHandler::WriteClusterData(const Access::SubjectDescriptor & aSubject, const ConcreteDataAttributePath & aPath, + TLV::TLVReader & aData) +{ + // Writes do not have a checked-path. If data model interface is enabled (both checked and only version) + // the write is done via the DataModel interface +#if CHIP_CONFIG_USE_DATA_MODEL_INTERFACE + VerifyOrReturnError(mDataModelProvider != nullptr, CHIP_ERROR_INCORRECT_STATE); + + DataModel::WriteAttributeRequest request; + + request.path = aPath; + request.subjectDescriptor = aSubject; + request.previousSuccessPath = mLastSuccessfullyWrittenPath; + request.writeFlags.Set(DataModel::WriteFlags::kTimed, IsTimedWrite()); + + AttributeValueDecoder decoder(aData, aSubject); + + DataModel::ActionReturnStatus status = mDataModelProvider->WriteAttribute(request, decoder); + + mLastSuccessfullyWrittenPath = status.IsSuccess() ? std::make_optional(aPath) : std::nullopt; + + return AddStatusInternal(aPath, StatusIB(status.GetStatusCode())); +#else + return WriteSingleClusterData(aSubject, aPath, aData, this); +#endif // CHIP_CONFIG_USE_DATA_MODEL_INTERFACE +} + } // namespace app } // namespace chip diff --git a/src/app/WriteHandler.h b/src/app/WriteHandler.h index 0723a316738498..fe63e028b8dfee 100644 --- a/src/app/WriteHandler.h +++ b/src/app/WriteHandler.h @@ -21,8 +21,10 @@ #include #include #include +#include #include #include +#include #include #include #include @@ -69,6 +71,7 @@ class WriteHandler : public Messaging::ExchangeDelegate * construction until a call to Close is made to terminate the * instance. * + * @param[in] apProvider A valid pointer to the model used to forward writes towards * @param[in] apWriteHandlerDelegate A Valid pointer to the WriteHandlerDelegate. * * @retval #CHIP_ERROR_INVALID_ARGUMENT on invalid pointers @@ -77,7 +80,7 @@ class WriteHandler : public Messaging::ExchangeDelegate * @retval #CHIP_NO_ERROR On success. * */ - CHIP_ERROR Init(WriteHandlerDelegate * apWriteHandlerDelegate); + CHIP_ERROR Init(DataModel::Provider * apProvider, WriteHandlerDelegate * apWriteHandlerDelegate); /** * Process a write request. Parts of the processing may end up being asynchronous, but the WriteHandler @@ -182,11 +185,20 @@ class WriteHandler : public Messaging::ExchangeDelegate System::PacketBufferHandle && aPayload) override; void OnResponseTimeout(Messaging::ExchangeContext * apExchangeContext) override; + // Write the given data to the given path + CHIP_ERROR WriteClusterData(const Access::SubjectDescriptor & aSubject, const ConcreteDataAttributePath & aPath, + TLV::TLVReader & aData); + Messaging::ExchangeHolder mExchangeCtx; WriteResponseMessage::Builder mWriteResponseBuilder; Optional mProcessingAttributePath; Optional mACLCheckCache = NullOptional; +#if CHIP_CONFIG_USE_DATA_MODEL_INTERFACE + DataModel::Provider * mDataModelProvider = nullptr; + std::optional mLastSuccessfullyWrittenPath; +#endif + // This may be a "fake" pointer or a real delegate pointer, depending // on CHIP_CONFIG_STATIC_GLOBAL_INTERACTION_MODEL_ENGINE setting. // diff --git a/src/app/codegen-data-model-provider/CodegenDataModelProvider_Write.cpp b/src/app/codegen-data-model-provider/CodegenDataModelProvider_Write.cpp index ffb44eb492dc5a..9eee57d1491fa5 100644 --- a/src/app/codegen-data-model-provider/CodegenDataModelProvider_Write.cpp +++ b/src/app/codegen-data-model-provider/CodegenDataModelProvider_Write.cpp @@ -15,6 +15,7 @@ * limitations under the License. */ #include +#include #include #include @@ -45,6 +46,17 @@ namespace { using namespace chip::app::Compatibility::Internal; using Protocols::InteractionModel::Status; +class ContextAttributesChangeListener : public AttributesChangedListener +{ +public: + ContextAttributesChangeListener(const DataModel::InteractionModelContext & context) : mListener(context.dataModelChangeListener) + {} + void MarkDirty(const AttributePathParams & path) override { mListener->MarkDirty(path); } + +private: + DataModel::ProviderChangeListener * mListener; +}; + /// Attempts to write via an attribute access interface (AAI) /// /// If it returns a CHIP_ERROR, then this is a FINAL result (i.e. either failure or success) @@ -273,27 +285,14 @@ DataModel::ActionReturnStatus CodegenDataModelProvider::WriteAttribute(const Dat ChipLogDetail(DataManagement, "Writing attribute: Cluster=" ChipLogFormatMEI " Endpoint=0x%x AttributeId=" ChipLogFormatMEI, ChipLogValueMEI(request.path.mClusterId), request.path.mEndpointId, ChipLogValueMEI(request.path.mAttributeId)); - // ACL check for non-internal requests - if (!request.operationFlags.Has(DataModel::OperationFlags::kInternal)) - { - ReturnErrorCodeIf(!request.subjectDescriptor.has_value(), Status::UnsupportedAccess); - - Access::RequestPath requestPath{ .cluster = request.path.mClusterId, - .endpoint = request.path.mEndpointId, - .requestType = Access::RequestType::kAttributeWriteRequest, - .entityId = request.path.mAttributeId }; - CHIP_ERROR err = Access::GetAccessControl().Check(*request.subjectDescriptor, requestPath, - RequiredPrivilege::ForWriteAttribute(request.path)); - - if (err != CHIP_NO_ERROR) - { - ReturnErrorCodeIf((err != CHIP_ERROR_ACCESS_DENIED) && (err != CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL), err); - - // TODO: when wildcard/group writes are supported, handle them to discard rather than fail with status - return err == CHIP_ERROR_ACCESS_DENIED ? Status::UnsupportedAccess : Status::AccessRestricted; - } - } - + // TODO: ordering is to check writability/existence BEFORE ACL and this seems wrong, however + // existing unit tests (TC_AcessChecker.py) validate that we get UnsupportedWrite instead of UnsupportedAccess + // + // This should likely be fixed in spec (probably already fixed by + // https://github.com/CHIP-Specifications/connectedhomeip-spec/pull/9024) + // and tests and implementation + // + // Open issue that needs fixing: https://github.com/project-chip/connectedhomeip/issues/33735 auto metadata = Ember::FindAttributeMetadata(request.path); // Explicit failure in finding a suitable metadata @@ -322,7 +321,56 @@ DataModel::ActionReturnStatus CodegenDataModelProvider::WriteAttribute(const Dat if (!request.operationFlags.Has(DataModel::OperationFlags::kInternal)) { VerifyOrReturnError(!isReadOnly, Status::UnsupportedWrite); + } + + // ACL check for non-internal requests + bool checkAcl = !request.operationFlags.Has(DataModel::OperationFlags::kInternal); + // For chunking, ACL check is not re-done if the previous write was successful for the exact same + // path. We apply this everywhere as a shortcut, although realistically this is only for AccessControl cluster + if (checkAcl && request.previousSuccessPath.has_value()) + { + // NOTE: explicit cast/check only for attribute path and nothing else. + // + // In particular `request.path` is a DATA path (contains a list index) + // and we do not want request.previousSuccessPath to be auto-cast to a + // data path with a empty list and fail the compare. + // + // This could be `request.previousSuccessPath != request.path` (where order + // is important) however that would seem more brittle (relying that a != b + // behaves differently than b != a due to casts). Overall Data paths are not + // the same as attribute paths. + // + // Also note that Concrete path have a mExpanded that is not used in compares. + const ConcreteAttributePath & attributePathA = request.path; + const ConcreteAttributePath & attributePathB = *request.previousSuccessPath; + + checkAcl = (attributePathA != attributePathB); + } + + if (checkAcl) + { + ReturnErrorCodeIf(!request.subjectDescriptor.has_value(), Status::UnsupportedAccess); + + Access::RequestPath requestPath{ .cluster = request.path.mClusterId, + .endpoint = request.path.mEndpointId, + .requestType = Access::RequestType::kAttributeWriteRequest, + .entityId = request.path.mAttributeId }; + CHIP_ERROR err = Access::GetAccessControl().Check(*request.subjectDescriptor, requestPath, + RequiredPrivilege::ForWriteAttribute(request.path)); + + if (err != CHIP_NO_ERROR) + { + VerifyOrReturnValue(err != CHIP_ERROR_ACCESS_DENIED, Status::UnsupportedAccess); + VerifyOrReturnValue(err != CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL, Status::AccessRestricted); + + return err; + } + } + + // Internal is allowed to bypass timed writes and read-only. + if (!request.operationFlags.Has(DataModel::OperationFlags::kInternal)) + { VerifyOrReturnError(!(*attributeMetadata)->MustUseTimedWrite() || request.writeFlags.Has(DataModel::WriteFlags::kTimed), Status::NeedsTimedInteraction); } @@ -349,6 +397,8 @@ DataModel::ActionReturnStatus CodegenDataModelProvider::WriteAttribute(const Dat } } + ContextAttributesChangeListener change_listener(CurrentContext()); + AttributeAccessInterface * aai = AttributeAccessInterfaceRegistry::Instance().Get(request.path.mEndpointId, request.path.mClusterId); std::optional aai_result = TryWriteViaAccessInterface(request.path, aai, decoder); @@ -356,11 +406,9 @@ DataModel::ActionReturnStatus CodegenDataModelProvider::WriteAttribute(const Dat { if (*aai_result == CHIP_NO_ERROR) { - // TODO: change callbacks should likely be routed through the context `MarkDirty` only - // however for now this is called directly because ember code does this call - // inside emberAfWriteAttribute. - MatterReportingAttributeChangeCallback(request.path); - CurrentContext().dataModelChangeListener->MarkDirty(request.path); + // TODO: this is awkward since it provides AAI no control over this, specifically + // AAI may not want to increase versions for some attributes that are Q + emberAfAttributeChanged(request.path.mEndpointId, request.path.mClusterId, request.path.mAttributeId, &change_listener); } return *aai_result; } @@ -376,18 +424,21 @@ DataModel::ActionReturnStatus CodegenDataModelProvider::WriteAttribute(const Dat return Status::InvalidValue; } + EmberAfWriteDataInput dataInput(dataBuffer.data(), (*attributeMetadata)->attributeType); + + dataInput.SetChangeListener(&change_listener); + // TODO: dataInput.SetMarkDirty() should be according to `ChangesOmmited` + if (request.operationFlags.Has(DataModel::OperationFlags::kInternal)) { // Internal requests use the non-External interface that has less enforcement // than the external version (e.g. does not check/enforce writable settings, does not // validate attribute types) - see attribute-table.h documentation for details. - status = emberAfWriteAttribute(request.path.mEndpointId, request.path.mClusterId, request.path.mAttributeId, - dataBuffer.data(), (*attributeMetadata)->attributeType); + status = emberAfWriteAttribute(request.path, dataInput); } else { - status = - emAfWriteAttributeExternal(request.path, EmberAfWriteDataInput(dataBuffer.data(), (*attributeMetadata)->attributeType)); + status = emAfWriteAttributeExternal(request.path, dataInput); } if (status != Protocols::InteractionModel::Status::Success) @@ -395,14 +446,6 @@ DataModel::ActionReturnStatus CodegenDataModelProvider::WriteAttribute(const Dat return status; } - // TODO: this WILL requre updates - // - // - Internal writes may need to be able to decide if to mark things dirty or not (see AAI as well) - // - Changes-ommited paths should not be marked dirty (ember is not aware of that flag) - // - This likely maps to `MatterReportingAttributeChangeCallback` HOWEVER current ember write functions - // will selectively call that one depending on old attribute state (i.e. calling every time is a - // change in behavior) - CurrentContext().dataModelChangeListener->MarkDirty(request.path); return CHIP_NO_ERROR; } diff --git a/src/app/codegen-data-model-provider/tests/EmberReadWriteOverride.cpp b/src/app/codegen-data-model-provider/tests/EmberReadWriteOverride.cpp index ac918049b860f4..5befbc14aa5715 100644 --- a/src/app/codegen-data-model-provider/tests/EmberReadWriteOverride.cpp +++ b/src/app/codegen-data-model-provider/tests/EmberReadWriteOverride.cpp @@ -16,6 +16,8 @@ */ #include "EmberReadWriteOverride.h" +#include +#include #include #include #include @@ -116,9 +118,15 @@ Status emAfWriteAttributeExternal(const chip::app::ConcreteAttributePath & path, // copy over as much data as possible // NOTE: we do NOT use (*metadata)->size since it is unclear if our mocks set that correctly size_t len = std::min(sizeof(gEmberIoBuffer), chip::app::Compatibility::Internal::gEmberAttributeIOBufferSpan.size()); + memcpy(gEmberIoBuffer, input.dataPtr, len); gEmberIoBufferFill = len; + if (input.changeListener != nullptr) + { + input.changeListener->MarkDirty(chip::app::AttributePathParams(path.mEndpointId, path.mClusterId, path.mAttributeId)); + } + return Status::Success; } @@ -128,3 +136,8 @@ Status emberAfWriteAttribute(chip::EndpointId endpoint, chip::ClusterId cluster, return emAfWriteAttributeExternal(chip::app::ConcreteAttributePath(endpoint, cluster, attributeID), EmberAfWriteDataInput(dataPtr, dataType)); } + +Status emberAfWriteAttribute(const chip::app::ConcreteAttributePath & path, const EmberAfWriteDataInput & input) +{ + return emAfWriteAttributeExternal(path, input); +} diff --git a/src/app/codegen-data-model-provider/tests/TestCodegenModelViaMocks.cpp b/src/app/codegen-data-model-provider/tests/TestCodegenModelViaMocks.cpp index 649c6f02195a27..c240b017071439 100644 --- a/src/app/codegen-data-model-provider/tests/TestCodegenModelViaMocks.cpp +++ b/src/app/codegen-data-model-provider/tests/TestCodegenModelViaMocks.cpp @@ -132,13 +132,13 @@ bool operator==(const Access::SubjectDescriptor & a, const Access::SubjectDescri class TestProviderChangeListener : public ProviderChangeListener { public: - void MarkDirty(const ConcreteAttributePath & path) override { mDirtyList.push_back(path); } + void MarkDirty(const AttributePathParams & path) override { mDirtyList.push_back(path); } - std::vector & DirtyList() { return mDirtyList; } - const std::vector & DirtyList() const { return mDirtyList; } + std::vector & DirtyList() { return mDirtyList; } + const std::vector & DirtyList() const { return mDirtyList; } private: - std::vector mDirtyList; + std::vector mDirtyList; }; class TestEventGenerator : public EventsGenerator @@ -802,12 +802,32 @@ void TestEmberScalarTypeWrite(const typename NumericAttributeTraits::WorkingT EXPECT_EQ(actual, value); ASSERT_EQ(model.ChangeListener().DirtyList().size(), 1u); - EXPECT_EQ(model.ChangeListener().DirtyList()[0], test.request.path); + EXPECT_EQ(model.ChangeListener().DirtyList()[0], + AttributePathParams(test.request.path.mEndpointId, test.request.path.mClusterId, test.request.path.mAttributeId)); // reset for the next test model.ChangeListener().DirtyList().clear(); } + // nullable test: write null to make sure content of buffer changed (otherwise it will be a noop for dirty checking) + { + TestWriteRequest test( + kAdminSubjectDescriptor, + ConcreteAttributePath(kMockEndpoint3, MockClusterId(4), MOCK_ATTRIBUTE_ID_FOR_NULLABLE_TYPE(ZclType))); + + using NumericType = NumericAttributeTraits; + using NullableType = chip::app::DataModel::Nullable; + AttributeValueDecoder decoder = test.DecoderFor(NullableType()); + + // write should succeed + ASSERT_EQ(model.WriteAttribute(test.request, decoder), CHIP_NO_ERROR); + + // dirty: we changed the value to null + ASSERT_EQ(model.ChangeListener().DirtyList().size(), 1u); + EXPECT_EQ(model.ChangeListener().DirtyList()[0], + AttributePathParams(test.request.path.mEndpointId, test.request.path.mClusterId, test.request.path.mAttributeId)); + } + // nullable test { TestWriteRequest test( @@ -827,8 +847,10 @@ void TestEmberScalarTypeWrite(const typename NumericAttributeTraits::WorkingT typename NumericAttributeTraits::WorkingType actual = NumericAttributeTraits::StorageToWorking(storage); ASSERT_EQ(actual, value); - ASSERT_EQ(model.ChangeListener().DirtyList().size(), 1u); - EXPECT_EQ(model.ChangeListener().DirtyList()[0], test.request.path); + // dirty a 2nd time when we moved from null to a real value + ASSERT_EQ(model.ChangeListener().DirtyList().size(), 2u); + EXPECT_EQ(model.ChangeListener().DirtyList()[1], + AttributePathParams(test.request.path.mEndpointId, test.request.path.mClusterId, test.request.path.mAttributeId)); } } @@ -1994,7 +2016,20 @@ TEST(TestCodegenModelViaMocks, EmberAttributeWriteAclDeny) CodegenDataModelProviderWithContext model; ScopedMockAccessControl accessControl; - TestWriteRequest test(kDenySubjectDescriptor, ConcreteDataAttributePath(kMockEndpoint1, MockClusterId(1), MockAttributeId(10))); + /* Using this path is also failing existence checks, so this cannot be enabled + * until we fix ordering of ACL to be done before existence checks + + TestWriteRequest test(kDenySubjectDescriptor, + ConcreteDataAttributePath(kMockEndpoint1, MockClusterId(1), MockAttributeId(10))); + AttributeValueDecoder decoder = test.DecoderFor(1234); + + ASSERT_EQ(model.WriteAttribute(test.request, decoder), Status::UnsupportedAccess); + ASSERT_TRUE(model.ChangeListener().DirtyList().empty()); + */ + + TestWriteRequest test(kDenySubjectDescriptor, + ConcreteDataAttributePath(kMockEndpoint3, MockClusterId(4), + MOCK_ATTRIBUTE_ID_FOR_NULLABLE_TYPE(ZCL_INT32U_ATTRIBUTE_TYPE))); AttributeValueDecoder decoder = test.DecoderFor(1234); ASSERT_EQ(model.WriteAttribute(test.request, decoder), Status::UnsupportedAccess); @@ -2431,12 +2466,13 @@ TEST(TestCodegenModelViaMocks, EmberWriteAttributeAccessInterfaceTest) // AAI marks dirty paths ASSERT_EQ(model.ChangeListener().DirtyList().size(), 1u); - EXPECT_EQ(model.ChangeListener().DirtyList()[0], kStructPath); + EXPECT_EQ(model.ChangeListener().DirtyList()[0], + AttributePathParams(kStructPath.mEndpointId, kStructPath.mClusterId, kStructPath.mAttributeId)); // AAI does not prevent read/write of regular attributes // validate that once AAI is added, we still can go through writing regular bits (i.e. // AAI returning "unknown" has fallback to ember) - TestEmberScalarTypeWrite(1234); + TestEmberScalarTypeWrite(4321); TestEmberScalarNullWrite(); } diff --git a/src/app/data-model-provider/OperationTypes.h b/src/app/data-model-provider/OperationTypes.h index 00ec0424763cf5..8758e02ef89c5d 100644 --- a/src/app/data-model-provider/OperationTypes.h +++ b/src/app/data-model-provider/OperationTypes.h @@ -70,15 +70,24 @@ struct ReadAttributeRequest : OperationRequest enum class WriteFlags : uint32_t { - kTimed = 0x0001, // Write is a timed write (i.e. a Timed Request Action preceeded it) - kListBegin = 0x0002, // This is the FIRST list of data elements - kListEnd = 0x0004, // This is the LAST list element to write + kTimed = 0x0001, // Write is a timed write (i.e. a Timed Request Action preceeded it) }; struct WriteAttributeRequest : OperationRequest { ConcreteDataAttributePath path; // NOTE: this also contains LIST operation options (i.e. "data" path type) BitFlags writeFlags; + + // The path of the previous successful write in the same write transaction, if any. + // + // In particular this means that a write to this path has succeeded before (i.e. it passed required ACL checks). + // The intent for this is to allow short-cutting ACL checks when ACL is in progress of being updated: + // - During write chunking, list writes can be of the form "reset list" followed by "append item by item" + // - When ACL is updating, a reset to empty would result in the entire ACL being deny and the "append" + // would fail. + // callers are expected to keep track of a `previousSuccessPath` whenever a write succeeds (otherwise ACL + // checks may fail) + std::optional previousSuccessPath; }; enum class InvokeFlags : uint32_t diff --git a/src/app/data-model-provider/ProviderChangeListener.h b/src/app/data-model-provider/ProviderChangeListener.h index 97061865921b0f..b0f6aab5f811f3 100644 --- a/src/app/data-model-provider/ProviderChangeListener.h +++ b/src/app/data-model-provider/ProviderChangeListener.h @@ -16,7 +16,7 @@ */ #pragma once -#include +#include namespace chip { namespace app { @@ -39,7 +39,7 @@ class ProviderChangeListener /// Mark all attributes matching the given path (which may be a wildcard) dirty. /// /// Wildcards are supported. - virtual void MarkDirty(const ConcreteAttributePath & path) = 0; + virtual void MarkDirty(const AttributePathParams & path) = 0; }; } // namespace DataModel diff --git a/src/app/reporting/Engine.cpp b/src/app/reporting/Engine.cpp index 072aa100e10312..cb4169420ee513 100644 --- a/src/app/reporting/Engine.cpp +++ b/src/app/reporting/Engine.cpp @@ -33,6 +33,7 @@ #include #include #include +#include #include #include @@ -1010,7 +1011,16 @@ void Engine::ScheduleUrgentEventDeliverySync(Optional fabricIndex) Run(); } -}; // namespace reporting +void Engine::MarkDirty(const AttributePathParams & path) +{ + CHIP_ERROR err = SetDirty(path); + if (err != CHIP_NO_ERROR) + { + ChipLogError(DataManagement, "Failed to set path dirty: %" CHIP_ERROR_FORMAT, err.Format()); + } +} + +} // namespace reporting } // namespace app } // namespace chip diff --git a/src/app/reporting/Engine.h b/src/app/reporting/Engine.h index cff36ff41ccb5b..a16b0d9151d3d9 100644 --- a/src/app/reporting/Engine.h +++ b/src/app/reporting/Engine.h @@ -27,6 +27,7 @@ #include #include #include +#include #include #include #include @@ -54,7 +55,7 @@ namespace reporting { * At its core, it tries to gather and pack as much relevant attributes changes and/or events as possible into a report * message before sending that to the reader. It continues to do so until it has no more work to do. */ -class Engine +class Engine : public DataModel::ProviderChangeListener { public: /** @@ -140,6 +141,9 @@ class Engine size_t GetGlobalDirtySetSize() { return mGlobalDirtySet.Allocated(); } #endif + /* ProviderChangeListener implementation */ + void MarkDirty(const AttributePathParams & path) override; + private: /** * Main work-horse function that executes the run-loop. diff --git a/src/app/tests/TestWriteInteraction.cpp b/src/app/tests/TestWriteInteraction.cpp index 625d6c3328d2f2..6500b5fe0db6a8 100644 --- a/src/app/tests/TestWriteInteraction.cpp +++ b/src/app/tests/TestWriteInteraction.cpp @@ -15,8 +15,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - -#include #include #include @@ -70,9 +68,12 @@ class TestWriteInteraction : public chip::Test::AppContext chip::MutableByteSpan span(buf); ASSERT_EQ(GetBobFabric()->GetCompressedFabricIdBytes(span), CHIP_NO_ERROR); ASSERT_EQ(chip::GroupTesting::InitData(&gGroupsProvider, GetBobFabricIndex(), span), CHIP_NO_ERROR); + + mOldProvider = InteractionModelEngine::GetInstance()->SetDataModelProvider(&TestImCustomDataModel::Instance()); } void TearDown() override { + InteractionModelEngine::GetInstance()->SetDataModelProvider(mOldProvider); chip::Credentials::GroupDataProvider * provider = chip::Credentials::GetGroupDataProvider(); if (provider != nullptr) { @@ -93,6 +94,9 @@ class TestWriteInteraction : public chip::Test::AppContext static void AddAttributeStatus(WriteHandler & aWriteHandler); static void GenerateWriteRequest(bool aIsTimedWrite, System::PacketBufferHandle & aPayload); static void GenerateWriteResponse(System::PacketBufferHandle & aPayload); + +private: + chip::app::DataModel::Provider * mOldProvider = nullptr; }; class TestExchangeDelegate : public Messaging::ExchangeDelegate @@ -296,7 +300,8 @@ TEST_F(TestWriteInteraction, TestWriteHandler) System::PacketBufferHandle buf = System::PacketBufferHandle::New(System::PacketBuffer::kMaxSize); - writeHandler.Init(chip::app::InteractionModelEngine::GetInstance()); + writeHandler.Init(chip::app::InteractionModelEngine::GetInstance()->GetDataModelProvider(), + chip::app::InteractionModelEngine::GetInstance()); GenerateWriteRequest(messageIsTimed, buf); diff --git a/src/app/tests/test-interaction-model-api.cpp b/src/app/tests/test-interaction-model-api.cpp index f483f4b3de88a4..92bc6df0f24740 100644 --- a/src/app/tests/test-interaction-model-api.cpp +++ b/src/app/tests/test-interaction-model-api.cpp @@ -46,6 +46,18 @@ class TestOnlyAttributeValueEncoderAccessor AttributeValueEncoder & mEncoder; }; +class TestOnlyAttributeValueDecoderAccessor +{ +public: + TestOnlyAttributeValueDecoderAccessor(AttributeValueDecoder & decoder) : mDecoder(decoder) {} + + TLV::TLVReader & GetTlvReader() { return mDecoder.mReader; } + void SetTriedDecode(bool triedDecode) { mDecoder.mTriedDecode = triedDecode; } + +private: + AttributeValueDecoder & mDecoder; +}; + // Used by the code in TestWriteInteraction.cpp (and generally tests that interact with the WriteHandler may need this). const EmberAfAttributeMetadata * GetAttributeMetadata(const ConcreteAttributePath & aConcreteClusterPath) { @@ -170,7 +182,21 @@ ActionReturnStatus TestImCustomDataModel::ReadAttribute(const ReadAttributeReque ActionReturnStatus TestImCustomDataModel::WriteAttribute(const WriteAttributeRequest & request, AttributeValueDecoder & decoder) { - return CHIP_ERROR_NOT_IMPLEMENTED; + if (request.path.mDataVersion.HasValue() && request.path.mDataVersion.Value() == Test::kRejectedDataVersion) + { + return CHIP_IM_GLOBAL_STATUS(DataVersionMismatch); + } + + TestOnlyAttributeValueDecoderAccessor decodeAccess(decoder); + + decodeAccess.SetTriedDecode(true); + + TLV::TLVWriter writer; + writer.Init(chip::Test::attributeDataTLV); + writer.CopyElement(TLV::AnonymousTag(), decodeAccess.GetTlvReader()); + chip::Test::attributeDataTLVLen = writer.GetLengthWritten(); + + return CHIP_NO_ERROR; } ActionReturnStatus TestImCustomDataModel::Invoke(const InvokeRequest & request, chip::TLV::TLVReader & input_arguments, diff --git a/src/app/util/mock/CodegenEmberMocks.cpp b/src/app/util/mock/CodegenEmberMocks.cpp index 521f36c9569647..a1963ab71d732e 100644 --- a/src/app/util/mock/CodegenEmberMocks.cpp +++ b/src/app/util/mock/CodegenEmberMocks.cpp @@ -15,6 +15,7 @@ * limitations under the License. */ #include +#include #include #include @@ -41,8 +42,13 @@ Status emAfWriteAttributeExternal(const chip::app::ConcreteAttributePath & path, } Status emberAfWriteAttribute(chip::EndpointId endpoint, chip::ClusterId cluster, chip::AttributeId attributeID, uint8_t * dataPtr, - EmberAfAttributeType dataType) + EmberAfAttributeType dataType, chip::app::MarkAttributeDirty markDirty) { return emAfWriteAttributeExternal(chip::app::ConcreteAttributePath(endpoint, cluster, attributeID), EmberAfWriteDataInput(dataPtr, dataType)); } + +Status emberAfWriteAttribute(const chip::app::ConcreteAttributePath & path, const EmberAfWriteDataInput & input) +{ + return emAfWriteAttributeExternal(path, input); +} diff --git a/src/app/util/mock/attribute-storage.cpp b/src/app/util/mock/attribute-storage.cpp index 93d3219fc04a41..87ffa8bbbdd89f 100644 --- a/src/app/util/mock/attribute-storage.cpp +++ b/src/app/util/mock/attribute-storage.cpp @@ -313,6 +313,13 @@ DataVersion * emberAfDataVersionStorage(const chip::app::ConcreteClusterPath & a return &dataVersion; } +void emberAfAttributeChanged(EndpointId endpoint, ClusterId clusterId, AttributeId attributeId, + AttributesChangedListener * listener) +{ + dataVersion++; + listener->MarkDirty(AttributePathParams(endpoint, clusterId, attributeId)); +} + namespace chip { namespace app { diff --git a/src/controller/tests/data_model/DataModelFixtures.cpp b/src/controller/tests/data_model/DataModelFixtures.cpp index 05fa16b956bb0c..3f75d8a7be3d53 100644 --- a/src/controller/tests/data_model/DataModelFixtures.cpp +++ b/src/controller/tests/data_model/DataModelFixtures.cpp @@ -50,6 +50,17 @@ class TestOnlyAttributeValueEncoderAccessor AttributeValueEncoder & mEncoder; }; +class TestOnlyAttributeValueDecoderAccessor +{ +public: + TestOnlyAttributeValueDecoderAccessor(AttributeValueDecoder & decoder) : mDecoder(decoder) {} + + TLV::TLVReader & GetTlvReader() { return mDecoder.mReader; } + +private: + AttributeValueDecoder & mDecoder; +}; + namespace DataModelTests { ScopedChangeOnly gReadResponseDirective(ReadResponseDirective::kSendDataResponse); @@ -300,7 +311,8 @@ CHIP_ERROR WriteSingleClusterData(const Access::SubjectDescriptor & aSubjectDesc } if (aPath.mClusterId == Clusters::UnitTesting::Id && aPath.mAttributeId == Attributes::ListFabricScoped::Id) { - // Mock a invalid SubjectDescriptor + // Mock an invalid SubjectDescriptor. + // NOTE: completely ignores the passed-in subjectDescriptor AttributeValueDecoder decoder(aReader, Access::SubjectDescriptor()); if (!aPath.IsListOperation() || aPath.mListOp == ConcreteDataAttributePath::ListOperation::ReplaceAll) { @@ -522,7 +534,132 @@ ActionReturnStatus CustomDataModel::ReadAttribute(const ReadAttributeRequest & r ActionReturnStatus CustomDataModel::WriteAttribute(const WriteAttributeRequest & request, AttributeValueDecoder & decoder) { - return CHIP_ERROR_NOT_IMPLEMENTED; + static ListIndex listStructOctetStringElementCount = 0; + + if (request.path.mDataVersion.HasValue() && request.path.mDataVersion.Value() == kRejectedDataVersion) + { + return InteractionModel::Status::DataVersionMismatch; + } + + if (request.path.mClusterId == Clusters::UnitTesting::Id && + request.path.mAttributeId == Attributes::ListStructOctetString::TypeInfo::GetAttributeId()) + { + if (gWriteResponseDirective == WriteResponseDirective::kSendAttributeSuccess) + { + if (!request.path.IsListOperation() || request.path.mListOp == ConcreteDataAttributePath::ListOperation::ReplaceAll) + { + + Attributes::ListStructOctetString::TypeInfo::DecodableType value; + + ReturnErrorOnFailure(decoder.Decode(value)); + + auto iter = value.begin(); + listStructOctetStringElementCount = 0; + while (iter.Next()) + { + auto & item = iter.GetValue(); + + VerifyOrReturnError(item.member1 == listStructOctetStringElementCount, CHIP_ERROR_INVALID_ARGUMENT); + listStructOctetStringElementCount++; + } + return CHIP_NO_ERROR; + } + + if (request.path.mListOp == ConcreteDataAttributePath::ListOperation::AppendItem) + { + Structs::TestListStructOctet::DecodableType item; + ReturnErrorOnFailure(decoder.Decode(item)); + VerifyOrReturnError(item.member1 == listStructOctetStringElementCount, CHIP_ERROR_INVALID_ARGUMENT); + listStructOctetStringElementCount++; + + return CHIP_NO_ERROR; + } + + return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE; + } + + return CHIP_IM_GLOBAL_STATUS(Failure); + } + if (request.path.mClusterId == Clusters::UnitTesting::Id && request.path.mAttributeId == Attributes::ListFabricScoped::Id) + { + // TODO(backwards compatibility): unit tests here undoes the subject descriptor usage + // - original tests were completely bypassing the passed in subject descriptor for this test + // and overriding it with a invalid subject descriptor + // - we do the same here, however this seems somewhat off: decoder.Decode() will fail for list + // items so we could just return the error directly without this extra step + + // Mock an invalid Subject Descriptor + AttributeValueDecoder invalidSubjectDescriptorDecoder(TestOnlyAttributeValueDecoderAccessor(decoder).GetTlvReader(), + Access::SubjectDescriptor()); + if (!request.path.IsListOperation() || request.path.mListOp == ConcreteDataAttributePath::ListOperation::ReplaceAll) + { + Attributes::ListFabricScoped::TypeInfo::DecodableType value; + + ReturnErrorOnFailure(invalidSubjectDescriptorDecoder.Decode(value)); + + auto iter = value.begin(); + while (iter.Next()) + { + auto & item = iter.GetValue(); + (void) item; + } + } + else if (request.path.mListOp == ConcreteDataAttributePath::ListOperation::AppendItem) + { + Structs::TestFabricScoped::DecodableType item; + ReturnErrorOnFailure(invalidSubjectDescriptorDecoder.Decode(item)); + } + else + { + return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE; + } + return CHIP_NO_ERROR; + } + + // Boolean attribute of unit testing cluster triggers "multiple errors" case. + if (request.path.mClusterId == Clusters::UnitTesting::Id && + request.path.mAttributeId == Attributes::Boolean::TypeInfo::GetAttributeId()) + { + // TODO(IMDM): this used to send 4 responses (hence the multiple status) + // + // for (size_t i = 0; i < 4; ++i) + // { + // aWriteHandler->AddStatus(request.path, status); + // } + // + // which are NOT encodable by a simple response. It is unclear how this is + // convertible (if at all): we write path by path only. Having multiple + // responses for the same path within the write code makes no sense + // + // This should NOT be possible anymore when one can only return a single + // status (nobody has access to multiple path status updates at this level) + switch (gWriteResponseDirective) + { + case WriteResponseDirective::kSendMultipleSuccess: + return InteractionModel::Status::Success; + case WriteResponseDirective::kSendMultipleErrors: + return InteractionModel::Status::Failure; + default: + chipDie(); + } + } + + if (request.path.mClusterId == Clusters::UnitTesting::Id && + request.path.mAttributeId == Attributes::Int8u::TypeInfo::GetAttributeId()) + { + switch (gWriteResponseDirective) + { + case WriteResponseDirective::kSendClusterSpecificSuccess: + return InteractionModel::ClusterStatusCode::ClusterSpecificSuccess(kExampleClusterSpecificSuccess); + case WriteResponseDirective::kSendClusterSpecificFailure: + return InteractionModel::ClusterStatusCode::ClusterSpecificFailure(kExampleClusterSpecificFailure); + default: + // this should not be reached, our tests only set up these for this test case + chipDie(); + } + } + + return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE; } ActionReturnStatus CustomDataModel::Invoke(const InvokeRequest & request, chip::TLV::TLVReader & input_arguments, diff --git a/src/controller/tests/data_model/TestWrite.cpp b/src/controller/tests/data_model/TestWrite.cpp index 03d100ab33dabb..218832702b230f 100644 --- a/src/controller/tests/data_model/TestWrite.cpp +++ b/src/controller/tests/data_model/TestWrite.cpp @@ -85,6 +85,19 @@ class SingleWriteCallback : public WriteClient::Callback class TestWrite : public chip::Test::AppContext { public: + void SetUp() override + { + chip::Test::AppContext::SetUp(); + mOldProvider = InteractionModelEngine::GetInstance()->SetDataModelProvider(&CustomDataModel::Instance()); + } + + // Performs teardown for each individual test in the test suite + void TearDown() override + { + InteractionModelEngine::GetInstance()->SetDataModelProvider(mOldProvider); + chip::Test::AppContext::TearDown(); + } + void ResetCallback() { mSingleWriteCallback.reset(); } void PrepareWriteCallback(ConcreteAttributePath path) { mSingleWriteCallback = std::make_unique(path); } @@ -93,6 +106,7 @@ class TestWrite : public chip::Test::AppContext protected: std::unique_ptr mSingleWriteCallback; + chip::app::DataModel::Provider * mOldProvider = nullptr; }; TEST_F(TestWrite, TestDataResponse) @@ -128,7 +142,8 @@ TEST_F(TestWrite, TestDataResponse) DrainAndServiceIO(); - EXPECT_TRUE(onSuccessCbInvoked && !onFailureCbInvoked); + EXPECT_TRUE(onSuccessCbInvoked); + EXPECT_FALSE(onFailureCbInvoked); EXPECT_EQ(chip::app::InteractionModelEngine::GetInstance()->GetNumActiveWriteHandlers(), 0u); EXPECT_EQ(GetExchangeManager().GetNumActiveExchanges(), 0u); }