Skip to content

Commit c9ce7f2

Browse files
lpbeliveau-silabsrestyled-commitsmkardous-silabsbzbarsky-apple
authored
[ReadHandler] Integration of ReportScheduler into the ReadHandler and IM engine. (#28104)
* Integration of ReportSchedulers into the ReadHandler and IM engine. Also added a static report scheduler to use in all test relying on the IM engine, also added test flags in ReportScheduler replace ReadHandler timing flags Refactored the TestReadInteraction to adopt correct behavior * Restyled by clang-format * Restyled by gn * Moved namespaces into class to avoid namespace pollution * Fix missing pragma once and copyright * Update src/app/reporting/ReportScheduler.h Co-authored-by: mkardous-silabs <84793247+mkardous-silabs@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Removed OnIntervalsChanged, removed TestReadInteraction from ReportScheduler's friend class and refactored the tests to use wrapper functions, refactored tests for the removal of OnIntervalsChanged, removed reportscheduler nullptr check form controllers * Restyled by clang-format * Fix for the TimerDelegates And also added a default Scheduler to controllers * Restyled by clang-format * Added a timer Context class to allow interface between timer delegates, ReadHandlerNodes and SynchronisedReport Scheduler * Fix using InteractionModel... error * Added Platform::Delete() for allocated TimerDelegate and ReportScheduler on controller * Update src/app/ReadHandler.cpp Co-authored-by: mkardous-silabs <84793247+mkardous-silabs@users.noreply.github.com> * Update src/app/TimerDelegates.h Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Addressed PR comments, next step trying to reduce RAM bloat * Restyled by clang-format * Removed IntrusiveList from the report Scheduler * Removed ReadHandlerNode's inheritance of IntrusiveListBase * Restyled by clang-format * Fix commented code and comment syntax * Restyled by clang-format * Update src/app/tests/TestReportScheduler.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Added extra check on timerDelegate in DeviceController, removed logging causing failure in linux CI * Restyled by clang-format --------- Co-authored-by: Restyled.io <commits@restyled.io> Co-authored-by: mkardous-silabs <84793247+mkardous-silabs@users.noreply.github.com> Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
1 parent fe4c679 commit c9ce7f2

30 files changed

+711
-504
lines changed

‎src/app/InteractionModelEngine.cpp

+5-3
Original file line numberDiff line numberDiff line change
@@ -51,16 +51,18 @@ InteractionModelEngine * InteractionModelEngine::GetInstance()
5151
}
5252

