Skip to content

Commit

Permalink
[ICD] Subscription report emission when becoming active (#28266)
Browse files Browse the repository at this point in the history
* 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 <bzbarsky@apple.com>

* 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 <bzbarsky@apple.com>

* 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 <commits@restyled.io>
Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
  • Loading branch information
3 people committed Aug 1, 2023
1 parent f71a48f commit e059202
Show file tree
Hide file tree
Showing 10 changed files with 82 additions and 10 deletions.
2 changes: 1 addition & 1 deletion src/app/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
4 changes: 2 additions & 2 deletions src/app/ReadHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 8 additions & 1 deletion src/app/icd/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Expand All @@ -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" ]
}

Expand Down
16 changes: 13 additions & 3 deletions src/app/icd/ICDManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,19 +33,27 @@
#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 {

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);
Expand Down Expand Up @@ -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
{
Expand Down
4 changes: 3 additions & 1 deletion src/app/icd/ICDManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
*/
#pragma once

#include <app/icd/ICDStateObserver.h>
#include <credentials/FabricTable.h>
#include <lib/support/BitFlags.h>
#include <platform/CHIPDeviceConfig.h>
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -86,6 +87,7 @@ class ICDManager
ICDMode mICDMode = ICDMode::SIT;
PersistentStorageDelegate * mStorage = nullptr;
FabricTable * mFabricTable = nullptr;
ICDStateObserver * mStateObserver = nullptr;
};

} // namespace app
Expand Down
30 changes: 30 additions & 0 deletions src/app/icd/ICDStateObserver.h
Original file line number Diff line number Diff line change
@@ -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
5 changes: 4 additions & 1 deletion src/app/reporting/ReportScheduler.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#pragma once

#include <app/ReadHandler.h>
#include <app/icd/ICDStateObserver.h>
#include <lib/core/CHIPError.h>
#include <system/SystemClock.h>

Expand All @@ -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
Expand Down Expand Up @@ -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(); }
Expand Down
17 changes: 17 additions & 0 deletions src/app/reporting/ReportSchedulerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
3 changes: 3 additions & 0 deletions src/app/reporting/ReportSchedulerImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/app/server/Server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit e059202

Please sign in to comment.