Skip to content

Commit

Permalink
Removed useless objects from tests as well as useless typecasting, an…
Browse files Browse the repository at this point in the history
…d unnecessary check
  • Loading branch information
lpbeliveau-silabs committed Jul 12, 2023
1 parent 64f5041 commit f015837
Show file tree
Hide file tree
Showing 8 changed files with 66 additions and 112 deletions.
2 changes: 1 addition & 1 deletion examples/platform/nrfconnect/util/ICDUtil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,5 @@ CHIP_ERROR ICDUtil::OnSubscriptionRequested(chip::app::ReadHandler & aReadHandle
agreedMaxInterval = kSubscriptionMaxIntervalPublisherLimit;
}

return aReadHandler.SetMaxReportingIntervals(agreedMaxInterval);
return aReadHandler.SetMaxReportingInterval(agreedMaxInterval);
}
2 changes: 1 addition & 1 deletion examples/platform/silabs/ICDSubscriptionCallback.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,5 +61,5 @@ CHIP_ERROR ICDSubscriptionCallback::OnSubscriptionRequested(chip::app::ReadHandl
decidedMaxInterval = maximumMaxInterval;
}

return aReadHandler.SetMaxReportingIntervals(decidedMaxInterval);
return aReadHandler.SetMaxReportingInterval(decidedMaxInterval);
}
3 changes: 2 additions & 1 deletion src/app/ReadHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
55 changes: 22 additions & 33 deletions src/app/reporting/ReportScheduler.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<>
Expand Down Expand Up @@ -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.
Expand All @@ -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(); }
Expand All @@ -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<ReadHandlerNode> mReadHandlerList;
ObjectPool<ReadHandlerNode, CHIP_IM_MAX_NUM_READS + CHIP_IM_MAX_NUM_SUBSCRIPTIONS> mNodesPool;
Expand Down
59 changes: 18 additions & 41 deletions src/app/reporting/ReportSchedulerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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);

Expand All @@ -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;
}
Expand All @@ -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);
Expand All @@ -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);
}
Expand Down
41 changes: 21 additions & 20 deletions src/app/reporting/ReportSchedulerImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 0 additions & 14 deletions src/app/tests/TestReportScheduler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion src/platform/telink/ICDUtil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,5 @@ CHIP_ERROR ICDUtil::OnSubscriptionRequested(chip::app::ReadHandler & aReadHandle
agreedMaxInterval = kSubscriptionMaxIntervalPublisherLimit;
}

return aReadHandler.SetMaxReportingIntervals(agreedMaxInterval);
return aReadHandler.SetMaxReportingInterval(agreedMaxInterval);
}

0 comments on commit f015837

Please sign in to comment.