5353
CHIP_ERROR InteractionModelEngine::Init(Messaging::ExchangeManager * apExchangeMgr, FabricTable * apFabricTable,
54-
CASESessionManager * apCASESessionMgr,
54+
reporting::ReportScheduler * reportScheduler, CASESessionManager * apCASESessionMgr,
5555
SubscriptionResumptionStorage * subscriptionResumptionStorage)
5656
{
5757
VerifyOrReturnError(apFabricTable != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
5858
VerifyOrReturnError(apExchangeMgr != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
59+
VerifyOrReturnError(reportScheduler != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
5960

6061
mpExchangeMgr = apExchangeMgr;
6162
mpFabricTable = apFabricTable;
6263
mpCASESessionMgr = apCASESessionMgr;
6364
mpSubscriptionResumptionStorage = subscriptionResumptionStorage;
65+
mReportScheduler = reportScheduler;
6466

6567
ReturnErrorOnFailure(mpFabricTable->AddFabricDelegate(this));
6668
ReturnErrorOnFailure(mpExchangeMgr->RegisterUnsolicitedMessageHandlerForProtocol(Protocols::InteractionModel::Id, this));
@@ -741,7 +743,7 @@ Protocols::InteractionModel::Status InteractionModelEngine::OnReadInitialRequest
741743

742744
// We have already reserved enough resources for read requests, and have granted enough resources for current subscriptions, so
743745
// we should be able to allocate resources requested by this request.
744-
ReadHandler * handler = mReadHandlers.CreateObject(*this, apExchangeContext, aInteractionType);
746+
ReadHandler * handler = mReadHandlers.CreateObject(*this, apExchangeContext, aInteractionType, mReportScheduler);
745747
if (handler == nullptr)
746748
{
747749
ChipLogProgress(InteractionModel, "no resource for %s interaction",
@@ -1845,7 +1847,7 @@ void InteractionModelEngine::ResumeSubscriptionsTimerCallback(System::Layer * ap
18451847
return;
18461848
}
18471849

1848-
ReadHandler * handler = imEngine->mReadHandlers.CreateObject(*imEngine);
1850+
ReadHandler * handler = imEngine->mReadHandlers.CreateObject(*imEngine, imEngine->GetReportScheduler());
18491851
if (handler == nullptr)
18501852
{
18511853
ChipLogProgress(InteractionModel, "no resource for ReadHandler creation");

‎src/app/InteractionModelEngine.h

+5-1
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@
5757
#include <app/WriteClient.h>
5858
#include <app/WriteHandler.h>
5959
#include <app/reporting/Engine.h>
60+
#include <app/reporting/ReportScheduler.h>
6061
#include <app/util/attribute-metadata.h>
6162
#include <app/util/basic-types.h>
6263

@@ -115,7 +116,7 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
115116
*
116117
*/
117118
CHIP_ERROR Init(Messaging::ExchangeManager * apExchangeMgr, FabricTable * apFabricTable,
118-
CASESessionManager * apCASESessionMgr = nullptr,
119+
reporting::ReportScheduler * reportScheduler, CASESessionManager * apCASESessionMgr = nullptr,
119120
SubscriptionResumptionStorage * subscriptionResumptionStorage = nullptr);
120121

121122
void Shutdown();
@@ -178,6 +179,8 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
178179

179180
reporting::Engine & GetReportingEngine() { return mReportingEngine; }
180181

182+
reporting::ReportScheduler * GetReportScheduler() { return mReportScheduler; }
183+
181184
void ReleaseAttributePathList(ObjectList<AttributePathParams> *& aAttributePathList);
182185

183186
CHIP_ERROR PushFrontAttributePathList(ObjectList<AttributePathParams> *& aAttributePathList,
@@ -566,6 +569,7 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
566569
ObjectPool<TimedHandler, CHIP_IM_MAX_NUM_TIMED_HANDLER> mTimedHandlers;
567570
WriteHandler mWriteHandlers[CHIP_IM_MAX_NUM_WRITE_HANDLER];
568571
reporting::Engine mReportingEngine;
572+
reporting::ReportScheduler * mReportScheduler = nullptr;
569573

570574
static constexpr size_t kReservedHandlersForReads = kMinSupportedReadRequestsPerFabric * (CHIP_CONFIG_MAX_FABRICS);
571575
static constexpr size_t kReservedPathsForReads = kMinSupportedPathsPerReadRequest * kReservedHandlersForReads;

‎src/app/ReadHandler.cpp

+24-135
Original file line numberDiff line numberDiff line change
@@ -77,16 +77,9 @@ ReadHandler::ReadHandler(ManagementCallback & apCallback, Messaging::ExchangeCon
7777

7878
mSessionHandle.Grab(mExchangeCtx->GetSessionHandle());
7979

80-
// TODO (#27672): Uncomment when the ReportScheduler is implemented
81-
#if 0
82-
if (nullptr != observer)
83-
{
84-
if (CHIP_NO_ERROR == SetObserver(observer))
85-
{
86-
mObserver->OnReadHandlerCreated(this);
87-
}
88-
}
89-
#endif
80+
VerifyOrDie(observer != nullptr);
81+
mObserver = observer;
82+
mObserver->OnReadHandlerCreated(this);
9083
}
9184

9285
#if CHIP_CONFIG_PERSIST_SUBSCRIPTIONS
@@ -97,16 +90,9 @@ ReadHandler::ReadHandler(ManagementCallback & apCallback, Observer * observer) :
9790
mInteractionType = InteractionType::Subscribe;
9891
mFlags.ClearAll();
9992

100-
// TODO (#27672): Uncomment when the ReportScheduler is implemented
101-
#if 0
102-
if (nullptr != observer)
103-
{
104-
if (CHIP_NO_ERROR == SetObserver(observer))
105-
{
106-
mObserver->OnReadHandlerCreated(this);
107-
}
108-
}
109-
#endif
93+
VerifyOrDie(observer != nullptr);
94+
mObserver = observer;
95+
mObserver->OnReadHandlerCreated(this);
11096
}
11197

11298
void ReadHandler::ResumeSubscription(CASESessionManager & caseSessionManager,
@@ -150,28 +136,14 @@ void ReadHandler::ResumeSubscription(CASESessionManager & caseSessionManager,
150136

151137
ReadHandler::~ReadHandler()
152138
{
153-
// TODO (#27672): Enable when the ReportScheduler is implemented and move in Close() after testing
154-
#if 0
155-
if (nullptr != mObserver)
156-
{
157-
mObserver->OnReadHandlerDestroyed(this);
158-
}
159-
#endif
139+
mObserver->OnReadHandlerDestroyed(this);
140+
160141
auto * appCallback = mManagementCallback.GetAppCallback();
161142
if (mFlags.Has(ReadHandlerFlags::ActiveSubscription) && appCallback)
162143
{
163144
appCallback->OnSubscriptionTerminated(*this);
164145
}
165146

166-
if (IsType(InteractionType::Subscribe))
167-
{
168-
InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->CancelTimer(
169-
MinIntervalExpiredCallback, this);
170-
171-
InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->CancelTimer(
172-
MaxIntervalExpiredCallback, this);
173-
}
174-
175147
if (IsAwaitingReportResponse())
176148
{
177149
InteractionModelEngine::GetInstance()->GetReportingEngine().OnReportConfirm();
@@ -290,7 +262,7 @@ CHIP_ERROR ReadHandler::OnStatusResponse(Messaging::ExchangeContext * apExchange
290262

291263
CHIP_ERROR ReadHandler::SendStatusReport(Protocols::InteractionModel::Status aStatus)
292264
{
293-
VerifyOrReturnLogError(IsReportableNow(), CHIP_ERROR_INCORRECT_STATE);
265+
VerifyOrReturnLogError(mState == HandlerState::GeneratingReports, CHIP_ERROR_INCORRECT_STATE);
294266
if (IsPriming() || IsChunkedReport())
295267
{
296268
mSessionHandle.Grab(mExchangeCtx->GetSessionHandle());
@@ -314,7 +286,7 @@ CHIP_ERROR ReadHandler::SendStatusReport(Protocols::InteractionModel::Status aSt
314286

315287
CHIP_ERROR ReadHandler::SendReportData(System::PacketBufferHandle && aPayload, bool aMoreChunks)
316288
{
317-
VerifyOrReturnLogError(IsReportableNow(), CHIP_ERROR_INCORRECT_STATE);
289+
VerifyOrReturnLogError(mState == HandlerState::GeneratingReports, CHIP_ERROR_INCORRECT_STATE);
318290
VerifyOrDie(!IsAwaitingReportResponse()); // Should not be reportable!
319291
if (IsPriming() || IsChunkedReport())
320292
{
@@ -359,21 +331,11 @@ CHIP_ERROR ReadHandler::SendReportData(System::PacketBufferHandle && aPayload, b
359331
InteractionModelEngine::GetInstance()->GetReportingEngine().OnReportConfirm();
360332
}
361333

362-
if (IsType(InteractionType::Subscribe) && !IsPriming())
334+
// If we just finished a non-priming subscription report, notify our observers.
335+
// Priming reports are handled when we send a SubscribeResponse.
336+
if (IsType(InteractionType::Subscribe) && !IsPriming() && !IsChunkedReport())
363337
{
364-
// TODO (#27672): Enable when the ReportScheduler is implemented and remove call to UpdateReportTimer, will be handled by
365-
// the report Scheduler
366-
#if 0
367-
if (nullptr != mObserver)
368-
{
369-
mObserver->OnSubscriptionAction(this);
370-
}
371-
#endif
372-
373-
// Ignore the error from UpdateReportTimer. If we've
374-
// successfully sent the message, we need to return success from
375-
// this method.
376-
UpdateReportTimer();
338+
mObserver->OnSubscriptionAction(this);
377339
}
378340
}
379341
if (!aMoreChunks)
@@ -641,16 +603,10 @@ void ReadHandler::MoveToState(const HandlerState aTargetState)
641603
// If we just unblocked sending reports, let's go ahead and schedule the reporting
642604
// engine to run to kick that off.
643605
//
644-
if (aTargetState == HandlerState::GeneratingReports && IsReportableNow())
606+
if (aTargetState == HandlerState::GeneratingReports)
645607
{
646-
// TODO (#27672): Enable when the ReportScheduler is implemented and remove the call to ScheduleRun()
647-
#if 0
648-
if(nullptr != mObserver)
649-
{
650-
mObserver->OnBecameReportable(this);
651-
}
652-
#endif
653-
InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun();
608+
// mObserver will take care of scheduling the report as soon as allowed
609+
mObserver->OnBecameReportable(this);
654610
}
655611
}
656612

@@ -691,15 +647,7 @@ CHIP_ERROR ReadHandler::SendSubscribeResponse()
691647
ReturnErrorOnFailure(writer.Finalize(&packet));
692648
VerifyOrReturnLogError(mExchangeCtx, CHIP_ERROR_INCORRECT_STATE);
693649

694-
// TODO (#27672): Uncomment when the ReportScheduler is implemented and remove call to UpdateReportTimer, handled by
695-
// the report Scheduler
696-
#if 0
697-
if (nullptr != mObserver)
698-
{
699-
mObserver->OnSubscriptionAction(this);
700-
}
701-
#endif
702-
ReturnErrorOnFailure(UpdateReportTimer());
650+
mObserver->OnSubscriptionAction(this);
703651

704652
ClearStateFlag(ReadHandlerFlags::PrimingReports);
705653
return mExchangeCtx->SendMessage(Protocols::InteractionModel::MsgType::SubscribeResponse, std::move(packet));
@@ -818,50 +766,6 @@ void ReadHandler::PersistSubscription()
818766
}
819767
}
820768

821-
// TODO (#27672): Remove when ReportScheduler is enabled as timing will now be handled by the ReportScheduler
822-
void ReadHandler::MinIntervalExpiredCallback(System::Layer * apSystemLayer, void * apAppState)
823-
{
824-
VerifyOrReturn(apAppState != nullptr);
825-
ReadHandler * readHandler = static_cast<ReadHandler *>(apAppState);
826-
ChipLogDetail(DataManagement, "Unblock report hold after min %d seconds", readHandler->mMinIntervalFloorSeconds);
827-
readHandler->ClearStateFlag(ReadHandlerFlags::WaitingUntilMinInterval);
828-
InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->StartTimer(
829-
System::Clock::Seconds16(readHandler->mMaxInterval - readHandler->mMinIntervalFloorSeconds), MaxIntervalExpiredCallback,
830-
readHandler);
831-
}
832-
833-
// TODO (#27672): Remove when ReportScheduler is enabled as timing will now be handled by the ReportScheduler
834-
void ReadHandler::MaxIntervalExpiredCallback(System::Layer * apSystemLayer, void * apAppState)
835-
{
836-
VerifyOrReturn(apAppState != nullptr);
837-
ReadHandler * readHandler = static_cast<ReadHandler *>(apAppState);
838-
readHandler->ClearStateFlag(ReadHandlerFlags::WaitingUntilMaxInterval);
839-
ChipLogProgress(DataManagement, "Refresh subscribe timer sync after %d seconds",
840-
readHandler->mMaxInterval - readHandler->mMinIntervalFloorSeconds);
841-
}
842-
843-
// TODO (#27672): Remove when ReportScheduler is enabled as timing will now be handled by the ReportScheduler
844-
CHIP_ERROR ReadHandler::UpdateReportTimer()
845-
{
846-
InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->CancelTimer(
847-
MinIntervalExpiredCallback, this);
848-
InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->CancelTimer(
849-
MaxIntervalExpiredCallback, this);
850-
851-
if (!IsChunkedReport())
852-
{
853-
ChipLogProgress(DataManagement, "Refresh Subscribe Sync Timer with min %d seconds and max %d seconds",
854-
mMinIntervalFloorSeconds, mMaxInterval);
855-
SetStateFlag(ReadHandlerFlags::WaitingUntilMinInterval);
856-
SetStateFlag(ReadHandlerFlags::WaitingUntilMaxInterval);
857-
ReturnErrorOnFailure(
858-
InteractionModelEngine::GetInstance()->GetExchangeManager()->GetSessionManager()->SystemLayer()->StartTimer(
859-
System::Clock::Seconds16(mMinIntervalFloorSeconds), MinIntervalExpiredCallback, this));
860-
}
861-
862-
return CHIP_NO_ERROR;
863-
}
864-
865769
void ReadHandler::ResetPathIterator()
866770
{
867771
mAttributePathExpandIterator = AttributePathExpandIterator(mpAttributePathList);
@@ -897,17 +801,8 @@ void ReadHandler::AttributePathIsDirty(const AttributePathParams & aAttributeCha
897801
mAttributeEncoderState = AttributeValueEncoder::AttributeEncodeState();
898802
}
899803

900-
if (IsReportableNow())
901-
{
902-
// TODO (#27672): Enable when the ReportScheduler is implemented and remove the call to ScheduleRun()
903-
#if 0
904-
if(nullptr != mObserver)
905-
{
906-
mObserver->OnBecameReportable(this);
907-
}
908-
#endif
909-
InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun();
910-
}
804+
// ReportScheduler will take care of verifying the reportability of the handler and schedule the run
805+
mObserver->OnBecameReportable(this);
911806
}
912807

913808
Transport::SecureSession * ReadHandler::GetSession() const
@@ -926,20 +821,14 @@ void ReadHandler::ForceDirtyState()
926821

927822
void ReadHandler::SetStateFlag(ReadHandlerFlags aFlag, bool aValue)
928823
{
929-
bool oldReportable = IsReportableNow();
824+
bool oldReportable = IsReportable();
930825
mFlags.Set(aFlag, aValue);
931826

932827
// If we became reportable, schedule a reporting run.
933-
if (!oldReportable && IsReportableNow())
828+
if (!oldReportable && IsReportable())
934829
{
935-
// TODO (#27672): Enable when the ReportScheduler is implemented and remove the call to ScheduleRun()
936-
#if 0
937-
if(nullptr != mObserver)
938-
{
939-
mObserver->OnBecameReportable(this);
940-
}
941-
#endif
942-
InteractionModelEngine::GetInstance()->GetReportingEngine().ScheduleRun();
830+
// If we became reportable, the scheduler will schedule a run as soon as allowed
831+
mObserver->OnBecameReportable(this);
943832
}
944833
}
945834

0 commit comments

Comments
 (0)