From 5f2ca43f6368c77f973e2a91c487f96a4f7d62cb Mon Sep 17 00:00:00 2001 From: Yuanyao Zhong Date: Thu, 12 Dec 2024 13:40:24 -0500 Subject: [PATCH 01/11] Inject event management into report engine --- src/app/BUILD.gn | 1 + src/app/EventManagement.cpp | 16 ++++++++++++++-- src/app/EventManagement.h | 8 +++++++- src/app/InteractionModelEngine.cpp | 5 +++-- src/app/InteractionModelEngine.h | 4 +++- src/app/reporting/Engine.cpp | 22 ++++++++++++++++------ src/app/reporting/Engine.h | 10 +++++++--- 7 files changed, 51 insertions(+), 15 deletions(-) diff --git a/src/app/BUILD.gn b/src/app/BUILD.gn index 14fcfc96e45543..fa0f70f952aa93 100644 --- a/src/app/BUILD.gn +++ b/src/app/BUILD.gn @@ -163,6 +163,7 @@ static_library("interaction-model") { "CommandSender.h", "DeviceProxy.cpp", "DeviceProxy.h", + "EventScheduler.h", "InteractionModelDelegatePointers.cpp", "InteractionModelDelegatePointers.h", "InteractionModelEngine.cpp", diff --git a/src/app/EventManagement.cpp b/src/app/EventManagement.cpp index 4c01bedf4b9a5b..3726ffaa316bdf 100644 --- a/src/app/EventManagement.cpp +++ b/src/app/EventManagement.cpp @@ -83,7 +83,7 @@ struct CopyAndAdjustDeltaTimeContext void EventManagement::Init(Messaging::ExchangeManager * apExchangeManager, uint32_t aNumBuffers, CircularEventBuffer * apCircularEventBuffer, const LogStorageResources * const apLogStorageResources, MonotonicallyIncreasingCounter * apEventNumberCounter, - System::Clock::Milliseconds64 aMonotonicStartupTime) + System::Clock::Milliseconds64 aMonotonicStartupTime, EventScheduler * apEventScheduler) { CircularEventBuffer * current = nullptr; CircularEventBuffer * prev = nullptr; @@ -124,6 +124,15 @@ void EventManagement::Init(Messaging::ExchangeManager * apExchangeManager, uint3 mBytesWritten = 0; mMonotonicStartupTime = aMonotonicStartupTime; + + if (apEventScheduler == nullptr) + { + mpEventScheduler = &InteractionModelEngine::GetInstance()->GetReportingEngine(); + } + else + { + mpEventScheduler = apEventScheduler; + } } CHIP_ERROR EventManagement::CopyToNextBuffer(CircularEventBuffer * apEventBuffer) @@ -490,7 +499,10 @@ CHIP_ERROR EventManagement::LogEventPrivate(EventLoggingDelegate * apDelegate, c opts.mTimestamp.mType == Timestamp::Type::kSystem ? "Sys" : "Epoch", ChipLogValueX64(opts.mTimestamp.mValue)); #endif // CHIP_CONFIG_EVENT_LOGGING_VERBOSE_DEBUG_LOGS - err = InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleEventDelivery(opts.mPath, mBytesWritten); + if (mpEventScheduler) + { + err = mpEventScheduler->ScheduleEventDelivery(opts.mPath, mBytesWritten); + } } return err; diff --git a/src/app/EventManagement.h b/src/app/EventManagement.h index 76220350fc2507..9ed0061715a61a 100644 --- a/src/app/EventManagement.h +++ b/src/app/EventManagement.h @@ -29,6 +29,7 @@ #include "EventLoggingDelegate.h" #include #include +#include #include #include #include @@ -224,12 +225,15 @@ class EventManagement : public DataModel::EventsGenerator * @param[in] aMonotonicStartupTime Time we should consider as "monotonic * time 0" for cases when we use * system-time event timestamps. + * + * @param[in] apEventScheduler Scheduler to deliver the event, default is the reporting + * engine in InteractionModelEngine. * */ void Init(Messaging::ExchangeManager * apExchangeManager, uint32_t aNumBuffers, CircularEventBuffer * apCircularEventBuffer, const LogStorageResources * const apLogStorageResources, MonotonicallyIncreasingCounter * apEventNumberCounter, - System::Clock::Milliseconds64 aMonotonicStartupTime); + System::Clock::Milliseconds64 aMonotonicStartupTime, EventScheduler * apEventScheduler = nullptr); static EventManagement & GetInstance(); @@ -563,6 +567,8 @@ class EventManagement : public DataModel::EventsGenerator Timestamp mLastEventTimestamp; ///< The timestamp of the last event in this buffer System::Clock::Milliseconds64 mMonotonicStartupTime; + + EventScheduler * mpEventScheduler = nullptr; }; } // namespace app diff --git a/src/app/InteractionModelEngine.cpp b/src/app/InteractionModelEngine.cpp index 78967c6d53c680..7fc2925e3b4962 100644 --- a/src/app/InteractionModelEngine.cpp +++ b/src/app/InteractionModelEngine.cpp @@ -149,7 +149,8 @@ InteractionModelEngine * InteractionModelEngine::GetInstance() CHIP_ERROR InteractionModelEngine::Init(Messaging::ExchangeManager * apExchangeMgr, FabricTable * apFabricTable, reporting::ReportScheduler * reportScheduler, CASESessionManager * apCASESessionMgr, - SubscriptionResumptionStorage * subscriptionResumptionStorage) + SubscriptionResumptionStorage * subscriptionResumptionStorage, + EventManagement * eventManagement) { VerifyOrReturnError(apFabricTable != nullptr, CHIP_ERROR_INVALID_ARGUMENT); VerifyOrReturnError(apExchangeMgr != nullptr, CHIP_ERROR_INVALID_ARGUMENT); @@ -165,7 +166,7 @@ CHIP_ERROR InteractionModelEngine::Init(Messaging::ExchangeManager * apExchangeM ReturnErrorOnFailure(mpFabricTable->AddFabricDelegate(this)); ReturnErrorOnFailure(mpExchangeMgr->RegisterUnsolicitedMessageHandlerForProtocol(Protocols::InteractionModel::Id, this)); - mReportingEngine.Init(); + mReportingEngine.Init(eventManagement); StatusIB::RegisterErrorFormatter(); diff --git a/src/app/InteractionModelEngine.h b/src/app/InteractionModelEngine.h index 36aa2ccc6e3dd9..dce25c5f352c58 100644 --- a/src/app/InteractionModelEngine.h +++ b/src/app/InteractionModelEngine.h @@ -126,11 +126,13 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler, * @param[in] apExchangeMgr A pointer to the ExchangeManager object. * @param[in] apFabricTable A pointer to the FabricTable object. * @param[in] apCASESessionMgr An optional pointer to a CASESessionManager (used for re-subscriptions). + * @parma[in] eventManagement An optional pointer to a EventManagement. Use the global instance if not presented. * */ CHIP_ERROR Init(Messaging::ExchangeManager * apExchangeMgr, FabricTable * apFabricTable, reporting::ReportScheduler * reportScheduler, CASESessionManager * apCASESessionMgr = nullptr, - SubscriptionResumptionStorage * subscriptionResumptionStorage = nullptr); + SubscriptionResumptionStorage * subscriptionResumptionStorage = nullptr, + EventManagement * eventManagement = nullptr); void Shutdown(); diff --git a/src/app/reporting/Engine.cpp b/src/app/reporting/Engine.cpp index e2691e519ac13d..a0482b93207c05 100644 --- a/src/app/reporting/Engine.cpp +++ b/src/app/reporting/Engine.cpp @@ -221,10 +221,20 @@ bool IsClusterDataVersionEqualTo(DataModel::Provider * dataModel, const Concrete Engine::Engine(InteractionModelEngine * apImEngine) : mpImEngine(apImEngine) {} -CHIP_ERROR Engine::Init() +CHIP_ERROR Engine::Init(EventManagement * apEventManagement) { mNumReportsInFlight = 0; mCurReadHandlerIdx = 0; + + if (apEventManagement == nullptr) + { + mpEventManagement = &EventManagement::GetInstance(); + } + else + { + mpEventManagement = apEventManagement; + } + return CHIP_NO_ERROR; } @@ -562,18 +572,18 @@ CHIP_ERROR Engine::BuildSingleReportDataEventReports(ReportDataMessage::Builder TLV::TLVWriter backup; bool eventClean = true; auto & eventMin = apReadHandler->GetEventMin(); - EventManagement & eventManager = EventManagement::GetInstance(); bool hasMoreChunks = false; aReportDataBuilder.Checkpoint(backup); VerifyOrExit(apReadHandler->GetEventPathList() != nullptr, ); - // If the eventManager is not valid or has not been initialized, + // If the mpEventManagement is not valid or has not been initialized, // skip the rest of processing - VerifyOrExit(eventManager.IsValid(), ChipLogError(DataManagement, "EventManagement has not yet initialized")); + VerifyOrExit(mpEventManagement != nullptr && mpEventManagement->IsValid(), + ChipLogError(DataManagement, "EventManagement has not yet initialized")); - eventClean = apReadHandler->CheckEventClean(eventManager); + eventClean = apReadHandler->CheckEventClean(*mpEventManagement); // proceed only if there are new events. if (eventClean) @@ -593,7 +603,7 @@ CHIP_ERROR Engine::BuildSingleReportDataEventReports(ReportDataMessage::Builder err = CheckAccessDeniedEventPaths(*(eventReportIBs.GetWriter()), hasEncodedStatus, apReadHandler); SuccessOrExit(err); - err = eventManager.FetchEventsSince(*(eventReportIBs.GetWriter()), apReadHandler->GetEventPathList(), eventMin, eventCount, + err = mpEventManagement->FetchEventsSince(*(eventReportIBs.GetWriter()), apReadHandler->GetEventPathList(), eventMin, eventCount, apReadHandler->GetSubjectDescriptor()); if ((err == CHIP_END_OF_TLV) || (err == CHIP_ERROR_TLV_UNDERRUN) || (err == CHIP_NO_ERROR)) diff --git a/src/app/reporting/Engine.h b/src/app/reporting/Engine.h index a16b0d9151d3d9..48060b7ba5f3e4 100644 --- a/src/app/reporting/Engine.h +++ b/src/app/reporting/Engine.h @@ -55,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 : public DataModel::ProviderChangeListener +class Engine : public DataModel::ProviderChangeListener, public EventScheduler { public: /** @@ -65,11 +65,13 @@ class Engine : public DataModel::ProviderChangeListener /** * Initializes the reporting engine. Should only be called once. + * + * @param[in] A pointer to EventManagement. Use the global one by default. * * @retval #CHIP_NO_ERROR On success. * @retval other Was unable to retrieve data and write it into the writer. */ - CHIP_ERROR Init(); + CHIP_ERROR Init(EventManagement * apEventManagement = nullptr); void Shutdown(); @@ -101,7 +103,7 @@ class Engine : public DataModel::ProviderChangeListener * Schedule the event delivery * */ - CHIP_ERROR ScheduleEventDelivery(ConcreteEventPath & aPath, uint32_t aBytesWritten); + CHIP_ERROR ScheduleEventDelivery(ConcreteEventPath & aPath, uint32_t aBytesWritten) override; /* * Resets the tracker that tracks the currently serviced read handler. @@ -287,6 +289,8 @@ class Engine : public DataModel::ProviderChangeListener #endif InteractionModelEngine * mpImEngine = nullptr; + + EventManagement * mpEventManagement = nullptr; }; }; // namespace reporting From 9f549dee26f8c886496b864454067bd6ab2a1e96 Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Thu, 12 Dec 2024 18:43:37 +0000 Subject: [PATCH 02/11] Restyled by whitespace --- src/app/EventManagement.h | 2 +- src/app/reporting/Engine.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/EventManagement.h b/src/app/EventManagement.h index 9ed0061715a61a..a9b685916b2c2f 100644 --- a/src/app/EventManagement.h +++ b/src/app/EventManagement.h @@ -225,7 +225,7 @@ class EventManagement : public DataModel::EventsGenerator * @param[in] aMonotonicStartupTime Time we should consider as "monotonic * time 0" for cases when we use * system-time event timestamps. - * + * * @param[in] apEventScheduler Scheduler to deliver the event, default is the reporting * engine in InteractionModelEngine. * diff --git a/src/app/reporting/Engine.h b/src/app/reporting/Engine.h index 48060b7ba5f3e4..b6271a821ecda6 100644 --- a/src/app/reporting/Engine.h +++ b/src/app/reporting/Engine.h @@ -65,7 +65,7 @@ class Engine : public DataModel::ProviderChangeListener, public EventScheduler /** * Initializes the reporting engine. Should only be called once. - * + * * @param[in] A pointer to EventManagement. Use the global one by default. * * @retval #CHIP_NO_ERROR On success. From 5cc696caadce86ea056d2948100934b0eb1b0450 Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Thu, 12 Dec 2024 18:43:42 +0000 Subject: [PATCH 03/11] Restyled by clang-format --- src/app/reporting/Engine.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/app/reporting/Engine.cpp b/src/app/reporting/Engine.cpp index a0482b93207c05..eb34b977a78976 100644 --- a/src/app/reporting/Engine.cpp +++ b/src/app/reporting/Engine.cpp @@ -570,9 +570,9 @@ CHIP_ERROR Engine::BuildSingleReportDataEventReports(ReportDataMessage::Builder size_t eventCount = 0; bool hasEncodedStatus = false; TLV::TLVWriter backup; - bool eventClean = true; - auto & eventMin = apReadHandler->GetEventMin(); - bool hasMoreChunks = false; + bool eventClean = true; + auto & eventMin = apReadHandler->GetEventMin(); + bool hasMoreChunks = false; aReportDataBuilder.Checkpoint(backup); @@ -603,8 +603,8 @@ CHIP_ERROR Engine::BuildSingleReportDataEventReports(ReportDataMessage::Builder err = CheckAccessDeniedEventPaths(*(eventReportIBs.GetWriter()), hasEncodedStatus, apReadHandler); SuccessOrExit(err); - err = mpEventManagement->FetchEventsSince(*(eventReportIBs.GetWriter()), apReadHandler->GetEventPathList(), eventMin, eventCount, - apReadHandler->GetSubjectDescriptor()); + err = mpEventManagement->FetchEventsSince(*(eventReportIBs.GetWriter()), apReadHandler->GetEventPathList(), eventMin, + eventCount, apReadHandler->GetSubjectDescriptor()); if ((err == CHIP_END_OF_TLV) || (err == CHIP_ERROR_TLV_UNDERRUN) || (err == CHIP_NO_ERROR)) { From f44a7a46accfdd266bf1177094675b9944ef0b31 Mon Sep 17 00:00:00 2001 From: Yuanyao Zhong Date: Thu, 12 Dec 2024 14:18:50 -0500 Subject: [PATCH 04/11] add event scheduler file --- src/app/EventScheduler.h | 50 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 src/app/EventScheduler.h diff --git a/src/app/EventScheduler.h b/src/app/EventScheduler.h new file mode 100644 index 00000000000000..0ef403b13a6d13 --- /dev/null +++ b/src/app/EventScheduler.h @@ -0,0 +1,50 @@ +/* + * + * Copyright (c) 2024 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * @file + * + * @brief + * Define EventScheduler interface. Event scheduler is used by EventManagement to notify that events are ready to be scheduled. + * Usually this is implemented by the Reporting Engine to find the proper ReadHandlers and deliver the events. + * + */ +#pragma once + +#include + +namespace chip { +namespace app { + +class EventScheduler +{ +public: + virtual ~EventScheduler() = default; + + /** + * @brief + * Schedule the event delivery + * + * @param[in] aPath The path to the event. + * @param[in] aBytesWritten Bytes that the event is written into the buffer in EventManagement. + */ + CHIP_ERROR virtual ScheduleEventDelivery(ConcreteEventPath & aPath, uint32_t aBytesWritten) = 0; +}; + +} // namespace app +} // namespace chip From d14ef631088f20965e9987b0fce44de5474741fc Mon Sep 17 00:00:00 2001 From: Yuanyao Zhong Date: Thu, 12 Dec 2024 14:42:59 -0500 Subject: [PATCH 05/11] add missing includes --- src/app/EventScheduler.h | 1 + src/app/reporting/Engine.h | 1 + 2 files changed, 2 insertions(+) diff --git a/src/app/EventScheduler.h b/src/app/EventScheduler.h index 0ef403b13a6d13..1d9abbc1029be3 100644 --- a/src/app/EventScheduler.h +++ b/src/app/EventScheduler.h @@ -27,6 +27,7 @@ #pragma once #include +#include namespace chip { namespace app { diff --git a/src/app/reporting/Engine.h b/src/app/reporting/Engine.h index b6271a821ecda6..c26701f4bc6bdd 100644 --- a/src/app/reporting/Engine.h +++ b/src/app/reporting/Engine.h @@ -25,6 +25,7 @@ #pragma once #include +#include #include #include #include From 2edc0ed7eb7023b63deaf628f8fda4cadaadd1f4 Mon Sep 17 00:00:00 2001 From: Yuanyao Zhong Date: Fri, 13 Dec 2024 14:45:13 -0500 Subject: [PATCH 06/11] Rename it to EventReporter --- src/app/BUILD.gn | 9 +++++++- src/app/EventManagement.cpp | 13 ++++++------ src/app/EventManagement.h | 8 +++---- src/app/{EventScheduler.h => EventReporter.h} | 10 ++++----- src/app/InteractionModelEngine.cpp | 2 +- src/app/reporting/Engine.cpp | 13 +++--------- src/app/reporting/Engine.h | 21 +++++++++---------- 7 files changed, 38 insertions(+), 38 deletions(-) rename src/app/{EventScheduler.h => EventReporter.h} (78%) diff --git a/src/app/BUILD.gn b/src/app/BUILD.gn index fa0f70f952aa93..96385f22266e9b 100644 --- a/src/app/BUILD.gn +++ b/src/app/BUILD.gn @@ -149,6 +149,12 @@ source_set("test-event-trigger") { sources = [ "TestEventTriggerDelegate.h" ] } +source_set("event-reporter") { + sources = [ + "EventReporter.h", + ] +} + # interaction-model is a static-library because it currently requires global functions (app/util/...) that are stubbed in different test files that depend on the app static_library # which in tern depens on the interaction-model. # Using source_set prevents the unit test to build correctly. @@ -163,7 +169,6 @@ static_library("interaction-model") { "CommandSender.h", "DeviceProxy.cpp", "DeviceProxy.h", - "EventScheduler.h", "InteractionModelDelegatePointers.cpp", "InteractionModelDelegatePointers.h", "InteractionModelEngine.cpp", @@ -211,6 +216,7 @@ static_library("interaction-model") { ":app_config", ":command-handler-impl", ":constants", + ":event-reporter", ":paths", ":subscription-info-provider", "${chip_root}/src/app/MessageDef", @@ -456,6 +462,7 @@ static_library("app") { ":app_config", ":attribute-access", ":constants", + ":event-reporter", ":global-attributes", ":interaction-model", "${chip_root}/src/app/data-model", diff --git a/src/app/EventManagement.cpp b/src/app/EventManagement.cpp index 3726ffaa316bdf..36b676af40b7bb 100644 --- a/src/app/EventManagement.cpp +++ b/src/app/EventManagement.cpp @@ -83,7 +83,7 @@ struct CopyAndAdjustDeltaTimeContext void EventManagement::Init(Messaging::ExchangeManager * apExchangeManager, uint32_t aNumBuffers, CircularEventBuffer * apCircularEventBuffer, const LogStorageResources * const apLogStorageResources, MonotonicallyIncreasingCounter * apEventNumberCounter, - System::Clock::Milliseconds64 aMonotonicStartupTime, EventScheduler * apEventScheduler) + System::Clock::Milliseconds64 aMonotonicStartupTime, EventReporter * apEventReporter) { CircularEventBuffer * current = nullptr; CircularEventBuffer * prev = nullptr; @@ -125,13 +125,14 @@ void EventManagement::Init(Messaging::ExchangeManager * apExchangeManager, uint3 mMonotonicStartupTime = aMonotonicStartupTime; - if (apEventScheduler == nullptr) + // Should remove using the global instance and rely only on passed in variable. + if (apEventReporter == nullptr) { - mpEventScheduler = &InteractionModelEngine::GetInstance()->GetReportingEngine(); + mpEventReporter = &InteractionModelEngine::GetInstance()->GetReportingEngine(); } else { - mpEventScheduler = apEventScheduler; + mpEventReporter = apEventReporter; } } @@ -499,9 +500,9 @@ CHIP_ERROR EventManagement::LogEventPrivate(EventLoggingDelegate * apDelegate, c opts.mTimestamp.mType == Timestamp::Type::kSystem ? "Sys" : "Epoch", ChipLogValueX64(opts.mTimestamp.mValue)); #endif // CHIP_CONFIG_EVENT_LOGGING_VERBOSE_DEBUG_LOGS - if (mpEventScheduler) + if (mpEventReporter) { - err = mpEventScheduler->ScheduleEventDelivery(opts.mPath, mBytesWritten); + err = mpEventReporter->NewEventGenerated(opts.mPath, mBytesWritten); } } diff --git a/src/app/EventManagement.h b/src/app/EventManagement.h index a9b685916b2c2f..fe5a14b6d04ee7 100644 --- a/src/app/EventManagement.h +++ b/src/app/EventManagement.h @@ -29,7 +29,7 @@ #include "EventLoggingDelegate.h" #include #include -#include +#include #include #include #include @@ -226,14 +226,14 @@ class EventManagement : public DataModel::EventsGenerator * time 0" for cases when we use * system-time event timestamps. * - * @param[in] apEventScheduler Scheduler to deliver the event, default is the reporting + * @param[in] apEventReporter Event reporter to deliver the event, default is the reporting * engine in InteractionModelEngine. * */ void Init(Messaging::ExchangeManager * apExchangeManager, uint32_t aNumBuffers, CircularEventBuffer * apCircularEventBuffer, const LogStorageResources * const apLogStorageResources, MonotonicallyIncreasingCounter * apEventNumberCounter, - System::Clock::Milliseconds64 aMonotonicStartupTime, EventScheduler * apEventScheduler = nullptr); + System::Clock::Milliseconds64 aMonotonicStartupTime, EventReporter * apEventReporter = nullptr); static EventManagement & GetInstance(); @@ -568,7 +568,7 @@ class EventManagement : public DataModel::EventsGenerator System::Clock::Milliseconds64 mMonotonicStartupTime; - EventScheduler * mpEventScheduler = nullptr; + EventReporter * mpEventReporter = nullptr; }; } // namespace app diff --git a/src/app/EventScheduler.h b/src/app/EventReporter.h similarity index 78% rename from src/app/EventScheduler.h rename to src/app/EventReporter.h index 1d9abbc1029be3..6ccc070fec0eb0 100644 --- a/src/app/EventScheduler.h +++ b/src/app/EventReporter.h @@ -20,7 +20,7 @@ * @file * * @brief - * Define EventScheduler interface. Event scheduler is used by EventManagement to notify that events are ready to be scheduled. + * Define EventReporter interface. Event reproter is used by EventManagement to notify that events are ready to be reported. * Usually this is implemented by the Reporting Engine to find the proper ReadHandlers and deliver the events. * */ @@ -32,19 +32,19 @@ namespace chip { namespace app { -class EventScheduler +class EventReporter { public: - virtual ~EventScheduler() = default; + virtual ~EventReporter() = default; /** * @brief - * Schedule the event delivery + * Notify of a new event generated. * * @param[in] aPath The path to the event. * @param[in] aBytesWritten Bytes that the event is written into the buffer in EventManagement. */ - CHIP_ERROR virtual ScheduleEventDelivery(ConcreteEventPath & aPath, uint32_t aBytesWritten) = 0; + CHIP_ERROR virtual NewEventGenerated(ConcreteEventPath & aPath, uint32_t aBytesWritten) = 0; }; } // namespace app diff --git a/src/app/InteractionModelEngine.cpp b/src/app/InteractionModelEngine.cpp index 7fc2925e3b4962..2fafe375f07e12 100644 --- a/src/app/InteractionModelEngine.cpp +++ b/src/app/InteractionModelEngine.cpp @@ -166,7 +166,7 @@ CHIP_ERROR InteractionModelEngine::Init(Messaging::ExchangeManager * apExchangeM ReturnErrorOnFailure(mpFabricTable->AddFabricDelegate(this)); ReturnErrorOnFailure(mpExchangeMgr->RegisterUnsolicitedMessageHandlerForProtocol(Protocols::InteractionModel::Id, this)); - mReportingEngine.Init(eventManagement); + mReportingEngine.Init((eventManagement != nullptr) ? eventManagement : &EventManagement::GetInstance()); StatusIB::RegisterErrorFormatter(); diff --git a/src/app/reporting/Engine.cpp b/src/app/reporting/Engine.cpp index eb34b977a78976..1afd88ed566f0a 100644 --- a/src/app/reporting/Engine.cpp +++ b/src/app/reporting/Engine.cpp @@ -223,17 +223,10 @@ Engine::Engine(InteractionModelEngine * apImEngine) : mpImEngine(apImEngine) {} CHIP_ERROR Engine::Init(EventManagement * apEventManagement) { + VerifyOrReturnError(apEventManagement != nullptr, CHIP_ERROR_INVALID_ARGUMENT); mNumReportsInFlight = 0; mCurReadHandlerIdx = 0; - - if (apEventManagement == nullptr) - { - mpEventManagement = &EventManagement::GetInstance(); - } - else - { - mpEventManagement = apEventManagement; - } + mpEventManagement = apEventManagement; return CHIP_NO_ERROR; } @@ -1138,7 +1131,7 @@ CHIP_ERROR Engine::ScheduleBufferPressureEventDelivery(uint32_t aBytesWritten) return CHIP_NO_ERROR; } -CHIP_ERROR Engine::ScheduleEventDelivery(ConcreteEventPath & aPath, uint32_t aBytesWritten) +CHIP_ERROR Engine::NewEventGenerated(ConcreteEventPath & aPath, uint32_t aBytesWritten) { // If we literally have no read handlers right now that care about any events, // we don't need to call schedule run for event. diff --git a/src/app/reporting/Engine.h b/src/app/reporting/Engine.h index c26701f4bc6bdd..69df9e4422241a 100644 --- a/src/app/reporting/Engine.h +++ b/src/app/reporting/Engine.h @@ -25,7 +25,7 @@ #pragma once #include -#include +#include #include #include #include @@ -56,7 +56,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 : public DataModel::ProviderChangeListener, public EventScheduler +class Engine : public DataModel::ProviderChangeListener, public EventReporter { public: /** @@ -67,12 +67,12 @@ class Engine : public DataModel::ProviderChangeListener, public EventScheduler /** * Initializes the reporting engine. Should only be called once. * - * @param[in] A pointer to EventManagement. Use the global one by default. + * @param[in] A pointer to EventManagement, should not be a nullptr. * * @retval #CHIP_NO_ERROR On success. * @retval other Was unable to retrieve data and write it into the writer. */ - CHIP_ERROR Init(EventManagement * apEventManagement = nullptr); + CHIP_ERROR Init(EventManagement * apEventManagement); void Shutdown(); @@ -99,13 +99,6 @@ class Engine : public DataModel::ProviderChangeListener, public EventScheduler */ CHIP_ERROR SetDirty(const AttributePathParams & aAttributePathParams); - /** - * @brief - * Schedule the event delivery - * - */ - CHIP_ERROR ScheduleEventDelivery(ConcreteEventPath & aPath, uint32_t aBytesWritten) override; - /* * Resets the tracker that tracks the currently serviced read handler. * apReadHandler can be non-null to indicate that the reset is due to a @@ -185,6 +178,12 @@ class Engine : public DataModel::ProviderChangeListener, public EventScheduler bool IsClusterDataVersionMatch(const SingleLinkedListNode * aDataVersionFilterList, const ConcreteReadAttributePath & aPath); + /** + * EventReporter implementations. + * + */ + CHIP_ERROR NewEventGenerated(ConcreteEventPath & aPath, uint32_t aBytesWritten) override; + /** * Send Report via ReadHandler * From 49c4df86b4d6485cedd1ab53f1e49f1353c15cc2 Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Fri, 13 Dec 2024 19:46:19 +0000 Subject: [PATCH 07/11] Restyled by clang-format --- src/app/reporting/Engine.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/reporting/Engine.cpp b/src/app/reporting/Engine.cpp index 1afd88ed566f0a..4f58a0450bf4cc 100644 --- a/src/app/reporting/Engine.cpp +++ b/src/app/reporting/Engine.cpp @@ -226,7 +226,7 @@ CHIP_ERROR Engine::Init(EventManagement * apEventManagement) VerifyOrReturnError(apEventManagement != nullptr, CHIP_ERROR_INVALID_ARGUMENT); mNumReportsInFlight = 0; mCurReadHandlerIdx = 0; - mpEventManagement = apEventManagement; + mpEventManagement = apEventManagement; return CHIP_NO_ERROR; } From 18515660e0e37d9712100bcdbc2123c75fbc4442 Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Fri, 13 Dec 2024 19:46:21 +0000 Subject: [PATCH 08/11] Restyled by gn --- src/app/BUILD.gn | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/app/BUILD.gn b/src/app/BUILD.gn index 96385f22266e9b..11c4a4ae60238b 100644 --- a/src/app/BUILD.gn +++ b/src/app/BUILD.gn @@ -150,9 +150,7 @@ source_set("test-event-trigger") { } source_set("event-reporter") { - sources = [ - "EventReporter.h", - ] + sources = [ "EventReporter.h" ] } # interaction-model is a static-library because it currently requires global functions (app/util/...) that are stubbed in different test files that depend on the app static_library From 880961865147fc594ef80b878a6c57e031b90878 Mon Sep 17 00:00:00 2001 From: Yuanyao Zhong Date: Mon, 16 Dec 2024 11:32:52 -0500 Subject: [PATCH 09/11] Modify comments and fix typo --- src/app/EventReporter.h | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/app/EventReporter.h b/src/app/EventReporter.h index 6ccc070fec0eb0..9ae267de232f51 100644 --- a/src/app/EventReporter.h +++ b/src/app/EventReporter.h @@ -16,14 +16,6 @@ * limitations under the License. */ -/** - * @file - * - * @brief - * Define EventReporter interface. Event reproter is used by EventManagement to notify that events are ready to be reported. - * Usually this is implemented by the Reporting Engine to find the proper ReadHandlers and deliver the events. - * - */ #pragma once #include @@ -32,13 +24,17 @@ namespace chip { namespace app { +/** + * Define EventReporter interface. Event reporter is used by EventManagement to notify that events are ready to be reported. + * Usually this is implemented by the Reporting Engine to find the proper ReadHandlers and deliver the events. + * + */ class EventReporter { public: virtual ~EventReporter() = default; /** - * @brief * Notify of a new event generated. * * @param[in] aPath The path to the event. From 749f4aa1f7ad88b93fcf54246c61e8981646f7e5 Mon Sep 17 00:00:00 2001 From: Yuanyao Zhong Date: Wed, 18 Dec 2024 11:30:57 -0500 Subject: [PATCH 10/11] Fix some comments --- src/app/EventManagement.cpp | 7 ++----- src/app/EventManagement.h | 3 +-- src/app/EventReporter.h | 11 +++++------ src/app/InteractionModelEngine.h | 2 +- src/app/reporting/Engine.cpp | 4 ++-- src/app/reporting/Engine.h | 4 ++-- 6 files changed, 13 insertions(+), 18 deletions(-) diff --git a/src/app/EventManagement.cpp b/src/app/EventManagement.cpp index 36b676af40b7bb..1321475d3e8314 100644 --- a/src/app/EventManagement.cpp +++ b/src/app/EventManagement.cpp @@ -125,7 +125,7 @@ void EventManagement::Init(Messaging::ExchangeManager * apExchangeManager, uint3 mMonotonicStartupTime = aMonotonicStartupTime; - // Should remove using the global instance and rely only on passed in variable. + // TODO(#36890): Should remove using the global instance and rely only on passed in variable. if (apEventReporter == nullptr) { mpEventReporter = &InteractionModelEngine::GetInstance()->GetReportingEngine(); @@ -500,10 +500,7 @@ CHIP_ERROR EventManagement::LogEventPrivate(EventLoggingDelegate * apDelegate, c opts.mTimestamp.mType == Timestamp::Type::kSystem ? "Sys" : "Epoch", ChipLogValueX64(opts.mTimestamp.mValue)); #endif // CHIP_CONFIG_EVENT_LOGGING_VERBOSE_DEBUG_LOGS - if (mpEventReporter) - { - err = mpEventReporter->NewEventGenerated(opts.mPath, mBytesWritten); - } + err = mpEventReporter->NewEventGenerated(opts.mPath, mBytesWritten); } return err; diff --git a/src/app/EventManagement.h b/src/app/EventManagement.h index fe5a14b6d04ee7..f5687b586e02f4 100644 --- a/src/app/EventManagement.h +++ b/src/app/EventManagement.h @@ -226,8 +226,7 @@ class EventManagement : public DataModel::EventsGenerator * time 0" for cases when we use * system-time event timestamps. * - * @param[in] apEventReporter Event reporter to deliver the event, default is the reporting - * engine in InteractionModelEngine. + * @param[in] apEventReporter Event reporter to be notified when events are generated. * */ void Init(Messaging::ExchangeManager * apExchangeManager, uint32_t aNumBuffers, CircularEventBuffer * apCircularEventBuffer, diff --git a/src/app/EventReporter.h b/src/app/EventReporter.h index 9ae267de232f51..aaba792b6a1d99 100644 --- a/src/app/EventReporter.h +++ b/src/app/EventReporter.h @@ -25,8 +25,7 @@ namespace chip { namespace app { /** - * Define EventReporter interface. Event reporter is used by EventManagement to notify that events are ready to be reported. - * Usually this is implemented by the Reporting Engine to find the proper ReadHandlers and deliver the events. + * Interface that EventManagement can use to notify when events are generated and may need reporting. * */ class EventReporter @@ -35,12 +34,12 @@ class EventReporter virtual ~EventReporter() = default; /** - * Notify of a new event generated. + * Notify that an event was generated. * - * @param[in] aPath The path to the event. - * @param[in] aBytesWritten Bytes that the event is written into the buffer in EventManagement. + * @param[in] aPath The path that identifies the kind of event that was generated. + * @param[in] aBytesConsumed The number of bytes needed to store the event in EventManagement. */ - CHIP_ERROR virtual NewEventGenerated(ConcreteEventPath & aPath, uint32_t aBytesWritten) = 0; + CHIP_ERROR virtual NewEventGenerated(ConcreteEventPath & aPath, uint32_t aBytesConsumed) = 0; }; } // namespace app diff --git a/src/app/InteractionModelEngine.h b/src/app/InteractionModelEngine.h index dce25c5f352c58..c5a4a0811d274b 100644 --- a/src/app/InteractionModelEngine.h +++ b/src/app/InteractionModelEngine.h @@ -126,7 +126,7 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler, * @param[in] apExchangeMgr A pointer to the ExchangeManager object. * @param[in] apFabricTable A pointer to the FabricTable object. * @param[in] apCASESessionMgr An optional pointer to a CASESessionManager (used for re-subscriptions). - * @parma[in] eventManagement An optional pointer to a EventManagement. Use the global instance if not presented. + * @parma[in] eventManagement An optional pointer to a EventManagement. If null, the global instance will be used. * */ CHIP_ERROR Init(Messaging::ExchangeManager * apExchangeMgr, FabricTable * apFabricTable, diff --git a/src/app/reporting/Engine.cpp b/src/app/reporting/Engine.cpp index 4f58a0450bf4cc..a9cef5c27dc333 100644 --- a/src/app/reporting/Engine.cpp +++ b/src/app/reporting/Engine.cpp @@ -1131,7 +1131,7 @@ CHIP_ERROR Engine::ScheduleBufferPressureEventDelivery(uint32_t aBytesWritten) return CHIP_NO_ERROR; } -CHIP_ERROR Engine::NewEventGenerated(ConcreteEventPath & aPath, uint32_t aBytesWritten) +CHIP_ERROR Engine::NewEventGenerated(ConcreteEventPath & aPath, uint32_t aBytesConsumed) { // If we literally have no read handlers right now that care about any events, // we don't need to call schedule run for event. @@ -1169,7 +1169,7 @@ CHIP_ERROR Engine::NewEventGenerated(ConcreteEventPath & aPath, uint32_t aBytesW return CHIP_NO_ERROR; } - return ScheduleBufferPressureEventDelivery(aBytesWritten); + return ScheduleBufferPressureEventDelivery(aBytesConsumed); } void Engine::ScheduleUrgentEventDeliverySync(Optional fabricIndex) diff --git a/src/app/reporting/Engine.h b/src/app/reporting/Engine.h index 69df9e4422241a..6f60da7d8f9caf 100644 --- a/src/app/reporting/Engine.h +++ b/src/app/reporting/Engine.h @@ -179,10 +179,10 @@ class Engine : public DataModel::ProviderChangeListener, public EventReporter const ConcreteReadAttributePath & aPath); /** - * EventReporter implementations. + * EventReporter implementation. * */ - CHIP_ERROR NewEventGenerated(ConcreteEventPath & aPath, uint32_t aBytesWritten) override; + CHIP_ERROR NewEventGenerated(ConcreteEventPath & aPath, uint32_t aBytesConsumed) override; /** * Send Report via ReadHandler From caa1dea5c79bda43832f44b9afb2f60d9ccc6846 Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Wed, 18 Dec 2024 16:32:17 +0000 Subject: [PATCH 11/11] Restyled by clang-format --- src/app/EventReporter.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/EventReporter.h b/src/app/EventReporter.h index aaba792b6a1d99..7a87b580931cdc 100644 --- a/src/app/EventReporter.h +++ b/src/app/EventReporter.h @@ -39,7 +39,7 @@ class EventReporter * @param[in] aPath The path that identifies the kind of event that was generated. * @param[in] aBytesConsumed The number of bytes needed to store the event in EventManagement. */ - CHIP_ERROR virtual NewEventGenerated(ConcreteEventPath & aPath, uint32_t aBytesConsumed) = 0; + CHIP_ERROR virtual NewEventGenerated(ConcreteEventPath & aPath, uint32_t aBytesConsumed) = 0; }; } // namespace app