From e059202613e91da78884aec4e1041d76da0af9bf Mon Sep 17 00:00:00 2001 From: lpbeliveau-silabs <112982107+lpbeliveau-silabs@users.noreply.github.com> Date: Mon, 31 Jul 2023 20:15:06 -0400 Subject: [PATCH] [ICD] Subscription report emission when becoming active (#28266) * Added mechanism to emit report when switching to active mode if necessary * Restyled by clang-format * Reworked the logic to force a report emission when min elapsed and inject the scheduler in the ICD manager from Server::Init() * Update src/app/reporting/ReportScheduler.h Co-authored-by: Boris Zbarsky * Created StateObserver as an interface class to fix dependencies issues in app/BUILD.gn * Restyled by whitespace * Update src/app/icd/ICDManager.cpp Co-authored-by: Boris Zbarsky * Moved the report on active mode ifdef to ReportSchedulerImpl and removed misleading comment * Restyled by clang-format * Put the now timestamp in the #if check to avoid unused variable error --------- Co-authored-by: Restyled.io Co-authored-by: Boris Zbarsky --- src/app/BUILD.gn | 2 +- src/app/ReadHandler.h | 4 +-- src/app/icd/BUILD.gn | 9 ++++++- src/app/icd/ICDManager.cpp | 16 +++++++++--- src/app/icd/ICDManager.h | 4 ++- src/app/icd/ICDStateObserver.h | 30 +++++++++++++++++++++++ src/app/reporting/ReportScheduler.h | 5 +++- src/app/reporting/ReportSchedulerImpl.cpp | 17 +++++++++++++ src/app/reporting/ReportSchedulerImpl.h | 3 +++ src/app/server/Server.cpp | 2 +- 10 files changed, 82 insertions(+), 10 deletions(-) create mode 100644 src/app/icd/ICDStateObserver.h diff --git a/src/app/BUILD.gn b/src/app/BUILD.gn index c8d20b58aff129..312a53aa6ee1a7 100644 --- a/src/app/BUILD.gn +++ b/src/app/BUILD.gn @@ -213,7 +213,7 @@ static_library("app") { public_deps = [ ":app_config", "${chip_root}/src/access", - "${chip_root}/src/app/icd:cluster-srcs", + "${chip_root}/src/app/icd:observer-srcs", "${chip_root}/src/lib/address_resolve", "${chip_root}/src/lib/support", "${chip_root}/src/messaging", diff --git a/src/app/ReadHandler.h b/src/app/ReadHandler.h index 5f1d3221886084..a87c121ffaf8b0 100644 --- a/src/app/ReadHandler.h +++ b/src/app/ReadHandler.h @@ -418,8 +418,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 IsReportable(), IsGeneratingReports() and - // IsDirty() 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 IsReportable(), IsGeneratingReports(), + // ForceDirtyState() and IsDirty() 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 diff --git a/src/app/icd/BUILD.gn b/src/app/icd/BUILD.gn index f4d5b9a7375dac..18aea3f155d845 100644 --- a/src/app/icd/BUILD.gn +++ b/src/app/icd/BUILD.gn @@ -17,6 +17,10 @@ import("icd.gni") # ICD Server sources and configurations +source_set("observer-srcs") { + sources = [ "ICDStateObserver.h" ] +} + # ICD Manager source-set is broken out of the main source-set to enable unit tests # All sources and configurations used by the ICDManager need to go in this source-set source_set("manager-srcs") { @@ -25,7 +29,10 @@ source_set("manager-srcs") { "ICDManager.h", ] - deps = [ ":cluster-srcs" ] + deps = [ + ":cluster-srcs", + ":observer-srcs", + ] public_deps = [ "${chip_root}/src/credentials:credentials" ] } diff --git a/src/app/icd/ICDManager.cpp b/src/app/icd/ICDManager.cpp index 853fd044d36732..e3787f2ddc747a 100644 --- a/src/app/icd/ICDManager.cpp +++ b/src/app/icd/ICDManager.cpp @@ -33,6 +33,11 @@ #define ICD_ENFORCE_SIT_SLOW_POLL_LIMIT 0 #endif +#ifndef ICD_REPORT_ON_ENTER_ACTIVE_MODE +// Enabling this makes the device emit subscription reports when transitioning from idle to active mode. +#define ICD_REPORT_ON_ENTER_ACTIVE_MODE 0 +#endif + namespace chip { namespace app { @@ -40,12 +45,15 @@ using namespace chip::app; using namespace chip::app::Clusters; using namespace chip::app::Clusters::IcdManagement; -void ICDManager::Init(PersistentStorageDelegate * storage, FabricTable * fabricTable) +void ICDManager::Init(PersistentStorageDelegate * storage, FabricTable * fabricTable, ICDStateObserver * stateObserver) { VerifyOrDie(storage != nullptr); VerifyOrDie(fabricTable != nullptr); - mStorage = storage; - mFabricTable = fabricTable; + VerifyOrDie(stateObserver != nullptr); + + mStorage = storage; + mFabricTable = fabricTable; + mStateObserver = stateObserver; uint32_t activeModeInterval = IcdManagementServer::GetInstance().GetActiveModeInterval(); VerifyOrDie(kFastPollingInterval.count() < activeModeInterval); @@ -152,6 +160,8 @@ void ICDManager::UpdateOperationState(OperationalState state) { ChipLogError(AppServer, "Failed to set Polling Interval: err %" CHIP_ERROR_FORMAT, err.Format()); } + + mStateObserver->OnEnterActiveMode(); } else { diff --git a/src/app/icd/ICDManager.h b/src/app/icd/ICDManager.h index 94127d645e9c18..10a1704a670cfa 100644 --- a/src/app/icd/ICDManager.h +++ b/src/app/icd/ICDManager.h @@ -16,6 +16,7 @@ */ #pragma once +#include #include #include #include @@ -51,7 +52,7 @@ class ICDManager }; ICDManager() {} - void Init(PersistentStorageDelegate * storage, FabricTable * fabricTable); + void Init(PersistentStorageDelegate * storage, FabricTable * fabricTable, ICDStateObserver * stateObserver); void Shutdown(); void UpdateIcdMode(); void UpdateOperationState(OperationalState state); @@ -86,6 +87,7 @@ class ICDManager ICDMode mICDMode = ICDMode::SIT; PersistentStorageDelegate * mStorage = nullptr; FabricTable * mFabricTable = nullptr; + ICDStateObserver * mStateObserver = nullptr; }; } // namespace app diff --git a/src/app/icd/ICDStateObserver.h b/src/app/icd/ICDStateObserver.h new file mode 100644 index 00000000000000..aa088a7b87f81a --- /dev/null +++ b/src/app/icd/ICDStateObserver.h @@ -0,0 +1,30 @@ +/* + * + * 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. + */ +#pragma once + +namespace chip { +namespace app { + +class ICDStateObserver +{ +public: + virtual ~ICDStateObserver() {} + virtual void OnEnterActiveMode() = 0; +}; + +} // namespace app +} // namespace chip diff --git a/src/app/reporting/ReportScheduler.h b/src/app/reporting/ReportScheduler.h index d691e4b36889ed..fc1858862bc4bd 100644 --- a/src/app/reporting/ReportScheduler.h +++ b/src/app/reporting/ReportScheduler.h @@ -19,6 +19,7 @@ #pragma once #include +#include #include #include @@ -38,7 +39,7 @@ class TimerContext virtual void TimerFired() = 0; }; -class ReportScheduler : public ReadHandler::Observer +class ReportScheduler : public ReadHandler::Observer, public ICDStateObserver { public: /// @brief This class acts as an interface between the report scheduler and the system timer to reduce dependencies on the @@ -172,6 +173,8 @@ class ReportScheduler : public ReadHandler::Observer 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->IsReportable(); } + /// @brief Sets the ForceDirty flag of a ReadHandler + void HandlerForceDirtyState(ReadHandler * aReadHandler) { aReadHandler->ForceDirtyState(); } /// @brief Get the number of ReadHandlers registered in the scheduler's node pool size_t GetNumReadHandlers() const { return mNodesPool.Allocated(); } diff --git a/src/app/reporting/ReportSchedulerImpl.cpp b/src/app/reporting/ReportSchedulerImpl.cpp index b6170fba9e0fdf..74f75a663277aa 100644 --- a/src/app/reporting/ReportSchedulerImpl.cpp +++ b/src/app/reporting/ReportSchedulerImpl.cpp @@ -37,6 +37,23 @@ ReportSchedulerImpl::ReportSchedulerImpl(TimerDelegate * aTimerDelegate) : Repor VerifyOrDie(nullptr != mTimerDelegate); } +/// @brief Method that triggers a report emission on each ReadHandler that is not blocked on its min interval. +/// Each read handler that is not blocked is immediately marked dirty so that it will report as soon as possible. +void ReportSchedulerImpl::OnEnterActiveMode() +{ +#if ICD_REPORT_ON_ENTER_ACTIVE_MODE + Timestamp now = mTimerDelegate->GetCurrentMonotonicTimestamp(); + mNodesPool.ForEachActiveObject([now, this](ReadHandlerNode * node) { + if (now >= node->GetMinTimestamp()) + { + this->HandlerForceDirtyState(node->GetReadHandler()); + } + + return Loop::Continue; + }); +#endif +} + /// @brief When a ReadHandler is added, register it, which will schedule an engine run void ReportSchedulerImpl::OnReadHandlerCreated(ReadHandler * aReadHandler) { diff --git a/src/app/reporting/ReportSchedulerImpl.h b/src/app/reporting/ReportSchedulerImpl.h index 3ac47449f834bc..573184ada02d5a 100644 --- a/src/app/reporting/ReportSchedulerImpl.h +++ b/src/app/reporting/ReportSchedulerImpl.h @@ -32,6 +32,9 @@ class ReportSchedulerImpl : public ReportScheduler ReportSchedulerImpl(TimerDelegate * aTimerDelegate); ~ReportSchedulerImpl() override { UnregisterAllHandlers(); } + // ICDStateObserver + void OnEnterActiveMode() override; + // ReadHandlerObserver void OnReadHandlerCreated(ReadHandler * aReadHandler) final; void OnBecameReportable(ReadHandler * aReadHandler) final; diff --git a/src/app/server/Server.cpp b/src/app/server/Server.cpp index a4c8b95d550f79..b416d5086c3096 100644 --- a/src/app/server/Server.cpp +++ b/src/app/server/Server.cpp @@ -250,7 +250,7 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams) #endif // CHIP_CONFIG_ENABLE_SERVER_IM_EVENT #if CHIP_CONFIG_ENABLE_ICD_SERVER - mICDManager.Init(mDeviceStorage, &GetFabricTable()); + mICDManager.Init(mDeviceStorage, &GetFabricTable(), &mReportScheduler); mICDEventManager.Init(&mICDManager); #endif // CHIP_CONFIG_ENABLE_ICD_SERVER