Skip to content

Commit 0b8ffb7

Browse files
andy31415andreilitvinbzbarsky-applerestyled-commitstcarmelveilleux
authored
Make use of DataModel::Provider in writes (#34754)
* Implement DM::Provider::Write usage * Fix compile * Fix java builds * Update src/app/InteractionModelEngine.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Make codegen data model call increasing the cluster data version * Restyle * Make sure that attribute changed information can be propagated out of ember * Optimize storage size of WriteHandler * Restyle * Make the code more obvious identical with what was there before * Change Provider to not double-call the dirty and version increase * Update src/app/InteractionModelEngine.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Cleaner usage: no need of a separate function that is used in one place only * Attempt an API update * Fix typos in the Accessors src * Fix typo and regen * More fixes on accessors * Update signature for emAfWriteAttributeExternal * Add a comment about all the checks being vague * Update src/app/util/af-types.h Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/app/util/af-types.h Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/app/util/attribute-storage.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/app/zap-templates/templates/app/attributes/Accessors-src.zapt Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/app/util/mock/CodegenEmberMocks.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/app/zap-templates/templates/app/attributes/Accessors-src.zapt Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/app/zap-templates/templates/app/attributes/Accessors-src.zapt Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * zap regen and restyle * Update src/app/zap-templates/templates/app/attributes/Accessors-src.zapt Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/app/util/attribute-table.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/app/util/attribute-storage.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/app/util/attribute-storage.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/app/util/attribute-table.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/app/util/attribute-table.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/app/util/attribute-storage.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/app/util/attribute-table.h Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Rename ChangedPathListener to AttributesChangedListener * Remove chip:: and chip::app * Update constructors of AttributePathParams and add nodiscard according to the linter to never call const methods without considering their return value * Restyled by clang-format * Remove auto-inserted include * Update again and zap regen: removed extra namespace prefixes in accessors.h/cpp * Add comment about uint8_t non-const usage... * Another rename given that the listener is now an attributes and not a path listener * Update src/app/util/attribute-table.h Co-authored-by: Tennessee Carmel-Veilleux <tennessee.carmelveilleux@gmail.com> * Update after merge to have more things compile * Everything compiles now * Restyle * Make unit tests pass: mocks also have to call the change listeners * Comment update to talk more about AttributesChangedListener * Restyle * Add support for a previous path write ... this is similar to what ACL caching and tokenizing currently does, but in a explicit manner describing the ACL use case * Remove some extra added includes * Another include fix * Follow the comment and do not restrict the cache to ACL cluster, since this is what current ember does * More self code review changes * Make tests pass, more code cleanup * Match ordering to ember-compatibility functions. This is ODD because we check attribute existance before access! * Adjust test ordering and add a large note that we are probably doing the wrong thing, however tests force us to do the wrong thing * Fix unit test * Remove odd comment * Add a few ending endifs * Renamed ScopedExchangeContext * Update src/app/InteractionModelEngine.h Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/app/WriteHandler.h Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/app/WriteHandler.h Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/app/data-model-provider/OperationTypes.h Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/app/data-model-provider/OperationTypes.h Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/app/data-model-provider/OperationTypes.h Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/app/codegen-data-model-provider/CodegenDataModelProvider_Write.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Restyle to reorder includes * Add issue link * Update equality logic * Rename member to mLastSuccessfullyWrittenPath * Update argument logic * Make ActionContext private in IME so it is not such a public API * Clean up comments * fix up compares * Restyle --------- Co-authored-by: Andrei Litvin <andreilitvin@google.com> Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> Co-authored-by: Restyled.io <commits@restyled.io> Co-authored-by: Tennessee Carmel-Veilleux <tennessee.carmelveilleux@gmail.com>
1 parent 2396bb4 commit 0b8ffb7

20 files changed

+513
-77
lines changed

src/app/AttributeValueDecoder.h

+2
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,8 @@ class AttributeValueDecoder
6565
const Access::SubjectDescriptor & GetSubjectDescriptor() const { return mSubjectDescriptor; }
6666

6767
private:
68+
friend class TestOnlyAttributeValueDecoderAccessor;
69+
6870
TLV::TLVReader & mReader;
6971
bool mTriedDecode = false;
7072
const Access::SubjectDescriptor mSubjectDescriptor;

src/app/EventManagement.cpp

+7
Original file line numberDiff line numberDiff line change
@@ -855,6 +855,12 @@ void EventManagement::SetScheduledEventInfo(EventNumber & aEventNumber, uint32_t
855855
aInitialWrittenEventBytes = mBytesWritten;
856856
}
857857

858+
CHIP_ERROR EventManagement::GenerateEvent(EventLoggingDelegate * eventPayloadWriter, const EventOptions & options,
859+
EventNumber & generatedEventNumber)
860+
{
861+
return LogEvent(eventPayloadWriter, options, generatedEventNumber);
862+
}
863+
858864
void CircularEventBuffer::Init(uint8_t * apBuffer, uint32_t aBufferLength, CircularEventBuffer * apPrev,
859865
CircularEventBuffer * apNext, PriorityLevel aPriorityLevel)
860866
{
@@ -914,5 +920,6 @@ CHIP_ERROR CircularEventBufferWrapper::GetNextBuffer(TLVReader & aReader, const
914920
exit:
915921
return err;
916922
}
923+
917924
} // namespace app
918925
} // namespace chip

