Skip to content

Commit e6f610b

Browse files
jmartinez-silabsjmartinez-silabs
authored
[ICD]Add needed elements to the ICD Manager to handle LIT mode (project-chip#27916)
* Add needed elements to the ICD Manager to handle LIT mode * separate IcdMonitorinTable in its own sourceset to fix build issues on examples that have the ICD cluster but not not enable chip_enable_icd_server * address comments. Don't force Slow Polling interval in SIT to 15s to respect the current SHOULD conformance * fix test build
1 parent 70bb397 commit e6f610b

File tree

13 files changed

+76
-47
lines changed

13 files changed

+76
-47
lines changed

‎examples/all-clusters-app/esp32/main/CMakeLists.txt

+1
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ set(SRC_DIRS_LIST
3434
"${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/examples/platform/esp32/lock"
3535
"${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/examples/platform/esp32/mode-support"
3636
"${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/server"
37+
"${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/icd"
3738
"${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/util"
3839
"${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/reporting"
3940
"${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/administrator-commissioning-server"

‎examples/all-clusters-minimal-app/esp32/main/CMakeLists.txt

+1
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ set(SRC_DIRS_LIST
3232
"${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/examples/platform/esp32/shell_extension"
3333
"${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/examples/providers"
3434
"${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/server"
35+
"${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/icd"
3536
"${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/util"
3637
"${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/reporting"
3738
"${CMAKE_SOURCE_DIR}/third_party/connectedhomeip/src/app/clusters/administrator-commissioning-server"

‎src/app/chip_data_model.cmake

+1-1
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ function(chip_configure_data_model APP_TARGET)
143143
${CHIP_APP_BASE_DIR}/util/attribute-storage.cpp
144144
${CHIP_APP_BASE_DIR}/util/attribute-table.cpp
145145
${CHIP_APP_BASE_DIR}/util/binding-table.cpp
146-
${CHIP_APP_BASE_DIR}/util/IcdMonitoringTable.cpp
146+
${CHIP_APP_BASE_DIR}/icd/IcdMonitoringTable.cpp
147147
${CHIP_APP_BASE_DIR}/util/DataModelHandler.cpp
148148
${CHIP_APP_BASE_DIR}/util/ember-compatibility-functions.cpp
149149
${CHIP_APP_BASE_DIR}/util/error-mapping.cpp

‎src/app/chip_data_model.gni

+8-2
Original file line numberDiff line numberDiff line change
@@ -165,8 +165,6 @@ template("chip_data_model") {
165165
"${_app_root}/clusters/scenes-server/SceneTableImpl.h",
166166
"${_app_root}/clusters/scenes-server/scenes-server.h",
167167
"${_app_root}/util/DataModelHandler.cpp",
168-
"${_app_root}/util/IcdMonitoringTable.cpp",
169-
"${_app_root}/util/IcdMonitoringTable.h",
170168
"${_app_root}/util/attribute-size-util.cpp",
171169
"${_app_root}/util/attribute-storage.cpp",
172170
"${_app_root}/util/attribute-table.cpp",
@@ -197,6 +195,10 @@ template("chip_data_model") {
197195
[ invoker.zap_file ])
198196
}
199197

198+
if (!defined(deps)) {
199+
deps = []
200+
}
201+
200202
foreach(cluster, _cluster_sources) {
201203
if (cluster == "door-lock-server") {
202204
sources += [
@@ -256,6 +258,10 @@ template("chip_data_model") {
256258
"${_app_root}/clusters/${cluster}/${cluster}.h",
257259
"${_app_root}/clusters/${cluster}/operational-state-delegate.h",
258260
]
261+
} else if (cluster == "icd-management-server") {
262+
sources += [ "${_app_root}/clusters/${cluster}/${cluster}.cpp" ]
263+
264+
deps += [ "${chip_root}/src/app/icd:monitoring-table" ]
259265
} else {
260266
sources += [ "${_app_root}/clusters/${cluster}/${cluster}.cpp" ]
261267
}

‎src/app/clusters/icd-management-server/icd-management-server.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
#include <app/AttributeAccessInterface.h>
2727
#include <app/CommandHandler.h>
2828
#include <app/ConcreteAttributePath.h>
29-
#include <app/util/IcdMonitoringTable.h>
29+
#include <app/icd/IcdMonitoringTable.h>
3030
#include <app/util/af.h>
3131
#include <app/util/attribute-storage.h>
3232

‎src/app/icd/BUILD.gn

+10
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,16 @@ source_set("manager-srcs") {
2525
"ICDManager.h",
2626
]
2727

28+
deps = [ ":monitoring-table" ]
29+
public_deps = [ "${chip_root}/src/credentials:credentials" ]
30+
}
31+
32+
source_set("monitoring-table") {
33+
sources = [
34+
"IcdMonitoringTable.cpp",
35+
"IcdMonitoringTable.h",
36+
]
37+
2838
public_deps = [
2939
"${chip_root}/src/lib/core",
3040
"${chip_root}/src/platform:platform",

‎src/app/icd/ICDManager.cpp

+39-21
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,13 @@
1919
#include <app-common/zap-generated/ids/Attributes.h>
2020
#include <app-common/zap-generated/ids/Clusters.h>
2121
#include <app/icd/ICDManager.h>
22+
#include <app/icd/IcdMonitoringTable.h>
2223
#include <lib/support/CodeUtils.h>
2324
#include <lib/support/logging/CHIPLogging.h>
2425
#include <platform/ConnectivityManager.h>
2526
#include <platform/LockTracker.h>
2627
#include <platform/internal/CHIPDeviceLayerInternal.h>
28+
#include <stdlib.h>
2729

2830
namespace chip {
2931
namespace app {
@@ -32,8 +34,13 @@ using namespace chip::app;
3234
using namespace chip::app::Clusters;
3335
using namespace chip::app::Clusters::IcdManagement;
3436

35-
void ICDManager::ICDManager::Init()
37+
void ICDManager::ICDManager::Init(PersistentStorageDelegate * storage, FabricTable * fabricTable)
3638
{
39+
VerifyOrDie(storage != nullptr);
40+
VerifyOrDie(fabricTable != nullptr);
41+
mStorage = storage;
42+
mFabricTable = fabricTable;
43+
3744
uint32_t activeModeInterval;
3845
if (Attributes::ActiveModeInterval::Get(kRootEndpointId, &activeModeInterval) != EMBER_ZCL_STATUS_SUCCESS)
3946
{
@@ -49,16 +56,17 @@ void ICDManager::ICDManager::Shutdown()
4956
// cancel any running timer of the icd
5057
DeviceLayer::SystemLayer().CancelTimer(OnIdleModeDone, this);
5158
DeviceLayer::SystemLayer().CancelTimer(OnActiveModeDone, this);
52-
mIcdMode = ICDMode::SIT;
59+
mICDMode = ICDMode::SIT;
5360
mOperationalState = OperationalState::IdleMode;
61+
mStorage = nullptr;
62+
mFabricTable = nullptr;
5463
}
5564

5665
bool ICDManager::SupportsCheckInProtocol()
5766
{
5867
bool success;
5968
uint32_t featureMap;
6069
success = (Attributes::FeatureMap::Get(kRootEndpointId, &featureMap) == EMBER_ZCL_STATUS_SUCCESS);
61-
6270
return success ? ((featureMap & to_underlying(Feature::kCheckInProtocolSupport)) != 0) : false;
6371
}
6472

@@ -68,25 +76,25 @@ void ICDManager::UpdateIcdMode()
6876

6977
ICDMode tempMode = ICDMode::SIT;
7078

71-
// TODO ICD LIT FIX DEPENDENCY ISSUE with app/util/IcdMonitoringTable.h and app/server:server
7279
// The Check In Protocol Feature is required and the slow polling interval shall also be greater than 15 seconds
7380
// to run an ICD in LIT mode.
74-
// if (kSlowPollingInterval > kICDSitModePollingThreashold && SupportsCheckInProtocol())
75-
// {
76-
// // We can only get to LIT Mode, if at least one client is registered to the ICD device
77-
// const auto & fabricTable = Server::GetInstance().GetFabricTable();
78-
// for (const auto & fabricInfo : fabricTable)
79-
// {
80-
// PersistentStorageDelegate & storage = Server::GetInstance().GetPersistentStorage();
81-
// IcdMonitoringTable table(storage, fabricInfo.GetFabricIndex(), 1);
82-
// if (!table.IsEmpty())
83-
// {
84-
// tempMode = ICDMode::LIT;
85-
// break;
86-
// }
87-
// }
88-
// }
89-
mIcdMode = tempMode;
81+
if (GetSlowPollingInterval() > GetSITPollingThreshold() && SupportsCheckInProtocol())
82+
{
83+
VerifyOrDie(mStorage != nullptr);
84+
VerifyOrDie(mFabricTable != nullptr);
85+
// We can only get to LIT Mode, if at least one client is registered with the ICD device
86+
for (const auto & fabricInfo : *mFabricTable)
87+
{
88+
// We only need 1 valid entry to ensure LIT compliance
89+
IcdMonitoringTable table(*mStorage, fabricInfo.GetFabricIndex(), 1 /*Table entry limit*/);
90+
if (!table.IsEmpty())
91+
{
92+
tempMode = ICDMode::LIT;
93+
break;
94+
}
95+
}
96+
}
97+
mICDMode = tempMode;
9098
}
9199

92100
void ICDManager::UpdateOperationState(OperationalState state)
@@ -105,7 +113,17 @@ void ICDManager::UpdateOperationState(OperationalState state)
105113
}
106114
DeviceLayer::SystemLayer().StartTimer(System::Clock::Timeout(idleModeInterval), OnIdleModeDone, this);
107115

108-
CHIP_ERROR err = DeviceLayer::ConnectivityMgr().SetPollingInterval(GetSlowPollingInterval());
116+
System::Clock::Milliseconds32 slowPollInterval = GetSlowPollingInterval();
117+
// When in SIT mode, the slow poll interval SHOULDN'T be greater than the SIT mode polling threshold, per spec.
118+
if (mICDMode == ICDMode::SIT && slowPollInterval > GetSITPollingThreshold())
119+
{
120+
ChipLogDetail(AppServer, "The Slow Polling Interval of an ICD in SIT mode should be <= %" PRIu32,
121+
(GetSITPollingThreshold().count() / 1000));
122+
// TODO Spec to define this conformance as a SHALL
123+
// slowPollInterval = GetSITPollingThreshold();
124+
}
125+
126+
CHIP_ERROR err = DeviceLayer::ConnectivityMgr().SetPollingInterval(slowPollInterval);
109127
if (err != CHIP_NO_ERROR)
110128
{
111129
ChipLogError(AppServer, "Failed to set Polling Interval: err %" CHIP_ERROR_FORMAT, err.Format());

‎src/app/icd/ICDManager.h

+12-7
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
*/
1717
#pragma once
1818

19+
#include <credentials/FabricTable.h>
1920
#include <lib/support/BitFlags.h>
2021
#include <platform/CHIPDeviceConfig.h>
2122
#include <platform/internal/CHIPDeviceLayerInternal.h>
@@ -50,15 +51,16 @@ class ICDManager
5051
};
5152

5253
ICDManager() {}
53-
void Init();
54+
void Init(PersistentStorageDelegate * storage, FabricTable * fabricTable);
5455
void Shutdown();
5556
void UpdateIcdMode();
5657
void UpdateOperationState(OperationalState state);
5758
void SetKeepActiveModeRequirements(KeepActiveFlags flag, bool state);
5859
bool IsKeepActive() { return mKeepActiveFlags.HasAny(); }
59-
ICDMode GetIcdMode() { return mIcdMode; }
60+
ICDMode GetICDMode() { return mICDMode; }
6061
OperationalState GetOperationalState() { return mOperationalState; }
6162

63+
static System::Clock::Milliseconds32 GetSITPollingThreshold() { return kSITPollingThreshold; }
6264
static System::Clock::Milliseconds32 GetSlowPollingInterval() { return kSlowPollingInterval; }
6365
static System::Clock::Milliseconds32 GetFastPollingInterval() { return kFastPollingInterval; }
6466

@@ -67,9 +69,10 @@ class ICDManager
6769
static void OnActiveModeDone(System::Layer * aLayer, void * appState);
6870

6971
private:
70-
static constexpr System::Clock::Milliseconds32 kICDSitModePollingThreashold = System::Clock::Milliseconds32(15000);
71-
static constexpr System::Clock::Milliseconds32 kSlowPollingInterval = CHIP_DEVICE_CONFIG_ICD_SLOW_POLL_INTERVAL;
72-
static constexpr System::Clock::Milliseconds32 kFastPollingInterval = CHIP_DEVICE_CONFIG_ICD_FAST_POLL_INTERVAL;
72+
// SIT ICDs should have a SlowPollingThreshold shorter than or equal to 15s (spec 9.16.1.5)
73+
static constexpr System::Clock::Milliseconds32 kSITPollingThreshold = System::Clock::Milliseconds32(15000);
74+
static constexpr System::Clock::Milliseconds32 kSlowPollingInterval = CHIP_DEVICE_CONFIG_ICD_SLOW_POLL_INTERVAL;
75+
static constexpr System::Clock::Milliseconds32 kFastPollingInterval = CHIP_DEVICE_CONFIG_ICD_FAST_POLL_INTERVAL;
7376

7477
// Minimal constraint value of the the ICD attributes.
7578
static constexpr uint32_t kMinIdleModeInterval = 500;
@@ -79,8 +82,10 @@ class ICDManager
7982
bool SupportsCheckInProtocol();
8083

8184
BitFlags<KeepActiveFlags> mKeepActiveFlags{ 0 };
82-
OperationalState mOperationalState = OperationalState::IdleMode;
83-
ICDMode mIcdMode = ICDMode::SIT;
85+
OperationalState mOperationalState = OperationalState::IdleMode;
86+
ICDMode mICDMode = ICDMode::SIT;
87+
PersistentStorageDelegate * mStorage = nullptr;
88+
FabricTable * mFabricTable = nullptr;
8489
};
8590

8691
} // namespace app

‎src/app/util/IcdMonitoringTable.cpp renamed to ‎src/app/icd/IcdMonitoringTable.cpp

File renamed without changes.

‎src/app/util/IcdMonitoringTable.h renamed to ‎src/app/icd/IcdMonitoringTable.h

File renamed without changes.

‎src/app/server/Server.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams)
249249
#endif // CHIP_CONFIG_ENABLE_SERVER_IM_EVENT
250250

251251
#if CHIP_CONFIG_ENABLE_ICD_SERVER
252-
mICDManager.Init();
252+
mICDManager.Init(mDeviceStorage, &GetFabricTable());
253253
mICDEventManager.Init(&mICDManager);
254254
#endif // CHIP_CONFIG_ENABLE_ICD_SERVER
255255

‎src/app/tests/BUILD.gn

+1-13
Original file line numberDiff line numberDiff line change
@@ -54,18 +54,6 @@ source_set("binding-test-srcs") {
5454
]
5555
}
5656

57-
source_set("icd-management-test-srcs") {
58-
sources = [
59-
"${chip_root}/src/app/util/IcdMonitoringTable.cpp",
60-
"${chip_root}/src/app/util/IcdMonitoringTable.h",
61-
]
62-
63-
public_deps = [
64-
"${chip_root}/src/app/common:cluster-objects",
65-
"${chip_root}/src/lib/core",
66-
]
67-
}
68-
6957
source_set("ota-requestor-test-srcs") {
7058
sources = [
7159
"${chip_root}/src/app/clusters/ota-requestor/DefaultOTARequestorStorage.cpp",
@@ -182,14 +170,14 @@ chip_test_suite("tests") {
182170

183171
public_deps = [
184172
":binding-test-srcs",
185-
":icd-management-test-srcs",
186173
":operational-state-test-srcs",
187174
":ota-requestor-test-srcs",
188175
":scenes-table-test-srcs",
189176
":time-sync-data-provider-test-srcs",
190177
"${chip_root}/src/app",
191178
"${chip_root}/src/app/common:cluster-objects",
192179
"${chip_root}/src/app/icd:manager-srcs",
180+
"${chip_root}/src/app/icd:monitoring-table",
193181
"${chip_root}/src/app/tests:helpers",
194182
"${chip_root}/src/app/util/mock:mock_ember",
195183
"${chip_root}/src/lib/core",

‎src/app/tests/TestIcdMonitoringTable.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
* limitations under the License.
1616
*/
1717

18-
#include <app/util/IcdMonitoringTable.h>
18+
#include <app/icd/IcdMonitoringTable.h>
1919
#include <lib/core/CHIPError.h>
2020
#include <lib/support/DefaultStorageKeyAllocator.h>
2121
#include <lib/support/TestPersistentStorageDelegate.h>

0 commit comments

Comments
 (0)