Fixed bug where chunked reports would fail on Synchronized report sch…

…eduler and added tests for Syncrhonized report scheduler interaction

src/app/ReadClient.h

@@ -500,10 +500,22 @@ class ReadClient : public Messaging::ExchangeDelegate
      *  Check if current read client is being used
      *
      */
-    bool IsIdle() const { return mState == ClientState::Idle; }
-    bool IsSubscriptionActive() const { return mState == ClientState::SubscriptionActive; }
-    bool IsAwaitingInitialReport() const { return mState == ClientState::AwaitingInitialReport; }
-    bool IsAwaitingSubscribeResponse() const { return mState == ClientState::AwaitingSubscribeResponse; }
+    bool IsIdle() const
+    {
+        return mState == ClientState::Idle;
+    }
+    bool IsSubscriptionActive() const
+    {
+        return mState == ClientState::SubscriptionActive;
+    }
+    bool IsAwaitingInitialReport() const
+    {
+        return mState == ClientState::AwaitingInitialReport;
+    }
+    bool IsAwaitingSubscribeResponse() const
+    {
+        return mState == ClientState::AwaitingSubscribeResponse;
+    }
 
     CHIP_ERROR GenerateEventPaths(EventPathIBs::Builder & aEventPathsBuilder, const Span<EventPathParams> & aEventPaths);
     CHIP_ERROR GenerateAttributePaths(AttributePathIBs::Builder & aAttributePathIBsBuilder,

src/app/ReadHandler.h

@@ -218,9 +218,18 @@ class ReadHandler : public Messaging::ExchangeDelegate
     ReadHandler(ManagementCallback & apCallback, Observer * observer);
 #endif
 
-    const ObjectList<AttributePathParams> * GetAttributePathList() const { return mpAttributePathList; }
-    const ObjectList<EventPathParams> * GetEventPathList() const { return mpEventPathList; }
-    const ObjectList<DataVersionFilter> * GetDataVersionFilterList() const { return mpDataVersionFilterList; }
+    const ObjectList<AttributePathParams> * GetAttributePathList() const
+    {
+        return mpAttributePathList;
+    }
+    const ObjectList<EventPathParams> * GetEventPathList() const
+    {
+        return mpEventPathList;
+    }
+    const ObjectList<DataVersionFilter> * GetDataVersionFilterList() const
+    {
+        return mpDataVersionFilterList;
+    }
 
     void GetReportingIntervals(uint16_t & aMinInterval, uint16_t & aMaxInterval) const
     {
@@ -254,8 +263,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;
+    }
 
     /**
      * Returns SUBSCRIPTION_MAX_INTERVAL_PUBLISHER_LIMIT
@@ -333,7 +348,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;
+    }
 
     /// @brief Returns whether the ReadHandler is in a state where it can send a report and there is data to report.
     bool ShouldStartReporting() const
@@ -345,14 +363,20 @@ class ReadHandler : public Messaging::ExchangeDelegate
     }
     /// @brief CanStartReporting() is true if the ReadHandler is in a state where it could generate
     /// a (possibly empty) report if someone asked it to.
-    bool CanStartReporting() const { return mState == HandlerState::CanStartReporting; }
+    bool CanStartReporting() const
+    {
+        return mState == HandlerState::CanStartReporting;
+    }
     /// @brief ShouldReportUnscheduled() is true if the ReadHandler should be asked to generate reports
     /// without consulting the report scheduler.
     bool ShouldReportUnscheduled() const
     {
         return CanStartReporting() && (IsType(ReadHandler::InteractionType::Read) || IsPriming());
     }
-    bool IsAwaitingReportResponse() const { return mState == HandlerState::AwaitingReportResponse; }
+    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();
@@ -364,17 +388,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.
@@ -384,7 +432,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();
@@ -398,23 +449,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);
 

src/app/reporting/Engine.h

@@ -67,9 +67,15 @@ class Engine
     void Shutdown();
 
 #if CONFIG_BUILD_FOR_HOST_UNIT_TEST
-    void SetWriterReserved(uint32_t aReservedSize) { mReservedSize = aReservedSize; }
+    void SetWriterReserved(uint32_t aReservedSize)
+    {
+        mReservedSize = aReservedSize;
+    }
 
-    void SetMaxAttributesPerChunk(uint32_t aMaxAttributesPerChunk) { mMaxAttributesPerChunk = aMaxAttributesPerChunk; }
+    void SetMaxAttributesPerChunk(uint32_t aMaxAttributesPerChunk)
+    {
+        mMaxAttributesPerChunk = aMaxAttributesPerChunk;
+    }
 #endif
 
     /**
@@ -118,9 +124,15 @@ class Engine
         }
     }
 
-    uint32_t GetNumReportsInFlight() const { return mNumReportsInFlight; }
+    uint32_t GetNumReportsInFlight() const
+    {
+        return mNumReportsInFlight;
+    }
 
-    uint64_t GetDirtySetGeneration() const { return mDirtyGeneration; }
+    uint64_t GetDirtySetGeneration() const
+    {
+        return mDirtyGeneration;
+    }
 
     /**
      * Schedule event delivery to happen immediately and run reporting to get
@@ -131,7 +143,10 @@ class Engine
     void ScheduleUrgentEventDeliverySync(Optional<FabricIndex> fabricIndex = NullOptional);
 
 #if CONFIG_BUILD_FOR_HOST_UNIT_TEST
-    size_t GetGlobalDirtySetSize() { return mGlobalDirtySet.Allocated(); }
+    size_t GetGlobalDirtySetSize()
+    {
+        return mGlobalDirtySet.Allocated();
+    }
 #endif
 
 private:
@@ -143,7 +158,10 @@ class Engine
     friend class TestReportingEngine;
     friend class ::chip::app::TestReadInteraction;
 
-    bool IsRunScheduled() const { return mRunScheduled; }
+    bool IsRunScheduled() const
+    {
+        return mRunScheduled;
+    }
 
     struct AttributePathParamsWithGeneration : public AttributePathParams
     {
@@ -224,7 +242,10 @@ class Engine
 
     CHIP_ERROR InsertPathIntoDirtySet(const AttributePathParams & aAttributePath);
 
-    inline void BumpDirtySetGeneration() { mDirtyGeneration++; }
+    inline void BumpDirtySetGeneration()
+    {
+        mDirtyGeneration++;
+    }
 
     /**
      * Boolean to indicate if ScheduleRun is pending. This flag is used to prevent calling ScheduleRun multiple times

src/app/reporting/ReportScheduler.h

@@ -96,6 +96,7 @@ class ReportScheduler : public ReadHandler::Observer, public ICDStateObserver
                      IsEngineRunScheduled()));
         }
 
+        bool IsChunkedReport() const { return mReadHandler->IsChunkedReport(); }
         bool IsEngineRunScheduled() const { return mFlags.Has(ReadHandlerNodeFlags::EngineRunScheduled); }
         void SetEngineRunScheduled(bool aEngineRunScheduled)
         {

src/app/reporting/ReportSchedulerImpl.h

@@ -44,7 +44,7 @@ class ReportSchedulerImpl : public ReportScheduler
     void OnSubscriptionReportSent(ReadHandler * aReadHandler) final;
     void OnReadHandlerDestroyed(ReadHandler * aReadHandler) override;
 
-    bool IsReportScheduled(ReadHandler * aReadHandler);
+    virtual bool IsReportScheduled(ReadHandler * aReadHandler);
 
     void ReportTimerCallback() override;
 

src/app/reporting/SynchronizedReportSchedulerImpl.cpp

@@ -79,7 +79,7 @@ void SynchronizedReportSchedulerImpl::CancelReport()
 }
 
 /// @brief Checks if the timer is active for the ReportScheduler
-bool SynchronizedReportSchedulerImpl::IsReportScheduled()
+bool SynchronizedReportSchedulerImpl::IsReportScheduled(ReadHandler * ReadHandler)
 {
     return mTimerDelegate->IsTimerActive(this);
 }
@@ -140,7 +140,7 @@ CHIP_ERROR SynchronizedReportSchedulerImpl::CalculateNextReportTimeout(Timeout &
     bool reportableAtMin = false;
 
     mNodesPool.ForEachActiveObject([&reportableNow, &reportableAtMin, this, now](ReadHandlerNode * node) {
-        if (!node->IsEngineRunScheduled())
+        if (!node->IsEngineRunScheduled() || node->IsChunkedReport())
         {
             if (node->IsReportableNow(now))
             {
@@ -185,6 +185,7 @@ void SynchronizedReportSchedulerImpl::TimerFired()
     InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun();
 
     mNodesPool.ForEachActiveObject([now](ReadHandlerNode * node) {
+        node->SetEngineRunScheduled(true);
         if (node->GetMinTimestamp() <= now)
         {
             node->SetCanBeSynced(true);

src/app/reporting/SynchronizedReportSchedulerImpl.h

@@ -40,7 +40,7 @@ class SynchronizedReportSchedulerImpl : public ReportSchedulerImpl, public Timer
 
     void OnTransitionToIdle() override;
 
-    bool IsReportScheduled();
+    bool IsReportScheduled(ReadHandler * ReadHandler) override;
 
     void TimerFired() override;
 

src/app/reporting/tests/MockReportScheduler.cpp

@@ -29,7 +29,7 @@ namespace reporting {
 
 static chip::app::DefaultTimerDelegate sTimerDelegate;
 static ReportSchedulerImpl sTestDefaultReportScheduler(&sTimerDelegate);
-static SynchronizedReportSchedulerImpl sTestReportScheduler(&sTimerDelegate);
+static SynchronizedReportSchedulerImpl sTestSyncReportScheduler(&sTimerDelegate);
 
 ReportSchedulerImpl * GetDefaultReportScheduler()
 {
@@ -38,7 +38,7 @@ ReportSchedulerImpl * GetDefaultReportScheduler()
 
 SynchronizedReportSchedulerImpl * GetSynchronizedReportScheduler()
 {
-    return &sTestReportScheduler;
+    return &sTestSyncReportScheduler;
 }
 
 } // namespace reporting