src/app/EventManagement.h

+7-1
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
#include <app/EventLoggingTypes.h>
3232
#include <app/MessageDef/EventDataIB.h>
3333
#include <app/MessageDef/StatusIB.h>
34+
#include <app/data-model-provider/EventsGenerator.h>
3435
#include <app/util/basic-types.h>
3536
#include <lib/core/TLVCircularBuffer.h>
3637
#include <lib/support/CHIPCounter.h>
@@ -196,7 +197,7 @@ struct LogStorageResources
196197
* more space for new events.
197198
*/
198199

199-
class EventManagement
200+
class EventManagement : public DataModel::EventsGenerator
200201
{
201202
public:
202203
/**
@@ -387,6 +388,10 @@ class EventManagement
387388
*/
388389
void SetScheduledEventInfo(EventNumber & aEventNumber, uint32_t & aInitialWrittenEventBytes) const;
389390

391+
/* EventsGenerator implementation */
392+
CHIP_ERROR GenerateEvent(EventLoggingDelegate * eventPayloadWriter, const EventOptions & options,
393+
EventNumber & generatedEventNumber) override;
394+
390395
private:
391396
/**
392397
* @brief
@@ -559,5 +564,6 @@ class EventManagement
559564

560565
System::Clock::Milliseconds64 mMonotonicStartupTime;
561566
};
567+
562568
} // namespace app
563569
} // namespace chip

src/app/InteractionModelEngine.cpp

+37-6
Original file line numberDiff line numberDiff line change
@@ -895,7 +895,7 @@ Protocols::InteractionModel::Status InteractionModelEngine::OnWriteRequest(Messa
895895
{
896896
if (writeHandler.IsFree())
897897
{
898-
VerifyOrReturnError(writeHandler.Init(this) == CHIP_NO_ERROR, Status::Busy);
898+
VerifyOrReturnError(writeHandler.Init(GetDataModelProvider(), this) == CHIP_NO_ERROR, Status::Busy);
899899
return writeHandler.OnWriteRequest(apExchangeContext, std::move(aPayload), aIsTimedWrite);
900900
}
901901
}
@@ -996,6 +996,9 @@ CHIP_ERROR InteractionModelEngine::OnMessageReceived(Messaging::ExchangeContext
996996

997997
Protocols::InteractionModel::Status status = Status::Failure;
998998

999+
// Ensure that DataModel::Provider has access to the exchange the message was received on.
1000+
CurrentExchangeValueScope scopedExchangeContext(*this, apExchangeContext);
1001+
9991002
// Group Message can only be an InvokeCommandRequest or WriteRequest
10001003
if (apExchangeContext->IsGroupExchangeContext() &&
10011004
!aPayloadHeader.HasMessageType(Protocols::InteractionModel::MsgType::InvokeCommandRequest) &&
@@ -1749,16 +1752,44 @@ DataModel::Provider * InteractionModelEngine::SetDataModelProvider(DataModel::Pr
17491752
// Alternting data model should not be done while IM is actively handling requests.
17501753
VerifyOrDie(mReadHandlers.begin() == mReadHandlers.end());
17511754

1752-
DataModel::Provider * oldModel = GetDataModelProvider();
1753-
mDataModelProvider = model;
1755+
DataModel::Provider * oldModel = mDataModelProvider;
1756+
if (oldModel != nullptr)
1757+
{
1758+
CHIP_ERROR err = oldModel->Shutdown();
1759+
if (err != CHIP_NO_ERROR)
1760+
{
1761+
ChipLogError(InteractionModel, "Failure on interaction model shutdown: %" CHIP_ERROR_FORMAT, err.Format());
1762+
}
1763+
}
1764+
1765+
mDataModelProvider = model;
1766+
if (mDataModelProvider != nullptr)
1767+
{
1768+
DataModel::InteractionModelContext context;
1769+
1770+
context.eventsGenerator = &EventManagement::GetInstance();
1771+
context.dataModelChangeListener = &mReportingEngine;
1772+
context.actionContext = this;
1773+
1774+
CHIP_ERROR err = mDataModelProvider->Startup(context);
1775+
if (err != CHIP_NO_ERROR)
1776+
{
1777+
ChipLogError(InteractionModel, "Failure on interaction model startup: %" CHIP_ERROR_FORMAT, err.Format());
1778+
}
1779+
}
1780+
17541781
return oldModel;
17551782
}
17561783

1757-
DataModel::Provider * InteractionModelEngine::GetDataModelProvider() const
1784+
DataModel::Provider * InteractionModelEngine::GetDataModelProvider()
17581785
{
17591786
#if CHIP_CONFIG_USE_DATA_MODEL_INTERFACE
1760-
// TODO: this should be temporary, we should fully inject the data model
1761-
VerifyOrReturnValue(mDataModelProvider != nullptr, CodegenDataModelProviderInstance());
1787+
if (mDataModelProvider == nullptr)
1788+
{
1789+
// These should be called within the CHIP processing loop.
1790+
assertChipStackLockedByCurrentThread();
1791+
SetDataModelProvider(CodegenDataModelProviderInstance());
1792+
}
17621793
#endif
17631794
return mDataModelProvider;
17641795
}

src/app/InteractionModelEngine.h

+25-2
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ namespace app {
8686
*/
8787
class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
8888
public Messaging::ExchangeDelegate,
89+
private DataModel::ActionContext,
8990
public CommandResponseSender::Callback,
9091
public CommandHandlerImpl::Callback,
9192
public ReadHandler::ManagementCallback,
@@ -402,7 +403,10 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
402403
}
403404
#endif
404405

