Completed renaming to eliminate compiling error, moved TestReporScehd…

…uler in reporting namespace, addressed some low hanging fruits

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());

src/app/ReadHandler.h

@@ -168,21 +168,22 @@ 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
         /// allowed.
         /// @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<ReadHandlerFlags> 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

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;

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);
 }

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

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(),
 };
 

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;
     }