From 73281726ee3ba67bed12b521c59118b7656a1216 Mon Sep 17 00:00:00 2001 From: lpbeliveau-silabs Date: Wed, 28 Jun 2023 01:19:58 +1000 Subject: [PATCH 01/21] Added a new class that will handle the scheduling of reports. --- src/app/BUILD.gn | 3 + src/app/ReadHandler.cpp | 62 +++- src/app/ReadHandler.h | 210 +++++++++++-- src/app/reporting/Engine.cpp | 1 + src/app/reporting/ReportScheduler.h | 134 ++++++++ src/app/reporting/ReportSchedulerImpl.cpp | 194 ++++++++++++ src/app/reporting/ReportSchedulerImpl.h | 60 ++++ src/app/tests/BUILD.gn | 1 + src/app/tests/TestReportScheduler.cpp | 315 +++++++++++++++++++ src/controller/tests/data_model/TestRead.cpp | 2 +- 10 files changed, 944 insertions(+), 38 deletions(-) create mode 100644 src/app/reporting/ReportScheduler.h create mode 100644 src/app/reporting/ReportSchedulerImpl.cpp create mode 100644 src/app/reporting/ReportSchedulerImpl.h create mode 100644 src/app/tests/TestReportScheduler.cpp diff --git a/src/app/BUILD.gn b/src/app/BUILD.gn index 38e534093d1599..9de687a81e2025 100644 --- a/src/app/BUILD.gn +++ b/src/app/BUILD.gn @@ -190,6 +190,9 @@ static_library("app") { "WriteHandler.cpp", "reporting/Engine.cpp", "reporting/Engine.h", + "reporting/ReportScheduler.h", + "reporting/ReportSchedulerImpl.cpp", + "reporting/ReportSchedulerImpl.h", "reporting/reporting.h", ] diff --git a/src/app/ReadHandler.cpp b/src/app/ReadHandler.cpp index 4a92fcfac91f9c..cedf72fcb4384d 100644 --- a/src/app/ReadHandler.cpp +++ b/src/app/ReadHandler.cpp @@ -39,7 +39,7 @@ namespace app { using Status = Protocols::InteractionModel::Status; ReadHandler::ReadHandler(ManagementCallback & apCallback, Messaging::ExchangeContext * apExchangeContext, - InteractionType aInteractionType) : + InteractionType aInteractionType, Observer * observer) : mExchangeCtx(*this), mManagementCallback(apCallback) #if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS @@ -63,15 +63,33 @@ ReadHandler::ReadHandler(ManagementCallback & apCallback, Messaging::ExchangeCon SetStateFlag(ReadHandlerFlags::PrimingReports); mSessionHandle.Grab(mExchangeCtx->GetSessionHandle()); + + // TODO Uncomment when the ReportScheduler is implemented + // if (nullptr != observer) + //{ + // if (CHIP_NO_ERROR == SetObserver(observer)) + // { + // mObserver->OnReadHandlerAdded(this); + // } + //} } #if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS -ReadHandler::ReadHandler(ManagementCallback & apCallback) : +ReadHandler::ReadHandler(ManagementCallback & apCallback, Observer * observer) : mExchangeCtx(*this), mManagementCallback(apCallback), mOnConnectedCallback(HandleDeviceConnected, this), mOnConnectionFailureCallback(HandleDeviceConnectionFailure, this) { mInteractionType = InteractionType::Subscribe; mFlags.ClearAll(); + + // TODO Uncomment when the ReportScheduler is implemented + // if (nullptr != observer) + //{ + // if (CHIP_NO_ERROR == SetObserver(observer)) + // { + // mObserver->OnReadHandlerAdded(this); + // } + //} } void ReadHandler::ResumeSubscription(CASESessionManager & caseSessionManager, @@ -137,6 +155,12 @@ ReadHandler::~ReadHandler() InteractionModelEngine::GetInstance()->ReleaseAttributePathList(mpAttributePathList); InteractionModelEngine::GetInstance()->ReleaseEventPathList(mpEventPathList); InteractionModelEngine::GetInstance()->ReleaseDataVersionFilterList(mpDataVersionFilterList); + + // TODO Uncomment when the ReportScheduler is implemented + // if (nullptr != mObserver) + //{ + // mObserver->OnReadHandlerRemoved(this); + //} } void ReadHandler::Close(CloseOptions options) @@ -319,6 +343,13 @@ CHIP_ERROR ReadHandler::SendReportData(System::PacketBufferHandle && aPayload, b if (IsType(InteractionType::Subscribe) && !IsPriming()) { + // TODO: Uncomment when the ReportScheduler is implemented and remove call to UpdateReportTimer, handled by + // the report Scheduler + // if (nullptr != mObserver) + //{ + // mObserver->OnReportSent(this); + //} + // Ignore the error from UpdateReportTimer. If we've // successfully sent the message, we need to return success from // this method. @@ -593,6 +624,11 @@ void ReadHandler::MoveToState(const HandlerState aTargetState) // if (aTargetState == HandlerState::GeneratingReports && IsReportableNow()) { + // TODO Uncomment when the ReportScheduler is implemented and remove the call to ScheduleRun() + // if(nullptr != mObserver) + //{ + // mObserver->OnBecameReportable(this); + //} InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun(); } } @@ -634,6 +670,12 @@ CHIP_ERROR ReadHandler::SendSubscribeResponse() ReturnErrorOnFailure(writer.Finalize(&packet)); VerifyOrReturnLogError(mExchangeCtx, CHIP_ERROR_INCORRECT_STATE); + // TODO: Uncomment when the ReportScheduler is implemented and remove call to UpdateReportTimer, handled by + // the report Scheduler + // if (nullptr != mObserver) + //{ + // mObserver->OnReportSent(this); + //} ReturnErrorOnFailure(UpdateReportTimer()); ClearStateFlag(ReadHandlerFlags::PrimingReports); @@ -753,6 +795,7 @@ void ReadHandler::PersistSubscription() } } +// TODO: Remove as timing will now be handled by the ReportScheduler void ReadHandler::MinIntervalExpiredCallback(System::Layer * apSystemLayer, void * apAppState) { VerifyOrReturn(apAppState != nullptr); @@ -764,6 +807,7 @@ void ReadHandler::MinIntervalExpiredCallback(System::Layer * apSystemLayer, void readHandler); } +// TODO: Remove as timing will now be handled by the ReportScheduler void ReadHandler::MaxIntervalExpiredCallback(System::Layer * apSystemLayer, void * apAppState) { VerifyOrReturn(apAppState != nullptr); @@ -773,6 +817,7 @@ void ReadHandler::MaxIntervalExpiredCallback(System::Layer * apSystemLayer, void readHandler->mMaxInterval - readHandler->mMinIntervalFloorSeconds); } +// TODO: Remove as timing will now be handled by the ReportScheduler CHIP_ERROR ReadHandler::UpdateReportTimer() { InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->CancelTimer( @@ -812,7 +857,7 @@ void ReadHandler::AttributePathIsDirty(const AttributePathParams & aAttributeCha // Here we just reset the iterator to the beginning of the current cluster, if the dirty path affects it. // This will ensure the reports are consistent within a single cluster generated from a single path in the request. - // TODO (#16699): Currently we can only gurentee the reports generated from a single path in the request are consistent. The + // TODO (#16699): Currently we can only guarantee the reports generated from a single path in the request are consistent. The // data might be inconsistent if the user send a request with two paths from the same cluster. We need to clearify the behavior // or make it consistent. if (mAttributePathExpandIterator.Get(path) && @@ -831,6 +876,11 @@ void ReadHandler::AttributePathIsDirty(const AttributePathParams & aAttributeCha if (IsReportableNow()) { + // TODO Uncomment when the ReportScheduler is implemented and remove the call to ScheduleRun() + // if(nullptr != mObserver) + //{ + // mObserver->OnBecameReportable(this); + //} InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun(); } } @@ -853,9 +903,15 @@ void ReadHandler::SetStateFlag(ReadHandlerFlags aFlag, bool aValue) { bool oldReportable = IsReportableNow(); mFlags.Set(aFlag, aValue); + // If we became reportable, schedule a reporting run. if (!oldReportable && IsReportableNow()) { + // TODO Uncomment when the ReportScheduler is implemented and remove the call to ScheduleRun() + // if(nullptr != mObserver) + //{ + // mObserver->OnBecameReportable(this); + //} InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun(); } } diff --git a/src/app/ReadHandler.h b/src/app/ReadHandler.h index f460188935223e..444244fb4e4967 100644 --- a/src/app/ReadHandler.h +++ b/src/app/ReadHandler.h @@ -64,6 +64,8 @@ namespace app { namespace reporting { class Engine; class TestReportingEngine; +class ReportScheduler; +class TestReportScheduler; } // namespace reporting class InteractionModelEngine; @@ -152,6 +154,36 @@ class ReadHandler : public Messaging::ExchangeDelegate virtual ApplicationCallback * GetAppCallback() = 0; }; + // TODO : Merge existing callback and observer into one class and have an observer pool in the Readhandler to notify every + /* + * Observer class for ReadHandler, meant to allow multiple objects to observe the ReadHandler. Currently only one observer is + * supported but all above callbacks should be merged into observer type and an observer pool should be added to allow multiple + * objects to observe ReadHandler + */ + class Observer + { + public: + virtual ~Observer() = default; + + /// @brief Callback invoked to notify a ReadHandler was created and can be registered + /// @param[in] apReadHandler ReadHandler getting added + virtual void OnReadHandlerAdded(ReadHandler * apReadHandler) = 0; + + /// @brief Callback invoked when a ReadHandler went from a non reportable state to a reportable state so a report can be + /// sent immediately if the minimal interval allows it. Otherwise the report should be rescheduled to the earliest time + /// allowed. + /// @param[in] apReadHandler ReadHandler that became dirty + virtual void OnBecameReportable(ReadHandler * apReadHandler) = 0; + + /// @brief Callback invoked when a Repport is sent so the next report can be scheduled + /// @param[in] apReadHandler ReadHandler that has generated a report + virtual void OnReportSent(ReadHandler * apReadHandler) = 0; + + /// @brief Callback invoked when a ReadHandler is getting removed so it can be unregistered + /// @param[in] apReadHandler ReadHandler getting destroyed + virtual void OnReadHandlerRemoved(ReadHandler * apReadHandler) = 0; + }; + /* * Destructor - as part of destruction, it will abort the exchange context * if a valid one still exists. @@ -167,7 +199,8 @@ class ReadHandler : public Messaging::ExchangeDelegate * The callback passed in has to outlive this handler object. * */ - ReadHandler(ManagementCallback & apCallback, Messaging::ExchangeContext * apExchangeContext, InteractionType aInteractionType); + ReadHandler(ManagementCallback & apCallback, Messaging::ExchangeContext * apExchangeContext, InteractionType aInteractionType, + Observer * observer = nullptr); #if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS /** @@ -177,12 +210,21 @@ class ReadHandler : public Messaging::ExchangeDelegate * The callback passed in has to outlive this handler object. * */ - ReadHandler(ManagementCallback & apCallback); + ReadHandler(ManagementCallback & apCallback, Observer * observer = nullptr); #endif - const ObjectList * GetAttributePathList() const { return mpAttributePathList; } - const ObjectList * GetEventPathList() const { return mpEventPathList; } - const ObjectList * GetDataVersionFilterList() const { return mpDataVersionFilterList; } + const ObjectList * GetAttributePathList() const + { + return mpAttributePathList; + } + const ObjectList * GetEventPathList() const + { + return mpEventPathList; + } + const ObjectList * GetDataVersionFilterList() const + { + return mpDataVersionFilterList; + } void GetReportingIntervals(uint16_t & aMinInterval, uint16_t & aMaxInterval) const { @@ -190,13 +232,21 @@ class ReadHandler : public Messaging::ExchangeDelegate aMaxInterval = mMaxInterval; } + CHIP_ERROR SetMinReportingIntervals(uint16_t aMinInterval) + { + VerifyOrReturnError(IsIdle(), CHIP_ERROR_INCORRECT_STATE); + VerifyOrReturnError(aMinInterval <= mMaxInterval, CHIP_ERROR_INVALID_ARGUMENT); + mMinIntervalFloorSeconds = aMinInterval; + return CHIP_NO_ERROR; + } + /* - * Set the reporting intervals for the subscription. This SHALL only be called + * Set the maximum reporting intervals for the subscription. This SHALL only be called * from the OnSubscriptionRequested callback above. The restriction is as below * MinIntervalFloor ≤ MaxInterval ≤ MAX(SUBSCRIPTION_MAX_INTERVAL_PUBLISHER_LIMIT, MaxIntervalCeiling) * Where SUBSCRIPTION_MAX_INTERVAL_PUBLISHER_LIMIT is set to 60m in the spec. */ - CHIP_ERROR SetReportingIntervals(uint16_t aMaxInterval) + CHIP_ERROR SetMaxReportingIntervals(uint16_t aMaxInterval) { VerifyOrReturnError(IsIdle(), CHIP_ERROR_INCORRECT_STATE); VerifyOrReturnError(mMinIntervalFloorSeconds <= aMaxInterval, CHIP_ERROR_INVALID_ARGUMENT); @@ -206,21 +256,39 @@ class ReadHandler : public Messaging::ExchangeDelegate return CHIP_NO_ERROR; } + /// @brief Add an observer to the read handler, currently only one observer is supported but all other callbacks should be + /// merged with a general observer type to allow multiple object to observe readhandlers + /// @param aObserver observer to be added + /// @return CHIP_ERROR_INVALID_ARGUMENT if passing in nullptr + CHIP_ERROR SetObserver(Observer * aObserver) + { + VerifyOrReturnError(nullptr != aObserver, CHIP_ERROR_INVALID_ARGUMENT); + // TODO : After merging the callbacks and observer, change so the method adds a new observer to an observer pool + mObserver = aObserver; + return CHIP_NO_ERROR; + } + private: - PriorityLevel GetCurrentPriority() const { return mCurrentPriority; } - EventNumber & GetEventMin() { return mEventMin; } + PriorityLevel GetCurrentPriority() const + { + return mCurrentPriority; + } + EventNumber & GetEventMin() + { + return mEventMin; + } enum class ReadHandlerFlags : uint8_t { // WaitingUntilMinInterval is used to prevent subscription data delivery while we are // waiting for the min reporting interval to elapse. - WaitingUntilMinInterval = (1 << 0), + WaitingUntilMinInterval = (1 << 0), // TODO : Remove once ReportScheduler is implemented or change to test flag // WaitingUntilMaxInterval is used to prevent subscription empty report delivery while we // are waiting for the max reporting interval to elaps. When WaitingUntilMaxInterval // becomes false, we are allowed to send an empty report to keep the // subscription alive on the client. - WaitingUntilMaxInterval = (1 << 1), + WaitingUntilMaxInterval = (1 << 1), // TODO : Remove once ReportScheduler is implemented // The flag indicating we are in the middle of a series of chunked report messages, this flag will be cleared during // sending last chunked message. @@ -289,8 +357,13 @@ class ReadHandler : public Messaging::ExchangeDelegate */ bool IsFromSubscriber(Messaging::ExchangeContext & apExchangeContext) const; - bool IsIdle() const { return mState == HandlerState::Idle; } + bool IsIdle() const + { + return mState == HandlerState::Idle; + } + // TODO: Change back to IsReportable once ReportScheduler is implemented so this can assess reportability without considering + // timing. The ReporScheduler will handle timing. /// @brief Returns whether the ReadHandler is in a state where it can immediately send a report. This function /// is used to determine whether a report generation should be scheduled for the handler. bool IsReportableNow() const @@ -301,8 +374,14 @@ class ReadHandler : public Messaging::ExchangeDelegate return mState == HandlerState::GeneratingReports && !mFlags.Has(ReadHandlerFlags::WaitingUntilMinInterval) && (IsDirty() || !mFlags.Has(ReadHandlerFlags::WaitingUntilMaxInterval)); } - bool IsGeneratingReports() const { return mState == HandlerState::GeneratingReports; } - bool IsAwaitingReportResponse() const { return mState == HandlerState::AwaitingReportResponse; } + bool IsGeneratingReports() const + { + return mState == HandlerState::GeneratingReports; + } + bool IsAwaitingReportResponse() const + { + return mState == HandlerState::AwaitingReportResponse; + } // Resets the path iterator to the beginning of the whole report for generating a series of new reports. void ResetPathIterator(); @@ -314,17 +393,41 @@ class ReadHandler : public Messaging::ExchangeDelegate // sanpshotted last event, check with latest last event number, re-setup snapshoted checkpoint, and compare again. bool CheckEventClean(EventManagement & aEventManager); - bool IsType(InteractionType type) const { return (mInteractionType == type); } - bool IsChunkedReport() const { return mFlags.Has(ReadHandlerFlags::ChunkedReport); } + bool IsType(InteractionType type) const + { + return (mInteractionType == type); + } + bool IsChunkedReport() const + { + return mFlags.Has(ReadHandlerFlags::ChunkedReport); + } // Is reporting indicates whether we are in the middle of a series chunks. As we will set mIsChunkedReport on the first chunk // and clear that flag on the last chunk, we can use mIsChunkedReport to indicate this state. - bool IsReporting() const { return mFlags.Has(ReadHandlerFlags::ChunkedReport); } - bool IsPriming() const { return mFlags.Has(ReadHandlerFlags::PrimingReports); } - bool IsActiveSubscription() const { return mFlags.Has(ReadHandlerFlags::ActiveSubscription); } - bool IsFabricFiltered() const { return mFlags.Has(ReadHandlerFlags::FabricFiltered); } + bool IsReporting() const + { + return mFlags.Has(ReadHandlerFlags::ChunkedReport); + } + bool IsPriming() const + { + return mFlags.Has(ReadHandlerFlags::PrimingReports); + } + bool IsActiveSubscription() const + { + return mFlags.Has(ReadHandlerFlags::ActiveSubscription); + } + bool IsFabricFiltered() const + { + return mFlags.Has(ReadHandlerFlags::FabricFiltered); + } CHIP_ERROR OnSubscribeRequest(Messaging::ExchangeContext * apExchangeContext, System::PacketBufferHandle && aPayload); - void GetSubscriptionId(SubscriptionId & aSubscriptionId) const { aSubscriptionId = mSubscriptionId; } - AttributePathExpandIterator * GetAttributePathExpandIterator() { return &mAttributePathExpandIterator; } + void GetSubscriptionId(SubscriptionId & aSubscriptionId) const + { + aSubscriptionId = mSubscriptionId; + } + AttributePathExpandIterator * GetAttributePathExpandIterator() + { + return &mAttributePathExpandIterator; + } /// @brief Notifies the read handler that a set of attribute paths has been marked dirty. This will schedule a reporting engine /// run if the change to the attribute path makes the ReadHandler reportable. @@ -334,7 +437,10 @@ class ReadHandler : public Messaging::ExchangeDelegate { return (mDirtyGeneration > mPreviousReportsBeginGeneration) || mFlags.Has(ReadHandlerFlags::ForceDirty); } - void ClearForceDirtyFlag() { ClearStateFlag(ReadHandlerFlags::ForceDirty); } + void ClearForceDirtyFlag() + { + ClearStateFlag(ReadHandlerFlags::ForceDirty); + } NodeId GetInitiatorNodeId() const { auto session = GetSession(); @@ -348,28 +454,53 @@ class ReadHandler : public Messaging::ExchangeDelegate } Transport::SecureSession * GetSession() const; - SubjectDescriptor GetSubjectDescriptor() const { return GetSession()->GetSubjectDescriptor(); } + SubjectDescriptor GetSubjectDescriptor() const + { + return GetSession()->GetSubjectDescriptor(); + } - auto GetTransactionStartGeneration() const { return mTransactionStartGeneration; } + auto GetTransactionStartGeneration() const + { + return mTransactionStartGeneration; + } /// @brief Forces the read handler into a dirty state, regardless of what's going on with attributes. /// This can lead to scheduling of a reporting run immediately, if the min interval has been reached, /// or after the min interval is reached if it has not yet been reached. void ForceDirtyState(); - const AttributeValueEncoder::AttributeEncodeState & GetAttributeEncodeState() const { return mAttributeEncoderState; } - void SetAttributeEncodeState(const AttributeValueEncoder::AttributeEncodeState & aState) { mAttributeEncoderState = aState; } - uint32_t GetLastWrittenEventsBytes() const { return mLastWrittenEventsBytes; } + const AttributeValueEncoder::AttributeEncodeState & GetAttributeEncodeState() const + { + return mAttributeEncoderState; + } + void SetAttributeEncodeState(const AttributeValueEncoder::AttributeEncodeState & aState) + { + mAttributeEncoderState = aState; + } + uint32_t GetLastWrittenEventsBytes() const + { + return mLastWrittenEventsBytes; + } // Returns the number of interested paths, including wildcard and concrete paths. - size_t GetAttributePathCount() const { return mpAttributePathList == nullptr ? 0 : mpAttributePathList->Count(); }; - size_t GetEventPathCount() const { return mpEventPathList == nullptr ? 0 : mpEventPathList->Count(); }; - size_t GetDataVersionFilterCount() const { return mpDataVersionFilterList == nullptr ? 0 : mpDataVersionFilterList->Count(); }; + size_t GetAttributePathCount() const + { + return mpAttributePathList == nullptr ? 0 : mpAttributePathList->Count(); + }; + size_t GetEventPathCount() const + { + return mpEventPathList == nullptr ? 0 : mpEventPathList->Count(); + }; + size_t GetDataVersionFilterCount() const + { + return mpDataVersionFilterList == nullptr ? 0 : mpDataVersionFilterList->Count(); + }; CHIP_ERROR SendStatusReport(Protocols::InteractionModel::Status aStatus); friend class TestReadInteraction; friend class chip::app::reporting::TestReportingEngine; + friend class TestReportScheduler; // // The engine needs to be able to Abort/Close a ReadHandler instance upon completion of work for a given read/subscribe @@ -379,6 +510,10 @@ class ReadHandler : public Messaging::ExchangeDelegate friend class chip::app::reporting::Engine; friend class chip::app::InteractionModelEngine; + // The report scheduler needs to be able to access StateFlag private functions to know when to schedule a run so it is declared + // as a friend class. + friend class chip::app::reporting::ReportScheduler; + enum class HandlerState : uint8_t { Idle, ///< The handler has been initialized and is ready @@ -404,10 +539,13 @@ class ReadHandler : public Messaging::ExchangeDelegate /// @brief This function is called when the min interval timer has expired, it restarts the timer on a timeout equal to the /// difference between the max interval and the min interval. - static void MinIntervalExpiredCallback(System::Layer * apSystemLayer, void * apAppState); - static void MaxIntervalExpiredCallback(System::Layer * apSystemLayer, void * apAppState); + static void MinIntervalExpiredCallback(System::Layer * apSystemLayer, void * apAppState); // TODO : Remove once ReportScheduler + // is implemented. + static void MaxIntervalExpiredCallback(System::Layer * apSystemLayer, void * apAppState); // TODO : Remove once ReportScheduler + // is implemented. /// @brief This function is called when a report is sent and it restarts the min interval timer. - CHIP_ERROR UpdateReportTimer(); + CHIP_ERROR UpdateReportTimer(); // TODO : Remove once ReportScheduler is implemented. + CHIP_ERROR SendSubscribeResponse(); CHIP_ERROR ProcessSubscribeRequest(System::PacketBufferHandle && aPayload); CHIP_ERROR ProcessReadRequest(System::PacketBufferHandle && aPayload); @@ -520,6 +658,10 @@ class ReadHandler : public Messaging::ExchangeDelegate BitFlags mFlags; InteractionType mInteractionType = InteractionType::Read; + // TODO : Several objects are behaving as observers for this class, there should be a single + // type for this and an array or a pool to store them. + Observer * mObserver = nullptr; + #if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS // Callbacks to handle server-initiated session success/failure chip::Callback::Callback mOnConnectedCallback; diff --git a/src/app/reporting/Engine.cpp b/src/app/reporting/Engine.cpp index a9853215332fd5..d1eb9936d653d0 100644 --- a/src/app/reporting/Engine.cpp +++ b/src/app/reporting/Engine.cpp @@ -636,6 +636,7 @@ void Engine::Run() ReadHandler * readHandler = imEngine->ActiveHandlerAt(mCurReadHandlerIdx % (uint32_t) imEngine->mReadHandlers.Allocated()); VerifyOrDie(readHandler != nullptr); + // TODO: Replace with check with Report Scheduler if the read handler is reportable if (readHandler->IsReportableNow()) { mRunningReadHandler = readHandler; diff --git a/src/app/reporting/ReportScheduler.h b/src/app/reporting/ReportScheduler.h new file mode 100644 index 00000000000000..17bad77c75488d --- /dev/null +++ b/src/app/reporting/ReportScheduler.h @@ -0,0 +1,134 @@ +/* + * + * Copyright (c) 2023 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. + */ + +#pragma once + +#include +#include +#include +#include +#include + +namespace chip { +namespace app { + +// Forward declaration of TestReportScheduler to allow it to be friend with ReportScheduler +class TestReportScheduler; + +namespace reporting { + +class ReportScheduler : public ReadHandler::Observer +{ +public: + class ReadHandlerNode : public IntrusiveListNodeBase<> + { + public: + ReadHandlerNode(ReadHandler * aReadHandler) + { + mReadHandler = aReadHandler; + SetIntervalsTimeStamp(aReadHandler); + } + ReadHandler * GetReadHandler() const { return mReadHandler; } + /// @brief Check if the Node is reportable now, meaning its readhandler was made reportable by attribute dirtying and + /// handler state, and minimal time interval last reporthas elapsed since, or the maximal time interval since last report + /// has elapsed + bool IsReportableNow() const + { + // TODO: Add flags to allow for test to simulate waiting for the min interval or max intrval to elapse when integrating + // the scheduler in the ReadHandler + System::Clock::Timestamp now = System::SystemClock().GetMonotonicTimestamp(); + return (mReadHandler->IsGeneratingReports() && + ((now >= mMinIntervalSeconds && mReadHandler->IsDirty()) || now >= mMaxIntervalSeconds)); + } + + void SetIntervalsTimeStamp(ReadHandler * aReadHandler) + { + uint16_t minInterval, maxInterval; + aReadHandler->GetReportingIntervals(minInterval, maxInterval); + System::Clock::Timestamp now = System::SystemClock().GetMonotonicTimestamp(); + mMinIntervalSeconds = now + System::Clock::Seconds16(minInterval); + mMaxIntervalSeconds = now + System::Clock::Seconds16(maxInterval); + } + + System::Clock::Timestamp GetMinInterval() const { return mMinIntervalSeconds; } + System::Clock::Timestamp GetMaxInterval() const { return mMaxIntervalSeconds; } + + private: + ReadHandler * mReadHandler; + System::Clock::Timestamp mMinIntervalSeconds; + System::Clock::Timestamp mMaxIntervalSeconds; + }; + /** + * Interface to act on changes in the ReadHandler reportability + */ + virtual ~ReportScheduler() = default; + + /// @brief Register a ReadHandler to the scheduler, will schedule report + /// @param aReadHandler read handler to register + virtual CHIP_ERROR RegisterReadHandler(ReadHandler * aReadHandler) = 0; + /// @brief Schedule a report for a given ReadHandler + /// @param timeout time in seconds before the next engine run is scheduled + /// @param aReadHandler read handler to schedule a report for + /// @return CHIP_ERROR not found if the ReadHandler is not registered in the scheduler, specific timer error if the timer + /// couldn't be started + virtual CHIP_ERROR ScheduleReport(System::Clock::Timeout timeout, ReadHandler * aReadHandler) = 0; + /// @brief Cancel a scheduled report for a given ReadHandler + /// @param aReadHandler readhandler to look for in the ReadHandler nodes list. If found, the timer started for this report will + /// be cancelled. + virtual void CancelReport(ReadHandler * aReadHandler) = 0; + /// @brief Unregister a ReadHandler from the scheduler + /// @param aReadHandler read handler to unregister + /// @return CHIP_NO_ERROR if the ReadHandler was successfully unregistered or not found, specific error otherwise + virtual void UnregisterReadHandler(ReadHandler * aReadHandler) = 0; + /// @brief Unregister all ReadHandlers from the scheduler + /// @return CHIP_NO_ERROR if all ReadHandlers were successfully unregistered, specific error otherwise + virtual void UnregisterAllHandlers() = 0; + /// @brief Check if a ReadHandler is scheduled for reporting + virtual bool IsReportScheduled(ReadHandler * aReadHandler) = 0; + /// @brief Check if a ReadHandler is reportable given its minimal and maximal intervals by using the node timestamps + /// @param aReadHandler read handler to check + bool IsReportableNow(ReadHandler * aReadHandler) { return FindReadHandlerNode(aReadHandler)->IsReportableNow(); }; + /// @brief Check if a ReadHandler is reportable without considering the timing + bool IsReadHandlerReportable(ReadHandler * aReadHandler) const + { + return aReadHandler->IsGeneratingReports() && aReadHandler->IsDirty(); + } + /// @brief Check the ReadHandler's ChunkedReport flag to prevent rescheduling if the Schedule is called when the engine is + /// processing a chunked report + bool IsChunkedReport(ReadHandler * aReadHandler) const { return aReadHandler->IsChunkedReport(); } + + /// @brief Get the number of ReadHandlers registered in the scheduler's node pool + size_t GetNumReadHandlers() const { return mNodesPool.Allocated(); } + +protected: + friend class chip::app::TestReportScheduler; + + /// @brief Find the ReadHandlerNode for a given ReadHandler pointer + /// @param [in] aReadHandler ReadHandler pointer to look for in the ReadHandler nodes list + /// @return Node Address if node was found, nullptr otherwise + virtual ReadHandlerNode * FindReadHandlerNode(const ReadHandler * aReadHandler) = 0; + + // virtual void ReportTimerCallback(System::Layer * aLayer, void * aAppState) = 0; + + IntrusiveList mReadHandlerList; + // TODO: Assess if possible to pass in the interaction model handler pool upon construction + ObjectPool mNodesPool; +}; +}; // namespace reporting +}; // namespace app +}; // namespace chip diff --git a/src/app/reporting/ReportSchedulerImpl.cpp b/src/app/reporting/ReportSchedulerImpl.cpp new file mode 100644 index 00000000000000..98024b5b7a6dbe --- /dev/null +++ b/src/app/reporting/ReportSchedulerImpl.cpp @@ -0,0 +1,194 @@ +/* + * + * Copyright (c) 2023 Project CHIP Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include + +namespace chip { +namespace app { +namespace reporting { + +using Seconds16 = System::Clock::Seconds16; +using Milliseconds32 = System::Clock::Milliseconds32; +using Timeout = System::Clock::Timeout; +using Timestamp = System::Clock::Timestamp; + +/// @brief Callback called when the report timer expires to schedule an engine run regardless of the state of the ReadHandlers, as +/// the engine already verifies that read handlers are reportable before sending a report +static void ReportTimerCallback(System::Layer * aLayer, void * aAppState) +{ + InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun(); +} + +/// @brief When a ReadHandler is added, register it, which will schedule an engine run on the max interval +void ReportSchedulerImpl::OnReadHandlerAdded(ReadHandler * aReadHandler) +{ + RegisterReadHandler(aReadHandler); +} + +/// @brief When a ReadHandler becomes reportable, schedule, verifies if the handler is the min interval is elapsed. If not, +/// reschedule the report to happen when the min interval is elapsed. If it is, schedule an engine run. +void ReportSchedulerImpl::OnBecameReportable(ReadHandler * aReadHandler) +{ + ReadHandlerNode * node = FindReadHandlerNode(aReadHandler); + VerifyOrReturn(nullptr != node); + if (node->IsReportableNow()) + { + // If the handler is reportable now, just schedule a report immediately + InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun(); + } + else + { + Milliseconds32 newTimeout = + Milliseconds32(node->GetMinInterval().count() - System::SystemClock().GetMonotonicTimestamp().count()); + // If the handler is not reportable now, schedule a report for the min interval + ScheduleReport(newTimeout, aReadHandler); + } +} + +void ReportSchedulerImpl::OnReportSent(ReadHandler * apReadHandler) +{ + ReadHandlerNode * node = FindReadHandlerNode(apReadHandler); + VerifyOrReturn(nullptr != node); + // Schedule callback for max interval + node->SetIntervalsTimeStamp(apReadHandler); + Milliseconds32 newTimeout = + Milliseconds32(node->GetMaxInterval().count() - System::SystemClock().GetMonotonicTimestamp().count()); + ScheduleReport(newTimeout, apReadHandler); +} + +/// @brief When a ReadHandler is removed, unregister it, which will cancel any scheduled report +void ReportSchedulerImpl::OnReadHandlerRemoved(ReadHandler * aReadHandler) +{ + UnregisterReadHandler(aReadHandler); +} + +CHIP_ERROR ReportSchedulerImpl::RegisterReadHandler(ReadHandler * aReadHandler) +{ + ReadHandlerNode * newNode = FindReadHandlerNode(aReadHandler); + // If the handler is already registered, no need to register it again + VerifyOrReturnValue(nullptr == newNode, CHIP_NO_ERROR); + newNode = mNodesPool.CreateObject(aReadHandler); + mReadHandlerList.PushBack(newNode); + + ChipLogProgress(DataManagement, + "Registered ReadHandler that will schedule a report between system Timestamp: %" PRIu64 + " and system Timestamp %" PRIu64 ".", + newNode->GetMinInterval().count(), newNode->GetMaxInterval().count()); + + Timestamp now = System::SystemClock().GetMonotonicTimestamp(); + // If the handler is reportable, schedule a report for the min interval, otherwise schdule a report for the max interval + if ((newNode->IsReportableNow())) + { + // If the handler is reportable now, just schedule a report immediately + InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun(); + } + else if (IsReadHandlerReportable(aReadHandler) && (newNode->GetMinInterval() > now)) + { + Milliseconds32 newTimeout = Milliseconds32(newNode->GetMinInterval().count() - now.count()); + // If the handler is reportable now, but the min interval is not elapsed, schedule a report for the min interval + ReturnErrorOnFailure(ScheduleReport(newTimeout, aReadHandler)); + } + else + { + Milliseconds32 newTimeout = Milliseconds32(newNode->GetMaxInterval().count() - now.count()); + // If the handler is not reportable now, schedule a report for the max interval + ReturnErrorOnFailure(ScheduleReport(newTimeout, aReadHandler)); + } + + return CHIP_NO_ERROR; +} + +CHIP_ERROR ReportSchedulerImpl::ScheduleReport(Timeout timeout, ReadHandler * aReadHandler) +{ + // Verify the handler is still registered + ReadHandlerNode * node = FindReadHandlerNode(aReadHandler); + VerifyOrReturnError(nullptr != node, CHIP_ERROR_NOT_FOUND); + + // Cancel Report if it is currently scheduled + InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->CancelTimer( + ReportTimerCallback, node); + + if (!IsChunkedReport(aReadHandler)) + { + // Schedule Report + ReturnErrorOnFailure( + InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->StartTimer( + timeout, ReportTimerCallback, node)); + } + + return CHIP_NO_ERROR; +} + +void ReportSchedulerImpl::CancelReport(ReadHandler * aReadHandler) +{ + ReadHandlerNode * node = FindReadHandlerNode(aReadHandler); + VerifyOrReturn(nullptr != node); + InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->CancelTimer( + ReportTimerCallback, node); +} + +void ReportSchedulerImpl::UnregisterReadHandler(ReadHandler * aReadHandler) +{ + // Verify list is populated and handler is not null + VerifyOrReturn((!mReadHandlerList.Empty() || (nullptr == aReadHandler))); + CancelReport(aReadHandler); + + ReadHandlerNode * removeNode = FindReadHandlerNode(aReadHandler); + // Nothing to remove if the handler is not found in the list + VerifyOrReturn(nullptr != removeNode); + + mReadHandlerList.Remove(removeNode); + mNodesPool.ReleaseObject(removeNode); +} + +void ReportSchedulerImpl::UnregisterAllHandlers() +{ + // Verify list is populated + VerifyOrReturn(!mReadHandlerList.Empty()); + while (!mReadHandlerList.Empty()) + { + ReadHandler * firstReadHandler = mReadHandlerList.begin()->GetReadHandler(); + UnregisterReadHandler(firstReadHandler); + } +} + +bool ReportSchedulerImpl::IsReportScheduled(ReadHandler * aReadHandler) +{ + ReadHandlerNode * node = FindReadHandlerNode(aReadHandler); + VerifyOrReturnValue(nullptr != node, false); + return InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->IsTimerActive( + ReportTimerCallback, node); +} + +ReportSchedulerImpl::ReadHandlerNode * ReportSchedulerImpl::FindReadHandlerNode(const ReadHandler * aReadHandler) +{ + // Verify list is populated and handler is not null + VerifyOrReturnValue(!mReadHandlerList.Empty() && (nullptr != aReadHandler), nullptr); + for (auto & iter : mReadHandlerList) + { + if (iter.GetReadHandler() == aReadHandler) + { + return &iter; + } + } + return nullptr; +} + +} // namespace reporting +} // namespace app +} // namespace chip diff --git a/src/app/reporting/ReportSchedulerImpl.h b/src/app/reporting/ReportSchedulerImpl.h new file mode 100644 index 00000000000000..df4cefbf0fef3f --- /dev/null +++ b/src/app/reporting/ReportSchedulerImpl.h @@ -0,0 +1,60 @@ +/* + * + * Copyright (c) 2023 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. + */ + +#pragma once + +#include + +namespace chip { +namespace app { + +namespace reporting { + +class ReportSchedulerImpl : public ReportScheduler +{ +public: + virtual ~ReportSchedulerImpl() override { UnregisterAllHandlers(); } + + // ReadHandlerObserver + virtual void OnReadHandlerAdded(ReadHandler * aReadHandler) override; + virtual void OnBecameReportable(ReadHandler * aReadHandler) override; + virtual void OnReportSent(ReadHandler * aReadHandler) override; + virtual void OnReadHandlerRemoved(ReadHandler * aReadHandler) override; + + // ReportScheduler specific + virtual CHIP_ERROR RegisterReadHandler(ReadHandler * aReadHandler) override; + virtual CHIP_ERROR ScheduleReport(System::Clock::Timeout timeout, ReadHandler * aReadHandler) override; + virtual void CancelReport(ReadHandler * aReadHandler) override; + virtual void UnregisterReadHandler(ReadHandler * aReadHandler) override; + virtual void UnregisterAllHandlers() override; + virtual bool IsReportScheduled(ReadHandler * aReadHandler) override; + +protected: + friend class chip::app::TestReportScheduler; + + /// @brief Find the ReadHandlerNode for a given ReadHandler pointer + /// @param [in] aReadHandler + /// @return Node Address if node was found, nullptr otherwise + virtual ReadHandlerNode * FindReadHandlerNode(const ReadHandler * aReadHandler) override; + + // void ReportTimerCallback(System::Layer * aLayer, void * aAppState) override; +}; + +} // namespace reporting +} // namespace app +} // namespace chip diff --git a/src/app/tests/BUILD.gn b/src/app/tests/BUILD.gn index 0faf6d0b06a05c..dc14b6d5a4c8da 100644 --- a/src/app/tests/BUILD.gn +++ b/src/app/tests/BUILD.gn @@ -124,6 +124,7 @@ chip_test_suite("tests") { "TestNumericAttributeTraits.cpp", "TestPendingNotificationMap.cpp", "TestReadInteraction.cpp", + "TestReportScheduler.cpp", "TestReportingEngine.cpp", "TestSceneTable.cpp", "TestStatusIB.cpp", diff --git a/src/app/tests/TestReportScheduler.cpp b/src/app/tests/TestReportScheduler.cpp new file mode 100644 index 00000000000000..2d5155329b85a5 --- /dev/null +++ b/src/app/tests/TestReportScheduler.cpp @@ -0,0 +1,315 @@ +/* + * + * Copyright (c) 2023 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include +#include +#include +#include +#include + +#define CONFIG_BUILD_FOR_HOST_UNIT_TEST 1 + +namespace { + +uint8_t gDebugEventBuffer[128]; +uint8_t gInfoEventBuffer[128]; +uint8_t gCritEventBuffer[128]; +chip::app::CircularEventBuffer gCircularEventBuffer[3]; + +class TestContext : public chip::Test::AppContext +{ +public: + static int Initialize(void * context) + { + if (AppContext::Initialize(context) != SUCCESS) + return FAILURE; + + auto * ctx = static_cast(context); + + if (ctx->mEventCounter.Init(0) != CHIP_NO_ERROR) + { + return FAILURE; + } + + chip::app::LogStorageResources logStorageResources[] = { + { &gDebugEventBuffer[0], sizeof(gDebugEventBuffer), chip::app::PriorityLevel::Debug }, + { &gInfoEventBuffer[0], sizeof(gInfoEventBuffer), chip::app::PriorityLevel::Info }, + { &gCritEventBuffer[0], sizeof(gCritEventBuffer), chip::app::PriorityLevel::Critical }, + }; + + chip::app::EventManagement::CreateEventManagement(&ctx->GetExchangeManager(), ArraySize(logStorageResources), + gCircularEventBuffer, logStorageResources, &ctx->mEventCounter); + + return SUCCESS; + } + + static int Finalize(void * context) + { + chip::app::EventManagement::DestroyEventManagement(); + + if (AppContext::Finalize(context) != SUCCESS) + return FAILURE; + + return SUCCESS; + } + +private: + chip::MonotonicallyIncreasingCounter mEventCounter; +}; + +class NullReadHandlerCallback : public chip::app::ReadHandler::ManagementCallback +{ +public: + void OnDone(chip::app::ReadHandler & apReadHandlerObj) override {} + chip::app::ReadHandler::ApplicationCallback * GetAppCallback() override { return nullptr; } +}; + +} // namespace + +namespace chip { +namespace app { + +using InteractionModelEngine = chip::app::InteractionModelEngine; +using ReportSchedulerImpl = chip::app::reporting::ReportSchedulerImpl; + +static const size_t kNumMaxReadHandlers = 30; + +ReportSchedulerImpl sScheduler; + +class TestReportScheduler +{ +public: + static void TestReadHandlerList(nlTestSuite * aSuite, void * aContext) + { + TestContext & ctx = *static_cast(aContext); + NullReadHandlerCallback nullCallback; + // exchange context + Messaging::ExchangeContext * exchangeCtx = ctx.NewExchangeToAlice(nullptr, false); + + // Read handler pool + ObjectPool readHandlerPool; + + for (size_t i = 0; i < kNumMaxReadHandlers; i++) + { + ReadHandler * readHandler = + readHandlerPool.CreateObject(nullCallback, exchangeCtx, chip::app::ReadHandler::InteractionType::Subscribe); + NL_TEST_ASSERT(aSuite, nullptr != readHandler); + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == sScheduler.RegisterReadHandler(readHandler)); + NL_TEST_ASSERT(aSuite, nullptr != sScheduler.FindReadHandlerNode(readHandler)); + } + + NL_TEST_ASSERT(aSuite, readHandlerPool.Allocated() == kNumMaxReadHandlers); + NL_TEST_ASSERT(aSuite, sScheduler.GetNumReadHandlers() == kNumMaxReadHandlers); + NL_TEST_ASSERT(aSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 1); + + // Test unregister first ReadHandler + ReadHandler * firstReadHandler = sScheduler.mReadHandlerList.begin()->GetReadHandler(); + sScheduler.UnregisterReadHandler(firstReadHandler); + NL_TEST_ASSERT(aSuite, sScheduler.GetNumReadHandlers() == kNumMaxReadHandlers - 1); + NL_TEST_ASSERT(aSuite, nullptr == sScheduler.FindReadHandlerNode(firstReadHandler)); + + // Test unregister middle ReadHandler + auto iter = sScheduler.mReadHandlerList.begin(); + for (size_t i = 0; i < static_cast(kNumMaxReadHandlers / 2); i++) + { + iter++; + } + ReadHandler * middleReadHandler = iter->GetReadHandler(); + sScheduler.UnregisterReadHandler(middleReadHandler); + NL_TEST_ASSERT(aSuite, sScheduler.GetNumReadHandlers() == kNumMaxReadHandlers - 2); + NL_TEST_ASSERT(aSuite, nullptr == sScheduler.FindReadHandlerNode(middleReadHandler)); + + // Test unregister last ReadHandler + iter = sScheduler.mReadHandlerList.end(); + iter--; + ReadHandler * lastReadHandler = iter->GetReadHandler(); + sScheduler.UnregisterReadHandler(lastReadHandler); + NL_TEST_ASSERT(aSuite, sScheduler.GetNumReadHandlers() == kNumMaxReadHandlers - 3); + NL_TEST_ASSERT(aSuite, nullptr == sScheduler.FindReadHandlerNode(lastReadHandler)); + + sScheduler.UnregisterAllHandlers(); + // Confirm all ReadHandlers are unregistered from the scheduler + NL_TEST_ASSERT(aSuite, sScheduler.GetNumReadHandlers() == 0); + readHandlerPool.ForEachActiveObject([&](ReadHandler * handler) { + NL_TEST_ASSERT(aSuite, nullptr == sScheduler.FindReadHandlerNode(handler)); + return Loop::Continue; + }); + + readHandlerPool.ReleaseAll(); + exchangeCtx->Close(); + NL_TEST_ASSERT(aSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0); + } + + static void TestReportTiming(nlTestSuite * aSuite, void * aContext) + { + TestContext & ctx = *static_cast(aContext); + NullReadHandlerCallback nullCallback; + // exchange context + Messaging::ExchangeContext * exchangeCtx = ctx.NewExchangeToAlice(nullptr, false); + + // Read handler pool + ObjectPool readHandlerPool; + + // Dirty read handler, will be triggered at min interval + ReadHandler * readHandler1 = + readHandlerPool.CreateObject(nullCallback, exchangeCtx, chip::app::ReadHandler::InteractionType::Subscribe); + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler1->SetMaxReportingIntervals(2)); + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler1->SetMinReportingIntervals(1)); + readHandler1->ForceDirtyState(); + readHandler1->MoveToState(ReadHandler::HandlerState::GeneratingReports); + + // Clean read handler, will be triggered at max interval + ReadHandler * readHandler2 = + readHandlerPool.CreateObject(nullCallback, exchangeCtx, chip::app::ReadHandler::InteractionType::Subscribe); + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler2->SetMaxReportingIntervals(3)); + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler2->SetMinReportingIntervals(0)); + readHandler2->MoveToState(ReadHandler::HandlerState::GeneratingReports); + + // Clean read handler, will be triggered at max interval, but will be cancelled before + ReadHandler * readHandler3 = + readHandlerPool.CreateObject(nullCallback, exchangeCtx, chip::app::ReadHandler::InteractionType::Subscribe); + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler3->SetMaxReportingIntervals(3)); + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler3->SetMinReportingIntervals(0)); + readHandler3->MoveToState(ReadHandler::HandlerState::GeneratingReports); + + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == sScheduler.RegisterReadHandler(readHandler1)); + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == sScheduler.RegisterReadHandler(readHandler2)); + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == sScheduler.RegisterReadHandler(readHandler3)); + + // Confirms that none of the ReadHandlers are currently reportable + NL_TEST_ASSERT(aSuite, !sScheduler.IsReportableNow(readHandler1)); + NL_TEST_ASSERT(aSuite, !sScheduler.IsReportableNow(readHandler2)); + NL_TEST_ASSERT(aSuite, !sScheduler.IsReportableNow(readHandler3)); + + ctx.GetIOContext().DriveIOUntil(System::Clock::Milliseconds32(1100), + [&]() -> bool { return sScheduler.IsReportableNow(readHandler1); }); + // Checks that the first ReadHandler is reportable after 1 second since it is dirty and min interval has expired + NL_TEST_ASSERT(aSuite, sScheduler.IsReportableNow(readHandler1)); + NL_TEST_ASSERT(aSuite, !sScheduler.IsReportableNow(readHandler2)); + NL_TEST_ASSERT(aSuite, !sScheduler.IsReportableNow(readHandler3)); + + NL_TEST_ASSERT(aSuite, sScheduler.IsReportScheduled(readHandler3)); + sScheduler.CancelReport(readHandler3); + NL_TEST_ASSERT(aSuite, !sScheduler.IsReportScheduled(readHandler3)); + + // Wait another 3 seconds to let the second ReadHandler become reportable + ctx.GetIOContext().DriveIOUntil(System::Clock::Milliseconds32(3100), + [&]() -> bool { return sScheduler.IsReportableNow(readHandler2); }); + + // Checks that all ReadHandlers are reportable + NL_TEST_ASSERT(aSuite, sScheduler.IsReportableNow(readHandler1)); + NL_TEST_ASSERT(aSuite, sScheduler.IsReportableNow(readHandler2)); + // Even if its timer got cancelled, readHandler3 should still be considered reportable as the max interval has expired and + // it is in generating report state + NL_TEST_ASSERT(aSuite, sScheduler.IsReportableNow(readHandler3)); + + sScheduler.UnregisterAllHandlers(); + readHandlerPool.ReleaseAll(); + exchangeCtx->Close(); + } + + static void TestObserverCallbacks(nlTestSuite * aSuite, void * aContext) + { + TestContext & ctx = *static_cast(aContext); + NullReadHandlerCallback nullCallback; + // exchange context + Messaging::ExchangeContext * exchangeCtx = ctx.NewExchangeToAlice(nullptr, false); + + // Read handler pool + ObjectPool readHandlerPool; + + ReadHandler * readHandler = + readHandlerPool.CreateObject(nullCallback, exchangeCtx, chip::app::ReadHandler::InteractionType::Subscribe); + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler->SetMaxReportingIntervals(2)); + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler->SetMinReportingIntervals(1)); + readHandler->MoveToState(ReadHandler::HandlerState::GeneratingReports); + readHandler->SetObserver(&sScheduler); + + // Test OnReadHandlerAdded + readHandler->mObserver->OnReadHandlerAdded(readHandler); + // Should have registered the read handler in the scheduler and scheduled a report + NL_TEST_ASSERT(aSuite, sScheduler.GetNumReadHandlers() == 1); + NL_TEST_ASSERT(aSuite, sScheduler.IsReportScheduled(readHandler)); + reporting::ReportScheduler::ReadHandlerNode * node = sScheduler.FindReadHandlerNode(readHandler); + NL_TEST_ASSERT(aSuite, nullptr != node); + NL_TEST_ASSERT(aSuite, node->GetReadHandler() == readHandler); + + // Test OnBecameReportable + readHandler->ForceDirtyState(); + readHandler->mObserver->OnBecameReportable(readHandler); + // Should have changed the scheduled timeout to the handlers min interval, to check, we wait for the min interval to expire + ctx.GetIOContext().DriveIOUntil(System::Clock::Milliseconds32(1100), + [&]() -> bool { return sScheduler.IsReportableNow(readHandler); }); + // Check that no report is scheduled since the min interval has expired, the timer should now be stopped + NL_TEST_ASSERT(aSuite, !sScheduler.IsReportScheduled(readHandler)); + + // Test OnReportSent + readHandler->ClearForceDirtyFlag(); + readHandler->mObserver->OnReportSent(readHandler); + // Should have changed the scheduled timeout to the handlers max interval, to check, we wait for the min interval to confirm + // it is not expired yet so the report should still be scheduled + NL_TEST_ASSERT(aSuite, sScheduler.IsReportScheduled(readHandler)); + ctx.GetIOContext().DriveIOUntil(System::Clock::Milliseconds32(1100), + [&]() -> bool { return sScheduler.IsReportableNow(readHandler); }); + // Check that the report is still scheduled as the max interval has not expired yet and the dirty flag was cleared + NL_TEST_ASSERT(aSuite, sScheduler.IsReportScheduled(readHandler)); + ctx.GetIOContext().DriveIOUntil(System::Clock::Milliseconds32(2100), + [&]() -> bool { return sScheduler.IsReportableNow(readHandler); }); + // Check that no report is scheduled since the max interval should have expired, the timer should now be stopped + NL_TEST_ASSERT(aSuite, !sScheduler.IsReportScheduled(readHandler)); + + // Test OnReadHandlerRemoved + readHandler->mObserver->OnReadHandlerRemoved(readHandler); + // Should have unregistered the read handler in the scheduler and cancelled the report + NL_TEST_ASSERT(aSuite, !sScheduler.IsReportScheduled(readHandler)); + NL_TEST_ASSERT(aSuite, sScheduler.GetNumReadHandlers() == 0); + NL_TEST_ASSERT(aSuite, nullptr == sScheduler.FindReadHandlerNode(readHandler)); + + sScheduler.UnregisterReadHandler(readHandler); + readHandlerPool.ReleaseAll(); + exchangeCtx->Close(); + } +}; + +} // namespace app +} // namespace chip + +namespace { + +/** + * Test Suite. It lists all the test functions. + */ + +static nlTest sTests[] = { + NL_TEST_DEF("TestReadHandlerList", chip::app::TestReportScheduler::TestReadHandlerList), + NL_TEST_DEF("TestReportTiming", chip::app::TestReportScheduler::TestReportTiming), + NL_TEST_DEF("TestObserverCallbacks", chip::app::TestReportScheduler::TestObserverCallbacks), + NL_TEST_SENTINEL(), +}; + +nlTestSuite sSuite = { "TestReportScheduler", &sTests[0], TestContext::Initialize, TestContext::Finalize }; + +} // namespace + +int TestReportScheduler() +{ + return chip::ExecuteTestsWithContext(&sSuite); +} + +CHIP_REGISTER_TEST_SUITE(TestReportScheduler); diff --git a/src/controller/tests/data_model/TestRead.cpp b/src/controller/tests/data_model/TestRead.cpp index 7be8008b7d7c9a..8b18f8625594f3 100644 --- a/src/controller/tests/data_model/TestRead.cpp +++ b/src/controller/tests/data_model/TestRead.cpp @@ -302,7 +302,7 @@ class TestReadInteraction : public app::ReadHandler::ApplicationCallback if (mAlterSubscriptionIntervals) { - ReturnErrorOnFailure(aReadHandler.SetReportingIntervals(mMaxInterval)); + ReturnErrorOnFailure(aReadHandler.SetMaxReportingIntervals(mMaxInterval)); } return CHIP_NO_ERROR; } From c1930a30531cef4f06c6bee12222c01eb0864b9a Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Thu, 29 Jun 2023 15:51:10 +0000 Subject: [PATCH 02/21] Restyled by clang-format --- src/app/ReadHandler.h | 125 +++++++++--------------------------------- 1 file changed, 25 insertions(+), 100 deletions(-) diff --git a/src/app/ReadHandler.h b/src/app/ReadHandler.h index 444244fb4e4967..909b3912848eb8 100644 --- a/src/app/ReadHandler.h +++ b/src/app/ReadHandler.h @@ -213,18 +213,9 @@ class ReadHandler : public Messaging::ExchangeDelegate ReadHandler(ManagementCallback & apCallback, Observer * observer = nullptr); #endif - const ObjectList * GetAttributePathList() const - { - return mpAttributePathList; - } - const ObjectList * GetEventPathList() const - { - return mpEventPathList; - } - const ObjectList * GetDataVersionFilterList() const - { - return mpDataVersionFilterList; - } + const ObjectList * GetAttributePathList() const { return mpAttributePathList; } + const ObjectList * GetEventPathList() const { return mpEventPathList; } + const ObjectList * GetDataVersionFilterList() const { return mpDataVersionFilterList; } void GetReportingIntervals(uint16_t & aMinInterval, uint16_t & aMaxInterval) const { @@ -269,14 +260,8 @@ class ReadHandler : public Messaging::ExchangeDelegate } private: - PriorityLevel GetCurrentPriority() const - { - return mCurrentPriority; - } - EventNumber & GetEventMin() - { - return mEventMin; - } + PriorityLevel GetCurrentPriority() const { return mCurrentPriority; } + EventNumber & GetEventMin() { return mEventMin; } enum class ReadHandlerFlags : uint8_t { @@ -357,10 +342,7 @@ class ReadHandler : public Messaging::ExchangeDelegate */ bool IsFromSubscriber(Messaging::ExchangeContext & apExchangeContext) const; - bool IsIdle() const - { - return mState == HandlerState::Idle; - } + bool IsIdle() const { return mState == HandlerState::Idle; } // TODO: Change back to IsReportable once ReportScheduler is implemented so this can assess reportability without considering // timing. The ReporScheduler will handle timing. @@ -374,14 +356,8 @@ class ReadHandler : public Messaging::ExchangeDelegate return mState == HandlerState::GeneratingReports && !mFlags.Has(ReadHandlerFlags::WaitingUntilMinInterval) && (IsDirty() || !mFlags.Has(ReadHandlerFlags::WaitingUntilMaxInterval)); } - bool IsGeneratingReports() const - { - return mState == HandlerState::GeneratingReports; - } - bool IsAwaitingReportResponse() const - { - return mState == HandlerState::AwaitingReportResponse; - } + bool IsGeneratingReports() const { return mState == HandlerState::GeneratingReports; } + bool IsAwaitingReportResponse() const { return mState == HandlerState::AwaitingReportResponse; } // Resets the path iterator to the beginning of the whole report for generating a series of new reports. void ResetPathIterator(); @@ -393,41 +369,17 @@ class ReadHandler : public Messaging::ExchangeDelegate // sanpshotted last event, check with latest last event number, re-setup snapshoted checkpoint, and compare again. bool CheckEventClean(EventManagement & aEventManager); - bool IsType(InteractionType type) const - { - return (mInteractionType == type); - } - bool IsChunkedReport() const - { - return mFlags.Has(ReadHandlerFlags::ChunkedReport); - } + bool IsType(InteractionType type) const { return (mInteractionType == type); } + bool IsChunkedReport() const { return mFlags.Has(ReadHandlerFlags::ChunkedReport); } // Is reporting indicates whether we are in the middle of a series chunks. As we will set mIsChunkedReport on the first chunk // and clear that flag on the last chunk, we can use mIsChunkedReport to indicate this state. - bool IsReporting() const - { - return mFlags.Has(ReadHandlerFlags::ChunkedReport); - } - bool IsPriming() const - { - return mFlags.Has(ReadHandlerFlags::PrimingReports); - } - bool IsActiveSubscription() const - { - return mFlags.Has(ReadHandlerFlags::ActiveSubscription); - } - bool IsFabricFiltered() const - { - return mFlags.Has(ReadHandlerFlags::FabricFiltered); - } + bool IsReporting() const { return mFlags.Has(ReadHandlerFlags::ChunkedReport); } + bool IsPriming() const { return mFlags.Has(ReadHandlerFlags::PrimingReports); } + bool IsActiveSubscription() const { return mFlags.Has(ReadHandlerFlags::ActiveSubscription); } + bool IsFabricFiltered() const { return mFlags.Has(ReadHandlerFlags::FabricFiltered); } CHIP_ERROR OnSubscribeRequest(Messaging::ExchangeContext * apExchangeContext, System::PacketBufferHandle && aPayload); - void GetSubscriptionId(SubscriptionId & aSubscriptionId) const - { - aSubscriptionId = mSubscriptionId; - } - AttributePathExpandIterator * GetAttributePathExpandIterator() - { - return &mAttributePathExpandIterator; - } + void GetSubscriptionId(SubscriptionId & aSubscriptionId) const { aSubscriptionId = mSubscriptionId; } + AttributePathExpandIterator * GetAttributePathExpandIterator() { return &mAttributePathExpandIterator; } /// @brief Notifies the read handler that a set of attribute paths has been marked dirty. This will schedule a reporting engine /// run if the change to the attribute path makes the ReadHandler reportable. @@ -437,10 +389,7 @@ class ReadHandler : public Messaging::ExchangeDelegate { return (mDirtyGeneration > mPreviousReportsBeginGeneration) || mFlags.Has(ReadHandlerFlags::ForceDirty); } - void ClearForceDirtyFlag() - { - ClearStateFlag(ReadHandlerFlags::ForceDirty); - } + void ClearForceDirtyFlag() { ClearStateFlag(ReadHandlerFlags::ForceDirty); } NodeId GetInitiatorNodeId() const { auto session = GetSession(); @@ -454,47 +403,23 @@ class ReadHandler : public Messaging::ExchangeDelegate } Transport::SecureSession * GetSession() const; - SubjectDescriptor GetSubjectDescriptor() const - { - return GetSession()->GetSubjectDescriptor(); - } + SubjectDescriptor GetSubjectDescriptor() const { return GetSession()->GetSubjectDescriptor(); } - auto GetTransactionStartGeneration() const - { - return mTransactionStartGeneration; - } + auto GetTransactionStartGeneration() const { return mTransactionStartGeneration; } /// @brief Forces the read handler into a dirty state, regardless of what's going on with attributes. /// This can lead to scheduling of a reporting run immediately, if the min interval has been reached, /// or after the min interval is reached if it has not yet been reached. void ForceDirtyState(); - const AttributeValueEncoder::AttributeEncodeState & GetAttributeEncodeState() const - { - return mAttributeEncoderState; - } - void SetAttributeEncodeState(const AttributeValueEncoder::AttributeEncodeState & aState) - { - mAttributeEncoderState = aState; - } - uint32_t GetLastWrittenEventsBytes() const - { - return mLastWrittenEventsBytes; - } + const AttributeValueEncoder::AttributeEncodeState & GetAttributeEncodeState() const { return mAttributeEncoderState; } + void SetAttributeEncodeState(const AttributeValueEncoder::AttributeEncodeState & aState) { mAttributeEncoderState = aState; } + uint32_t GetLastWrittenEventsBytes() const { return mLastWrittenEventsBytes; } // Returns the number of interested paths, including wildcard and concrete paths. - size_t GetAttributePathCount() const - { - return mpAttributePathList == nullptr ? 0 : mpAttributePathList->Count(); - }; - size_t GetEventPathCount() const - { - return mpEventPathList == nullptr ? 0 : mpEventPathList->Count(); - }; - size_t GetDataVersionFilterCount() const - { - return mpDataVersionFilterList == nullptr ? 0 : mpDataVersionFilterList->Count(); - }; + size_t GetAttributePathCount() const { return mpAttributePathList == nullptr ? 0 : mpAttributePathList->Count(); }; + size_t GetEventPathCount() const { return mpEventPathList == nullptr ? 0 : mpEventPathList->Count(); }; + size_t GetDataVersionFilterCount() const { return mpDataVersionFilterList == nullptr ? 0 : mpDataVersionFilterList->Count(); }; CHIP_ERROR SendStatusReport(Protocols::InteractionModel::Status aStatus); From ed7bfac9895dd40d80b014ff0084e6a86b263e3d Mon Sep 17 00:00:00 2001 From: lpbeliveau-silabs Date: Thu, 29 Jun 2023 15:04:16 -0400 Subject: [PATCH 03/21] Removed un-necessary define in TestReportScheduler and applied refactor of SetReportingIntervals to SetMaxReportingIntervals to platform code --- examples/platform/nrfconnect/util/ICDUtil.cpp | 2 +- examples/platform/silabs/ICDSubscriptionCallback.cpp | 2 +- src/app/tests/TestReportScheduler.cpp | 2 -- src/platform/telink/ICDUtil.cpp | 2 +- 4 files changed, 3 insertions(+), 5 deletions(-) diff --git a/examples/platform/nrfconnect/util/ICDUtil.cpp b/examples/platform/nrfconnect/util/ICDUtil.cpp index fd2130c6c22dcb..b1c2c4797adb80 100644 --- a/examples/platform/nrfconnect/util/ICDUtil.cpp +++ b/examples/platform/nrfconnect/util/ICDUtil.cpp @@ -36,5 +36,5 @@ CHIP_ERROR ICDUtil::OnSubscriptionRequested(chip::app::ReadHandler & aReadHandle agreedMaxInterval = kSubscriptionMaxIntervalPublisherLimit; } - return aReadHandler.SetReportingIntervals(agreedMaxInterval); + return aReadHandler.SetMaxReportingIntervals(agreedMaxInterval); } diff --git a/examples/platform/silabs/ICDSubscriptionCallback.cpp b/examples/platform/silabs/ICDSubscriptionCallback.cpp index 30fbf683e6e8dc..8551277fe19885 100644 --- a/examples/platform/silabs/ICDSubscriptionCallback.cpp +++ b/examples/platform/silabs/ICDSubscriptionCallback.cpp @@ -61,5 +61,5 @@ CHIP_ERROR ICDSubscriptionCallback::OnSubscriptionRequested(chip::app::ReadHandl decidedMaxInterval = maximumMaxInterval; } - return aReadHandler.SetReportingIntervals(decidedMaxInterval); + return aReadHandler.SetMaxReportingIntervals(decidedMaxInterval); } diff --git a/src/app/tests/TestReportScheduler.cpp b/src/app/tests/TestReportScheduler.cpp index 2d5155329b85a5..e9645cba860b76 100644 --- a/src/app/tests/TestReportScheduler.cpp +++ b/src/app/tests/TestReportScheduler.cpp @@ -23,8 +23,6 @@ #include #include -#define CONFIG_BUILD_FOR_HOST_UNIT_TEST 1 - namespace { uint8_t gDebugEventBuffer[128]; diff --git a/src/platform/telink/ICDUtil.cpp b/src/platform/telink/ICDUtil.cpp index fd2130c6c22dcb..b1c2c4797adb80 100644 --- a/src/platform/telink/ICDUtil.cpp +++ b/src/platform/telink/ICDUtil.cpp @@ -36,5 +36,5 @@ CHIP_ERROR ICDUtil::OnSubscriptionRequested(chip::app::ReadHandler & aReadHandle agreedMaxInterval = kSubscriptionMaxIntervalPublisherLimit; } - return aReadHandler.SetReportingIntervals(agreedMaxInterval); + return aReadHandler.SetMaxReportingIntervals(agreedMaxInterval); } From e9812aed08f6a97ca8abad132d63780643dd1075 Mon Sep 17 00:00:00 2001 From: lpbeliveau-silabs Date: Thu, 29 Jun 2023 18:13:48 -0400 Subject: [PATCH 04/21] Added TimerDelegate and wrapper functions around calls to Timer. Remove unnecessary checks for nullptr --- src/app/reporting/ReportScheduler.h | 23 ++++++++++- src/app/reporting/ReportSchedulerImpl.cpp | 47 ++++++++++++++--------- src/app/reporting/ReportSchedulerImpl.h | 4 +- 3 files changed, 54 insertions(+), 20 deletions(-) diff --git a/src/app/reporting/ReportScheduler.h b/src/app/reporting/ReportScheduler.h index 17bad77c75488d..9ce03ea2851d00 100644 --- a/src/app/reporting/ReportScheduler.h +++ b/src/app/reporting/ReportScheduler.h @@ -73,6 +73,24 @@ class ReportScheduler : public ReadHandler::Observer System::Clock::Timestamp mMinIntervalSeconds; System::Clock::Timestamp mMaxIntervalSeconds; }; + + // TODO: Completer the TimerDelegate class in tests and use calls to it in the ReportSchedulerImpl + class TimerDelegate + { + public: + using TimerCompleteCallback = void (*)(ReadHandlerNode * node); + virtual ~TimerDelegate() {} + virtual void StartTimer(ReadHandlerNode * node, System::Clock::Timeout aTimeout) = 0; + virtual void CancelTimer(ReadHandlerNode * node) = 0; + virtual void IsTimerActive(ReadHandlerNode * node) = 0; + + virtual void SetCallback(TimerCompleteCallback callback) = 0; + + protected: + TimerCompleteCallback mCallback; + }; + + // TODO: Add constructor that requires a TimerDelegate /** * Interface to act on changes in the ReadHandler reportability */ @@ -123,11 +141,14 @@ class ReportScheduler : public ReadHandler::Observer /// @return Node Address if node was found, nullptr otherwise virtual ReadHandlerNode * FindReadHandlerNode(const ReadHandler * aReadHandler) = 0; - // virtual void ReportTimerCallback(System::Layer * aLayer, void * aAppState) = 0; + virtual CHIP_ERROR StartTimerForHandler(ReadHandlerNode * node, System::Clock::Timeout aTimeout) = 0; + virtual void CancelTimerForHandler(ReadHandlerNode * node) = 0; + virtual bool CheckTimerActiveForHandler(ReadHandlerNode * node) = 0; IntrusiveList mReadHandlerList; // TODO: Assess if possible to pass in the interaction model handler pool upon construction ObjectPool mNodesPool; + TimerDelegate * mDelegate; }; }; // namespace reporting }; // namespace app diff --git a/src/app/reporting/ReportSchedulerImpl.cpp b/src/app/reporting/ReportSchedulerImpl.cpp index 98024b5b7a6dbe..c04892a5804f22 100644 --- a/src/app/reporting/ReportSchedulerImpl.cpp +++ b/src/app/reporting/ReportSchedulerImpl.cpp @@ -22,10 +22,11 @@ namespace chip { namespace app { namespace reporting { -using Seconds16 = System::Clock::Seconds16; -using Milliseconds32 = System::Clock::Milliseconds32; -using Timeout = System::Clock::Timeout; -using Timestamp = System::Clock::Timestamp; +using Seconds16 = System::Clock::Seconds16; +using Milliseconds32 = System::Clock::Milliseconds32; +using Timeout = System::Clock::Timeout; +using Timestamp = System::Clock::Timestamp; +using ReadHandlerNode = ReportScheduler::ReadHandlerNode; /// @brief Callback called when the report timer expires to schedule an engine run regardless of the state of the ReadHandlers, as /// the engine already verifies that read handlers are reportable before sending a report @@ -120,15 +121,11 @@ CHIP_ERROR ReportSchedulerImpl::ScheduleReport(Timeout timeout, ReadHandler * aR VerifyOrReturnError(nullptr != node, CHIP_ERROR_NOT_FOUND); // Cancel Report if it is currently scheduled - InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->CancelTimer( - ReportTimerCallback, node); + CancelTimerForHandler(node); if (!IsChunkedReport(aReadHandler)) { - // Schedule Report - ReturnErrorOnFailure( - InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->StartTimer( - timeout, ReportTimerCallback, node)); + StartTimerForHandler(node, timeout); } return CHIP_NO_ERROR; @@ -138,8 +135,7 @@ void ReportSchedulerImpl::CancelReport(ReadHandler * aReadHandler) { ReadHandlerNode * node = FindReadHandlerNode(aReadHandler); VerifyOrReturn(nullptr != node); - InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->CancelTimer( - ReportTimerCallback, node); + CancelTimerForHandler(node); } void ReportSchedulerImpl::UnregisterReadHandler(ReadHandler * aReadHandler) @@ -158,8 +154,6 @@ void ReportSchedulerImpl::UnregisterReadHandler(ReadHandler * aReadHandler) void ReportSchedulerImpl::UnregisterAllHandlers() { - // Verify list is populated - VerifyOrReturn(!mReadHandlerList.Empty()); while (!mReadHandlerList.Empty()) { ReadHandler * firstReadHandler = mReadHandlerList.begin()->GetReadHandler(); @@ -171,14 +165,11 @@ bool ReportSchedulerImpl::IsReportScheduled(ReadHandler * aReadHandler) { ReadHandlerNode * node = FindReadHandlerNode(aReadHandler); VerifyOrReturnValue(nullptr != node, false); - return InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->IsTimerActive( - ReportTimerCallback, node); + return CheckTimerActiveForHandler(node); } ReportSchedulerImpl::ReadHandlerNode * ReportSchedulerImpl::FindReadHandlerNode(const ReadHandler * aReadHandler) { - // Verify list is populated and handler is not null - VerifyOrReturnValue(!mReadHandlerList.Empty() && (nullptr != aReadHandler), nullptr); for (auto & iter : mReadHandlerList) { if (iter.GetReadHandler() == aReadHandler) @@ -189,6 +180,26 @@ ReportSchedulerImpl::ReadHandlerNode * ReportSchedulerImpl::FindReadHandlerNode( return nullptr; } +// TODO: Replace calls to SystemLayer()-> with calls to TimerDelegate-> +CHIP_ERROR ReportSchedulerImpl::StartTimerForHandler(ReadHandlerNode * node, System::Clock::Timeout aTimeout) +{ + // Schedule Report + return InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->StartTimer( + aTimeout, ReportTimerCallback, node); +} + +void ReportSchedulerImpl::CancelTimerForHandler(ReadHandlerNode * node) +{ + InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->CancelTimer( + ReportTimerCallback, node); +} + +bool ReportSchedulerImpl::CheckTimerActiveForHandler(ReadHandlerNode * node) +{ + return InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->IsTimerActive( + ReportTimerCallback, node); +} + } // namespace reporting } // namespace app } // namespace chip diff --git a/src/app/reporting/ReportSchedulerImpl.h b/src/app/reporting/ReportSchedulerImpl.h index df4cefbf0fef3f..ecf93bd417ab6a 100644 --- a/src/app/reporting/ReportSchedulerImpl.h +++ b/src/app/reporting/ReportSchedulerImpl.h @@ -52,7 +52,9 @@ class ReportSchedulerImpl : public ReportScheduler /// @return Node Address if node was found, nullptr otherwise virtual ReadHandlerNode * FindReadHandlerNode(const ReadHandler * aReadHandler) override; - // void ReportTimerCallback(System::Layer * aLayer, void * aAppState) override; + virtual CHIP_ERROR StartTimerForHandler(ReadHandlerNode * node, System::Clock::Timeout aTimeout) override; + virtual void CancelTimerForHandler(ReadHandlerNode * node) override; + virtual bool CheckTimerActiveForHandler(ReadHandlerNode * node) override; }; } // namespace reporting From 498716e4f69585ea46cdef3cc62d33193d93ddc5 Mon Sep 17 00:00:00 2001 From: lpbeliveau-silabs Date: Fri, 30 Jun 2023 09:53:14 -0400 Subject: [PATCH 05/21] Added VerifyOrReturn after NL_TEST_ASSERTS for nullptr --- src/app/tests/TestReportScheduler.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/app/tests/TestReportScheduler.cpp b/src/app/tests/TestReportScheduler.cpp index e9645cba860b76..a7ea075a6a48c1 100644 --- a/src/app/tests/TestReportScheduler.cpp +++ b/src/app/tests/TestReportScheduler.cpp @@ -108,6 +108,7 @@ class TestReportScheduler ReadHandler * readHandler = readHandlerPool.CreateObject(nullCallback, exchangeCtx, chip::app::ReadHandler::InteractionType::Subscribe); NL_TEST_ASSERT(aSuite, nullptr != readHandler); + VerifyOrReturn(nullptr != readHandler); NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == sScheduler.RegisterReadHandler(readHandler)); NL_TEST_ASSERT(aSuite, nullptr != sScheduler.FindReadHandlerNode(readHandler)); } @@ -246,6 +247,7 @@ class TestReportScheduler NL_TEST_ASSERT(aSuite, sScheduler.IsReportScheduled(readHandler)); reporting::ReportScheduler::ReadHandlerNode * node = sScheduler.FindReadHandlerNode(readHandler); NL_TEST_ASSERT(aSuite, nullptr != node); + VerifyOrReturn(nullptr != node); NL_TEST_ASSERT(aSuite, node->GetReadHandler() == readHandler); // Test OnBecameReportable From 9a16c68ceac663bc96d5efc1aef018b18c1cf722 Mon Sep 17 00:00:00 2001 From: lpbeliveau-silabs Date: Fri, 30 Jun 2023 12:26:27 -0400 Subject: [PATCH 06/21] Completed TimerDelegate class and modified ReadHandlerNodes so they carry their own callback --- src/app/reporting/ReportScheduler.h | 26 +++++----- src/app/reporting/ReportSchedulerImpl.cpp | 21 ++++---- src/app/reporting/ReportSchedulerImpl.h | 1 + src/app/tests/TestReportScheduler.cpp | 59 +++++++++++++++++------ 4 files changed, 67 insertions(+), 40 deletions(-) diff --git a/src/app/reporting/ReportScheduler.h b/src/app/reporting/ReportScheduler.h index 9ce03ea2851d00..e33c61a068ea67 100644 --- a/src/app/reporting/ReportScheduler.h +++ b/src/app/reporting/ReportScheduler.h @@ -19,7 +19,6 @@ #pragma once #include -#include #include #include #include @@ -38,7 +37,9 @@ class ReportScheduler : public ReadHandler::Observer class ReadHandlerNode : public IntrusiveListNodeBase<> { public: - ReadHandlerNode(ReadHandler * aReadHandler) + using TimerCompleteCallback = void (*)(); + + ReadHandlerNode(ReadHandler * aReadHandler, TimerCompleteCallback aCallback) : mCallback(aCallback) { mReadHandler = aReadHandler; SetIntervalsTimeStamp(aReadHandler); @@ -65,32 +66,28 @@ class ReportScheduler : public ReadHandler::Observer mMaxIntervalSeconds = now + System::Clock::Seconds16(maxInterval); } + void RunCallback() { mCallback(); } + System::Clock::Timestamp GetMinInterval() const { return mMinIntervalSeconds; } System::Clock::Timestamp GetMaxInterval() const { return mMaxIntervalSeconds; } private: + TimerCompleteCallback mCallback; ReadHandler * mReadHandler; System::Clock::Timestamp mMinIntervalSeconds; System::Clock::Timestamp mMaxIntervalSeconds; }; - // TODO: Completer the TimerDelegate class in tests and use calls to it in the ReportSchedulerImpl class TimerDelegate { public: - using TimerCompleteCallback = void (*)(ReadHandlerNode * node); virtual ~TimerDelegate() {} - virtual void StartTimer(ReadHandlerNode * node, System::Clock::Timeout aTimeout) = 0; - virtual void CancelTimer(ReadHandlerNode * node) = 0; - virtual void IsTimerActive(ReadHandlerNode * node) = 0; - - virtual void SetCallback(TimerCompleteCallback callback) = 0; - - protected: - TimerCompleteCallback mCallback; + virtual CHIP_ERROR StartTimer(ReadHandlerNode * node, System::Clock::Timeout aTimeout) = 0; + virtual void CancelTimer(ReadHandlerNode * node) = 0; + virtual bool IsTimerActive(ReadHandlerNode * node) = 0; }; - // TODO: Add constructor that requires a TimerDelegate + ReportScheduler(TimerDelegate * aTimerDelegate) : mTimerDelegate(aTimerDelegate) {} /** * Interface to act on changes in the ReadHandler reportability */ @@ -146,9 +143,8 @@ class ReportScheduler : public ReadHandler::Observer virtual bool CheckTimerActiveForHandler(ReadHandlerNode * node) = 0; IntrusiveList mReadHandlerList; - // TODO: Assess if possible to pass in the interaction model handler pool upon construction ObjectPool mNodesPool; - TimerDelegate * mDelegate; + TimerDelegate * mTimerDelegate; }; }; // namespace reporting }; // namespace app diff --git a/src/app/reporting/ReportSchedulerImpl.cpp b/src/app/reporting/ReportSchedulerImpl.cpp index c04892a5804f22..a5bf38917de24d 100644 --- a/src/app/reporting/ReportSchedulerImpl.cpp +++ b/src/app/reporting/ReportSchedulerImpl.cpp @@ -30,11 +30,16 @@ using ReadHandlerNode = ReportScheduler::ReadHandlerNode; /// @brief Callback called when the report timer expires to schedule an engine run regardless of the state of the ReadHandlers, as /// the engine already verifies that read handlers are reportable before sending a report -static void ReportTimerCallback(System::Layer * aLayer, void * aAppState) +static void ReportTimerCallback() { InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun(); } +ReportSchedulerImpl::ReportSchedulerImpl(TimerDelegate * aTimerDelegate) : ReportScheduler(aTimerDelegate) +{ + VerifyOrDie(nullptr != mTimerDelegate); +} + /// @brief When a ReadHandler is added, register it, which will schedule an engine run on the max interval void ReportSchedulerImpl::OnReadHandlerAdded(ReadHandler * aReadHandler) { @@ -83,7 +88,7 @@ CHIP_ERROR ReportSchedulerImpl::RegisterReadHandler(ReadHandler * aReadHandler) ReadHandlerNode * newNode = FindReadHandlerNode(aReadHandler); // If the handler is already registered, no need to register it again VerifyOrReturnValue(nullptr == newNode, CHIP_NO_ERROR); - newNode = mNodesPool.CreateObject(aReadHandler); + newNode = mNodesPool.CreateObject(aReadHandler, ReportTimerCallback); mReadHandlerList.PushBack(newNode); ChipLogProgress(DataManagement, @@ -96,7 +101,7 @@ CHIP_ERROR ReportSchedulerImpl::RegisterReadHandler(ReadHandler * aReadHandler) if ((newNode->IsReportableNow())) { // If the handler is reportable now, just schedule a report immediately - InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun(); + ReturnErrorOnFailure(InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun()); } else if (IsReadHandlerReportable(aReadHandler) && (newNode->GetMinInterval() > now)) { @@ -180,24 +185,20 @@ ReportSchedulerImpl::ReadHandlerNode * ReportSchedulerImpl::FindReadHandlerNode( return nullptr; } -// TODO: Replace calls to SystemLayer()-> with calls to TimerDelegate-> CHIP_ERROR ReportSchedulerImpl::StartTimerForHandler(ReadHandlerNode * node, System::Clock::Timeout aTimeout) { // Schedule Report - return InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->StartTimer( - aTimeout, ReportTimerCallback, node); + return mTimerDelegate->StartTimer(node, aTimeout); } void ReportSchedulerImpl::CancelTimerForHandler(ReadHandlerNode * node) { - InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->CancelTimer( - ReportTimerCallback, node); + mTimerDelegate->CancelTimer(node); } bool ReportSchedulerImpl::CheckTimerActiveForHandler(ReadHandlerNode * node) { - return InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->IsTimerActive( - ReportTimerCallback, node); + return mTimerDelegate->IsTimerActive(node); } } // namespace reporting diff --git a/src/app/reporting/ReportSchedulerImpl.h b/src/app/reporting/ReportSchedulerImpl.h index ecf93bd417ab6a..fdd54f3cba132a 100644 --- a/src/app/reporting/ReportSchedulerImpl.h +++ b/src/app/reporting/ReportSchedulerImpl.h @@ -28,6 +28,7 @@ namespace reporting { class ReportSchedulerImpl : public ReportScheduler { public: + ReportSchedulerImpl(TimerDelegate * aTimerDelegate); virtual ~ReportSchedulerImpl() override { UnregisterAllHandlers(); } // ReadHandlerObserver diff --git a/src/app/tests/TestReportScheduler.cpp b/src/app/tests/TestReportScheduler.cpp index a7ea075a6a48c1..bf1356b9287ed3 100644 --- a/src/app/tests/TestReportScheduler.cpp +++ b/src/app/tests/TestReportScheduler.cpp @@ -83,12 +83,40 @@ class NullReadHandlerCallback : public chip::app::ReadHandler::ManagementCallbac namespace chip { namespace app { -using InteractionModelEngine = chip::app::InteractionModelEngine; -using ReportSchedulerImpl = chip::app::reporting::ReportSchedulerImpl; +using InteractionModelEngine = InteractionModelEngine; +using ReportScheduler = reporting::ReportScheduler; +using ReportSchedulerImpl = reporting::ReportSchedulerImpl; +using ReadHandlerNode = reporting::ReportScheduler::ReadHandlerNode; -static const size_t kNumMaxReadHandlers = 30; +class TestTimerDelegate : public ReportScheduler::TimerDelegate +{ +public: + static void TimerCallbackInterface(System::Layer * aLayer, void * aAppState) + { + ReadHandlerNode * node = static_cast(aAppState); + node->RunCallback(); + } + virtual CHIP_ERROR StartTimer(ReadHandlerNode * node, System::Clock::Timeout aTimeout) override + { + return InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->StartTimer( + aTimeout, TimerCallbackInterface, node); + } + virtual void CancelTimer(ReadHandlerNode * node) override + { + InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->CancelTimer( + TimerCallbackInterface, node); + } + virtual bool IsTimerActive(ReadHandlerNode * node) override + { + return InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->IsTimerActive( + TimerCallbackInterface, node); + } +}; + +static const size_t kNumMaxReadHandlers = 16; -ReportSchedulerImpl sScheduler; +TestTimerDelegate sTestTimerDelegate; +ReportSchedulerImpl sScheduler(&sTestTimerDelegate); class TestReportScheduler { @@ -106,7 +134,7 @@ class TestReportScheduler for (size_t i = 0; i < kNumMaxReadHandlers; i++) { ReadHandler * readHandler = - readHandlerPool.CreateObject(nullCallback, exchangeCtx, chip::app::ReadHandler::InteractionType::Subscribe); + readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe); NL_TEST_ASSERT(aSuite, nullptr != readHandler); VerifyOrReturn(nullptr != readHandler); NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == sScheduler.RegisterReadHandler(readHandler)); @@ -167,7 +195,7 @@ class TestReportScheduler // Dirty read handler, will be triggered at min interval ReadHandler * readHandler1 = - readHandlerPool.CreateObject(nullCallback, exchangeCtx, chip::app::ReadHandler::InteractionType::Subscribe); + readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe); NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler1->SetMaxReportingIntervals(2)); NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler1->SetMinReportingIntervals(1)); readHandler1->ForceDirtyState(); @@ -175,14 +203,14 @@ class TestReportScheduler // Clean read handler, will be triggered at max interval ReadHandler * readHandler2 = - readHandlerPool.CreateObject(nullCallback, exchangeCtx, chip::app::ReadHandler::InteractionType::Subscribe); + readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe); NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler2->SetMaxReportingIntervals(3)); NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler2->SetMinReportingIntervals(0)); readHandler2->MoveToState(ReadHandler::HandlerState::GeneratingReports); // Clean read handler, will be triggered at max interval, but will be cancelled before ReadHandler * readHandler3 = - readHandlerPool.CreateObject(nullCallback, exchangeCtx, chip::app::ReadHandler::InteractionType::Subscribe); + readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe); NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler3->SetMaxReportingIntervals(3)); NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler3->SetMinReportingIntervals(0)); readHandler3->MoveToState(ReadHandler::HandlerState::GeneratingReports); @@ -214,8 +242,8 @@ class TestReportScheduler // Checks that all ReadHandlers are reportable NL_TEST_ASSERT(aSuite, sScheduler.IsReportableNow(readHandler1)); NL_TEST_ASSERT(aSuite, sScheduler.IsReportableNow(readHandler2)); - // Even if its timer got cancelled, readHandler3 should still be considered reportable as the max interval has expired and - // it is in generating report state + // Even if its timer got cancelled, readHandler3 should still be considered reportable as the max interval has expired + // and it is in generating report state NL_TEST_ASSERT(aSuite, sScheduler.IsReportableNow(readHandler3)); sScheduler.UnregisterAllHandlers(); @@ -234,7 +262,7 @@ class TestReportScheduler ObjectPool readHandlerPool; ReadHandler * readHandler = - readHandlerPool.CreateObject(nullCallback, exchangeCtx, chip::app::ReadHandler::InteractionType::Subscribe); + readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe); NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler->SetMaxReportingIntervals(2)); NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler->SetMinReportingIntervals(1)); readHandler->MoveToState(ReadHandler::HandlerState::GeneratingReports); @@ -245,7 +273,7 @@ class TestReportScheduler // Should have registered the read handler in the scheduler and scheduled a report NL_TEST_ASSERT(aSuite, sScheduler.GetNumReadHandlers() == 1); NL_TEST_ASSERT(aSuite, sScheduler.IsReportScheduled(readHandler)); - reporting::ReportScheduler::ReadHandlerNode * node = sScheduler.FindReadHandlerNode(readHandler); + ReadHandlerNode * node = sScheduler.FindReadHandlerNode(readHandler); NL_TEST_ASSERT(aSuite, nullptr != node); VerifyOrReturn(nullptr != node); NL_TEST_ASSERT(aSuite, node->GetReadHandler() == readHandler); @@ -253,7 +281,8 @@ class TestReportScheduler // Test OnBecameReportable readHandler->ForceDirtyState(); readHandler->mObserver->OnBecameReportable(readHandler); - // Should have changed the scheduled timeout to the handlers min interval, to check, we wait for the min interval to expire + // Should have changed the scheduled timeout to the handlers min interval, to check, we wait for the min interval to + // expire ctx.GetIOContext().DriveIOUntil(System::Clock::Milliseconds32(1100), [&]() -> bool { return sScheduler.IsReportableNow(readHandler); }); // Check that no report is scheduled since the min interval has expired, the timer should now be stopped @@ -262,8 +291,8 @@ class TestReportScheduler // Test OnReportSent readHandler->ClearForceDirtyFlag(); readHandler->mObserver->OnReportSent(readHandler); - // Should have changed the scheduled timeout to the handlers max interval, to check, we wait for the min interval to confirm - // it is not expired yet so the report should still be scheduled + // Should have changed the scheduled timeout to the handlers max interval, to check, we wait for the min interval to + // confirm it is not expired yet so the report should still be scheduled NL_TEST_ASSERT(aSuite, sScheduler.IsReportScheduled(readHandler)); ctx.GetIOContext().DriveIOUntil(System::Clock::Milliseconds32(1100), [&]() -> bool { return sScheduler.IsReportableNow(readHandler); }); From 48454fb44e9bdde4e24890bed9174a633ae49042 Mon Sep 17 00:00:00 2001 From: lpbeliveau-silabs Date: Tue, 4 Jul 2023 15:48:21 -0400 Subject: [PATCH 07/21] Modified TimerDelegate to allow to pass different objects as context --- src/app/reporting/ReportScheduler.h | 6 +++--- src/app/tests/TestReportScheduler.cpp | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/app/reporting/ReportScheduler.h b/src/app/reporting/ReportScheduler.h index e33c61a068ea67..95825c82e228cc 100644 --- a/src/app/reporting/ReportScheduler.h +++ b/src/app/reporting/ReportScheduler.h @@ -82,9 +82,9 @@ class ReportScheduler : public ReadHandler::Observer { public: virtual ~TimerDelegate() {} - virtual CHIP_ERROR StartTimer(ReadHandlerNode * node, System::Clock::Timeout aTimeout) = 0; - virtual void CancelTimer(ReadHandlerNode * node) = 0; - virtual bool IsTimerActive(ReadHandlerNode * node) = 0; + virtual CHIP_ERROR StartTimer(void * context, System::Clock::Timeout aTimeout) = 0; + virtual void CancelTimer(void * context) = 0; + virtual bool IsTimerActive(void * context) = 0; }; ReportScheduler(TimerDelegate * aTimerDelegate) : mTimerDelegate(aTimerDelegate) {} diff --git a/src/app/tests/TestReportScheduler.cpp b/src/app/tests/TestReportScheduler.cpp index bf1356b9287ed3..3d012bb4c8be25 100644 --- a/src/app/tests/TestReportScheduler.cpp +++ b/src/app/tests/TestReportScheduler.cpp @@ -96,20 +96,20 @@ class TestTimerDelegate : public ReportScheduler::TimerDelegate ReadHandlerNode * node = static_cast(aAppState); node->RunCallback(); } - virtual CHIP_ERROR StartTimer(ReadHandlerNode * node, System::Clock::Timeout aTimeout) override + virtual CHIP_ERROR StartTimer(void * context, System::Clock::Timeout aTimeout) override { return InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->StartTimer( - aTimeout, TimerCallbackInterface, node); + aTimeout, TimerCallbackInterface, context); } - virtual void CancelTimer(ReadHandlerNode * node) override + virtual void CancelTimer(void * context) override { InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->CancelTimer( - TimerCallbackInterface, node); + TimerCallbackInterface, context); } - virtual bool IsTimerActive(ReadHandlerNode * node) override + virtual bool IsTimerActive(void * context) override { return InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->IsTimerActive( - TimerCallbackInterface, node); + TimerCallbackInterface, context); } }; From 02bf98aeca05c30768182947117a4bab91771392 Mon Sep 17 00:00:00 2001 From: lpbeliveau-silabs Date: Wed, 5 Jul 2023 13:47:50 -0400 Subject: [PATCH 08/21] ifdefing out ScheduleRun() to debug failing CI --- src/app/reporting/ReportSchedulerImpl.cpp | 2 ++ src/app/tests/TestReportScheduler.cpp | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/app/reporting/ReportSchedulerImpl.cpp b/src/app/reporting/ReportSchedulerImpl.cpp index a5bf38917de24d..be1c0c0db119b8 100644 --- a/src/app/reporting/ReportSchedulerImpl.cpp +++ b/src/app/reporting/ReportSchedulerImpl.cpp @@ -32,7 +32,9 @@ using ReadHandlerNode = ReportScheduler::ReadHandlerNode; /// the engine already verifies that read handlers are reportable before sending a report static void ReportTimerCallback() { +#ifndef CONFIG_BUILD_FOR_HOST_UNIT_TEST InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun(); +#endif } ReportSchedulerImpl::ReportSchedulerImpl(TimerDelegate * aTimerDelegate) : ReportScheduler(aTimerDelegate) diff --git a/src/app/tests/TestReportScheduler.cpp b/src/app/tests/TestReportScheduler.cpp index 3d012bb4c8be25..6ef5224a2e68f8 100644 --- a/src/app/tests/TestReportScheduler.cpp +++ b/src/app/tests/TestReportScheduler.cpp @@ -249,6 +249,7 @@ class TestReportScheduler sScheduler.UnregisterAllHandlers(); readHandlerPool.ReleaseAll(); exchangeCtx->Close(); + NL_TEST_ASSERT(aSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0); } static void TestObserverCallbacks(nlTestSuite * aSuite, void * aContext) @@ -313,6 +314,7 @@ class TestReportScheduler sScheduler.UnregisterReadHandler(readHandler); readHandlerPool.ReleaseAll(); exchangeCtx->Close(); + NL_TEST_ASSERT(aSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0); } }; From 5e38e01aa3a47dd92962d778efc26129cbb0bc7a Mon Sep 17 00:00:00 2001 From: lpbeliveau-silabs Date: Thu, 6 Jul 2023 16:53:10 -0400 Subject: [PATCH 09/21] Added issue # to TODOs, refactored Min/Max Intervals to Min/Max Timestamp --- src/app/ReadHandler.cpp | 116 ++++++++++------- src/app/ReadHandler.h | 152 ++++++++++++++++------ src/app/reporting/Engine.cpp | 2 +- src/app/reporting/ReportScheduler.h | 14 +- src/app/reporting/ReportSchedulerImpl.cpp | 30 +++-- src/app/tests/TestReportScheduler.cpp | 7 +- 6 files changed, 209 insertions(+), 112 deletions(-) diff --git a/src/app/ReadHandler.cpp b/src/app/ReadHandler.cpp index cedf72fcb4384d..e6e273f1692e2e 100644 --- a/src/app/ReadHandler.cpp +++ b/src/app/ReadHandler.cpp @@ -64,14 +64,16 @@ ReadHandler::ReadHandler(ManagementCallback & apCallback, Messaging::ExchangeCon mSessionHandle.Grab(mExchangeCtx->GetSessionHandle()); - // TODO Uncomment when the ReportScheduler is implemented - // if (nullptr != observer) - //{ - // if (CHIP_NO_ERROR == SetObserver(observer)) - // { - // mObserver->OnReadHandlerAdded(this); - // } - //} +// TODO (#27672): Uncomment when the ReportScheduler is implemented +#if 0 + if (nullptr != observer) + { + if (CHIP_NO_ERROR == SetObserver(observer)) + { + mObserver->OnReadHandlerAdded(this); + } + } +#endif } #if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS @@ -82,14 +84,16 @@ ReadHandler::ReadHandler(ManagementCallback & apCallback, Observer * observer) : mInteractionType = InteractionType::Subscribe; mFlags.ClearAll(); - // TODO Uncomment when the ReportScheduler is implemented - // if (nullptr != observer) - //{ - // if (CHIP_NO_ERROR == SetObserver(observer)) - // { - // mObserver->OnReadHandlerAdded(this); - // } - //} +// TODO (#27672): Uncomment when the ReportScheduler is implemented +#if 0 + if (nullptr != observer) + { + if (CHIP_NO_ERROR == SetObserver(observer)) + { + mObserver->OnReadHandlerAdded(this); + } + } +#endif } void ReadHandler::ResumeSubscription(CASESessionManager & caseSessionManager, @@ -156,11 +160,13 @@ ReadHandler::~ReadHandler() InteractionModelEngine::GetInstance()->ReleaseEventPathList(mpEventPathList); InteractionModelEngine::GetInstance()->ReleaseDataVersionFilterList(mpDataVersionFilterList); - // TODO Uncomment when the ReportScheduler is implemented - // if (nullptr != mObserver) - //{ - // mObserver->OnReadHandlerRemoved(this); - //} +// TODO (#27672): Enable when the ReportScheduler is implemented +#if 0 + if (nullptr != mObserver) + { + mObserver->OnReadHandlerRemoved(this); + } +#endif } void ReadHandler::Close(CloseOptions options) @@ -343,12 +349,14 @@ CHIP_ERROR ReadHandler::SendReportData(System::PacketBufferHandle && aPayload, b if (IsType(InteractionType::Subscribe) && !IsPriming()) { - // TODO: Uncomment when the ReportScheduler is implemented and remove call to UpdateReportTimer, handled by - // the report Scheduler - // if (nullptr != mObserver) - //{ - // mObserver->OnReportSent(this); - //} +// TODO (#27672): Enable when the ReportScheduler is implemented and remove call to UpdateReportTimer, will be handled by +// the report Scheduler +#if 0 + if (nullptr != mObserver) + { + mObserver->OnReportSent(this); + } +#endif // Ignore the error from UpdateReportTimer. If we've // successfully sent the message, we need to return success from @@ -624,11 +632,13 @@ void ReadHandler::MoveToState(const HandlerState aTargetState) // if (aTargetState == HandlerState::GeneratingReports && IsReportableNow()) { - // TODO Uncomment when the ReportScheduler is implemented and remove the call to ScheduleRun() - // if(nullptr != mObserver) - //{ - // mObserver->OnBecameReportable(this); - //} +// TODO (#27672): Enable when the ReportScheduler is implemented and remove the call to ScheduleRun() +#if 0 + if(nullptr != mObserver) + { + mObserver->OnBecameReportable(this); + } +#endif InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun(); } } @@ -670,12 +680,14 @@ CHIP_ERROR ReadHandler::SendSubscribeResponse() ReturnErrorOnFailure(writer.Finalize(&packet)); VerifyOrReturnLogError(mExchangeCtx, CHIP_ERROR_INCORRECT_STATE); - // TODO: Uncomment when the ReportScheduler is implemented and remove call to UpdateReportTimer, handled by + // TODO (#27672): Uncomment when the ReportScheduler is implemented and remove call to UpdateReportTimer, handled by // the report Scheduler - // if (nullptr != mObserver) - //{ - // mObserver->OnReportSent(this); - //} +#if 0 + if (nullptr != mObserver) + { + mObserver->OnReportSent(this); + } +#endif ReturnErrorOnFailure(UpdateReportTimer()); ClearStateFlag(ReadHandlerFlags::PrimingReports); @@ -795,7 +807,7 @@ void ReadHandler::PersistSubscription() } } -// TODO: Remove as timing will now be handled by the ReportScheduler +// TODO (#27672): Remove when ReportScheduler is enabled as timing will now be handled by the ReportScheduler void ReadHandler::MinIntervalExpiredCallback(System::Layer * apSystemLayer, void * apAppState) { VerifyOrReturn(apAppState != nullptr); @@ -807,7 +819,7 @@ void ReadHandler::MinIntervalExpiredCallback(System::Layer * apSystemLayer, void readHandler); } -// TODO: Remove as timing will now be handled by the ReportScheduler +// TODO (#27672): Remove when ReportScheduler is enabled as timing will now be handled by the ReportScheduler void ReadHandler::MaxIntervalExpiredCallback(System::Layer * apSystemLayer, void * apAppState) { VerifyOrReturn(apAppState != nullptr); @@ -817,7 +829,7 @@ void ReadHandler::MaxIntervalExpiredCallback(System::Layer * apSystemLayer, void readHandler->mMaxInterval - readHandler->mMinIntervalFloorSeconds); } -// TODO: Remove as timing will now be handled by the ReportScheduler +// TODO (#27672): Remove when ReportScheduler is enabled as timing will now be handled by the ReportScheduler CHIP_ERROR ReadHandler::UpdateReportTimer() { InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->CancelTimer( @@ -876,11 +888,13 @@ void ReadHandler::AttributePathIsDirty(const AttributePathParams & aAttributeCha if (IsReportableNow()) { - // TODO Uncomment when the ReportScheduler is implemented and remove the call to ScheduleRun() - // if(nullptr != mObserver) - //{ - // mObserver->OnBecameReportable(this); - //} + // TODO (#27672): Enable when the ReportScheduler is implemented and remove the call to ScheduleRun() +#if 0 + if(nullptr != mObserver) + { + mObserver->OnBecameReportable(this); + } +#endif InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun(); } } @@ -907,11 +921,13 @@ void ReadHandler::SetStateFlag(ReadHandlerFlags aFlag, bool aValue) // If we became reportable, schedule a reporting run. if (!oldReportable && IsReportableNow()) { - // TODO Uncomment when the ReportScheduler is implemented and remove the call to ScheduleRun() - // if(nullptr != mObserver) - //{ - // mObserver->OnBecameReportable(this); - //} +// TODO (#27672): Enable when the ReportScheduler is implemented and remove the call to ScheduleRun() +#if 0 + if(nullptr != mObserver) + { + mObserver->OnBecameReportable(this); + } +#endif InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun(); } } diff --git a/src/app/ReadHandler.h b/src/app/ReadHandler.h index 909b3912848eb8..8c9849c78c46ae 100644 --- a/src/app/ReadHandler.h +++ b/src/app/ReadHandler.h @@ -154,7 +154,8 @@ class ReadHandler : public Messaging::ExchangeDelegate virtual ApplicationCallback * GetAppCallback() = 0; }; - // TODO : Merge existing callback and observer into one class and have an observer pool in the Readhandler to notify every + // TODO (#27675) : Merge existing callback and observer into one class and have an observer pool in the Readhandler to notify + // every /* * Observer class for ReadHandler, meant to allow multiple objects to observe the ReadHandler. Currently only one observer is * supported but all above callbacks should be merged into observer type and an observer pool should be added to allow multiple @@ -213,9 +214,18 @@ class ReadHandler : public Messaging::ExchangeDelegate ReadHandler(ManagementCallback & apCallback, Observer * observer = nullptr); #endif - const ObjectList * GetAttributePathList() const { return mpAttributePathList; } - const ObjectList * GetEventPathList() const { return mpEventPathList; } - const ObjectList * GetDataVersionFilterList() const { return mpDataVersionFilterList; } + const ObjectList * GetAttributePathList() const + { + return mpAttributePathList; + } + const ObjectList * GetEventPathList() const + { + return mpEventPathList; + } + const ObjectList * GetDataVersionFilterList() const + { + return mpDataVersionFilterList; + } void GetReportingIntervals(uint16_t & aMinInterval, uint16_t & aMaxInterval) const { @@ -254,26 +264,32 @@ class ReadHandler : public Messaging::ExchangeDelegate CHIP_ERROR SetObserver(Observer * aObserver) { VerifyOrReturnError(nullptr != aObserver, CHIP_ERROR_INVALID_ARGUMENT); - // TODO : After merging the callbacks and observer, change so the method adds a new observer to an observer pool + // TODO (#27675) : After merging the callbacks and observer, change so the method adds a new observer to an observer pool mObserver = aObserver; return CHIP_NO_ERROR; } private: - PriorityLevel GetCurrentPriority() const { return mCurrentPriority; } - EventNumber & GetEventMin() { return mEventMin; } + PriorityLevel GetCurrentPriority() const + { + return mCurrentPriority; + } + EventNumber & GetEventMin() + { + return mEventMin; + } enum class ReadHandlerFlags : uint8_t { // WaitingUntilMinInterval is used to prevent subscription data delivery while we are // waiting for the min reporting interval to elapse. - WaitingUntilMinInterval = (1 << 0), // TODO : Remove once ReportScheduler is implemented or change to test flag + WaitingUntilMinInterval = (1 << 0), // TODO (#27672): Remove once ReportScheduler is implemented or change to test flag // WaitingUntilMaxInterval is used to prevent subscription empty report delivery while we // are waiting for the max reporting interval to elaps. When WaitingUntilMaxInterval // becomes false, we are allowed to send an empty report to keep the // subscription alive on the client. - WaitingUntilMaxInterval = (1 << 1), // TODO : Remove once ReportScheduler is implemented + WaitingUntilMaxInterval = (1 << 1), // TODO (#27672): Remove once ReportScheduler is implemented // The flag indicating we are in the middle of a series of chunked report messages, this flag will be cleared during // sending last chunked message. @@ -342,10 +358,13 @@ class ReadHandler : public Messaging::ExchangeDelegate */ bool IsFromSubscriber(Messaging::ExchangeContext & apExchangeContext) const; - bool IsIdle() const { return mState == HandlerState::Idle; } + bool IsIdle() const + { + return mState == HandlerState::Idle; + } - // TODO: Change back to IsReportable once ReportScheduler is implemented so this can assess reportability without considering - // timing. The ReporScheduler will handle timing. + // TODO (#27672): Change back to IsReportable once ReportScheduler is implemented so this can assess reportability without + // considering timing. The ReporScheduler will handle timing. /// @brief Returns whether the ReadHandler is in a state where it can immediately send a report. This function /// is used to determine whether a report generation should be scheduled for the handler. bool IsReportableNow() const @@ -356,8 +375,14 @@ class ReadHandler : public Messaging::ExchangeDelegate return mState == HandlerState::GeneratingReports && !mFlags.Has(ReadHandlerFlags::WaitingUntilMinInterval) && (IsDirty() || !mFlags.Has(ReadHandlerFlags::WaitingUntilMaxInterval)); } - bool IsGeneratingReports() const { return mState == HandlerState::GeneratingReports; } - bool IsAwaitingReportResponse() const { return mState == HandlerState::AwaitingReportResponse; } + bool IsGeneratingReports() const + { + return mState == HandlerState::GeneratingReports; + } + bool IsAwaitingReportResponse() const + { + return mState == HandlerState::AwaitingReportResponse; + } // Resets the path iterator to the beginning of the whole report for generating a series of new reports. void ResetPathIterator(); @@ -369,17 +394,41 @@ class ReadHandler : public Messaging::ExchangeDelegate // sanpshotted last event, check with latest last event number, re-setup snapshoted checkpoint, and compare again. bool CheckEventClean(EventManagement & aEventManager); - bool IsType(InteractionType type) const { return (mInteractionType == type); } - bool IsChunkedReport() const { return mFlags.Has(ReadHandlerFlags::ChunkedReport); } + bool IsType(InteractionType type) const + { + return (mInteractionType == type); + } + bool IsChunkedReport() const + { + return mFlags.Has(ReadHandlerFlags::ChunkedReport); + } // Is reporting indicates whether we are in the middle of a series chunks. As we will set mIsChunkedReport on the first chunk // and clear that flag on the last chunk, we can use mIsChunkedReport to indicate this state. - bool IsReporting() const { return mFlags.Has(ReadHandlerFlags::ChunkedReport); } - bool IsPriming() const { return mFlags.Has(ReadHandlerFlags::PrimingReports); } - bool IsActiveSubscription() const { return mFlags.Has(ReadHandlerFlags::ActiveSubscription); } - bool IsFabricFiltered() const { return mFlags.Has(ReadHandlerFlags::FabricFiltered); } + bool IsReporting() const + { + return mFlags.Has(ReadHandlerFlags::ChunkedReport); + } + bool IsPriming() const + { + return mFlags.Has(ReadHandlerFlags::PrimingReports); + } + bool IsActiveSubscription() const + { + return mFlags.Has(ReadHandlerFlags::ActiveSubscription); + } + bool IsFabricFiltered() const + { + return mFlags.Has(ReadHandlerFlags::FabricFiltered); + } CHIP_ERROR OnSubscribeRequest(Messaging::ExchangeContext * apExchangeContext, System::PacketBufferHandle && aPayload); - void GetSubscriptionId(SubscriptionId & aSubscriptionId) const { aSubscriptionId = mSubscriptionId; } - AttributePathExpandIterator * GetAttributePathExpandIterator() { return &mAttributePathExpandIterator; } + void GetSubscriptionId(SubscriptionId & aSubscriptionId) const + { + aSubscriptionId = mSubscriptionId; + } + AttributePathExpandIterator * GetAttributePathExpandIterator() + { + return &mAttributePathExpandIterator; + } /// @brief Notifies the read handler that a set of attribute paths has been marked dirty. This will schedule a reporting engine /// run if the change to the attribute path makes the ReadHandler reportable. @@ -389,7 +438,10 @@ class ReadHandler : public Messaging::ExchangeDelegate { return (mDirtyGeneration > mPreviousReportsBeginGeneration) || mFlags.Has(ReadHandlerFlags::ForceDirty); } - void ClearForceDirtyFlag() { ClearStateFlag(ReadHandlerFlags::ForceDirty); } + void ClearForceDirtyFlag() + { + ClearStateFlag(ReadHandlerFlags::ForceDirty); + } NodeId GetInitiatorNodeId() const { auto session = GetSession(); @@ -403,23 +455,47 @@ class ReadHandler : public Messaging::ExchangeDelegate } Transport::SecureSession * GetSession() const; - SubjectDescriptor GetSubjectDescriptor() const { return GetSession()->GetSubjectDescriptor(); } + SubjectDescriptor GetSubjectDescriptor() const + { + return GetSession()->GetSubjectDescriptor(); + } - auto GetTransactionStartGeneration() const { return mTransactionStartGeneration; } + auto GetTransactionStartGeneration() const + { + return mTransactionStartGeneration; + } /// @brief Forces the read handler into a dirty state, regardless of what's going on with attributes. /// This can lead to scheduling of a reporting run immediately, if the min interval has been reached, /// or after the min interval is reached if it has not yet been reached. void ForceDirtyState(); - const AttributeValueEncoder::AttributeEncodeState & GetAttributeEncodeState() const { return mAttributeEncoderState; } - void SetAttributeEncodeState(const AttributeValueEncoder::AttributeEncodeState & aState) { mAttributeEncoderState = aState; } - uint32_t GetLastWrittenEventsBytes() const { return mLastWrittenEventsBytes; } + const AttributeValueEncoder::AttributeEncodeState & GetAttributeEncodeState() const + { + return mAttributeEncoderState; + } + void SetAttributeEncodeState(const AttributeValueEncoder::AttributeEncodeState & aState) + { + mAttributeEncoderState = aState; + } + uint32_t GetLastWrittenEventsBytes() const + { + return mLastWrittenEventsBytes; + } // Returns the number of interested paths, including wildcard and concrete paths. - size_t GetAttributePathCount() const { return mpAttributePathList == nullptr ? 0 : mpAttributePathList->Count(); }; - size_t GetEventPathCount() const { return mpEventPathList == nullptr ? 0 : mpEventPathList->Count(); }; - size_t GetDataVersionFilterCount() const { return mpDataVersionFilterList == nullptr ? 0 : mpDataVersionFilterList->Count(); }; + size_t GetAttributePathCount() const + { + return mpAttributePathList == nullptr ? 0 : mpAttributePathList->Count(); + }; + size_t GetEventPathCount() const + { + return mpEventPathList == nullptr ? 0 : mpEventPathList->Count(); + }; + size_t GetDataVersionFilterCount() const + { + return mpDataVersionFilterList == nullptr ? 0 : mpDataVersionFilterList->Count(); + }; CHIP_ERROR SendStatusReport(Protocols::InteractionModel::Status aStatus); @@ -464,12 +540,12 @@ class ReadHandler : public Messaging::ExchangeDelegate /// @brief This function is called when the min interval timer has expired, it restarts the timer on a timeout equal to the /// difference between the max interval and the min interval. - static void MinIntervalExpiredCallback(System::Layer * apSystemLayer, void * apAppState); // TODO : Remove once ReportScheduler - // is implemented. - static void MaxIntervalExpiredCallback(System::Layer * apSystemLayer, void * apAppState); // TODO : Remove once ReportScheduler - // is implemented. + static void MinIntervalExpiredCallback(System::Layer * apSystemLayer, void * apAppState); // TODO (#27672): Remove once + // ReportScheduler is implemented. + static void MaxIntervalExpiredCallback(System::Layer * apSystemLayer, void * apAppState); // TODO (#27672): Remove once + // ReportScheduler is implemented. /// @brief This function is called when a report is sent and it restarts the min interval timer. - CHIP_ERROR UpdateReportTimer(); // TODO : Remove once ReportScheduler is implemented. + CHIP_ERROR UpdateReportTimer(); // TODO (#27672) : Remove once ReportScheduler is implemented. CHIP_ERROR SendSubscribeResponse(); CHIP_ERROR ProcessSubscribeRequest(System::PacketBufferHandle && aPayload); @@ -555,7 +631,7 @@ class ReadHandler : public Messaging::ExchangeDelegate // The last schedule event number snapshoted in the beginning when preparing to fill new events to reports EventNumber mLastScheduledEventNumber = 0; - // TODO: We should shutdown the transaction when the session expires. + // TODO : We should shutdown the transaction when the session expires. SessionHolder mSessionHandle; Messaging::ExchangeHolder mExchangeCtx; @@ -583,7 +659,7 @@ class ReadHandler : public Messaging::ExchangeDelegate BitFlags mFlags; InteractionType mInteractionType = InteractionType::Read; - // TODO : Several objects are behaving as observers for this class, there should be a single + // TODO (#27675): Several objects are behaving as observers for this class, there should be a single // type for this and an array or a pool to store them. Observer * mObserver = nullptr; diff --git a/src/app/reporting/Engine.cpp b/src/app/reporting/Engine.cpp index d1eb9936d653d0..f0fea66cc0b16c 100644 --- a/src/app/reporting/Engine.cpp +++ b/src/app/reporting/Engine.cpp @@ -636,7 +636,7 @@ void Engine::Run() ReadHandler * readHandler = imEngine->ActiveHandlerAt(mCurReadHandlerIdx % (uint32_t) imEngine->mReadHandlers.Allocated()); VerifyOrDie(readHandler != nullptr); - // TODO: Replace with check with Report Scheduler if the read handler is reportable + // TODO (#27672): Replace with check with Report Scheduler if the read handler is reportable if (readHandler->IsReportableNow()) { mRunningReadHandler = readHandler; diff --git a/src/app/reporting/ReportScheduler.h b/src/app/reporting/ReportScheduler.h index 95825c82e228cc..3ec78db2ceddfe 100644 --- a/src/app/reporting/ReportScheduler.h +++ b/src/app/reporting/ReportScheduler.h @@ -54,7 +54,7 @@ class ReportScheduler : public ReadHandler::Observer // the scheduler in the ReadHandler System::Clock::Timestamp now = System::SystemClock().GetMonotonicTimestamp(); return (mReadHandler->IsGeneratingReports() && - ((now >= mMinIntervalSeconds && mReadHandler->IsDirty()) || now >= mMaxIntervalSeconds)); + ((now >= mMinTimestamp && mReadHandler->IsDirty()) || now >= mMaxTimestamp)); } void SetIntervalsTimeStamp(ReadHandler * aReadHandler) @@ -62,20 +62,20 @@ class ReportScheduler : public ReadHandler::Observer uint16_t minInterval, maxInterval; aReadHandler->GetReportingIntervals(minInterval, maxInterval); System::Clock::Timestamp now = System::SystemClock().GetMonotonicTimestamp(); - mMinIntervalSeconds = now + System::Clock::Seconds16(minInterval); - mMaxIntervalSeconds = now + System::Clock::Seconds16(maxInterval); + mMinTimestamp = now + System::Clock::Seconds16(minInterval); + mMaxTimestamp = now + System::Clock::Seconds16(maxInterval); } void RunCallback() { mCallback(); } - System::Clock::Timestamp GetMinInterval() const { return mMinIntervalSeconds; } - System::Clock::Timestamp GetMaxInterval() const { return mMaxIntervalSeconds; } + System::Clock::Timestamp GetMinTimestamp() const { return mMinTimestamp; } + System::Clock::Timestamp GetMaxTimestamp() const { return mMaxTimestamp; } private: TimerCompleteCallback mCallback; ReadHandler * mReadHandler; - System::Clock::Timestamp mMinIntervalSeconds; - System::Clock::Timestamp mMaxIntervalSeconds; + System::Clock::Timestamp mMinTimestamp; + System::Clock::Timestamp mMaxTimestamp; }; class TimerDelegate diff --git a/src/app/reporting/ReportSchedulerImpl.cpp b/src/app/reporting/ReportSchedulerImpl.cpp index be1c0c0db119b8..b6639363b19d55 100644 --- a/src/app/reporting/ReportSchedulerImpl.cpp +++ b/src/app/reporting/ReportSchedulerImpl.cpp @@ -32,9 +32,7 @@ using ReadHandlerNode = ReportScheduler::ReadHandlerNode; /// the engine already verifies that read handlers are reportable before sending a report static void ReportTimerCallback() { -#ifndef CONFIG_BUILD_FOR_HOST_UNIT_TEST InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun(); -#endif } ReportSchedulerImpl::ReportSchedulerImpl(TimerDelegate * aTimerDelegate) : ReportScheduler(aTimerDelegate) @@ -54,18 +52,20 @@ void ReportSchedulerImpl::OnBecameReportable(ReadHandler * aReadHandler) { ReadHandlerNode * node = FindReadHandlerNode(aReadHandler); VerifyOrReturn(nullptr != node); + + Milliseconds32 newTimeout; if (node->IsReportableNow()) { // If the handler is reportable now, just schedule a report immediately - InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun(); + newTimeout = Milliseconds32(0); } else { - Milliseconds32 newTimeout = - Milliseconds32(node->GetMinInterval().count() - System::SystemClock().GetMonotonicTimestamp().count()); // If the handler is not reportable now, schedule a report for the min interval - ScheduleReport(newTimeout, aReadHandler); + newTimeout = Milliseconds32(node->GetMinTimestamp().count() - System::SystemClock().GetMonotonicTimestamp().count()); } + + ScheduleReport(newTimeout, aReadHandler); } void ReportSchedulerImpl::OnReportSent(ReadHandler * apReadHandler) @@ -75,7 +75,7 @@ void ReportSchedulerImpl::OnReportSent(ReadHandler * apReadHandler) // Schedule callback for max interval node->SetIntervalsTimeStamp(apReadHandler); Milliseconds32 newTimeout = - Milliseconds32(node->GetMaxInterval().count() - System::SystemClock().GetMonotonicTimestamp().count()); + Milliseconds32(node->GetMaxTimestamp().count() - System::SystemClock().GetMonotonicTimestamp().count()); ScheduleReport(newTimeout, apReadHandler); } @@ -94,30 +94,32 @@ CHIP_ERROR ReportSchedulerImpl::RegisterReadHandler(ReadHandler * aReadHandler) mReadHandlerList.PushBack(newNode); ChipLogProgress(DataManagement, - "Registered ReadHandler that will schedule a report between system Timestamp: %" PRIu64 + "Registered a ReadHandler that will schedule a report between system Timestamp: %" PRIu64 " and system Timestamp %" PRIu64 ".", - newNode->GetMinInterval().count(), newNode->GetMaxInterval().count()); + newNode->GetMinTimestamp().count(), newNode->GetMaxTimestamp().count()); Timestamp now = System::SystemClock().GetMonotonicTimestamp(); + Milliseconds32 newTimeout; // If the handler is reportable, schedule a report for the min interval, otherwise schdule a report for the max interval if ((newNode->IsReportableNow())) { // If the handler is reportable now, just schedule a report immediately - ReturnErrorOnFailure(InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun()); + newTimeout = Milliseconds32(0); } - else if (IsReadHandlerReportable(aReadHandler) && (newNode->GetMinInterval() > now)) + else if (IsReadHandlerReportable(aReadHandler) && (newNode->GetMinTimestamp() > now)) { - Milliseconds32 newTimeout = Milliseconds32(newNode->GetMinInterval().count() - now.count()); // If the handler is reportable now, but the min interval is not elapsed, schedule a report for the min interval + newTimeout = Milliseconds32(newNode->GetMinTimestamp().count() - now.count()); + ReturnErrorOnFailure(ScheduleReport(newTimeout, aReadHandler)); } else { - Milliseconds32 newTimeout = Milliseconds32(newNode->GetMaxInterval().count() - now.count()); // If the handler is not reportable now, schedule a report for the max interval - ReturnErrorOnFailure(ScheduleReport(newTimeout, aReadHandler)); + newTimeout = Milliseconds32(newNode->GetMaxTimestamp().count() - now.count()); } + ReturnErrorOnFailure(ScheduleReport(newTimeout, aReadHandler)); return CHIP_NO_ERROR; } diff --git a/src/app/tests/TestReportScheduler.cpp b/src/app/tests/TestReportScheduler.cpp index 6ef5224a2e68f8..a17cb7fbbc9fe4 100644 --- a/src/app/tests/TestReportScheduler.cpp +++ b/src/app/tests/TestReportScheduler.cpp @@ -93,8 +93,11 @@ class TestTimerDelegate : public ReportScheduler::TimerDelegate public: static void TimerCallbackInterface(System::Layer * aLayer, void * aAppState) { - ReadHandlerNode * node = static_cast(aAppState); - node->RunCallback(); + // Normaly we would call the callback here, thus scheduling an engine run, but we don't need it for this test as we simulate + // all the callbacks related to report emissions. The actual callback should look like this: + // + // ReadHandlerNode * node = static_cast(aAppState); + // node->RunCallback(); } virtual CHIP_ERROR StartTimer(void * context, System::Clock::Timeout aTimeout) override { From 7c2cf2f9ede042072261b43001b30ead8b169b21 Mon Sep 17 00:00:00 2001 From: lpbeliveau-silabs Date: Thu, 6 Jul 2023 16:57:49 -0400 Subject: [PATCH 10/21] Clarified some comments regarding timing --- src/app/reporting/ReportSchedulerImpl.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/app/reporting/ReportSchedulerImpl.cpp b/src/app/reporting/ReportSchedulerImpl.cpp index b6639363b19d55..6a75e9d60777ca 100644 --- a/src/app/reporting/ReportSchedulerImpl.cpp +++ b/src/app/reporting/ReportSchedulerImpl.cpp @@ -40,13 +40,13 @@ ReportSchedulerImpl::ReportSchedulerImpl(TimerDelegate * aTimerDelegate) : Repor VerifyOrDie(nullptr != mTimerDelegate); } -/// @brief When a ReadHandler is added, register it, which will schedule an engine run on the max interval +/// @brief When a ReadHandler is added, register it, which will schedule an engine run void ReportSchedulerImpl::OnReadHandlerAdded(ReadHandler * aReadHandler) { RegisterReadHandler(aReadHandler); } -/// @brief When a ReadHandler becomes reportable, schedule, verifies if the handler is the min interval is elapsed. If not, +/// @brief When a ReadHandler becomes reportable, schedule, verifies if the min interval of a handleris elapsed. If not, /// reschedule the report to happen when the min interval is elapsed. If it is, schedule an engine run. void ReportSchedulerImpl::OnBecameReportable(ReadHandler * aReadHandler) { @@ -72,7 +72,7 @@ void ReportSchedulerImpl::OnReportSent(ReadHandler * apReadHandler) { ReadHandlerNode * node = FindReadHandlerNode(apReadHandler); VerifyOrReturn(nullptr != node); - // Schedule callback for max interval + // Schedule callback for max interval by computing the difference between the max timestamp and the current timestamp node->SetIntervalsTimeStamp(apReadHandler); Milliseconds32 newTimeout = Milliseconds32(node->GetMaxTimestamp().count() - System::SystemClock().GetMonotonicTimestamp().count()); @@ -100,7 +100,7 @@ CHIP_ERROR ReportSchedulerImpl::RegisterReadHandler(ReadHandler * aReadHandler) Timestamp now = System::SystemClock().GetMonotonicTimestamp(); Milliseconds32 newTimeout; - // If the handler is reportable, schedule a report for the min interval, otherwise schdule a report for the max interval + // If the handler is reportable, schedule a report for the min interval, otherwise schedule a report for the max interval if ((newNode->IsReportableNow())) { // If the handler is reportable now, just schedule a report immediately @@ -108,7 +108,8 @@ CHIP_ERROR ReportSchedulerImpl::RegisterReadHandler(ReadHandler * aReadHandler) } else if (IsReadHandlerReportable(aReadHandler) && (newNode->GetMinTimestamp() > now)) { - // If the handler is reportable now, but the min interval is not elapsed, schedule a report for the min interval + // If the handler is reportable now, but the min interval is not elapsed, schedule a report for the moment the min interval + // has elapsed newTimeout = Milliseconds32(newNode->GetMinTimestamp().count() - now.count()); ReturnErrorOnFailure(ScheduleReport(newTimeout, aReadHandler)); From 58df486e763e43329d7a1abb755a48a641c702f6 Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Thu, 6 Jul 2023 20:58:35 +0000 Subject: [PATCH 11/21] Restyled by whitespace --- src/app/ReadHandler.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/ReadHandler.cpp b/src/app/ReadHandler.cpp index e6e273f1692e2e..7d4e7cc1a5bbd7 100644 --- a/src/app/ReadHandler.cpp +++ b/src/app/ReadHandler.cpp @@ -633,7 +633,7 @@ void ReadHandler::MoveToState(const HandlerState aTargetState) if (aTargetState == HandlerState::GeneratingReports && IsReportableNow()) { // TODO (#27672): Enable when the ReportScheduler is implemented and remove the call to ScheduleRun() -#if 0 +#if 0 if(nullptr != mObserver) { mObserver->OnBecameReportable(this); @@ -922,7 +922,7 @@ void ReadHandler::SetStateFlag(ReadHandlerFlags aFlag, bool aValue) if (!oldReportable && IsReportableNow()) { // TODO (#27672): Enable when the ReportScheduler is implemented and remove the call to ScheduleRun() -#if 0 +#if 0 if(nullptr != mObserver) { mObserver->OnBecameReportable(this); From bfb69d9daef99ea21f6fdae6470e4e9cf76722da Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Thu, 6 Jul 2023 20:58:36 +0000 Subject: [PATCH 12/21] Restyled by clang-format --- src/app/ReadHandler.h | 125 +++++++++--------------------------------- 1 file changed, 25 insertions(+), 100 deletions(-) diff --git a/src/app/ReadHandler.h b/src/app/ReadHandler.h index 8c9849c78c46ae..5657be730fd517 100644 --- a/src/app/ReadHandler.h +++ b/src/app/ReadHandler.h @@ -214,18 +214,9 @@ class ReadHandler : public Messaging::ExchangeDelegate ReadHandler(ManagementCallback & apCallback, Observer * observer = nullptr); #endif - const ObjectList * GetAttributePathList() const - { - return mpAttributePathList; - } - const ObjectList * GetEventPathList() const - { - return mpEventPathList; - } - const ObjectList * GetDataVersionFilterList() const - { - return mpDataVersionFilterList; - } + const ObjectList * GetAttributePathList() const { return mpAttributePathList; } + const ObjectList * GetEventPathList() const { return mpEventPathList; } + const ObjectList * GetDataVersionFilterList() const { return mpDataVersionFilterList; } void GetReportingIntervals(uint16_t & aMinInterval, uint16_t & aMaxInterval) const { @@ -270,14 +261,8 @@ class ReadHandler : public Messaging::ExchangeDelegate } private: - PriorityLevel GetCurrentPriority() const - { - return mCurrentPriority; - } - EventNumber & GetEventMin() - { - return mEventMin; - } + PriorityLevel GetCurrentPriority() const { return mCurrentPriority; } + EventNumber & GetEventMin() { return mEventMin; } enum class ReadHandlerFlags : uint8_t { @@ -358,10 +343,7 @@ class ReadHandler : public Messaging::ExchangeDelegate */ bool IsFromSubscriber(Messaging::ExchangeContext & apExchangeContext) const; - bool IsIdle() const - { - return mState == HandlerState::Idle; - } + bool IsIdle() const { return mState == HandlerState::Idle; } // TODO (#27672): Change back to IsReportable once ReportScheduler is implemented so this can assess reportability without // considering timing. The ReporScheduler will handle timing. @@ -375,14 +357,8 @@ class ReadHandler : public Messaging::ExchangeDelegate return mState == HandlerState::GeneratingReports && !mFlags.Has(ReadHandlerFlags::WaitingUntilMinInterval) && (IsDirty() || !mFlags.Has(ReadHandlerFlags::WaitingUntilMaxInterval)); } - bool IsGeneratingReports() const - { - return mState == HandlerState::GeneratingReports; - } - bool IsAwaitingReportResponse() const - { - return mState == HandlerState::AwaitingReportResponse; - } + bool IsGeneratingReports() const { return mState == HandlerState::GeneratingReports; } + bool IsAwaitingReportResponse() const { return mState == HandlerState::AwaitingReportResponse; } // Resets the path iterator to the beginning of the whole report for generating a series of new reports. void ResetPathIterator(); @@ -394,41 +370,17 @@ class ReadHandler : public Messaging::ExchangeDelegate // sanpshotted last event, check with latest last event number, re-setup snapshoted checkpoint, and compare again. bool CheckEventClean(EventManagement & aEventManager); - bool IsType(InteractionType type) const - { - return (mInteractionType == type); - } - bool IsChunkedReport() const - { - return mFlags.Has(ReadHandlerFlags::ChunkedReport); - } + bool IsType(InteractionType type) const { return (mInteractionType == type); } + bool IsChunkedReport() const { return mFlags.Has(ReadHandlerFlags::ChunkedReport); } // Is reporting indicates whether we are in the middle of a series chunks. As we will set mIsChunkedReport on the first chunk // and clear that flag on the last chunk, we can use mIsChunkedReport to indicate this state. - bool IsReporting() const - { - return mFlags.Has(ReadHandlerFlags::ChunkedReport); - } - bool IsPriming() const - { - return mFlags.Has(ReadHandlerFlags::PrimingReports); - } - bool IsActiveSubscription() const - { - return mFlags.Has(ReadHandlerFlags::ActiveSubscription); - } - bool IsFabricFiltered() const - { - return mFlags.Has(ReadHandlerFlags::FabricFiltered); - } + bool IsReporting() const { return mFlags.Has(ReadHandlerFlags::ChunkedReport); } + bool IsPriming() const { return mFlags.Has(ReadHandlerFlags::PrimingReports); } + bool IsActiveSubscription() const { return mFlags.Has(ReadHandlerFlags::ActiveSubscription); } + bool IsFabricFiltered() const { return mFlags.Has(ReadHandlerFlags::FabricFiltered); } CHIP_ERROR OnSubscribeRequest(Messaging::ExchangeContext * apExchangeContext, System::PacketBufferHandle && aPayload); - void GetSubscriptionId(SubscriptionId & aSubscriptionId) const - { - aSubscriptionId = mSubscriptionId; - } - AttributePathExpandIterator * GetAttributePathExpandIterator() - { - return &mAttributePathExpandIterator; - } + void GetSubscriptionId(SubscriptionId & aSubscriptionId) const { aSubscriptionId = mSubscriptionId; } + AttributePathExpandIterator * GetAttributePathExpandIterator() { return &mAttributePathExpandIterator; } /// @brief Notifies the read handler that a set of attribute paths has been marked dirty. This will schedule a reporting engine /// run if the change to the attribute path makes the ReadHandler reportable. @@ -438,10 +390,7 @@ class ReadHandler : public Messaging::ExchangeDelegate { return (mDirtyGeneration > mPreviousReportsBeginGeneration) || mFlags.Has(ReadHandlerFlags::ForceDirty); } - void ClearForceDirtyFlag() - { - ClearStateFlag(ReadHandlerFlags::ForceDirty); - } + void ClearForceDirtyFlag() { ClearStateFlag(ReadHandlerFlags::ForceDirty); } NodeId GetInitiatorNodeId() const { auto session = GetSession(); @@ -455,47 +404,23 @@ class ReadHandler : public Messaging::ExchangeDelegate } Transport::SecureSession * GetSession() const; - SubjectDescriptor GetSubjectDescriptor() const - { - return GetSession()->GetSubjectDescriptor(); - } + SubjectDescriptor GetSubjectDescriptor() const { return GetSession()->GetSubjectDescriptor(); } - auto GetTransactionStartGeneration() const - { - return mTransactionStartGeneration; - } + auto GetTransactionStartGeneration() const { return mTransactionStartGeneration; } /// @brief Forces the read handler into a dirty state, regardless of what's going on with attributes. /// This can lead to scheduling of a reporting run immediately, if the min interval has been reached, /// or after the min interval is reached if it has not yet been reached. void ForceDirtyState(); - const AttributeValueEncoder::AttributeEncodeState & GetAttributeEncodeState() const - { - return mAttributeEncoderState; - } - void SetAttributeEncodeState(const AttributeValueEncoder::AttributeEncodeState & aState) - { - mAttributeEncoderState = aState; - } - uint32_t GetLastWrittenEventsBytes() const - { - return mLastWrittenEventsBytes; - } + const AttributeValueEncoder::AttributeEncodeState & GetAttributeEncodeState() const { return mAttributeEncoderState; } + void SetAttributeEncodeState(const AttributeValueEncoder::AttributeEncodeState & aState) { mAttributeEncoderState = aState; } + uint32_t GetLastWrittenEventsBytes() const { return mLastWrittenEventsBytes; } // Returns the number of interested paths, including wildcard and concrete paths. - size_t GetAttributePathCount() const - { - return mpAttributePathList == nullptr ? 0 : mpAttributePathList->Count(); - }; - size_t GetEventPathCount() const - { - return mpEventPathList == nullptr ? 0 : mpEventPathList->Count(); - }; - size_t GetDataVersionFilterCount() const - { - return mpDataVersionFilterList == nullptr ? 0 : mpDataVersionFilterList->Count(); - }; + size_t GetAttributePathCount() const { return mpAttributePathList == nullptr ? 0 : mpAttributePathList->Count(); }; + size_t GetEventPathCount() const { return mpEventPathList == nullptr ? 0 : mpEventPathList->Count(); }; + size_t GetDataVersionFilterCount() const { return mpDataVersionFilterList == nullptr ? 0 : mpDataVersionFilterList->Count(); }; CHIP_ERROR SendStatusReport(Protocols::InteractionModel::Status aStatus); From 6d92863c0b3bcc169fe0b983e74af3f935e752ad Mon Sep 17 00:00:00 2001 From: lpbeliveau-silabs Date: Tue, 11 Jul 2023 11:28:31 -0400 Subject: [PATCH 13/21] Added interface to GetMonotonicTimestamp in the timer delegate --- src/app/reporting/ReportScheduler.h | 45 ++++++++++++++--------- src/app/reporting/ReportSchedulerImpl.cpp | 8 ++-- src/app/tests/TestReportScheduler.cpp | 5 +++ 3 files changed, 36 insertions(+), 22 deletions(-) diff --git a/src/app/reporting/ReportScheduler.h b/src/app/reporting/ReportScheduler.h index 3ec78db2ceddfe..17a4ef470866cb 100644 --- a/src/app/reporting/ReportScheduler.h +++ b/src/app/reporting/ReportScheduler.h @@ -31,16 +31,33 @@ class TestReportScheduler; namespace reporting { +using Timestamp = System::Clock::Timestamp; + class ReportScheduler : public ReadHandler::Observer { public: + class TimerDelegate + { + public: + virtual ~TimerDelegate() {} + virtual CHIP_ERROR StartTimer(void * context, System::Clock::Timeout aTimeout) = 0; + virtual void CancelTimer(void * context) = 0; + virtual bool IsTimerActive(void * context) = 0; + virtual Timestamp GetCurrentMonotonicTimestamp() = 0; + }; + class ReadHandlerNode : public IntrusiveListNodeBase<> { public: using TimerCompleteCallback = void (*)(); - ReadHandlerNode(ReadHandler * aReadHandler, TimerCompleteCallback aCallback) : mCallback(aCallback) + ReadHandlerNode(ReadHandler * aReadHandler, TimerDelegate * aTimerDelegate, TimerCompleteCallback aCallback) : + mTimerDelegate(aTimerDelegate), mCallback(aCallback) { + VerifyOrDie(aReadHandler != nullptr); + VerifyOrDie(aTimerDelegate != nullptr); + VerifyOrDie(aCallback != nullptr); + mReadHandler = aReadHandler; SetIntervalsTimeStamp(aReadHandler); } @@ -52,7 +69,7 @@ class ReportScheduler : public ReadHandler::Observer { // TODO: Add flags to allow for test to simulate waiting for the min interval or max intrval to elapse when integrating // the scheduler in the ReadHandler - System::Clock::Timestamp now = System::SystemClock().GetMonotonicTimestamp(); + Timestamp now = mTimerDelegate->GetCurrentMonotonicTimestamp(); return (mReadHandler->IsGeneratingReports() && ((now >= mMinTimestamp && mReadHandler->IsDirty()) || now >= mMaxTimestamp)); } @@ -61,30 +78,22 @@ class ReportScheduler : public ReadHandler::Observer { uint16_t minInterval, maxInterval; aReadHandler->GetReportingIntervals(minInterval, maxInterval); - System::Clock::Timestamp now = System::SystemClock().GetMonotonicTimestamp(); - mMinTimestamp = now + System::Clock::Seconds16(minInterval); - mMaxTimestamp = now + System::Clock::Seconds16(maxInterval); + Timestamp now = mTimerDelegate->GetCurrentMonotonicTimestamp(); + mMinTimestamp = now + System::Clock::Seconds16(minInterval); + mMaxTimestamp = now + System::Clock::Seconds16(maxInterval); } void RunCallback() { mCallback(); } - System::Clock::Timestamp GetMinTimestamp() const { return mMinTimestamp; } - System::Clock::Timestamp GetMaxTimestamp() const { return mMaxTimestamp; } + Timestamp GetMinTimestamp() const { return mMinTimestamp; } + Timestamp GetMaxTimestamp() const { return mMaxTimestamp; } private: + TimerDelegate * mTimerDelegate; TimerCompleteCallback mCallback; ReadHandler * mReadHandler; - System::Clock::Timestamp mMinTimestamp; - System::Clock::Timestamp mMaxTimestamp; - }; - - class TimerDelegate - { - public: - virtual ~TimerDelegate() {} - virtual CHIP_ERROR StartTimer(void * context, System::Clock::Timeout aTimeout) = 0; - virtual void CancelTimer(void * context) = 0; - virtual bool IsTimerActive(void * context) = 0; + Timestamp mMinTimestamp; + Timestamp mMaxTimestamp; }; ReportScheduler(TimerDelegate * aTimerDelegate) : mTimerDelegate(aTimerDelegate) {} diff --git a/src/app/reporting/ReportSchedulerImpl.cpp b/src/app/reporting/ReportSchedulerImpl.cpp index 6a75e9d60777ca..4c8358590fb45b 100644 --- a/src/app/reporting/ReportSchedulerImpl.cpp +++ b/src/app/reporting/ReportSchedulerImpl.cpp @@ -62,7 +62,7 @@ void ReportSchedulerImpl::OnBecameReportable(ReadHandler * aReadHandler) else { // If the handler is not reportable now, schedule a report for the min interval - newTimeout = Milliseconds32(node->GetMinTimestamp().count() - System::SystemClock().GetMonotonicTimestamp().count()); + newTimeout = Milliseconds32(node->GetMinTimestamp().count() - mTimerDelegate->GetCurrentMonotonicTimestamp().count()); } ScheduleReport(newTimeout, aReadHandler); @@ -75,7 +75,7 @@ void ReportSchedulerImpl::OnReportSent(ReadHandler * apReadHandler) // Schedule callback for max interval by computing the difference between the max timestamp and the current timestamp node->SetIntervalsTimeStamp(apReadHandler); Milliseconds32 newTimeout = - Milliseconds32(node->GetMaxTimestamp().count() - System::SystemClock().GetMonotonicTimestamp().count()); + Milliseconds32(node->GetMaxTimestamp().count() - mTimerDelegate->GetCurrentMonotonicTimestamp().count()); ScheduleReport(newTimeout, apReadHandler); } @@ -90,7 +90,7 @@ CHIP_ERROR ReportSchedulerImpl::RegisterReadHandler(ReadHandler * aReadHandler) ReadHandlerNode * newNode = FindReadHandlerNode(aReadHandler); // If the handler is already registered, no need to register it again VerifyOrReturnValue(nullptr == newNode, CHIP_NO_ERROR); - newNode = mNodesPool.CreateObject(aReadHandler, ReportTimerCallback); + newNode = mNodesPool.CreateObject(aReadHandler, mTimerDelegate, ReportTimerCallback); mReadHandlerList.PushBack(newNode); ChipLogProgress(DataManagement, @@ -98,7 +98,7 @@ CHIP_ERROR ReportSchedulerImpl::RegisterReadHandler(ReadHandler * aReadHandler) " and system Timestamp %" PRIu64 ".", newNode->GetMinTimestamp().count(), newNode->GetMaxTimestamp().count()); - Timestamp now = System::SystemClock().GetMonotonicTimestamp(); + Timestamp now = mTimerDelegate->GetCurrentMonotonicTimestamp(); Milliseconds32 newTimeout; // If the handler is reportable, schedule a report for the min interval, otherwise schedule a report for the max interval if ((newNode->IsReportableNow())) diff --git a/src/app/tests/TestReportScheduler.cpp b/src/app/tests/TestReportScheduler.cpp index a17cb7fbbc9fe4..31b699a1b83b81 100644 --- a/src/app/tests/TestReportScheduler.cpp +++ b/src/app/tests/TestReportScheduler.cpp @@ -114,6 +114,11 @@ class TestTimerDelegate : public ReportScheduler::TimerDelegate return InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->IsTimerActive( TimerCallbackInterface, context); } + + virtual System::Clock::Timestamp GetCurrentMonotonicTimestamp() override + { + return System::SystemClock().GetMonotonicTimestamp(); + } }; static const size_t kNumMaxReadHandlers = 16; From 86aec7643be3438517acf5679333927abe3622f2 Mon Sep 17 00:00:00 2001 From: lpbeliveau-silabs <112982107+lpbeliveau-silabs@users.noreply.github.com> Date: Tue, 11 Jul 2023 18:16:01 -0400 Subject: [PATCH 14/21] Apply suggestions from code review Co-authored-by: Boris Zbarsky --- src/app/ReadHandler.h | 6 +++--- src/app/reporting/ReportScheduler.h | 6 +++--- src/app/tests/TestReportScheduler.cpp | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/app/ReadHandler.h b/src/app/ReadHandler.h index 5657be730fd517..af8bf029b027ab 100644 --- a/src/app/ReadHandler.h +++ b/src/app/ReadHandler.h @@ -176,7 +176,7 @@ class ReadHandler : public Messaging::ExchangeDelegate /// @param[in] apReadHandler ReadHandler that became dirty virtual void OnBecameReportable(ReadHandler * apReadHandler) = 0; - /// @brief Callback invoked when a Repport is sent so the next report can be scheduled + /// @brief Callback invoked when the read handler needs to make sure to send a message to the subscriber within the next maxInterval time period. /// @param[in] apReadHandler ReadHandler that has generated a report virtual void OnReportSent(ReadHandler * apReadHandler) = 0; @@ -224,7 +224,7 @@ class ReadHandler : public Messaging::ExchangeDelegate aMaxInterval = mMaxInterval; } - CHIP_ERROR SetMinReportingIntervals(uint16_t aMinInterval) + CHIP_ERROR SetMinReportingInterval(uint16_t aMinInterval) { VerifyOrReturnError(IsIdle(), CHIP_ERROR_INCORRECT_STATE); VerifyOrReturnError(aMinInterval <= mMaxInterval, CHIP_ERROR_INVALID_ARGUMENT); @@ -238,7 +238,7 @@ class ReadHandler : public Messaging::ExchangeDelegate * MinIntervalFloor ≤ MaxInterval ≤ MAX(SUBSCRIPTION_MAX_INTERVAL_PUBLISHER_LIMIT, MaxIntervalCeiling) * Where SUBSCRIPTION_MAX_INTERVAL_PUBLISHER_LIMIT is set to 60m in the spec. */ - CHIP_ERROR SetMaxReportingIntervals(uint16_t aMaxInterval) + CHIP_ERROR SetMaxReportingInterval(uint16_t aMaxInterval) { VerifyOrReturnError(IsIdle(), CHIP_ERROR_INCORRECT_STATE); VerifyOrReturnError(mMinIntervalFloorSeconds <= aMaxInterval, CHIP_ERROR_INVALID_ARGUMENT); diff --git a/src/app/reporting/ReportScheduler.h b/src/app/reporting/ReportScheduler.h index 17a4ef470866cb..d1bc23bf81bbe3 100644 --- a/src/app/reporting/ReportScheduler.h +++ b/src/app/reporting/ReportScheduler.h @@ -63,7 +63,7 @@ class ReportScheduler : public ReadHandler::Observer } ReadHandler * GetReadHandler() const { return mReadHandler; } /// @brief Check if the Node is reportable now, meaning its readhandler was made reportable by attribute dirtying and - /// handler state, and minimal time interval last reporthas elapsed since, or the maximal time interval since last report + /// handler state, and minimal time interval last since last report has elapsed, or the maximal time interval since last report /// has elapsed bool IsReportableNow() const { @@ -74,7 +74,7 @@ class ReportScheduler : public ReadHandler::Observer ((now >= mMinTimestamp && mReadHandler->IsDirty()) || now >= mMaxTimestamp)); } - void SetIntervalsTimeStamp(ReadHandler * aReadHandler) + void SetIntervalTimeStamps(ReadHandler * aReadHandler) { uint16_t minInterval, maxInterval; aReadHandler->GetReportingIntervals(minInterval, maxInterval); @@ -124,7 +124,7 @@ class ReportScheduler : public ReadHandler::Observer virtual void UnregisterAllHandlers() = 0; /// @brief Check if a ReadHandler is scheduled for reporting virtual bool IsReportScheduled(ReadHandler * aReadHandler) = 0; - /// @brief Check if a ReadHandler is reportable given its minimal and maximal intervals by using the node timestamps + /// @brief Check whether a ReadHandler is reportable right now, taking into account its minimum and maximum intervals. /// @param aReadHandler read handler to check bool IsReportableNow(ReadHandler * aReadHandler) { return FindReadHandlerNode(aReadHandler)->IsReportableNow(); }; /// @brief Check if a ReadHandler is reportable without considering the timing diff --git a/src/app/tests/TestReportScheduler.cpp b/src/app/tests/TestReportScheduler.cpp index 31b699a1b83b81..2ee87ebff16ae2 100644 --- a/src/app/tests/TestReportScheduler.cpp +++ b/src/app/tests/TestReportScheduler.cpp @@ -290,7 +290,7 @@ class TestReportScheduler // Test OnBecameReportable readHandler->ForceDirtyState(); readHandler->mObserver->OnBecameReportable(readHandler); - // Should have changed the scheduled timeout to the handlers min interval, to check, we wait for the min interval to + // Should have changed the scheduled timeout to the handler's min interval, to check, we wait for the min interval to // expire ctx.GetIOContext().DriveIOUntil(System::Clock::Milliseconds32(1100), [&]() -> bool { return sScheduler.IsReportableNow(readHandler); }); From 64f50417f9a541d115b2b57f87cc44fc24d086d3 Mon Sep 17 00:00:00 2001 From: lpbeliveau-silabs Date: Tue, 11 Jul 2023 19:21:06 -0400 Subject: [PATCH 15/21] Completed renaming to eliminate compiling error, moved TestReporScehduler in reporting namespace, addressed some low hanging fruits --- src/app/ReadHandler.cpp | 23 ++++++------- src/app/ReadHandler.h | 20 +++++------ src/app/reporting/ReportScheduler.h | 15 ++++---- src/app/reporting/ReportSchedulerImpl.cpp | 8 ++--- src/app/reporting/ReportSchedulerImpl.h | 9 +++-- src/app/tests/TestReportScheduler.cpp | 36 +++++++++++--------- src/controller/tests/data_model/TestRead.cpp | 2 +- 7 files changed, 57 insertions(+), 56 deletions(-) diff --git a/src/app/ReadHandler.cpp b/src/app/ReadHandler.cpp index 7d4e7cc1a5bbd7..a5209ae45d588e 100644 --- a/src/app/ReadHandler.cpp +++ b/src/app/ReadHandler.cpp @@ -70,7 +70,7 @@ ReadHandler::ReadHandler(ManagementCallback & apCallback, Messaging::ExchangeCon { if (CHIP_NO_ERROR == SetObserver(observer)) { - mObserver->OnReadHandlerAdded(this); + mObserver->OnReadHandlerCreated(this); } } #endif @@ -90,7 +90,7 @@ ReadHandler::ReadHandler(ManagementCallback & apCallback, Observer * observer) : { if (CHIP_NO_ERROR == SetObserver(observer)) { - mObserver->OnReadHandlerAdded(this); + mObserver->OnReadHandlerCreated(this); } } #endif @@ -137,6 +137,13 @@ void ReadHandler::ResumeSubscription(CASESessionManager & caseSessionManager, ReadHandler::~ReadHandler() { + // TODO (#27672): Enable when the ReportScheduler is implemented and move in Close() after testing +#if 0 + if (nullptr != mObserver) + { + mObserver->OnReadHandlerDestroyed(this); + } +#endif auto * appCallback = mManagementCallback.GetAppCallback(); if (mFlags.Has(ReadHandlerFlags::ActiveSubscription) && appCallback) { @@ -159,14 +166,6 @@ ReadHandler::~ReadHandler() InteractionModelEngine::GetInstance()->ReleaseAttributePathList(mpAttributePathList); InteractionModelEngine::GetInstance()->ReleaseEventPathList(mpEventPathList); InteractionModelEngine::GetInstance()->ReleaseDataVersionFilterList(mpDataVersionFilterList); - -// TODO (#27672): Enable when the ReportScheduler is implemented -#if 0 - if (nullptr != mObserver) - { - mObserver->OnReadHandlerRemoved(this); - } -#endif } void ReadHandler::Close(CloseOptions options) @@ -354,7 +353,7 @@ CHIP_ERROR ReadHandler::SendReportData(System::PacketBufferHandle && aPayload, b #if 0 if (nullptr != mObserver) { - mObserver->OnReportSent(this); + mObserver->OnSubscriptionAction(this); } #endif @@ -685,7 +684,7 @@ CHIP_ERROR ReadHandler::SendSubscribeResponse() #if 0 if (nullptr != mObserver) { - mObserver->OnReportSent(this); + mObserver->OnSubscriptionAction(this); } #endif ReturnErrorOnFailure(UpdateReportTimer()); diff --git a/src/app/ReadHandler.h b/src/app/ReadHandler.h index af8bf029b027ab..579c34880bfc4d 100644 --- a/src/app/ReadHandler.h +++ b/src/app/ReadHandler.h @@ -168,7 +168,7 @@ class ReadHandler : public Messaging::ExchangeDelegate /// @brief Callback invoked to notify a ReadHandler was created and can be registered /// @param[in] apReadHandler ReadHandler getting added - virtual void OnReadHandlerAdded(ReadHandler * apReadHandler) = 0; + virtual void OnReadHandlerCreated(ReadHandler * apReadHandler) = 0; /// @brief Callback invoked when a ReadHandler went from a non reportable state to a reportable state so a report can be /// sent immediately if the minimal interval allows it. Otherwise the report should be rescheduled to the earliest time @@ -176,13 +176,14 @@ class ReadHandler : public Messaging::ExchangeDelegate /// @param[in] apReadHandler ReadHandler that became dirty virtual void OnBecameReportable(ReadHandler * apReadHandler) = 0; - /// @brief Callback invoked when the read handler needs to make sure to send a message to the subscriber within the next maxInterval time period. + /// @brief Callback invoked when the read handler needs to make sure to send a message to the subscriber within the next + /// maxInterval time period. /// @param[in] apReadHandler ReadHandler that has generated a report - virtual void OnReportSent(ReadHandler * apReadHandler) = 0; + virtual void OnSubscriptionAction(ReadHandler * apReadHandler) = 0; /// @brief Callback invoked when a ReadHandler is getting removed so it can be unregistered /// @param[in] apReadHandler ReadHandler getting destroyed - virtual void OnReadHandlerRemoved(ReadHandler * apReadHandler) = 0; + virtual void OnReadHandlerDestroyed(ReadHandler * apReadHandler) = 0; }; /* @@ -426,7 +427,7 @@ class ReadHandler : public Messaging::ExchangeDelegate friend class TestReadInteraction; friend class chip::app::reporting::TestReportingEngine; - friend class TestReportScheduler; + friend class chip::app::reporting::TestReportScheduler; // // The engine needs to be able to Abort/Close a ReadHandler instance upon completion of work for a given read/subscribe @@ -436,8 +437,8 @@ class ReadHandler : public Messaging::ExchangeDelegate friend class chip::app::reporting::Engine; friend class chip::app::InteractionModelEngine; - // The report scheduler needs to be able to access StateFlag private functions to know when to schedule a run so it is declared - // as a friend class. + // The report scheduler needs to be able to access StateFlag private functions IsReadHandlerReportable and IsChunkedReport to + // know when to schedule a run so it is declared as a friend class. friend class chip::app::reporting::ReportScheduler; enum class HandlerState : uint8_t @@ -556,7 +557,7 @@ class ReadHandler : public Messaging::ExchangeDelegate // The last schedule event number snapshoted in the beginning when preparing to fill new events to reports EventNumber mLastScheduledEventNumber = 0; - // TODO : We should shutdown the transaction when the session expires. + // TODO: We should shutdown the transaction when the session expires. SessionHolder mSessionHandle; Messaging::ExchangeHolder mExchangeCtx; @@ -584,8 +585,7 @@ class ReadHandler : public Messaging::ExchangeDelegate BitFlags mFlags; InteractionType mInteractionType = InteractionType::Read; - // TODO (#27675): Several objects are behaving as observers for this class, there should be a single - // type for this and an array or a pool to store them. + // TODO (#27675): Merge all observers into one and that one will dispatch the callbacks to the right place. Observer * mObserver = nullptr; #if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS diff --git a/src/app/reporting/ReportScheduler.h b/src/app/reporting/ReportScheduler.h index d1bc23bf81bbe3..36dbc3147cbba4 100644 --- a/src/app/reporting/ReportScheduler.h +++ b/src/app/reporting/ReportScheduler.h @@ -25,12 +25,11 @@ namespace chip { namespace app { +namespace reporting { // Forward declaration of TestReportScheduler to allow it to be friend with ReportScheduler class TestReportScheduler; -namespace reporting { - using Timestamp = System::Clock::Timestamp; class ReportScheduler : public ReadHandler::Observer @@ -59,12 +58,12 @@ class ReportScheduler : public ReadHandler::Observer VerifyOrDie(aCallback != nullptr); mReadHandler = aReadHandler; - SetIntervalsTimeStamp(aReadHandler); + SetIntervalTimeStamps(aReadHandler); } ReadHandler * GetReadHandler() const { return mReadHandler; } /// @brief Check if the Node is reportable now, meaning its readhandler was made reportable by attribute dirtying and - /// handler state, and minimal time interval last since last report has elapsed, or the maximal time interval since last report - /// has elapsed + /// handler state, and minimal time interval last since last report has elapsed, or the maximal time interval since last + /// report has elapsed bool IsReportableNow() const { // TODO: Add flags to allow for test to simulate waiting for the min interval or max intrval to elapse when integrating @@ -140,13 +139,15 @@ class ReportScheduler : public ReadHandler::Observer size_t GetNumReadHandlers() const { return mNodesPool.Allocated(); } protected: - friend class chip::app::TestReportScheduler; + friend class chip::app::reporting::TestReportScheduler; /// @brief Find the ReadHandlerNode for a given ReadHandler pointer /// @param [in] aReadHandler ReadHandler pointer to look for in the ReadHandler nodes list /// @return Node Address if node was found, nullptr otherwise virtual ReadHandlerNode * FindReadHandlerNode(const ReadHandler * aReadHandler) = 0; - + /// @brief Start a timer for a given ReadHandlerNode, ensures that if a timer is already running for this node, it is cancelled + /// @param node Node of the ReadHandler list to start a timer for + /// @param aTimeout Delay before the timer expires virtual CHIP_ERROR StartTimerForHandler(ReadHandlerNode * node, System::Clock::Timeout aTimeout) = 0; virtual void CancelTimerForHandler(ReadHandlerNode * node) = 0; virtual bool CheckTimerActiveForHandler(ReadHandlerNode * node) = 0; diff --git a/src/app/reporting/ReportSchedulerImpl.cpp b/src/app/reporting/ReportSchedulerImpl.cpp index 4c8358590fb45b..ec46a6c1c89a72 100644 --- a/src/app/reporting/ReportSchedulerImpl.cpp +++ b/src/app/reporting/ReportSchedulerImpl.cpp @@ -41,7 +41,7 @@ ReportSchedulerImpl::ReportSchedulerImpl(TimerDelegate * aTimerDelegate) : Repor } /// @brief When a ReadHandler is added, register it, which will schedule an engine run -void ReportSchedulerImpl::OnReadHandlerAdded(ReadHandler * aReadHandler) +void ReportSchedulerImpl::OnReadHandlerCreated(ReadHandler * aReadHandler) { RegisterReadHandler(aReadHandler); } @@ -68,19 +68,19 @@ void ReportSchedulerImpl::OnBecameReportable(ReadHandler * aReadHandler) ScheduleReport(newTimeout, aReadHandler); } -void ReportSchedulerImpl::OnReportSent(ReadHandler * apReadHandler) +void ReportSchedulerImpl::OnSubscriptionAction(ReadHandler * apReadHandler) { ReadHandlerNode * node = FindReadHandlerNode(apReadHandler); VerifyOrReturn(nullptr != node); // Schedule callback for max interval by computing the difference between the max timestamp and the current timestamp - node->SetIntervalsTimeStamp(apReadHandler); + node->SetIntervalTimeStamps(apReadHandler); Milliseconds32 newTimeout = Milliseconds32(node->GetMaxTimestamp().count() - mTimerDelegate->GetCurrentMonotonicTimestamp().count()); ScheduleReport(newTimeout, apReadHandler); } /// @brief When a ReadHandler is removed, unregister it, which will cancel any scheduled report -void ReportSchedulerImpl::OnReadHandlerRemoved(ReadHandler * aReadHandler) +void ReportSchedulerImpl::OnReadHandlerDestroyed(ReadHandler * aReadHandler) { UnregisterReadHandler(aReadHandler); } diff --git a/src/app/reporting/ReportSchedulerImpl.h b/src/app/reporting/ReportSchedulerImpl.h index fdd54f3cba132a..60deca82bd40ce 100644 --- a/src/app/reporting/ReportSchedulerImpl.h +++ b/src/app/reporting/ReportSchedulerImpl.h @@ -22,7 +22,6 @@ namespace chip { namespace app { - namespace reporting { class ReportSchedulerImpl : public ReportScheduler @@ -32,10 +31,10 @@ class ReportSchedulerImpl : public ReportScheduler virtual ~ReportSchedulerImpl() override { UnregisterAllHandlers(); } // ReadHandlerObserver - virtual void OnReadHandlerAdded(ReadHandler * aReadHandler) override; + virtual void OnReadHandlerCreated(ReadHandler * aReadHandler) override; virtual void OnBecameReportable(ReadHandler * aReadHandler) override; - virtual void OnReportSent(ReadHandler * aReadHandler) override; - virtual void OnReadHandlerRemoved(ReadHandler * aReadHandler) override; + virtual void OnSubscriptionAction(ReadHandler * aReadHandler) override; + virtual void OnReadHandlerDestroyed(ReadHandler * aReadHandler) override; // ReportScheduler specific virtual CHIP_ERROR RegisterReadHandler(ReadHandler * aReadHandler) override; @@ -46,7 +45,7 @@ class ReportSchedulerImpl : public ReportScheduler virtual bool IsReportScheduled(ReadHandler * aReadHandler) override; protected: - friend class chip::app::TestReportScheduler; + friend class chip::app::reporting::TestReportScheduler; /// @brief Find the ReadHandlerNode for a given ReadHandler pointer /// @param [in] aReadHandler diff --git a/src/app/tests/TestReportScheduler.cpp b/src/app/tests/TestReportScheduler.cpp index 2ee87ebff16ae2..1c43ada639022d 100644 --- a/src/app/tests/TestReportScheduler.cpp +++ b/src/app/tests/TestReportScheduler.cpp @@ -82,6 +82,7 @@ class NullReadHandlerCallback : public chip::app::ReadHandler::ManagementCallbac namespace chip { namespace app { +namespace reporting { using InteractionModelEngine = InteractionModelEngine; using ReportScheduler = reporting::ReportScheduler; @@ -204,23 +205,23 @@ class TestReportScheduler // Dirty read handler, will be triggered at min interval ReadHandler * readHandler1 = readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler1->SetMaxReportingIntervals(2)); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler1->SetMinReportingIntervals(1)); + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler1->SetMaxReportingInterval(2)); + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler1->SetMinReportingInterval(1)); readHandler1->ForceDirtyState(); readHandler1->MoveToState(ReadHandler::HandlerState::GeneratingReports); // Clean read handler, will be triggered at max interval ReadHandler * readHandler2 = readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler2->SetMaxReportingIntervals(3)); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler2->SetMinReportingIntervals(0)); + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler2->SetMaxReportingInterval(3)); + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler2->SetMinReportingInterval(0)); readHandler2->MoveToState(ReadHandler::HandlerState::GeneratingReports); // Clean read handler, will be triggered at max interval, but will be cancelled before ReadHandler * readHandler3 = readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler3->SetMaxReportingIntervals(3)); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler3->SetMinReportingIntervals(0)); + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler3->SetMaxReportingInterval(3)); + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler3->SetMinReportingInterval(0)); readHandler3->MoveToState(ReadHandler::HandlerState::GeneratingReports); NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == sScheduler.RegisterReadHandler(readHandler1)); @@ -272,13 +273,13 @@ class TestReportScheduler ReadHandler * readHandler = readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler->SetMaxReportingIntervals(2)); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler->SetMinReportingIntervals(1)); + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler->SetMaxReportingInterval(2)); + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler->SetMinReportingInterval(1)); readHandler->MoveToState(ReadHandler::HandlerState::GeneratingReports); readHandler->SetObserver(&sScheduler); - // Test OnReadHandlerAdded - readHandler->mObserver->OnReadHandlerAdded(readHandler); + // Test OnReadHandlerCreated + readHandler->mObserver->OnReadHandlerCreated(readHandler); // Should have registered the read handler in the scheduler and scheduled a report NL_TEST_ASSERT(aSuite, sScheduler.GetNumReadHandlers() == 1); NL_TEST_ASSERT(aSuite, sScheduler.IsReportScheduled(readHandler)); @@ -297,9 +298,9 @@ class TestReportScheduler // Check that no report is scheduled since the min interval has expired, the timer should now be stopped NL_TEST_ASSERT(aSuite, !sScheduler.IsReportScheduled(readHandler)); - // Test OnReportSent + // Test OnSubscriptionAction readHandler->ClearForceDirtyFlag(); - readHandler->mObserver->OnReportSent(readHandler); + readHandler->mObserver->OnSubscriptionAction(readHandler); // Should have changed the scheduled timeout to the handlers max interval, to check, we wait for the min interval to // confirm it is not expired yet so the report should still be scheduled NL_TEST_ASSERT(aSuite, sScheduler.IsReportScheduled(readHandler)); @@ -312,8 +313,8 @@ class TestReportScheduler // Check that no report is scheduled since the max interval should have expired, the timer should now be stopped NL_TEST_ASSERT(aSuite, !sScheduler.IsReportScheduled(readHandler)); - // Test OnReadHandlerRemoved - readHandler->mObserver->OnReadHandlerRemoved(readHandler); + // Test OnReadHandlerDestroyed + readHandler->mObserver->OnReadHandlerDestroyed(readHandler); // Should have unregistered the read handler in the scheduler and cancelled the report NL_TEST_ASSERT(aSuite, !sScheduler.IsReportScheduled(readHandler)); NL_TEST_ASSERT(aSuite, sScheduler.GetNumReadHandlers() == 0); @@ -326,6 +327,7 @@ class TestReportScheduler } }; +} // namespace reporting } // namespace app } // namespace chip @@ -336,9 +338,9 @@ namespace { */ static nlTest sTests[] = { - NL_TEST_DEF("TestReadHandlerList", chip::app::TestReportScheduler::TestReadHandlerList), - NL_TEST_DEF("TestReportTiming", chip::app::TestReportScheduler::TestReportTiming), - NL_TEST_DEF("TestObserverCallbacks", chip::app::TestReportScheduler::TestObserverCallbacks), + NL_TEST_DEF("TestReadHandlerList", chip::app::reporting::TestReportScheduler::TestReadHandlerList), + NL_TEST_DEF("TestReportTiming", chip::app::reporting::TestReportScheduler::TestReportTiming), + NL_TEST_DEF("TestObserverCallbacks", chip::app::reporting::TestReportScheduler::TestObserverCallbacks), NL_TEST_SENTINEL(), }; diff --git a/src/controller/tests/data_model/TestRead.cpp b/src/controller/tests/data_model/TestRead.cpp index 8b18f8625594f3..fe1bc18c86259e 100644 --- a/src/controller/tests/data_model/TestRead.cpp +++ b/src/controller/tests/data_model/TestRead.cpp @@ -302,7 +302,7 @@ class TestReadInteraction : public app::ReadHandler::ApplicationCallback if (mAlterSubscriptionIntervals) { - ReturnErrorOnFailure(aReadHandler.SetMaxReportingIntervals(mMaxInterval)); + ReturnErrorOnFailure(aReadHandler.SetMaxReportingInterval(mMaxInterval)); } return CHIP_NO_ERROR; } From f015837fab40e04bdf1c5d638ad0fa7f56da83d1 Mon Sep 17 00:00:00 2001 From: lpbeliveau-silabs Date: Wed, 12 Jul 2023 09:40:13 -0400 Subject: [PATCH 16/21] Removed useless objects from tests as well as useless typecasting, and unnecessary check --- examples/platform/nrfconnect/util/ICDUtil.cpp | 2 +- .../silabs/ICDSubscriptionCallback.cpp | 2 +- src/app/ReadHandler.h | 3 +- src/app/reporting/ReportScheduler.h | 55 +++++++---------- src/app/reporting/ReportSchedulerImpl.cpp | 59 ++++++------------- src/app/reporting/ReportSchedulerImpl.h | 41 ++++++------- src/app/tests/TestReportScheduler.cpp | 14 ----- src/platform/telink/ICDUtil.cpp | 2 +- 8 files changed, 66 insertions(+), 112 deletions(-) diff --git a/examples/platform/nrfconnect/util/ICDUtil.cpp b/examples/platform/nrfconnect/util/ICDUtil.cpp index b1c2c4797adb80..b3dc9c80bbc9dd 100644 --- a/examples/platform/nrfconnect/util/ICDUtil.cpp +++ b/examples/platform/nrfconnect/util/ICDUtil.cpp @@ -36,5 +36,5 @@ CHIP_ERROR ICDUtil::OnSubscriptionRequested(chip::app::ReadHandler & aReadHandle agreedMaxInterval = kSubscriptionMaxIntervalPublisherLimit; } - return aReadHandler.SetMaxReportingIntervals(agreedMaxInterval); + return aReadHandler.SetMaxReportingInterval(agreedMaxInterval); } diff --git a/examples/platform/silabs/ICDSubscriptionCallback.cpp b/examples/platform/silabs/ICDSubscriptionCallback.cpp index 8551277fe19885..318300472ecfaa 100644 --- a/examples/platform/silabs/ICDSubscriptionCallback.cpp +++ b/examples/platform/silabs/ICDSubscriptionCallback.cpp @@ -61,5 +61,5 @@ CHIP_ERROR ICDSubscriptionCallback::OnSubscriptionRequested(chip::app::ReadHandl decidedMaxInterval = maximumMaxInterval; } - return aReadHandler.SetMaxReportingIntervals(decidedMaxInterval); + return aReadHandler.SetMaxReportingInterval(decidedMaxInterval); } diff --git a/src/app/ReadHandler.h b/src/app/ReadHandler.h index 579c34880bfc4d..40b240180f7689 100644 --- a/src/app/ReadHandler.h +++ b/src/app/ReadHandler.h @@ -229,7 +229,8 @@ class ReadHandler : public Messaging::ExchangeDelegate { VerifyOrReturnError(IsIdle(), CHIP_ERROR_INCORRECT_STATE); VerifyOrReturnError(aMinInterval <= mMaxInterval, CHIP_ERROR_INVALID_ARGUMENT); - mMinIntervalFloorSeconds = aMinInterval; + // Ensures the new min interval is higher than the subscriber established one. + mMinIntervalFloorSeconds = std::max(mMinIntervalFloorSeconds, aMinInterval); return CHIP_NO_ERROR; } diff --git a/src/app/reporting/ReportScheduler.h b/src/app/reporting/ReportScheduler.h index 36dbc3147cbba4..bbe10fb56e6155 100644 --- a/src/app/reporting/ReportScheduler.h +++ b/src/app/reporting/ReportScheduler.h @@ -35,14 +35,22 @@ using Timestamp = System::Clock::Timestamp; class ReportScheduler : public ReadHandler::Observer { public: + /// @brief This class acts as an interface between the report scheduler and the system timer to reduce dependencies on the + /// system layer. class TimerDelegate { public: virtual ~TimerDelegate() {} + /// @brief Start a timer for a given context, the report scheduler will always start by trying to cancel an existing timer + /// before starting a new one + /// @param context context to pass to the timer callback, in this case, a read handler node is passed + /// @param aTimeout time in miliseconds before the timer expires virtual CHIP_ERROR StartTimer(void * context, System::Clock::Timeout aTimeout) = 0; - virtual void CancelTimer(void * context) = 0; - virtual bool IsTimerActive(void * context) = 0; - virtual Timestamp GetCurrentMonotonicTimestamp() = 0; + /// @brief Cancel a timer for a given context + /// @param context used to identify the timer to cancel + virtual void CancelTimer(void * context) = 0; + virtual bool IsTimerActive(void * context) = 0; + virtual Timestamp GetCurrentMonotonicTimestamp() = 0; }; class ReadHandlerNode : public IntrusiveListNodeBase<> @@ -101,26 +109,6 @@ class ReportScheduler : public ReadHandler::Observer */ virtual ~ReportScheduler() = default; - /// @brief Register a ReadHandler to the scheduler, will schedule report - /// @param aReadHandler read handler to register - virtual CHIP_ERROR RegisterReadHandler(ReadHandler * aReadHandler) = 0; - /// @brief Schedule a report for a given ReadHandler - /// @param timeout time in seconds before the next engine run is scheduled - /// @param aReadHandler read handler to schedule a report for - /// @return CHIP_ERROR not found if the ReadHandler is not registered in the scheduler, specific timer error if the timer - /// couldn't be started - virtual CHIP_ERROR ScheduleReport(System::Clock::Timeout timeout, ReadHandler * aReadHandler) = 0; - /// @brief Cancel a scheduled report for a given ReadHandler - /// @param aReadHandler readhandler to look for in the ReadHandler nodes list. If found, the timer started for this report will - /// be cancelled. - virtual void CancelReport(ReadHandler * aReadHandler) = 0; - /// @brief Unregister a ReadHandler from the scheduler - /// @param aReadHandler read handler to unregister - /// @return CHIP_NO_ERROR if the ReadHandler was successfully unregistered or not found, specific error otherwise - virtual void UnregisterReadHandler(ReadHandler * aReadHandler) = 0; - /// @brief Unregister all ReadHandlers from the scheduler - /// @return CHIP_NO_ERROR if all ReadHandlers were successfully unregistered, specific error otherwise - virtual void UnregisterAllHandlers() = 0; /// @brief Check if a ReadHandler is scheduled for reporting virtual bool IsReportScheduled(ReadHandler * aReadHandler) = 0; /// @brief Check whether a ReadHandler is reportable right now, taking into account its minimum and maximum intervals. @@ -131,9 +119,6 @@ class ReportScheduler : public ReadHandler::Observer { return aReadHandler->IsGeneratingReports() && aReadHandler->IsDirty(); } - /// @brief Check the ReadHandler's ChunkedReport flag to prevent rescheduling if the Schedule is called when the engine is - /// processing a chunked report - bool IsChunkedReport(ReadHandler * aReadHandler) const { return aReadHandler->IsChunkedReport(); } /// @brief Get the number of ReadHandlers registered in the scheduler's node pool size_t GetNumReadHandlers() const { return mNodesPool.Allocated(); } @@ -144,13 +129,17 @@ class ReportScheduler : public ReadHandler::Observer /// @brief Find the ReadHandlerNode for a given ReadHandler pointer /// @param [in] aReadHandler ReadHandler pointer to look for in the ReadHandler nodes list /// @return Node Address if node was found, nullptr otherwise - virtual ReadHandlerNode * FindReadHandlerNode(const ReadHandler * aReadHandler) = 0; - /// @brief Start a timer for a given ReadHandlerNode, ensures that if a timer is already running for this node, it is cancelled - /// @param node Node of the ReadHandler list to start a timer for - /// @param aTimeout Delay before the timer expires - virtual CHIP_ERROR StartTimerForHandler(ReadHandlerNode * node, System::Clock::Timeout aTimeout) = 0; - virtual void CancelTimerForHandler(ReadHandlerNode * node) = 0; - virtual bool CheckTimerActiveForHandler(ReadHandlerNode * node) = 0; + ReadHandlerNode * FindReadHandlerNode(const ReadHandler * aReadHandler) + { + for (auto & iter : mReadHandlerList) + { + if (iter.GetReadHandler() == aReadHandler) + { + return &iter; + } + } + return nullptr; + } IntrusiveList mReadHandlerList; ObjectPool mNodesPool; diff --git a/src/app/reporting/ReportSchedulerImpl.cpp b/src/app/reporting/ReportSchedulerImpl.cpp index ec46a6c1c89a72..8b62ad49536791 100644 --- a/src/app/reporting/ReportSchedulerImpl.cpp +++ b/src/app/reporting/ReportSchedulerImpl.cpp @@ -62,10 +62,10 @@ void ReportSchedulerImpl::OnBecameReportable(ReadHandler * aReadHandler) else { // If the handler is not reportable now, schedule a report for the min interval - newTimeout = Milliseconds32(node->GetMinTimestamp().count() - mTimerDelegate->GetCurrentMonotonicTimestamp().count()); + newTimeout = node->GetMinTimestamp() - mTimerDelegate->GetCurrentMonotonicTimestamp(); } - ScheduleReport(newTimeout, aReadHandler); + ScheduleReport(newTimeout, node); } void ReportSchedulerImpl::OnSubscriptionAction(ReadHandler * apReadHandler) @@ -74,9 +74,8 @@ void ReportSchedulerImpl::OnSubscriptionAction(ReadHandler * apReadHandler) VerifyOrReturn(nullptr != node); // Schedule callback for max interval by computing the difference between the max timestamp and the current timestamp node->SetIntervalTimeStamps(apReadHandler); - Milliseconds32 newTimeout = - Milliseconds32(node->GetMaxTimestamp().count() - mTimerDelegate->GetCurrentMonotonicTimestamp().count()); - ScheduleReport(newTimeout, apReadHandler); + Milliseconds32 newTimeout = node->GetMaxTimestamp() - mTimerDelegate->GetCurrentMonotonicTimestamp(); + ScheduleReport(newTimeout, node); } /// @brief When a ReadHandler is removed, unregister it, which will cancel any scheduled report @@ -89,7 +88,9 @@ CHIP_ERROR ReportSchedulerImpl::RegisterReadHandler(ReadHandler * aReadHandler) { ReadHandlerNode * newNode = FindReadHandlerNode(aReadHandler); // If the handler is already registered, no need to register it again - VerifyOrReturnValue(nullptr == newNode, CHIP_NO_ERROR); + VerifyOrDie(nullptr == newNode); + // The NodePool is the same size as the ReadHandler pool from the IM Engine, so we don't need a check for size here since if a + // ReadHandler was created, space should be available. newNode = mNodesPool.CreateObject(aReadHandler, mTimerDelegate, ReportTimerCallback); mReadHandlerList.PushBack(newNode); @@ -110,33 +111,23 @@ CHIP_ERROR ReportSchedulerImpl::RegisterReadHandler(ReadHandler * aReadHandler) { // If the handler is reportable now, but the min interval is not elapsed, schedule a report for the moment the min interval // has elapsed - newTimeout = Milliseconds32(newNode->GetMinTimestamp().count() - now.count()); - - ReturnErrorOnFailure(ScheduleReport(newTimeout, aReadHandler)); + newTimeout = newNode->GetMinTimestamp() - now; } else { // If the handler is not reportable now, schedule a report for the max interval - newTimeout = Milliseconds32(newNode->GetMaxTimestamp().count() - now.count()); + newTimeout = newNode->GetMaxTimestamp() - now; } - ReturnErrorOnFailure(ScheduleReport(newTimeout, aReadHandler)); + ReturnErrorOnFailure(ScheduleReport(newTimeout, newNode)); return CHIP_NO_ERROR; } -CHIP_ERROR ReportSchedulerImpl::ScheduleReport(Timeout timeout, ReadHandler * aReadHandler) +CHIP_ERROR ReportSchedulerImpl::ScheduleReport(Timeout timeout, ReadHandlerNode * node) { - // Verify the handler is still registered - ReadHandlerNode * node = FindReadHandlerNode(aReadHandler); - VerifyOrReturnError(nullptr != node, CHIP_ERROR_NOT_FOUND); - // Cancel Report if it is currently scheduled - CancelTimerForHandler(node); - - if (!IsChunkedReport(aReadHandler)) - { - StartTimerForHandler(node, timeout); - } + CancelSchedulerTimer(node); + StartSchedulerTimer(node, timeout); return CHIP_NO_ERROR; } @@ -145,13 +136,11 @@ void ReportSchedulerImpl::CancelReport(ReadHandler * aReadHandler) { ReadHandlerNode * node = FindReadHandlerNode(aReadHandler); VerifyOrReturn(nullptr != node); - CancelTimerForHandler(node); + CancelSchedulerTimer(node); } void ReportSchedulerImpl::UnregisterReadHandler(ReadHandler * aReadHandler) { - // Verify list is populated and handler is not null - VerifyOrReturn((!mReadHandlerList.Empty() || (nullptr == aReadHandler))); CancelReport(aReadHandler); ReadHandlerNode * removeNode = FindReadHandlerNode(aReadHandler); @@ -175,33 +164,21 @@ bool ReportSchedulerImpl::IsReportScheduled(ReadHandler * aReadHandler) { ReadHandlerNode * node = FindReadHandlerNode(aReadHandler); VerifyOrReturnValue(nullptr != node, false); - return CheckTimerActiveForHandler(node); -} - -ReportSchedulerImpl::ReadHandlerNode * ReportSchedulerImpl::FindReadHandlerNode(const ReadHandler * aReadHandler) -{ - for (auto & iter : mReadHandlerList) - { - if (iter.GetReadHandler() == aReadHandler) - { - return &iter; - } - } - return nullptr; + return CheckSchedulerTimerActive(node); } -CHIP_ERROR ReportSchedulerImpl::StartTimerForHandler(ReadHandlerNode * node, System::Clock::Timeout aTimeout) +CHIP_ERROR ReportSchedulerImpl::StartSchedulerTimer(ReadHandlerNode * node, System::Clock::Timeout aTimeout) { // Schedule Report return mTimerDelegate->StartTimer(node, aTimeout); } -void ReportSchedulerImpl::CancelTimerForHandler(ReadHandlerNode * node) +void ReportSchedulerImpl::CancelSchedulerTimer(ReadHandlerNode * node) { mTimerDelegate->CancelTimer(node); } -bool ReportSchedulerImpl::CheckTimerActiveForHandler(ReadHandlerNode * node) +bool ReportSchedulerImpl::CheckSchedulerTimerActive(ReadHandlerNode * node) { return mTimerDelegate->IsTimerActive(node); } diff --git a/src/app/reporting/ReportSchedulerImpl.h b/src/app/reporting/ReportSchedulerImpl.h index 60deca82bd40ce..849f9b797b5f93 100644 --- a/src/app/reporting/ReportSchedulerImpl.h +++ b/src/app/reporting/ReportSchedulerImpl.h @@ -28,33 +28,34 @@ class ReportSchedulerImpl : public ReportScheduler { public: ReportSchedulerImpl(TimerDelegate * aTimerDelegate); - virtual ~ReportSchedulerImpl() override { UnregisterAllHandlers(); } + ~ReportSchedulerImpl() override { UnregisterAllHandlers(); } // ReadHandlerObserver - virtual void OnReadHandlerCreated(ReadHandler * aReadHandler) override; - virtual void OnBecameReportable(ReadHandler * aReadHandler) override; - virtual void OnSubscriptionAction(ReadHandler * aReadHandler) override; - virtual void OnReadHandlerDestroyed(ReadHandler * aReadHandler) override; - - // ReportScheduler specific - virtual CHIP_ERROR RegisterReadHandler(ReadHandler * aReadHandler) override; - virtual CHIP_ERROR ScheduleReport(System::Clock::Timeout timeout, ReadHandler * aReadHandler) override; - virtual void CancelReport(ReadHandler * aReadHandler) override; - virtual void UnregisterReadHandler(ReadHandler * aReadHandler) override; - virtual void UnregisterAllHandlers() override; - virtual bool IsReportScheduled(ReadHandler * aReadHandler) override; + void OnReadHandlerCreated(ReadHandler * aReadHandler) override; + void OnBecameReportable(ReadHandler * aReadHandler) override; + void OnSubscriptionAction(ReadHandler * aReadHandler) override; + void OnReadHandlerDestroyed(ReadHandler * aReadHandler) override; protected: + virtual CHIP_ERROR RegisterReadHandler(ReadHandler * aReadHandler); + virtual CHIP_ERROR ScheduleReport(System::Clock::Timeout timeout, ReadHandlerNode * node); + virtual void CancelReport(ReadHandler * aReadHandler); + virtual void UnregisterReadHandler(ReadHandler * aReadHandler); + virtual void UnregisterAllHandlers(); + +private: friend class chip::app::reporting::TestReportScheduler; - /// @brief Find the ReadHandlerNode for a given ReadHandler pointer - /// @param [in] aReadHandler - /// @return Node Address if node was found, nullptr otherwise - virtual ReadHandlerNode * FindReadHandlerNode(const ReadHandler * aReadHandler) override; + bool IsReportScheduled(ReadHandler * aReadHandler) override; - virtual CHIP_ERROR StartTimerForHandler(ReadHandlerNode * node, System::Clock::Timeout aTimeout) override; - virtual void CancelTimerForHandler(ReadHandlerNode * node) override; - virtual bool CheckTimerActiveForHandler(ReadHandlerNode * node) override; + /// @brief Start a timer for a given ReadHandlerNode, ensures that if a timer is already running for this node, it is cancelled + /// @param node Node of the ReadHandler list to start a timer for + /// @param aTimeout Delay before the timer expires + virtual CHIP_ERROR StartSchedulerTimer(ReadHandlerNode * node, System::Clock::Timeout aTimeout); + /// @brief Cancel the timer for a given ReadHandlerNode + virtual void CancelSchedulerTimer(ReadHandlerNode * node); + /// @brief Check if the timer for a given ReadHandlerNode is active + virtual bool CheckSchedulerTimerActive(ReadHandlerNode * node); }; } // namespace reporting diff --git a/src/app/tests/TestReportScheduler.cpp b/src/app/tests/TestReportScheduler.cpp index 1c43ada639022d..308c2cea83eaae 100644 --- a/src/app/tests/TestReportScheduler.cpp +++ b/src/app/tests/TestReportScheduler.cpp @@ -25,11 +25,6 @@ namespace { -uint8_t gDebugEventBuffer[128]; -uint8_t gInfoEventBuffer[128]; -uint8_t gCritEventBuffer[128]; -chip::app::CircularEventBuffer gCircularEventBuffer[3]; - class TestContext : public chip::Test::AppContext { public: @@ -45,15 +40,6 @@ class TestContext : public chip::Test::AppContext return FAILURE; } - chip::app::LogStorageResources logStorageResources[] = { - { &gDebugEventBuffer[0], sizeof(gDebugEventBuffer), chip::app::PriorityLevel::Debug }, - { &gInfoEventBuffer[0], sizeof(gInfoEventBuffer), chip::app::PriorityLevel::Info }, - { &gCritEventBuffer[0], sizeof(gCritEventBuffer), chip::app::PriorityLevel::Critical }, - }; - - chip::app::EventManagement::CreateEventManagement(&ctx->GetExchangeManager(), ArraySize(logStorageResources), - gCircularEventBuffer, logStorageResources, &ctx->mEventCounter); - return SUCCESS; } diff --git a/src/platform/telink/ICDUtil.cpp b/src/platform/telink/ICDUtil.cpp index b1c2c4797adb80..b3dc9c80bbc9dd 100644 --- a/src/platform/telink/ICDUtil.cpp +++ b/src/platform/telink/ICDUtil.cpp @@ -36,5 +36,5 @@ CHIP_ERROR ICDUtil::OnSubscriptionRequested(chip::app::ReadHandler & aReadHandle agreedMaxInterval = kSubscriptionMaxIntervalPublisherLimit; } - return aReadHandler.SetMaxReportingIntervals(agreedMaxInterval); + return aReadHandler.SetMaxReportingInterval(agreedMaxInterval); } From 97683adfb21383cce86b69400e06ace89b0b0420 Mon Sep 17 00:00:00 2001 From: lpbeliveau-silabs Date: Wed, 12 Jul 2023 17:24:12 -0400 Subject: [PATCH 17/21] Fixed comment about private methods used in ReportScheduler as a friend class --- src/app/ReadHandler.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/ReadHandler.h b/src/app/ReadHandler.h index 40b240180f7689..40b31bd0acb1e2 100644 --- a/src/app/ReadHandler.h +++ b/src/app/ReadHandler.h @@ -438,7 +438,7 @@ class ReadHandler : public Messaging::ExchangeDelegate friend class chip::app::reporting::Engine; friend class chip::app::InteractionModelEngine; - // The report scheduler needs to be able to access StateFlag private functions IsReadHandlerReportable and IsChunkedReport to + // The report scheduler needs to be able to access StateFlag private functions IsGeneratingReports() and IsDirty() to // know when to schedule a run so it is declared as a friend class. friend class chip::app::reporting::ReportScheduler; From e81641dfe22f7f3f20284ac85f0689c845e8aaac Mon Sep 17 00:00:00 2001 From: lpbeliveau-silabs Date: Wed, 12 Jul 2023 20:22:57 -0400 Subject: [PATCH 18/21] Changed to SetMinReportInterval to SetMinReportingIntervalForTests, removed the IsChunkedReport from comment about friend class, added a mock timestamp and timer to test to better control time in simulation for specific timing test cases --- src/app/ReadHandler.h | 127 +++++++++++++++++++----- src/app/tests/TestReportScheduler.cpp | 133 ++++++++++++++++++++------ 2 files changed, 206 insertions(+), 54 deletions(-) diff --git a/src/app/ReadHandler.h b/src/app/ReadHandler.h index 40b31bd0acb1e2..edfdc0b8c1f8cd 100644 --- a/src/app/ReadHandler.h +++ b/src/app/ReadHandler.h @@ -215,9 +215,18 @@ class ReadHandler : public Messaging::ExchangeDelegate ReadHandler(ManagementCallback & apCallback, Observer * observer = nullptr); #endif - const ObjectList * GetAttributePathList() const { return mpAttributePathList; } - const ObjectList * GetEventPathList() const { return mpEventPathList; } - const ObjectList * GetDataVersionFilterList() const { return mpDataVersionFilterList; } + const ObjectList * GetAttributePathList() const + { + return mpAttributePathList; + } + const ObjectList * GetEventPathList() const + { + return mpEventPathList; + } + const ObjectList * GetDataVersionFilterList() const + { + return mpDataVersionFilterList; + } void GetReportingIntervals(uint16_t & aMinInterval, uint16_t & aMaxInterval) const { @@ -225,7 +234,7 @@ class ReadHandler : public Messaging::ExchangeDelegate aMaxInterval = mMaxInterval; } - CHIP_ERROR SetMinReportingInterval(uint16_t aMinInterval) + CHIP_ERROR SetMinReportingIntervalForTests(uint16_t aMinInterval) { VerifyOrReturnError(IsIdle(), CHIP_ERROR_INCORRECT_STATE); VerifyOrReturnError(aMinInterval <= mMaxInterval, CHIP_ERROR_INVALID_ARGUMENT); @@ -263,8 +272,14 @@ class ReadHandler : public Messaging::ExchangeDelegate } private: - PriorityLevel GetCurrentPriority() const { return mCurrentPriority; } - EventNumber & GetEventMin() { return mEventMin; } + PriorityLevel GetCurrentPriority() const + { + return mCurrentPriority; + } + EventNumber & GetEventMin() + { + return mEventMin; + } enum class ReadHandlerFlags : uint8_t { @@ -345,7 +360,10 @@ class ReadHandler : public Messaging::ExchangeDelegate */ bool IsFromSubscriber(Messaging::ExchangeContext & apExchangeContext) const; - bool IsIdle() const { return mState == HandlerState::Idle; } + bool IsIdle() const + { + return mState == HandlerState::Idle; + } // TODO (#27672): Change back to IsReportable once ReportScheduler is implemented so this can assess reportability without // considering timing. The ReporScheduler will handle timing. @@ -359,8 +377,14 @@ class ReadHandler : public Messaging::ExchangeDelegate return mState == HandlerState::GeneratingReports && !mFlags.Has(ReadHandlerFlags::WaitingUntilMinInterval) && (IsDirty() || !mFlags.Has(ReadHandlerFlags::WaitingUntilMaxInterval)); } - bool IsGeneratingReports() const { return mState == HandlerState::GeneratingReports; } - bool IsAwaitingReportResponse() const { return mState == HandlerState::AwaitingReportResponse; } + bool IsGeneratingReports() const + { + return mState == HandlerState::GeneratingReports; + } + bool IsAwaitingReportResponse() const + { + return mState == HandlerState::AwaitingReportResponse; + } // Resets the path iterator to the beginning of the whole report for generating a series of new reports. void ResetPathIterator(); @@ -372,17 +396,41 @@ class ReadHandler : public Messaging::ExchangeDelegate // sanpshotted last event, check with latest last event number, re-setup snapshoted checkpoint, and compare again. bool CheckEventClean(EventManagement & aEventManager); - bool IsType(InteractionType type) const { return (mInteractionType == type); } - bool IsChunkedReport() const { return mFlags.Has(ReadHandlerFlags::ChunkedReport); } + bool IsType(InteractionType type) const + { + return (mInteractionType == type); + } + bool IsChunkedReport() const + { + return mFlags.Has(ReadHandlerFlags::ChunkedReport); + } // Is reporting indicates whether we are in the middle of a series chunks. As we will set mIsChunkedReport on the first chunk // and clear that flag on the last chunk, we can use mIsChunkedReport to indicate this state. - bool IsReporting() const { return mFlags.Has(ReadHandlerFlags::ChunkedReport); } - bool IsPriming() const { return mFlags.Has(ReadHandlerFlags::PrimingReports); } - bool IsActiveSubscription() const { return mFlags.Has(ReadHandlerFlags::ActiveSubscription); } - bool IsFabricFiltered() const { return mFlags.Has(ReadHandlerFlags::FabricFiltered); } + bool IsReporting() const + { + return mFlags.Has(ReadHandlerFlags::ChunkedReport); + } + bool IsPriming() const + { + return mFlags.Has(ReadHandlerFlags::PrimingReports); + } + bool IsActiveSubscription() const + { + return mFlags.Has(ReadHandlerFlags::ActiveSubscription); + } + bool IsFabricFiltered() const + { + return mFlags.Has(ReadHandlerFlags::FabricFiltered); + } CHIP_ERROR OnSubscribeRequest(Messaging::ExchangeContext * apExchangeContext, System::PacketBufferHandle && aPayload); - void GetSubscriptionId(SubscriptionId & aSubscriptionId) const { aSubscriptionId = mSubscriptionId; } - AttributePathExpandIterator * GetAttributePathExpandIterator() { return &mAttributePathExpandIterator; } + void GetSubscriptionId(SubscriptionId & aSubscriptionId) const + { + aSubscriptionId = mSubscriptionId; + } + AttributePathExpandIterator * GetAttributePathExpandIterator() + { + return &mAttributePathExpandIterator; + } /// @brief Notifies the read handler that a set of attribute paths has been marked dirty. This will schedule a reporting engine /// run if the change to the attribute path makes the ReadHandler reportable. @@ -392,7 +440,10 @@ class ReadHandler : public Messaging::ExchangeDelegate { return (mDirtyGeneration > mPreviousReportsBeginGeneration) || mFlags.Has(ReadHandlerFlags::ForceDirty); } - void ClearForceDirtyFlag() { ClearStateFlag(ReadHandlerFlags::ForceDirty); } + void ClearForceDirtyFlag() + { + ClearStateFlag(ReadHandlerFlags::ForceDirty); + } NodeId GetInitiatorNodeId() const { auto session = GetSession(); @@ -406,23 +457,47 @@ class ReadHandler : public Messaging::ExchangeDelegate } Transport::SecureSession * GetSession() const; - SubjectDescriptor GetSubjectDescriptor() const { return GetSession()->GetSubjectDescriptor(); } + SubjectDescriptor GetSubjectDescriptor() const + { + return GetSession()->GetSubjectDescriptor(); + } - auto GetTransactionStartGeneration() const { return mTransactionStartGeneration; } + auto GetTransactionStartGeneration() const + { + return mTransactionStartGeneration; + } /// @brief Forces the read handler into a dirty state, regardless of what's going on with attributes. /// This can lead to scheduling of a reporting run immediately, if the min interval has been reached, /// or after the min interval is reached if it has not yet been reached. void ForceDirtyState(); - const AttributeValueEncoder::AttributeEncodeState & GetAttributeEncodeState() const { return mAttributeEncoderState; } - void SetAttributeEncodeState(const AttributeValueEncoder::AttributeEncodeState & aState) { mAttributeEncoderState = aState; } - uint32_t GetLastWrittenEventsBytes() const { return mLastWrittenEventsBytes; } + const AttributeValueEncoder::AttributeEncodeState & GetAttributeEncodeState() const + { + return mAttributeEncoderState; + } + void SetAttributeEncodeState(const AttributeValueEncoder::AttributeEncodeState & aState) + { + mAttributeEncoderState = aState; + } + uint32_t GetLastWrittenEventsBytes() const + { + return mLastWrittenEventsBytes; + } // Returns the number of interested paths, including wildcard and concrete paths. - size_t GetAttributePathCount() const { return mpAttributePathList == nullptr ? 0 : mpAttributePathList->Count(); }; - size_t GetEventPathCount() const { return mpEventPathList == nullptr ? 0 : mpEventPathList->Count(); }; - size_t GetDataVersionFilterCount() const { return mpDataVersionFilterList == nullptr ? 0 : mpDataVersionFilterList->Count(); }; + size_t GetAttributePathCount() const + { + return mpAttributePathList == nullptr ? 0 : mpAttributePathList->Count(); + }; + size_t GetEventPathCount() const + { + return mpEventPathList == nullptr ? 0 : mpEventPathList->Count(); + }; + size_t GetDataVersionFilterCount() const + { + return mpDataVersionFilterList == nullptr ? 0 : mpDataVersionFilterList->Count(); + }; CHIP_ERROR SendStatusReport(Protocols::InteractionModel::Status aStatus); diff --git a/src/app/tests/TestReportScheduler.cpp b/src/app/tests/TestReportScheduler.cpp index 308c2cea83eaae..e110d020a1160a 100644 --- a/src/app/tests/TestReportScheduler.cpp +++ b/src/app/tests/TestReportScheduler.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include namespace { @@ -74,10 +75,64 @@ using InteractionModelEngine = InteractionModelEngine; using ReportScheduler = reporting::ReportScheduler; using ReportSchedulerImpl = reporting::ReportSchedulerImpl; using ReadHandlerNode = reporting::ReportScheduler::ReadHandlerNode; +using Milliseconds64 = System::Clock::Milliseconds64; + +static const size_t kNumMaxReadHandlers = 16; class TestTimerDelegate : public ReportScheduler::TimerDelegate { public: + struct NodeTimeoutPair + { + ReadHandlerNode * node; + System::Clock::Timeout timeout; + }; + + NodeTimeoutPair mPairArray[kNumMaxReadHandlers]; + size_t mPairArraySize = 0; + System::Clock::Timestamp mMockSystemTimestamp = System::Clock::Milliseconds64(0); + + NodeTimeoutPair * FindPair(ReadHandlerNode * node, size_t & position) + { + for (size_t i = 0; i < mPairArraySize; i++) + { + if (mPairArray[i].node == node) + { + position = i; + return &mPairArray[i]; + } + } + return nullptr; + } + + CHIP_ERROR insertPair(ReadHandlerNode * node, System::Clock::Timeout timeout) + { + VerifyOrReturnError(mPairArraySize < kNumMaxReadHandlers, CHIP_ERROR_NO_MEMORY); + mPairArray[mPairArraySize].node = node; + mPairArray[mPairArraySize].timeout = timeout; + mPairArraySize++; + + return CHIP_NO_ERROR; + } + + void removePair(ReadHandlerNode * node) + { + size_t position; + NodeTimeoutPair * pair = FindPair(node, position); + VerifyOrReturn(pair != nullptr); + + size_t nextPos = static_cast(position + 1); + size_t moveNum = static_cast(mPairArraySize - nextPos); + + // Compress array after removal, if the removed position is not the last + if (moveNum) + { + memmove(&mPairArray[position], &mPairArray[nextPos], sizeof(NodeTimeoutPair) * moveNum); + } + + mPairArraySize--; + } + static void TimerCallbackInterface(System::Layer * aLayer, void * aAppState) { // Normaly we would call the callback here, thus scheduling an engine run, but we don't need it for this test as we simulate @@ -85,31 +140,40 @@ class TestTimerDelegate : public ReportScheduler::TimerDelegate // // ReadHandlerNode * node = static_cast(aAppState); // node->RunCallback(); + ChipLogProgress(DataManagement, "Simluating engine run for Handler: %p", aAppState); } virtual CHIP_ERROR StartTimer(void * context, System::Clock::Timeout aTimeout) override { - return InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->StartTimer( - aTimeout, TimerCallbackInterface, context); - } - virtual void CancelTimer(void * context) override - { - InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->CancelTimer( - TimerCallbackInterface, context); + return insertPair(static_cast(context), aTimeout + mMockSystemTimestamp); } + virtual void CancelTimer(void * context) override { removePair(static_cast(context)); } virtual bool IsTimerActive(void * context) override { - return InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->IsTimerActive( - TimerCallbackInterface, context); + size_t position; + NodeTimeoutPair * pair = FindPair(static_cast(context), position); + VerifyOrReturnValue(pair != nullptr, false); + + return pair->timeout > mMockSystemTimestamp; } - virtual System::Clock::Timestamp GetCurrentMonotonicTimestamp() override + virtual System::Clock::Timestamp GetCurrentMonotonicTimestamp() override { return mMockSystemTimestamp; } + + void SetMockSystemTimestamp(System::Clock::Timestamp aMockTimestamp) { mMockSystemTimestamp = aMockTimestamp; } + + // Increment the mock timestamp one milisecond at a time for a total of aTime miliseconds. Checks if + void IncrementMockTimestamp(System::Clock::Milliseconds64 aTime) { - return System::SystemClock().GetMonotonicTimestamp(); + mMockSystemTimestamp = mMockSystemTimestamp + aTime; + for (size_t i = 0; i < mPairArraySize; i++) + { + if (mPairArray[i].timeout <= mMockSystemTimestamp) + { + TimerCallbackInterface(nullptr, mPairArray[i].node); + } + } } }; -static const size_t kNumMaxReadHandlers = 16; - TestTimerDelegate sTestTimerDelegate; ReportSchedulerImpl sScheduler(&sTestTimerDelegate); @@ -126,6 +190,9 @@ class TestReportScheduler // Read handler pool ObjectPool readHandlerPool; + // Initialize mock timestamp + sTestTimerDelegate.SetMockSystemTimestamp(Milliseconds64(0)); + for (size_t i = 0; i < kNumMaxReadHandlers; i++) { ReadHandler * readHandler = @@ -188,11 +255,14 @@ class TestReportScheduler // Read handler pool ObjectPool readHandlerPool; + // Initialize mock timestamp + sTestTimerDelegate.SetMockSystemTimestamp(Milliseconds64(0)); + // Dirty read handler, will be triggered at min interval ReadHandler * readHandler1 = readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe); NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler1->SetMaxReportingInterval(2)); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler1->SetMinReportingInterval(1)); + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler1->SetMinReportingIntervalForTests(1)); readHandler1->ForceDirtyState(); readHandler1->MoveToState(ReadHandler::HandlerState::GeneratingReports); @@ -200,14 +270,14 @@ class TestReportScheduler ReadHandler * readHandler2 = readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe); NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler2->SetMaxReportingInterval(3)); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler2->SetMinReportingInterval(0)); + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler2->SetMinReportingIntervalForTests(0)); readHandler2->MoveToState(ReadHandler::HandlerState::GeneratingReports); // Clean read handler, will be triggered at max interval, but will be cancelled before ReadHandler * readHandler3 = readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe); NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler3->SetMaxReportingInterval(3)); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler3->SetMinReportingInterval(0)); + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler3->SetMinReportingIntervalForTests(0)); readHandler3->MoveToState(ReadHandler::HandlerState::GeneratingReports); NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == sScheduler.RegisterReadHandler(readHandler1)); @@ -219,8 +289,9 @@ class TestReportScheduler NL_TEST_ASSERT(aSuite, !sScheduler.IsReportableNow(readHandler2)); NL_TEST_ASSERT(aSuite, !sScheduler.IsReportableNow(readHandler3)); - ctx.GetIOContext().DriveIOUntil(System::Clock::Milliseconds32(1100), - [&]() -> bool { return sScheduler.IsReportableNow(readHandler1); }); + // Simulate system clock increment + sTestTimerDelegate.IncrementMockTimestamp(Milliseconds64(1100)); + // Checks that the first ReadHandler is reportable after 1 second since it is dirty and min interval has expired NL_TEST_ASSERT(aSuite, sScheduler.IsReportableNow(readHandler1)); NL_TEST_ASSERT(aSuite, !sScheduler.IsReportableNow(readHandler2)); @@ -230,9 +301,8 @@ class TestReportScheduler sScheduler.CancelReport(readHandler3); NL_TEST_ASSERT(aSuite, !sScheduler.IsReportScheduled(readHandler3)); - // Wait another 3 seconds to let the second ReadHandler become reportable - ctx.GetIOContext().DriveIOUntil(System::Clock::Milliseconds32(3100), - [&]() -> bool { return sScheduler.IsReportableNow(readHandler2); }); + // Simulate system clock increment + sTestTimerDelegate.IncrementMockTimestamp(Milliseconds64(2000)); // Checks that all ReadHandlers are reportable NL_TEST_ASSERT(aSuite, sScheduler.IsReportableNow(readHandler1)); @@ -257,10 +327,13 @@ class TestReportScheduler // Read handler pool ObjectPool readHandlerPool; + // Initialize mock timestamp + sTestTimerDelegate.SetMockSystemTimestamp(Milliseconds64(0)); + ReadHandler * readHandler = readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe); NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler->SetMaxReportingInterval(2)); - NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler->SetMinReportingInterval(1)); + NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler->SetMinReportingIntervalForTests(1)); readHandler->MoveToState(ReadHandler::HandlerState::GeneratingReports); readHandler->SetObserver(&sScheduler); @@ -279,8 +352,9 @@ class TestReportScheduler readHandler->mObserver->OnBecameReportable(readHandler); // Should have changed the scheduled timeout to the handler's min interval, to check, we wait for the min interval to // expire - ctx.GetIOContext().DriveIOUntil(System::Clock::Milliseconds32(1100), - [&]() -> bool { return sScheduler.IsReportableNow(readHandler); }); + // Simulate system clock increment + sTestTimerDelegate.IncrementMockTimestamp(Milliseconds64(1100)); + // Check that no report is scheduled since the min interval has expired, the timer should now be stopped NL_TEST_ASSERT(aSuite, !sScheduler.IsReportScheduled(readHandler)); @@ -289,13 +363,16 @@ class TestReportScheduler readHandler->mObserver->OnSubscriptionAction(readHandler); // Should have changed the scheduled timeout to the handlers max interval, to check, we wait for the min interval to // confirm it is not expired yet so the report should still be scheduled + NL_TEST_ASSERT(aSuite, sScheduler.IsReportScheduled(readHandler)); - ctx.GetIOContext().DriveIOUntil(System::Clock::Milliseconds32(1100), - [&]() -> bool { return sScheduler.IsReportableNow(readHandler); }); + // Simulate system clock increment + sTestTimerDelegate.IncrementMockTimestamp(Milliseconds64(1100)); + // Check that the report is still scheduled as the max interval has not expired yet and the dirty flag was cleared NL_TEST_ASSERT(aSuite, sScheduler.IsReportScheduled(readHandler)); - ctx.GetIOContext().DriveIOUntil(System::Clock::Milliseconds32(2100), - [&]() -> bool { return sScheduler.IsReportableNow(readHandler); }); + // Simulate system clock increment + sTestTimerDelegate.IncrementMockTimestamp(Milliseconds64(2100)); + // Check that no report is scheduled since the max interval should have expired, the timer should now be stopped NL_TEST_ASSERT(aSuite, !sScheduler.IsReportScheduled(readHandler)); From 2a55ac59060f2bf201af689d4b3c3c853e2786e2 Mon Sep 17 00:00:00 2001 From: lpbeliveau-silabs <112982107+lpbeliveau-silabs@users.noreply.github.com> Date: Wed, 12 Jul 2023 20:24:09 -0400 Subject: [PATCH 19/21] Apply suggestions from code review Co-authored-by: Boris Zbarsky --- src/app/ReadHandler.h | 2 +- src/app/reporting/ReportScheduler.h | 8 ++++---- src/app/reporting/ReportSchedulerImpl.cpp | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/app/ReadHandler.h b/src/app/ReadHandler.h index edfdc0b8c1f8cd..3a23de49c70f90 100644 --- a/src/app/ReadHandler.h +++ b/src/app/ReadHandler.h @@ -244,7 +244,7 @@ class ReadHandler : public Messaging::ExchangeDelegate } /* - * Set the maximum reporting intervals for the subscription. This SHALL only be called + * Set the maximum reporting interval for the subscription. This SHALL only be called * from the OnSubscriptionRequested callback above. The restriction is as below * MinIntervalFloor ≤ MaxInterval ≤ MAX(SUBSCRIPTION_MAX_INTERVAL_PUBLISHER_LIMIT, MaxIntervalCeiling) * Where SUBSCRIPTION_MAX_INTERVAL_PUBLISHER_LIMIT is set to 60m in the spec. diff --git a/src/app/reporting/ReportScheduler.h b/src/app/reporting/ReportScheduler.h index bbe10fb56e6155..0a5902d7ae7639 100644 --- a/src/app/reporting/ReportScheduler.h +++ b/src/app/reporting/ReportScheduler.h @@ -41,9 +41,9 @@ class ReportScheduler : public ReadHandler::Observer { public: virtual ~TimerDelegate() {} - /// @brief Start a timer for a given context, the report scheduler will always start by trying to cancel an existing timer - /// before starting a new one - /// @param context context to pass to the timer callback, in this case, a read handler node is passed + /// @brief Start a timer for a given context. The report scheduler must always cancel an existing timer for a context (using CancelTimer) + /// before starting a new one for that context. + /// @param context context to pass to the timer callback. /// @param aTimeout time in miliseconds before the timer expires virtual CHIP_ERROR StartTimer(void * context, System::Clock::Timeout aTimeout) = 0; /// @brief Cancel a timer for a given context @@ -70,7 +70,7 @@ class ReportScheduler : public ReadHandler::Observer } ReadHandler * GetReadHandler() const { return mReadHandler; } /// @brief Check if the Node is reportable now, meaning its readhandler was made reportable by attribute dirtying and - /// handler state, and minimal time interval last since last report has elapsed, or the maximal time interval since last + /// handler state, and minimal time interval since last report has elapsed, or the maximal time interval since last /// report has elapsed bool IsReportableNow() const { diff --git a/src/app/reporting/ReportSchedulerImpl.cpp b/src/app/reporting/ReportSchedulerImpl.cpp index 8b62ad49536791..4b45ab9d1b6deb 100644 --- a/src/app/reporting/ReportSchedulerImpl.cpp +++ b/src/app/reporting/ReportSchedulerImpl.cpp @@ -87,7 +87,7 @@ void ReportSchedulerImpl::OnReadHandlerDestroyed(ReadHandler * aReadHandler) CHIP_ERROR ReportSchedulerImpl::RegisterReadHandler(ReadHandler * aReadHandler) { ReadHandlerNode * newNode = FindReadHandlerNode(aReadHandler); - // If the handler is already registered, no need to register it again + // Handler must not be registered yet; it's just being constructed. VerifyOrDie(nullptr == newNode); // The NodePool is the same size as the ReadHandler pool from the IM Engine, so we don't need a check for size here since if a // ReadHandler was created, space should be available. @@ -102,7 +102,7 @@ CHIP_ERROR ReportSchedulerImpl::RegisterReadHandler(ReadHandler * aReadHandler) Timestamp now = mTimerDelegate->GetCurrentMonotonicTimestamp(); Milliseconds32 newTimeout; // If the handler is reportable, schedule a report for the min interval, otherwise schedule a report for the max interval - if ((newNode->IsReportableNow())) + if (newNode->IsReportableNow()) { // If the handler is reportable now, just schedule a report immediately newTimeout = Milliseconds32(0); From a0b9d6ed588e9c97915147fb45f897e43df9c506 Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Thu, 13 Jul 2023 00:24:26 +0000 Subject: [PATCH 20/21] Restyled by clang-format --- src/app/ReadHandler.h | 125 ++++++---------------------- src/app/reporting/ReportScheduler.h | 4 +- 2 files changed, 27 insertions(+), 102 deletions(-) diff --git a/src/app/ReadHandler.h b/src/app/ReadHandler.h index 3a23de49c70f90..77c88c12cd1ff9 100644 --- a/src/app/ReadHandler.h +++ b/src/app/ReadHandler.h @@ -215,18 +215,9 @@ class ReadHandler : public Messaging::ExchangeDelegate ReadHandler(ManagementCallback & apCallback, Observer * observer = nullptr); #endif - const ObjectList * GetAttributePathList() const - { - return mpAttributePathList; - } - const ObjectList * GetEventPathList() const - { - return mpEventPathList; - } - const ObjectList * GetDataVersionFilterList() const - { - return mpDataVersionFilterList; - } + const ObjectList * GetAttributePathList() const { return mpAttributePathList; } + const ObjectList * GetEventPathList() const { return mpEventPathList; } + const ObjectList * GetDataVersionFilterList() const { return mpDataVersionFilterList; } void GetReportingIntervals(uint16_t & aMinInterval, uint16_t & aMaxInterval) const { @@ -272,14 +263,8 @@ class ReadHandler : public Messaging::ExchangeDelegate } private: - PriorityLevel GetCurrentPriority() const - { - return mCurrentPriority; - } - EventNumber & GetEventMin() - { - return mEventMin; - } + PriorityLevel GetCurrentPriority() const { return mCurrentPriority; } + EventNumber & GetEventMin() { return mEventMin; } enum class ReadHandlerFlags : uint8_t { @@ -360,10 +345,7 @@ class ReadHandler : public Messaging::ExchangeDelegate */ bool IsFromSubscriber(Messaging::ExchangeContext & apExchangeContext) const; - bool IsIdle() const - { - return mState == HandlerState::Idle; - } + bool IsIdle() const { return mState == HandlerState::Idle; } // TODO (#27672): Change back to IsReportable once ReportScheduler is implemented so this can assess reportability without // considering timing. The ReporScheduler will handle timing. @@ -377,14 +359,8 @@ class ReadHandler : public Messaging::ExchangeDelegate return mState == HandlerState::GeneratingReports && !mFlags.Has(ReadHandlerFlags::WaitingUntilMinInterval) && (IsDirty() || !mFlags.Has(ReadHandlerFlags::WaitingUntilMaxInterval)); } - bool IsGeneratingReports() const - { - return mState == HandlerState::GeneratingReports; - } - bool IsAwaitingReportResponse() const - { - return mState == HandlerState::AwaitingReportResponse; - } + bool IsGeneratingReports() const { return mState == HandlerState::GeneratingReports; } + bool IsAwaitingReportResponse() const { return mState == HandlerState::AwaitingReportResponse; } // Resets the path iterator to the beginning of the whole report for generating a series of new reports. void ResetPathIterator(); @@ -396,41 +372,17 @@ class ReadHandler : public Messaging::ExchangeDelegate // sanpshotted last event, check with latest last event number, re-setup snapshoted checkpoint, and compare again. bool CheckEventClean(EventManagement & aEventManager); - bool IsType(InteractionType type) const - { - return (mInteractionType == type); - } - bool IsChunkedReport() const - { - return mFlags.Has(ReadHandlerFlags::ChunkedReport); - } + bool IsType(InteractionType type) const { return (mInteractionType == type); } + bool IsChunkedReport() const { return mFlags.Has(ReadHandlerFlags::ChunkedReport); } // Is reporting indicates whether we are in the middle of a series chunks. As we will set mIsChunkedReport on the first chunk // and clear that flag on the last chunk, we can use mIsChunkedReport to indicate this state. - bool IsReporting() const - { - return mFlags.Has(ReadHandlerFlags::ChunkedReport); - } - bool IsPriming() const - { - return mFlags.Has(ReadHandlerFlags::PrimingReports); - } - bool IsActiveSubscription() const - { - return mFlags.Has(ReadHandlerFlags::ActiveSubscription); - } - bool IsFabricFiltered() const - { - return mFlags.Has(ReadHandlerFlags::FabricFiltered); - } + bool IsReporting() const { return mFlags.Has(ReadHandlerFlags::ChunkedReport); } + bool IsPriming() const { return mFlags.Has(ReadHandlerFlags::PrimingReports); } + bool IsActiveSubscription() const { return mFlags.Has(ReadHandlerFlags::ActiveSubscription); } + bool IsFabricFiltered() const { return mFlags.Has(ReadHandlerFlags::FabricFiltered); } CHIP_ERROR OnSubscribeRequest(Messaging::ExchangeContext * apExchangeContext, System::PacketBufferHandle && aPayload); - void GetSubscriptionId(SubscriptionId & aSubscriptionId) const - { - aSubscriptionId = mSubscriptionId; - } - AttributePathExpandIterator * GetAttributePathExpandIterator() - { - return &mAttributePathExpandIterator; - } + void GetSubscriptionId(SubscriptionId & aSubscriptionId) const { aSubscriptionId = mSubscriptionId; } + AttributePathExpandIterator * GetAttributePathExpandIterator() { return &mAttributePathExpandIterator; } /// @brief Notifies the read handler that a set of attribute paths has been marked dirty. This will schedule a reporting engine /// run if the change to the attribute path makes the ReadHandler reportable. @@ -440,10 +392,7 @@ class ReadHandler : public Messaging::ExchangeDelegate { return (mDirtyGeneration > mPreviousReportsBeginGeneration) || mFlags.Has(ReadHandlerFlags::ForceDirty); } - void ClearForceDirtyFlag() - { - ClearStateFlag(ReadHandlerFlags::ForceDirty); - } + void ClearForceDirtyFlag() { ClearStateFlag(ReadHandlerFlags::ForceDirty); } NodeId GetInitiatorNodeId() const { auto session = GetSession(); @@ -457,47 +406,23 @@ class ReadHandler : public Messaging::ExchangeDelegate } Transport::SecureSession * GetSession() const; - SubjectDescriptor GetSubjectDescriptor() const - { - return GetSession()->GetSubjectDescriptor(); - } + SubjectDescriptor GetSubjectDescriptor() const { return GetSession()->GetSubjectDescriptor(); } - auto GetTransactionStartGeneration() const - { - return mTransactionStartGeneration; - } + auto GetTransactionStartGeneration() const { return mTransactionStartGeneration; } /// @brief Forces the read handler into a dirty state, regardless of what's going on with attributes. /// This can lead to scheduling of a reporting run immediately, if the min interval has been reached, /// or after the min interval is reached if it has not yet been reached. void ForceDirtyState(); - const AttributeValueEncoder::AttributeEncodeState & GetAttributeEncodeState() const - { - return mAttributeEncoderState; - } - void SetAttributeEncodeState(const AttributeValueEncoder::AttributeEncodeState & aState) - { - mAttributeEncoderState = aState; - } - uint32_t GetLastWrittenEventsBytes() const - { - return mLastWrittenEventsBytes; - } + const AttributeValueEncoder::AttributeEncodeState & GetAttributeEncodeState() const { return mAttributeEncoderState; } + void SetAttributeEncodeState(const AttributeValueEncoder::AttributeEncodeState & aState) { mAttributeEncoderState = aState; } + uint32_t GetLastWrittenEventsBytes() const { return mLastWrittenEventsBytes; } // Returns the number of interested paths, including wildcard and concrete paths. - size_t GetAttributePathCount() const - { - return mpAttributePathList == nullptr ? 0 : mpAttributePathList->Count(); - }; - size_t GetEventPathCount() const - { - return mpEventPathList == nullptr ? 0 : mpEventPathList->Count(); - }; - size_t GetDataVersionFilterCount() const - { - return mpDataVersionFilterList == nullptr ? 0 : mpDataVersionFilterList->Count(); - }; + size_t GetAttributePathCount() const { return mpAttributePathList == nullptr ? 0 : mpAttributePathList->Count(); }; + size_t GetEventPathCount() const { return mpEventPathList == nullptr ? 0 : mpEventPathList->Count(); }; + size_t GetDataVersionFilterCount() const { return mpDataVersionFilterList == nullptr ? 0 : mpDataVersionFilterList->Count(); }; CHIP_ERROR SendStatusReport(Protocols::InteractionModel::Status aStatus); diff --git a/src/app/reporting/ReportScheduler.h b/src/app/reporting/ReportScheduler.h index 0a5902d7ae7639..80d391c171c7e5 100644 --- a/src/app/reporting/ReportScheduler.h +++ b/src/app/reporting/ReportScheduler.h @@ -41,8 +41,8 @@ class ReportScheduler : public ReadHandler::Observer { public: virtual ~TimerDelegate() {} - /// @brief Start a timer for a given context. The report scheduler must always cancel an existing timer for a context (using CancelTimer) - /// before starting a new one for that context. + /// @brief Start a timer for a given context. The report scheduler must always cancel an existing timer for a context (using + /// CancelTimer) before starting a new one for that context. /// @param context context to pass to the timer callback. /// @param aTimeout time in miliseconds before the timer expires virtual CHIP_ERROR StartTimer(void * context, System::Clock::Timeout aTimeout) = 0; From 4df646cc5cbbba13614c679b4f367cdeef62f78b Mon Sep 17 00:00:00 2001 From: lpbeliveau-silabs Date: Thu, 13 Jul 2023 09:52:45 -0400 Subject: [PATCH 21/21] Removed all calls to ReadHandler States to prevent Engine calls from the Test as it seems to impact the CI --- src/app/tests/TestReportScheduler.cpp | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/app/tests/TestReportScheduler.cpp b/src/app/tests/TestReportScheduler.cpp index e110d020a1160a..a24422010c47c9 100644 --- a/src/app/tests/TestReportScheduler.cpp +++ b/src/app/tests/TestReportScheduler.cpp @@ -263,22 +263,25 @@ class TestReportScheduler readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe); NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler1->SetMaxReportingInterval(2)); NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler1->SetMinReportingIntervalForTests(1)); - readHandler1->ForceDirtyState(); - readHandler1->MoveToState(ReadHandler::HandlerState::GeneratingReports); + // Do those manually to avoid scheduling an engine run + readHandler1->mFlags.Set(ReadHandler::ReadHandlerFlags::ForceDirty, true); + readHandler1->mState = ReadHandler::HandlerState::GeneratingReports; // Clean read handler, will be triggered at max interval ReadHandler * readHandler2 = readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe); NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler2->SetMaxReportingInterval(3)); NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler2->SetMinReportingIntervalForTests(0)); - readHandler2->MoveToState(ReadHandler::HandlerState::GeneratingReports); + // Do those manually to avoid scheduling an engine run + readHandler2->mState = ReadHandler::HandlerState::GeneratingReports; // Clean read handler, will be triggered at max interval, but will be cancelled before ReadHandler * readHandler3 = readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe); NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler3->SetMaxReportingInterval(3)); NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler3->SetMinReportingIntervalForTests(0)); - readHandler3->MoveToState(ReadHandler::HandlerState::GeneratingReports); + // Do those manually to avoid scheduling an engine run + readHandler3->mState = ReadHandler::HandlerState::GeneratingReports; NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == sScheduler.RegisterReadHandler(readHandler1)); NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == sScheduler.RegisterReadHandler(readHandler2)); @@ -334,7 +337,8 @@ class TestReportScheduler readHandlerPool.CreateObject(nullCallback, exchangeCtx, ReadHandler::InteractionType::Subscribe); NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler->SetMaxReportingInterval(2)); NL_TEST_ASSERT(aSuite, CHIP_NO_ERROR == readHandler->SetMinReportingIntervalForTests(1)); - readHandler->MoveToState(ReadHandler::HandlerState::GeneratingReports); + // Do those manually to avoid scheduling an engine run + readHandler->mState = ReadHandler::HandlerState::GeneratingReports; readHandler->SetObserver(&sScheduler); // Test OnReadHandlerCreated @@ -348,7 +352,7 @@ class TestReportScheduler NL_TEST_ASSERT(aSuite, node->GetReadHandler() == readHandler); // Test OnBecameReportable - readHandler->ForceDirtyState(); + readHandler->mFlags.Set(ReadHandler::ReadHandlerFlags::ForceDirty, true); readHandler->mObserver->OnBecameReportable(readHandler); // Should have changed the scheduled timeout to the handler's min interval, to check, we wait for the min interval to // expire @@ -359,7 +363,7 @@ class TestReportScheduler NL_TEST_ASSERT(aSuite, !sScheduler.IsReportScheduled(readHandler)); // Test OnSubscriptionAction - readHandler->ClearForceDirtyFlag(); + readHandler->mFlags.Set(ReadHandler::ReadHandlerFlags::ForceDirty, false); readHandler->mObserver->OnSubscriptionAction(readHandler); // Should have changed the scheduled timeout to the handlers max interval, to check, we wait for the min interval to // confirm it is not expired yet so the report should still be scheduled