405-
DataModel::Provider * GetDataModelProvider() const;
406+
// Temporarily NOT const because the data model provider will be auto-set
407+
// to codegen on first usage. This behaviour will be changed once each
408+
// application must explicitly set the data model provider.
409+
DataModel::Provider * GetDataModelProvider();
406410

407411
// MUST NOT be used while the interaction model engine is running as interaction
408412
// model functionality (e.g. active reads/writes/subscriptions) rely on data model
@@ -412,6 +416,9 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
412416
DataModel::Provider * SetDataModelProvider(DataModel::Provider * model);
413417

414418
private:
419+
/* DataModel::ActionContext implementation */
420+
Messaging::ExchangeContext * CurrentExchange() override { return mCurrentExchange; }
421+
415422
friend class reporting::Engine;
416423
friend class TestCommandInteraction;
417424
friend class TestInteractionModelEngine;
@@ -698,7 +705,23 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
698705

699706
SubscriptionResumptionStorage * mpSubscriptionResumptionStorage = nullptr;
700707

701-
DataModel::Provider * mDataModelProvider = nullptr;
708+
DataModel::Provider * mDataModelProvider = nullptr;
709+
Messaging::ExchangeContext * mCurrentExchange = nullptr;
710+
711+
// Changes the current exchange context of a InteractionModelEngine to a given context
712+
class CurrentExchangeValueScope
713+
{
714+
public:
715+
CurrentExchangeValueScope(InteractionModelEngine & engine, Messaging::ExchangeContext * context) : mEngine(engine)
716+
{
717+
mEngine.mCurrentExchange = context;
718+
}
719+
720+
~CurrentExchangeValueScope() { mEngine.mCurrentExchange = nullptr; }
721+
722+
private:
723+
InteractionModelEngine & mEngine;
724+
};
702725
};
703726

704727
} // namespace app

src/app/WriteHandler.cpp

+48-4
Original file line numberDiff line numberDiff line change
@@ -18,30 +18,40 @@
1818

1919
#include <app/AppConfig.h>
2020
#include <app/AttributeAccessInterfaceRegistry.h>
21+
#include <app/AttributeValueDecoder.h>
2122
#include <app/InteractionModelEngine.h>
2223
#include <app/MessageDef/EventPathIB.h>
2324
#include <app/MessageDef/StatusIB.h>
2425
#include <app/StatusResponse.h>
2526
#include <app/WriteHandler.h>
27+
#include <app/data-model-provider/OperationTypes.h>
2628
#include <app/reporting/Engine.h>
2729
#include <app/util/MatterCallbacks.h>
2830
#include <app/util/ember-compatibility-functions.h>
2931
#include <credentials/GroupDataProvider.h>
32+
#include <lib/core/CHIPError.h>
3033
#include <lib/support/CodeUtils.h>
3134
#include <lib/support/TypeTraits.h>
35+
#include <messaging/ExchangeContext.h>
3236
#include <protocols/interaction_model/StatusCode.h>
3337

38+
#include <optional>
39+
3440
namespace chip {
3541
namespace app {
3642

3743
using namespace Protocols::InteractionModel;
3844
using Status = Protocols::InteractionModel::Status;
3945
constexpr uint8_t kListAttributeType = 0x48;
4046

41-
CHIP_ERROR WriteHandler::Init(WriteHandlerDelegate * apWriteHandlerDelegate)
47+
CHIP_ERROR WriteHandler::Init(DataModel::Provider * apProvider, WriteHandlerDelegate * apWriteHandlerDelegate)
4248
{
4349
VerifyOrReturnError(!mExchangeCtx, CHIP_ERROR_INCORRECT_STATE);
4450
VerifyOrReturnError(apWriteHandlerDelegate, CHIP_ERROR_INVALID_ARGUMENT);
51+
#if CHIP_CONFIG_USE_DATA_MODEL_INTERFACE
52+
VerifyOrReturnError(apProvider, CHIP_ERROR_INVALID_ARGUMENT);
53+
mDataModelProvider = apProvider;
54+
#endif // CHIP_CONFIG_USE_DATA_MODEL_INTERFACE
4555

4656
mDelegate = apWriteHandlerDelegate;
4757
MoveToState(State::Initialized);
@@ -63,6 +73,9 @@ void WriteHandler::Close()
6373
DeliverFinalListWriteEnd(false /* wasSuccessful */);
6474
mExchangeCtx.Release();
6575
mStateFlags.Clear(StateBits::kSuppressResponse);
76+
#if CHIP_CONFIG_USE_DATA_MODEL_INTERFACE
77+
mDataModelProvider = nullptr;
78+
#endif // CHIP_CONFIG_USE_DATA_MODEL_INTERFACE
6679
MoveToState(State::Uninitialized);
6780
}
6881

@@ -354,7 +367,7 @@ CHIP_ERROR WriteHandler::ProcessAttributeDataIBs(TLV::TLVReader & aAttributeData
354367
err = CHIP_NO_ERROR;
355368
}
356369
SuccessOrExit(err);
357-
err = WriteSingleClusterData(subjectDescriptor, dataAttributePath, dataReader, this);
370+
err = WriteClusterData(subjectDescriptor, dataAttributePath, dataReader);
358371
if (err != CHIP_NO_ERROR)
359372
{
360373
mWriteResponseBuilder.GetWriteResponses().Rollback(backup);
@@ -501,7 +514,7 @@ CHIP_ERROR WriteHandler::ProcessGroupAttributeDataIBs(TLV::TLVReader & aAttribut
501514

502515
DataModelCallbacks::GetInstance()->AttributeOperation(DataModelCallbacks::OperationType::Write,
503516
DataModelCallbacks::OperationOrder::Pre, dataAttributePath);
504-
err = WriteSingleClusterData(subjectDescriptor, dataAttributePath, tmpDataReader, this);
517+
err = WriteClusterData(subjectDescriptor, dataAttributePath, tmpDataReader);
505518
if (err != CHIP_NO_ERROR)
506519
{
507520
ChipLogError(DataManagement,
@@ -552,14 +565,18 @@ Status WriteHandler::ProcessWriteRequest(System::PacketBufferHandle && aPayload,
552565
// our callees hand out Status as well.
553566
Status status = Status::InvalidAction;
554567

568+
#if CHIP_CONFIG_USE_DATA_MODEL_INTERFACE
569+
mLastSuccessfullyWrittenPath = std::nullopt;
570+
#endif // CHIP_CONFIG_USE_DATA_MODEL_INTERFACE
571+
555572
reader.Init(std::move(aPayload));
556573

557574
err = writeRequestParser.Init(reader);
558575
SuccessOrExit(err);
559576

560577
#if CHIP_CONFIG_IM_PRETTY_PRINT
561578
writeRequestParser.PrettyPrint();
562-
#endif
579+
#endif // CHIP_CONFIG_IM_PRETTY_PRINT
563580
bool boolValue;
564581

565582
boolValue = mStateFlags.Has(StateBits::kSuppressResponse);
@@ -703,5 +720,32 @@ void WriteHandler::MoveToState(const State aTargetState)
703720
ChipLogDetail(DataManagement, "IM WH moving to [%s]", GetStateStr());
704721
}
705722

723+
CHIP_ERROR WriteHandler::WriteClusterData(const Access::SubjectDescriptor & aSubject, const ConcreteDataAttributePath & aPath,
724+
TLV::TLVReader & aData)
725+
{
726+
// Writes do not have a checked-path. If data model interface is enabled (both checked and only version)
727+
// the write is done via the DataModel interface
728+
#if CHIP_CONFIG_USE_DATA_MODEL_INTERFACE
729+
VerifyOrReturnError(mDataModelProvider != nullptr, CHIP_ERROR_INCORRECT_STATE);
730+
731+
DataModel::WriteAttributeRequest request;
732+
733+
request.path = aPath;
734+
request.subjectDescriptor = aSubject;
735+
request.previousSuccessPath = mLastSuccessfullyWrittenPath;
736+
request.writeFlags.Set(DataModel::WriteFlags::kTimed, IsTimedWrite());
737+
738+
AttributeValueDecoder decoder(aData, aSubject);
739+
740+
DataModel::ActionReturnStatus status = mDataModelProvider->WriteAttribute(request, decoder);
741+
742+
mLastSuccessfullyWrittenPath = status.IsSuccess() ? std::make_optional(aPath) : std::nullopt;
743+
744+
return AddStatusInternal(aPath, StatusIB(status.GetStatusCode()));
745+
#else
746+
return WriteSingleClusterData(aSubject, aPath, aData, this);
747+
#endif // CHIP_CONFIG_USE_DATA_MODEL_INTERFACE
748+
}
749+
706750
} // namespace app
707751
} // namespace chip

src/app/WriteHandler.h

+13-1
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,10 @@
2121
#include <app/AppConfig.h>
2222
#include <app/AttributeAccessToken.h>
2323
#include <app/AttributePathParams.h>
24+
#include <app/ConcreteAttributePath.h>
2425
#include <app/InteractionModelDelegatePointers.h>
2526
#include <app/MessageDef/WriteResponseMessage.h>
27+
#include <app/data-model-provider/Provider.h>
2628
#include <lib/core/CHIPCore.h>
2729
#include <lib/core/TLVDebug.h>
2830
#include <lib/support/BitFlags.h>
@@ -69,6 +71,7 @@ class WriteHandler : public Messaging::ExchangeDelegate
6971
* construction until a call to Close is made to terminate the
7072
* instance.
7173
*
74+
* @param[in] apProvider A valid pointer to the model used to forward writes towards
7275
* @param[in] apWriteHandlerDelegate A Valid pointer to the WriteHandlerDelegate.
7376
*
7477
* @retval #CHIP_ERROR_INVALID_ARGUMENT on invalid pointers
@@ -77,7 +80,7 @@ class WriteHandler : public Messaging::ExchangeDelegate
7780
* @retval #CHIP_NO_ERROR On success.
7881
*
7982
*/
80-
CHIP_ERROR Init(WriteHandlerDelegate * apWriteHandlerDelegate);
83+
CHIP_ERROR Init(DataModel::Provider * apProvider, WriteHandlerDelegate * apWriteHandlerDelegate);
8184

8285
/**
8386
* 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
182185
System::PacketBufferHandle && aPayload) override;
183186
void OnResponseTimeout(Messaging::ExchangeContext * apExchangeContext) override;
184187

188+
// Write the given data to the given path
189+
CHIP_ERROR WriteClusterData(const Access::SubjectDescriptor & aSubject, const ConcreteDataAttributePath & aPath,
190+
TLV::TLVReader & aData);
191+
185192
Messaging::ExchangeHolder mExchangeCtx;
186193
WriteResponseMessage::Builder mWriteResponseBuilder;
187194
Optional<ConcreteAttributePath> mProcessingAttributePath;
188195
Optional<AttributeAccessToken> mACLCheckCache = NullOptional;
189196

197+
#if CHIP_CONFIG_USE_DATA_MODEL_INTERFACE
198+
DataModel::Provider * mDataModelProvider = nullptr;
199+
std::optional<ConcreteAttributePath> mLastSuccessfullyWrittenPath;
200+
#endif
201+
190202
// This may be a "fake" pointer or a real delegate pointer, depending
191203
// on CHIP_CONFIG_STATIC_GLOBAL_INTERACTION_MODEL_ENGINE setting.
192204
//

0 commit comments

Comments
 (0)